tweaks so vars are still reset

gchq · Jan 16, 2025 · 1f0d16e · 1f0d16e
1 parent a8c955d
commit 1f0d16e
Show file tree

Hide file tree

Showing 3 changed files with 72 additions and 99 deletions.
diff --git a/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopGraph.java b/library/tinkerpop/src/main/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopGraph.java
@@ -457,7 +457,7 @@ public Iterator<Vertex> vertices(final Object... vertexIds) {
                 variables.getElementsLimit());
         }
 
-        // Translate results to Gafferpop elements
+        // Translate results to GafferPop elements
         final GafferPopElementGenerator generator = new GafferPopElementGenerator(this);
         final Set<Vertex> translatedResults = StreamSupport.stream(result.spliterator(), false)
                 .map(generator::_apply)
@@ -677,60 +677,6 @@ public Iterator<Edge> edgesWithView(final Object id, final Direction direction,
         return edgesWithView(Collections.singletonList(id), direction, view);
     }
 
-    /**
-     * This performs a GetElements operation filtering edges by direction and view.
-     *
-     * @param ids vertex IDs or edge IDs to be queried for.
-     * Supports input as a {@link Vertex}, {@link Edge}, List of Edge IDs or individual Vertex IDs.
-     * @param direction {@link Direction} of edges to return.
-     * @param view Gaffer {@link View} to filter edges by
-     * @return iterator of {@link GafferPopEdge}s.
-     * @see #edges(Object...)
-     */
-    public Iterator<Edge> edgesWithView(final Iterable<Object> ids, final Direction direction, final View view) {
-        return edgesWithSeedsAndView(getElementSeeds(ids), direction, view);
-    }
-
-    @Override
-    public <C extends GraphComputer> C compute(final Class<C> graphComputerClass) throws IllegalArgumentException {
-        throw Exceptions.graphComputerNotSupported();
-    }
-
-    @Override
-    public GraphComputer compute() throws IllegalArgumentException {
-        throw Exceptions.graphComputerNotSupported();
-    }
-
-    @Override
-    public Transaction tx() {
-        throw Exceptions.transactionsNotSupported();
-    }
-
-    @Override
-    public Variables variables() {
-        return this.variables;
-    }
-
-    @Override
-    public Configuration configuration() {
-        return configuration;
-    }
-
-    @Override
-    public void close() throws Exception {
-        serviceRegistry.close();
-    }
-
-    @Override
-    public ServiceRegistry getServiceRegistry() {
-        return serviceRegistry;
-    }
-
-    @Override
-    public Features features() {
-        return features;
-    }
-
     public <T> T execute(final OperationChain<T> opChain) {
         // Set options at opChain level
         opChain.setOptions(variables.getOperationOptions());
@@ -765,12 +711,16 @@ public <T> T execute(final OperationChain<T> opChain) {
 
     /**
      * Sets the {@link GafferPopGraphVariables} to default values for this
-     * graph
+     * graph optionally preserving the previous user.
      *
+     * @param preserveUser keep the current set user.
      */
-    public void setDefaultVariables() {
+    public void setDefaultVariables(final boolean preserveUser) {
         LOGGER.debug("Resetting graph variables to defaults");
-        variables.set(GafferPopGraphVariables.USER, defaultUser);
+        if (!preserveUser) {
+            LOGGER.debug("Resetting graph user to default");
+            variables.set(GafferPopGraphVariables.USER, defaultUser);
+        }
         variables.set(GafferPopGraphVariables.OP_OPTIONS, Collections.unmodifiableMap(opOptions));
         variables.set(GafferPopGraphVariables.GET_ELEMENTS_LIMIT,
                 configuration().getInteger(GET_ELEMENTS_LIMIT, DEFAULT_GET_ELEMENTS_LIMIT));
@@ -781,6 +731,14 @@ public void setDefaultVariables() {
         variables.set(GafferPopGraphVariables.LAST_OPERATION_CHAIN, new OperationChain<Object>());
     }
 
+    /**
+     * Sets the {@link GafferPopGraphVariables} to default values for this
+     * graph.
+     */
+    public void setDefaultVariables() {
+        setDefaultVariables(false);
+    }
+
     /**
      * Get the underlying Gaffer graph this GafferPop graph is connected to.
      *
@@ -790,6 +748,60 @@ public Graph getGafferGraph() {
         return graph;
     }
 
+    /**
+     * This performs a GetElements operation filtering edges by direction and view.
+     *
+     * @param ids vertex IDs or edge IDs to be queried for.
+     * Supports input as a {@link Vertex}, {@link Edge}, List of Edge IDs or individual Vertex IDs.
+     * @param direction {@link Direction} of edges to return.
+     * @param view Gaffer {@link View} to filter edges by
+     * @return iterator of {@link GafferPopEdge}s.
+     * @see #edges(Object...)
+     */
+    public Iterator<Edge> edgesWithView(final Iterable<Object> ids, final Direction direction, final View view) {
+        return edgesWithSeedsAndView(getElementSeeds(ids), direction, view);
+    }
+
+    @Override
+    public <C extends GraphComputer> C compute(final Class<C> graphComputerClass) throws IllegalArgumentException {
+        throw Exceptions.graphComputerNotSupported();
+    }
+
+    @Override
+    public GraphComputer compute() throws IllegalArgumentException {
+        throw Exceptions.graphComputerNotSupported();
+    }
+
+    @Override
+    public Transaction tx() {
+        throw Exceptions.transactionsNotSupported();
+    }
+
+    @Override
+    public Variables variables() {
+        return this.variables;
+    }
+
+    @Override
+    public Configuration configuration() {
+        return configuration;
+    }
+
+    @Override
+    public void close() throws Exception {
+        serviceRegistry.close();
+    }
+
+    @Override
+    public ServiceRegistry getServiceRegistry() {
+        return serviceRegistry;
+    }
+
+    @Override
+    public Features features() {
+        return features;
+    }
+
     private Iterator<GafferPopVertex> verticesWithSeedsAndView(final List<ElementSeed> seeds, final View view) {
         final boolean getAll = null == seeds || seeds.isEmpty();
         final LinkedList<GafferPopVertex> idVertices = new LinkedList<>();

diff --git a/...src/main/java/uk/gov/gchq/gaffer/tinkerpop/process/traversal/step/GafferPopGraphStep.java b/...src/main/java/uk/gov/gchq/gaffer/tinkerpop/process/traversal/step/GafferPopGraphStep.java
@@ -73,6 +73,8 @@ public GafferPopGraphStep(final GraphStep<S, E> originalGraphStep) {
 
         // Save reference to the graph
         GafferPopGraph graph = (GafferPopGraph) originalGraphStep.getTraversal().getGraph().get();
+        // Reset vars on the graph but preserving the user in case it was set externally
+        graph.setDefaultVariables(true);
 
         // Find any options on the traversal
         Optional<OptionsStrategy> optionsStrategy = originalGraphStep.getTraversal().getStrategies().getStrategy(OptionsStrategy.class);

diff --git a/library/tinkerpop/src/test/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopGraphIT.java b/library/tinkerpop/src/test/java/uk/gov/gchq/gaffer/tinkerpop/GafferPopGraphIT.java
@@ -27,16 +27,12 @@
 import org.junit.jupiter.params.provider.Arguments;
 import org.junit.jupiter.params.provider.MethodSource;
 
-import io.opentelemetry.context.Context;
 import uk.gov.gchq.gaffer.cache.CacheServiceLoader;
 import uk.gov.gchq.gaffer.tinkerpop.util.GafferPopTestUtil.StoreType;
 import uk.gov.gchq.gaffer.tinkerpop.util.modern.GafferPopModernTestUtils;
-import uk.gov.gchq.gaffer.user.User;
 
 import java.util.List;
 import java.util.Map;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -355,43 +351,6 @@ void shouldTraverseEdgeWithVertexOnlyEdge(String graph, GraphTraversalSource g)
         reset();
     }
 
-    @ParameterizedTest(name = TEST_NAME_FORMAT)
-    @MethodSource("provideTraversals")
-    void shouldPreserveExternallySetUser(String graph, GraphTraversalSource g) {
-        ExecutorService executorService = Context.taskWrapping(Executors.newFixedThreadPool(2));
-
-        User testUser = new User("shouldPreserveExternallySetUser");
-        User testUser2 = new User("shouldPreserveExternallySetUser2");
-
-        executorService.submit(() -> {
-            GraphTraversalSource g2 = ((GafferPopGraph) g.getGraph()).newInstance().traversal();
-            g2.getGraph().variables().set(GafferPopGraphVariables.USER, testUser);
-            g2.V().toList();
-            assertThat(((GafferPopGraphVariables) g.getGraph().variables()).getUser()).isEqualTo(testUser);
-        });
-
-        executorService.submit(() -> {
-            GraphTraversalSource g2 = ((GafferPopGraph) g.getGraph()).newInstance().traversal();
-            g2.getGraph().variables().set(GafferPopGraphVariables.USER, testUser2);
-            g2.V().toList();
-            assertThat(((GafferPopGraphVariables) g.getGraph().variables()).getUser()).isEqualTo(testUser2);
-        });
-
-        // Set the user
-        // g.getGraph().variables().set(GafferPopGraphVariables.USER, testUser);
-
-        // // Run a Query
-        // g.V().toList();
-
-        // Ensure user is still set
-        //assertThat(((GafferPopGraphVariables) g.getGraph().variables()).getUser()).isEqualTo(testUser);
-
-        // Reset vars
-        ((GafferPopGraph) g.getGraph()).setDefaultVariables();
-        assertThat(((GafferPopGraphVariables) g.getGraph().variables()).getUser()).isNotEqualTo(testUser);
-        reset();
-    }
-
 
     void reset() {
         // reset cache for federation