Merge branch 'TINKERPOP-2994' into 3.6-dev

CHANGELOG.asciidoc

@@ -31,6 +31,10 @@ This release also includes changes from <<release-3-5-8, 3.5.8>>.
 * Fixed a bug in `StarGraph` where `EdgeFilter` did not remove associated Edge Properties.
 * Added translator to the Go GLV.
 * Fixed bug with filtering for `group()` when the side-effect label was defined for it.
+* Fixed bug in `DotNetTranslator` where `PartitionStrategy` usage was not translating properly when specifying the `readPartitions`.
+* Fixed bug in `PythonTranslator` where `Set` syntax was not being generated properly.
+* Fixed bug in configuration object given to `PartitionStrategy` for Go that prevented `readPartitions` from behing set properly.
+* Fixed bug where the `partitionKey` was not being written when using `PartitionStrategy` with `mergeV()` and `mergeE()`
 
 [[release-3-6-5]]
 === TinkerPop 3.6.5 (Release Date: July 31, 2023)

docs/src/dev/developer/for-committers.asciidoc

@@ -580,9 +580,11 @@ best example of this sort of test would be one that uses the remote `Lambda` syn
 * `@UserSuppliedEdgeIds` - The scenario relies on the edge IDs specified in the dataset used by the scenario.
 * `@UserSuppliedVertexPropertyIds` - The scenario relies on the vertex property IDs specified in the dataset used by the scenario.
 * `@With*` - The scenario uses some `with()` based configuration like strategies:
+** `@WithPartitionStrategy`
 ** `@WithProductiveByStrategy`
 ** `@WithReadOnlyStrategy`
 ** `@WithSeedStrategy`
+** `@WithSubgraphStrategy`
 
 Tag filters can be applied to Intellij at execution time by adding a system properties of
 `-Dcucumber.filter.tags=<step-filter>`.

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeStep.java

@@ -41,7 +41,9 @@
 import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.CallbackRegistry;
 import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.Event;
 import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.ListCallbackRegistry;
+import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.PartitionStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement;
+import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil;
 import org.apache.tinkerpop.gremlin.structure.Direction;
 import org.apache.tinkerpop.gremlin.structure.Graph;
@@ -65,6 +67,10 @@ public abstract class MergeStep<S, E, C> extends FlatMapStep<S, E>
 
     protected CallbackRegistry<Event> callbackRegistry;
 
+    private Parameters parameters = new Parameters();
+
+    private boolean usesPartitionStrategy;
+
     public MergeStep(final Traversal.Admin traversal, final boolean isStart) {
         this(traversal, isStart, new IdentityTraversal<>());
     }
@@ -79,6 +85,13 @@ public MergeStep(final Traversal.Admin traversal, final boolean isStart,
         super(traversal);
         this.isStart = isStart;
         this.mergeTraversal = integrateChild(mergeTraversal);
+
+        // determines if this step uses PartitionStrategy. it's not great that merge needs to know about a particular
+        // strategy but if it doesn't then it can't determine if Parameters are being used properly or not. to not have
+        // this check seems to invite problems. in some sense, this is not the first time steps have had to know more
+        // about strategies than is probably preferred - EventStrategy comes to mind
+        this.usesPartitionStrategy = TraversalHelper.getRootTraversal(traversal).
+                getStrategies().getStrategy(PartitionStrategy.class).isPresent();
     }
 
     /**
@@ -144,20 +157,20 @@ public <S, C> List<Traversal.Admin<S, C>> getLocalChildren() {
         return children;
     }
 
+    /**
+     * This implementation should only be used as a mechanism for supporting {@link PartitionStrategy}. Using this
+     * with {@link GraphTraversal#with(String,Object)} will have an ill effect of simply acting like a call to
+     * {@link GraphTraversal#property(Object, Object, Object...)}. No mutating steps currently support use of
+     * {@link GraphTraversal#with(String,Object)} so perhaps it's best to not start with that now.
+     */
     @Override
     public void configure(final Object... keyValues) {
-        // this is a Mutating step but property() should not be folded into this step.  The main issue here is that
-        // this method won't know what step called it - property() or with() or something else so it can't make the
-        // choice easily to throw an exception, write the keys/values to parameters, etc. It really is up to the
-        // caller to make sure it is handled properly at this point. this may best be left as a do-nothing method for
-        // now.
+        this.parameters.set(this, keyValues);
     }
 
     @Override
     public Parameters getParameters() {
-        // merge doesn't take fold ups of property() calls. those need to get treated as regular old PropertyStep
-        // instances. not sure if this should support with() though.....none of the other Mutating steps do.
-        return null;
+        return this.parameters;
     }
 
     @Override
@@ -301,7 +314,19 @@ protected void validateNoOverrides(final Map<?,?> mergeMap, final Map<?,?> onCre
      * null Map == empty Map
      */
     protected Map materializeMap(final Traverser.Admin<S> traverser, Traversal.Admin<S, ?> mapTraversal) {
-        final Map map = (Map) TraversalUtil.apply(traverser, mapTraversal);
+        Map map = (Map) TraversalUtil.apply(traverser, mapTraversal);
+
+        // PartitionStrategy uses parameters as a mechanism for setting the partition key. trying to be as specific
+        // as possible here wrt parameters usage to avoid misuse
+        if (usesPartitionStrategy) {
+            map = null == map ? new LinkedHashMap() : map;
+            for (Map.Entry<Object, List<Object>> entry : parameters.getRaw().entrySet()) {
+                final Object k = entry.getKey();
+                final List<Object> v = entry.getValue();
+                map.put(k, v.get(0));
+            }
+        }
+
         return map == null ? new LinkedHashMap() : map;
     }
 

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategy.java

@@ -39,6 +39,8 @@
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.EdgeVertexStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.GraphStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.LambdaMapStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.map.MergeEdgeStep;
+import org.apache.tinkerpop.gremlin.process.traversal.step.map.MergeVertexStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.PropertiesStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.PropertyMapStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.map.VertexStep;
@@ -226,6 +228,7 @@ public void apply(final Traversal.Admin<?, ?> traversal) {
         }
 
         final List<Step> stepsToInsertPropertyMutations = traversal.getSteps().stream().filter(step ->
+                step instanceof MergeVertexStep || step instanceof MergeEdgeStep ||
                 step instanceof AddEdgeStep || step instanceof AddVertexStep ||
                         step instanceof AddEdgeStartStep || step instanceof AddVertexStartStep ||
                         (includeMetaProperties && step instanceof AddPropertyStep)

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/DotNetTranslator.java

@@ -34,6 +34,7 @@
 import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.TraversalStrategyProxy;
+import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.PartitionStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.util.ConnectiveP;
 import org.apache.tinkerpop.gremlin.process.traversal.util.OrP;
 import org.apache.tinkerpop.gremlin.structure.Direction;
@@ -194,8 +195,14 @@ protected String getSyntax(final Number o) {
 
         @Override
         protected Script produceScript(final Set<?> o) {
+            return produceScriptForHashSet(o, "object");
+        }
+
+        private Script produceScriptForHashSet(final Set<?> o, final String generic) {
             final Iterator<?> iterator = o.iterator();
-            script.append("new HashSet<object> {");
+            script.append("new HashSet<");
+            script.append(generic);
+            script.append("> {");
 
             while (iterator.hasNext()) {
                 final Object nextItem = iterator.next();
@@ -292,7 +299,13 @@ protected Script produceScript(final TraversalStrategyProxy<?> o) {
                     final String k = keys.next();
                     script.append(k);
                     script.append(": ");
-                    convertToScript(o.getConfiguration().getProperty(k));
+
+                    // special handle PartitionStrategy since the Set it uses must be of String for readPartitions
+                    if (k.equals("readPartitions") && o.getStrategyClass().equals(PartitionStrategy.class))
+                        produceScriptForHashSet((Set) o.getConfiguration().getProperty(k), "string");
+                    else
+                        convertToScript(o.getConfiguration().getProperty(k));
+
                     if (keys.hasNext())
                         script.append(", ");
                 }

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/PythonTranslator.java

@@ -205,13 +205,13 @@ protected String getSyntax(final Pick o) {
         @Override
         protected Script produceScript(final Set<?> o) {
             final Iterator<?> iterator = o.iterator();
-            script.append("set(");
+            script.append("set((");
             while(iterator.hasNext()) {
                 convertToScript(iterator.next());
                 if (iterator.hasNext())
                     script.append(",");
             }
-            return script.append(")");
+            return script.append("))");
         }
 
         @Override

gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeStepTest.java

@@ -21,7 +21,10 @@
 import org.apache.tinkerpop.gremlin.process.traversal.Merge;
 import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
 import org.apache.tinkerpop.gremlin.process.traversal.TraversalSideEffects;
+import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies;
 import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
+import org.apache.tinkerpop.gremlin.process.traversal.step.util.EmptyStep;
+import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalStrategies;
 import org.apache.tinkerpop.gremlin.structure.Direction;
 import org.apache.tinkerpop.gremlin.structure.T;
 import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceVertex;
@@ -116,6 +119,8 @@ public void shouldFailToValidateWithoutTokensBecauseOfWeirdKey() {
     public void shouldWorkWithImmutableMap() {
         final Traversal.Admin traversal = mock(Traversal.Admin.class);
         when(traversal.getTraverserSetSupplier()).thenReturn(TraverserSetSupplier.instance());
+        when(traversal.getParent()).thenReturn(EmptyStep.instance());
+        when(traversal.getStrategies()).thenReturn(new DefaultTraversalStrategies());
         final Traverser.Admin traverser = mock(Traverser.Admin.class);
         when(traverser.split()).thenReturn(mock(Traverser.Admin.class));
         final Traversal.Admin onCreateTraversal = mock(Traversal.Admin.class);

gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeVertexStepTest.java

@@ -22,6 +22,8 @@
 import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
 import org.apache.tinkerpop.gremlin.process.traversal.TraversalSideEffects;
 import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
+import org.apache.tinkerpop.gremlin.process.traversal.step.util.EmptyStep;
+import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalStrategies;
 import org.apache.tinkerpop.gremlin.structure.Direction;
 import org.apache.tinkerpop.gremlin.structure.T;
 import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceVertex;
@@ -90,6 +92,8 @@ public void shouldFailToValidateWithoutTokensBecauseOfWeirdKey() {
     public void shouldWorkWithImmutableMap() {
         final Traversal.Admin traversal = mock(Traversal.Admin.class);
         when(traversal.getTraverserSetSupplier()).thenReturn(TraverserSetSupplier.instance());
+        when(traversal.getParent()).thenReturn(EmptyStep.instance());
+        when(traversal.getStrategies()).thenReturn(new DefaultTraversalStrategies());
         final Traverser.Admin traverser = mock(Traverser.Admin.class);
         when(traverser.split()).thenReturn(mock(Traverser.Admin.class));
         final Traversal.Admin onCreateTraversal = mock(Traversal.Admin.class);