TINKERPOP-3148 Prevent multiple by modulators from being used for the dedup step by throwing IllegalStateException if modulateBy is called more than once. (#3105)

Author: andreachild

CHANGELOG.asciidoc

@@ -66,7 +66,7 @@ This release also includes changes from <<release-3-7-XXX, 3.7.XXX>>.
 * Added missing strategies in `gremlin-go`, updated certain strategies to take varargs and updated `GoTranslatorVisitor` for corresponding translations.
 * Fixed `BigInt` and `BigDecimal` parsing in `gremlin-go` cucumber test suite, fixed `UnscaledValue` type in `BigDecimal` struct and added `ParseBigDecimal` method.
 * Added validation to `valueMap()`/`propertyMap()`/`groupCount()` to prevent an invalid usage of multiple `by()` modulators.
-* Added validation to `groupCount()` and `sack()` to prevent an invalid usage of multiple `by()` modulators.
+* Added validation to `groupCount()`, `sack()` and `dedup()` to prevent an invalid usage of multiple `by()` modulators.
 * Deprecated `ProcessLimitedStandardSuite` and `ProcessLimitedComputerSuite` in favor of `ProcessEmbeddedStandardSuite` and `ProcessEmbeddedComputerSuite` respectively.
 * Deprecated `ProcessStandardSuite` and the `ProcessComputerSuite` in favor of Gherkin testing and the `ProcessEmbeddedStandardSuite` and `ProcessEmbeddedComputerSuite` for testing JVM-specific Gremlin behaviors.
 * Removed lambda oriented Gremlin testing from Gherkin test suite.

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/DedupGlobalStep.java

@@ -122,6 +122,9 @@ public List<Traversal<S, Object>> getLocalChildren() {
 
     @Override
     public void modulateBy(final Traversal.Admin<?, ?> dedupTraversal) {
+        if (this.dedupTraversal != null) {
+            throw new IllegalStateException("Dedup step can only have one by modulator");
+        }
         this.dedupTraversal = this.integrateChild(dedupTraversal);
     }
 

gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/DedupGlobalStepTest.java

@@ -24,6 +24,7 @@
 
 import java.util.Arrays;
 import java.util.List;
+import org.junit.Test;
 
 /**
  * @author Daniel Kuppitz (http://gremlin.guru)
@@ -37,4 +38,9 @@ protected List<Traversal> getTraversals() {
                 __.dedup().by("name")
         );
     }
+
+    @Test(expected = IllegalStateException.class)
+    public void shouldThrowForMultipleByModulators() {
+        __.dedup().by("name").by("age");
+    }
 }

gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs

@@ -200,6 +200,7 @@ private static IDictionary<string, List<Func<GraphTraversalSource, IDictionary<s
                {"g_VX1X_valuesXageX_dedupXlocalX_unfold", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V(p["vid1"]).Values<object>("age").Dedup(Scope.Local).Unfold<object>()}}, 
                {"g_V_properties_dedup_count", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV("person").Property("name", "josh").AddV("person").Property("name", "josh").AddV("person").Property("name", "josh"), (g,p) =>g.V().Properties<object>("name").Dedup().Count()}}, 
                {"g_V_properties_dedup_byXvalueX_count", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV("person").Property("name", "josh").AddV("person").Property("name", "josh").AddV("person").Property("name", "josh"), (g,p) =>g.V().Properties<object>("name").Dedup().By(T.Value).Count()}}, 
+               {"g_V_both_hasXlabel_softwareX_dedup_byXlangX_byXnameX_name", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Both().Has(T.Label, "software").Dedup().By("lang").By("name").Values<object>("name")}}, 
                {"g_V_drop", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV().As("a").AddV().As("b").AddE("knows").To("a"), (g,p) =>g.V().Drop(), (g,p) =>g.V(), (g,p) =>g.E()}}, 
                {"g_V_outE_drop", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV().As("a").AddV().As("b").AddE("knows").To("a"), (g,p) =>g.V().OutE().Drop(), (g,p) =>g.V(), (g,p) =>g.E()}}, 
                {"g_V_properties_drop", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.AddV().Property("name", "bob").AddV().Property("name", "alice"), (g,p) =>g.V().Properties<object>().Drop(), (g,p) =>g.V(), (g,p) =>g.V().Properties<object>()}}, 

gremlin-go/driver/cucumber/gremlin.go

@@ -169,6 +169,7 @@ var translationMap = map[string][]func(g *gremlingo.GraphTraversalSource, p map[
     "g_VX1X_valuesXageX_dedupXlocalX_unfold": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V(p["vid1"]).Values("age").Dedup(gremlingo.Scope.Local).Unfold()}}, 
     "g_V_properties_dedup_count": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.AddV("person").Property("name", "josh").AddV("person").Property("name", "josh").AddV("person").Property("name", "josh")}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Properties("name").Dedup().Count()}}, 
     "g_V_properties_dedup_byXvalueX_count": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.AddV("person").Property("name", "josh").AddV("person").Property("name", "josh").AddV("person").Property("name", "josh")}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Properties("name").Dedup().By(gremlingo.T.Value).Count()}}, 
+    "g_V_both_hasXlabel_softwareX_dedup_byXlangX_byXnameX_name": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Both().Has(gremlingo.T.Label, "software").Dedup().By("lang").By("name").Values("name")}}, 
     "g_V_drop": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.AddV().As("a").AddV().As("b").AddE("knows").To("a")}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Drop()}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V()}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.E()}}, 
     "g_V_outE_drop": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.AddV().As("a").AddV().As("b").AddE("knows").To("a")}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().OutE().Drop()}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V()}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.E()}}, 
     "g_V_properties_drop": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.AddV().Property("name", "bob").AddV().Property("name", "alice")}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Properties().Drop()}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V()}, func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Properties()}}, 

gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js

@@ -172,6 +172,7 @@
     'g_VX1X_valuesXageX_dedupXlocalX_unfold': [(lambda g, vid1=None:g.V(vid1).values('age').dedup(Scope.local).unfold())], 
     'g_V_properties_dedup_count': [(lambda g:g.add_v('person').property('name', 'josh').add_v('person').property('name', 'josh').add_v('person').property('name', 'josh')), (lambda g:g.V().properties('name').dedup().count())], 
     'g_V_properties_dedup_byXvalueX_count': [(lambda g:g.add_v('person').property('name', 'josh').add_v('person').property('name', 'josh').add_v('person').property('name', 'josh')), (lambda g:g.V().properties('name').dedup().by(T.value).count())], 
+    'g_V_both_hasXlabel_softwareX_dedup_byXlangX_byXnameX_name': [(lambda g:g.V().both().has(T.label, 'software').dedup().by('lang').by('name').values('name'))], 
     'g_V_drop': [(lambda g:g.add_v().as_('a').add_v().as_('b').add_e('knows').to('a')), (lambda g:g.V().drop()), (lambda g:g.V()), (lambda g:g.E())], 
     'g_V_outE_drop': [(lambda g:g.add_v().as_('a').add_v().as_('b').add_e('knows').to('a')), (lambda g:g.V().out_e().drop()), (lambda g:g.V()), (lambda g:g.E())], 
     'g_V_properties_drop': [(lambda g:g.add_v().property('name', 'bob').add_v().property('name', 'alice')), (lambda g:g.V().properties().drop()), (lambda g:g.V()), (lambda g:g.V().properties())], 

gremlin-python/src/main/python/radish/gremlin.py

@@ -421,4 +421,13 @@ Feature: Step - dedup()
     When iterated to list
     Then the result should be unordered
       | result |
-      | d[1].l |
+      | d[1].l |
+
+  Scenario: g_V_both_hasXlabel_softwareX_dedup_byXlangX_byXnameX_name
+    Given the modern graph
+    And the traversal of
+      """
+      g.V().both().has(T.label, "software").dedup().by("lang").by("name").values("name")
+      """
+    When iterated to list
+    Then the traversal will raise an error with message containing text of "Dedup step can only have one by modulator"