Merge branch '3.8-dev'

Author: xiazcy

CHANGELOG.asciidoc

@@ -135,8 +135,7 @@ This release also includes changes from <<release-3-7-XXX, 3.7.XXX>>.
 * Added and made `OffsetDateTime` serializers the default for existing date types in Python, Go, JavaScript, and .NET. `Date` is only used to deserialize from server.
 * 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()`, `sack()` and `dedup()` to prevent an invalid usage of multiple `by()` modulators.
+* Added validation to `valueMap()`, `propertyMap()`, `groupCount()`, `sack()`, `dedup()`, `sample()`, and `aggregate()` to prevent the 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.

docs/src/upgrade/release-3.8.x.asciidoc

@@ -303,13 +303,16 @@ The previous behavior attempted to round-robin the by() but this wasn't possible
 
 See: link:https://issues.apache.org/jira/browse/TINKERPOP-2974[TINKERPOP-2974]
 
-==== groupCount() By Modulation Semantics
+==== groupCount(), dedup(), sack(), sample(), aggregate() By Modulation Semantics
 
-The `groupCount()` step has been changed to throw an error if multiple `by()` modulators are applied. The previous 
-behavior would ignore previous `by()` modulators and apply the last one, which was not intuitive.
+The `groupCount()`, `dedup()`, `sack()`, `sample()`, and `aggregate()` steps has been changed to throw an error if
+multiple `by()` modulators are applied. The previous behavior would ignore previous `by()` modulators and apply the
+last one, which was not intuitive.
 
 See: link:https://issues.apache.org/jira/browse/TINKERPOP-3121[TINKERPOP-3121]
 
+==== Additional By Modulation Semantics
+
 === Upgrading for Providers
 
 ==== Graph System Providers

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

@@ -43,7 +43,8 @@
  */
 public final class SampleGlobalStep<S> extends CollectingBarrierStep<S> implements TraversalParent, ByModulating, Seedable {
 
-    private Traversal.Admin<S, Number> probabilityTraversal = new ConstantTraversal<>(1.0d);
+    private boolean detectedMutipleBy = false;
+    private Traversal.Admin<S, Number> probabilityTraversal = new ConstantTraversal<>(1.0d);;
     private final int amountToSample;
     private final Random random = new Random();
 
@@ -68,13 +69,18 @@ public List<Traversal.Admin<S, Number>> getLocalChildren() {
 
     @Override
     public void modulateBy(final Traversal.Admin<?, ?> probabilityTraversal) {
+        if (this.detectedMutipleBy)
+            throw new IllegalStateException("Sample step can only have one by modulator");
+        this.detectedMutipleBy = true;
         this.probabilityTraversal = this.integrateChild(probabilityTraversal);
     }
 
     @Override
     public void replaceLocalChild(final Traversal.Admin<?, ?> oldTraversal, final Traversal.Admin<?, ?> newTraversal) {
-        if (null != this.probabilityTraversal && this.probabilityTraversal.equals(oldTraversal))
+        if (null != this.probabilityTraversal && this.probabilityTraversal.equals(oldTraversal)) {
+            this.detectedMutipleBy = true;
             this.probabilityTraversal = this.integrateChild(newTraversal);
+        }
     }
 
     @Override
@@ -155,6 +161,7 @@ public Set<TraverserRequirement> getRequirements() {
     public SampleGlobalStep<S> clone() {
         final SampleGlobalStep<S> clone = (SampleGlobalStep<S>) super.clone();
         clone.probabilityTraversal = this.probabilityTraversal.clone();
+        clone.detectedMutipleBy = this.detectedMutipleBy;
         return clone;
     }
 

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AggregateGlobalStep.java

@@ -69,6 +69,8 @@ public String toString() {
 
     @Override
     public void modulateBy(final Traversal.Admin<?, ?> aggregateTraversal) {
+        if (this.aggregateTraversal != null)
+            throw new IllegalStateException("Aggregate step can only have one by modulator");
         this.aggregateTraversal = this.integrateChild(aggregateTraversal);
     }
 

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AggregateLocalStep.java

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

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

@@ -22,6 +22,7 @@
 import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__;
 import org.apache.tinkerpop.gremlin.process.traversal.step.StepTest;
+import org.apache.tinkerpop.gremlin.structure.T;
 import org.junit.Test;
 
 import java.util.ArrayList;
@@ -59,4 +60,9 @@ public void shouldSelectSubsetsCorrectly() {
             assertTrue(list.containsAll(result));
         }
     }
+
+    @Test(expected = IllegalStateException.class)
+    public void shouldThrowForMultipleByModulators() {
+        __.V().sample(1).by("age").by(T.id);
+    }
 }

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

@@ -365,8 +365,11 @@ private static IDictionary<string, List<Func<GraphTraversalSource, IDictionary<s
                {"g_V_hasLabelXpersonX_order_byXageX_valuesXnameX_skipX1X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().HasLabel("person").Order().By("age").Values<object>("name").Skip<object>(1)}},
                {"g_VX1X_valuesXageX_rangeXlocal_20_30X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V(p["vid1"]).Values<object>("age").Range<object>(Scope.Local, 20, 30)}}, 
                {"g_V_mapXin_hasIdX1XX_limitX2X_valuesXnameX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Map<object>(__.In().HasId(p["vid1"])).Limit<object>(2).Values<object>("name")}}, 
+               {"g_V_sampleX1X_byXageX_byXT_idX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Sample(1).By("age").By(T.Id)}}, 
                {"g_V_rangeX2_1X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Range<object>(2, 1)}}, 
                {"g_V_rangeX3_2X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Range<object>(3, 2)}}, 
+               {"g_V_aggregateXxX_byXnameX_byXageX_capXxX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Aggregate("x").By("name").By("age").Cap<object>("x")}}, 
+               {"g_V_aggregateXScope_local_xX_byXnameX_byXageX_capXxX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Aggregate(Scope.Local, "x").By("name").By("age").Cap<object>("x")}}, 
                {"g_E_sampleX1X", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.E().Sample(1)}}, 
                {"g_E_sampleX2X_byXweightX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.E().Sample(2).By("weight")}}, 
                {"g_V_localXoutE_sampleX1X_byXweightXX", new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>> {(g,p) =>g.V().Local<object>(__.OutE().Sample(1).By("weight"))}}, 

gremlin-go/driver/cucumber/gremlin.go

@@ -339,8 +339,11 @@ var translationMap = map[string][]func(g *gremlingo.GraphTraversalSource, p map[
     "g_V_hasLabelXpersonX_order_byXageX_valuesXnameX_skipX1X": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().HasLabel("person").Order().By("age").Values("name").Skip(1)}},
     "g_VX1X_valuesXageX_rangeXlocal_20_30X": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V(p["vid1"]).Values("age").Range(gremlingo.Scope.Local, 20, 30)}}, 
     "g_V_mapXin_hasIdX1XX_limitX2X_valuesXnameX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Map(gremlingo.T__.In().HasId(p["vid1"])).Limit(2).Values("name")}}, 
+    "g_V_sampleX1X_byXageX_byXT_idX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Sample(1).By("age").By(gremlingo.T.Id)}}, 
     "g_V_rangeX2_1X": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Range(2, 1)}}, 
     "g_V_rangeX3_2X": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Range(3, 2)}}, 
+    "g_V_aggregateXxX_byXnameX_byXageX_capXxX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Aggregate("x").By("name").By("age").Cap("x")}}, 
+    "g_V_aggregateXScope_local_xX_byXnameX_byXageX_capXxX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Aggregate(gremlingo.Scope.Local, "x").By("name").By("age").Cap("x")}}, 
     "g_E_sampleX1X": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.E().Sample(1)}}, 
     "g_E_sampleX2X_byXweightX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.E().Sample(2).By("weight")}}, 
     "g_V_localXoutE_sampleX1X_byXweightXX": {func(g *gremlingo.GraphTraversalSource, p map[string]interface{}) *gremlingo.GraphTraversal {return g.V().Local(gremlingo.T__.OutE().Sample(1).By("weight"))}}, 

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

@@ -350,8 +350,11 @@
     'g_V_hasLabelXpersonX_order_byXageX_valuesXnameX_skipX1X': [(lambda g:g.V().has_label('person').order().by('age').values('name').skip(1))], 
     'g_VX1X_valuesXageX_rangeXlocal_20_30X': [(lambda g, vid1=None:g.V(vid1).values('age').range_(Scope.local, 20, 30))], 
     'g_V_mapXin_hasIdX1XX_limitX2X_valuesXnameX': [(lambda g, vid1=None:g.V().map(__.in_().has_id(vid1)).limit(2).values('name'))], 
+    'g_V_sampleX1X_byXageX_byXT_idX': [(lambda g:g.V().sample(1).by('age').by(T.id_))], 
     'g_V_rangeX2_1X': [(lambda g:g.V().range_(2, 1))], 
     'g_V_rangeX3_2X': [(lambda g:g.V().range_(3, 2))], 
+    'g_V_aggregateXxX_byXnameX_byXageX_capXxX': [(lambda g:g.V().aggregate('x').by('name').by('age').cap('x'))], 
+    'g_V_aggregateXScope_local_xX_byXnameX_byXageX_capXxX': [(lambda g:g.V().aggregate(Scope.local, 'x').by('name').by('age').cap('x'))], 
     'g_E_sampleX1X': [(lambda g:g.E().sample(1))], 
     'g_E_sampleX2X_byXweightX': [(lambda g:g.E().sample(2).by('weight'))], 
     'g_V_localXoutE_sampleX1X_byXweightXX': [(lambda g:g.V().local(__.out_e().sample(1).by('weight')))],