This is an automated email from the ASF dual-hosted git repository.
colegreer pushed a commit to branch 3.8-dev
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/3.8-dev by this push:
new 007fd228a9 [TINKERPOP-3149] Prevent multiple by modulators for sack
step and changed GroupCount.feature test to verify the error message received.
(#3102)
007fd228a9 is described below
commit 007fd228a992f1dccba22e25a88e647168e5ba4f
Author: andreachild <[email protected]>
AuthorDate: Mon May 5 09:58:23 2025 -0700
[TINKERPOP-3149] Prevent multiple by modulators for sack step and changed
GroupCount.feature test to verify the error message received. (#3102)
---
CHANGELOG.asciidoc | 2 +-
.../process/traversal/step/sideEffect/SackValueStep.java | 3 +++
.../process/traversal/step/sideEffect/SackValueStepTest.java | 6 ++++++
.../test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs | 1 +
gremlin-go/driver/cucumber/gremlin.go | 1 +
.../javascript/gremlin-javascript/test/cucumber/gremlin.js | 1 +
gremlin-python/src/main/python/radish/gremlin.py | 1 +
.../gremlin/test/features/sideEffect/GroupCount.feature | 2 +-
.../tinkerpop/gremlin/test/features/sideEffect/Sack.feature | 11 ++++++++++-
9 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index aead724472..6219bf76bf 100644
--- a/CHANGELOG.asciidoc
+++ b/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()` to prevent an invalid usage of multiple
`by()` modulators.
+* Added validation to `groupCount()` and `sack()` 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.
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/SackValueStep.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/SackValueStep.java
index b19f099bfe..f4f4383eb9 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/SackValueStep.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/SackValueStep.java
@@ -51,6 +51,9 @@ public final class SackValueStep<S, A, B> extends
AbstractStep<S, S> implements
@Override
public void modulateBy(final Traversal.Admin<?, ?> sackTraversal) {
+ if (this.sackTraversal != null) {
+ throw new IllegalStateException("Sack step can only have one by
modulator");
+ }
this.sackTraversal = this.integrateChild(sackTraversal);
}
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/SackValueStepTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/SackValueStepTest.java
index 0f6731bba9..51cb9048b0 100644
---
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/SackValueStepTest.java
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/SackValueStepTest.java
@@ -27,6 +27,7 @@ import org.apache.tinkerpop.gremlin.structure.Vertex;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
+import org.junit.Test;
/**
* @author Daniel Kuppitz (http://gremlin.guru)
@@ -49,4 +50,9 @@ public class SackValueStepTest extends StepTest {
})
);
}
+
+ @Test(expected = IllegalStateException.class)
+ public void shouldThrowForMultipleByModulators() {
+ __.sack(Operator.assign).by("age").by("name");
+ }
}
diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs
b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs
index 774cd7043c..ea2ce560b8 100644
--- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs
+++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/Gremlin.cs
@@ -1640,6 +1640,7 @@ namespace Gremlin.Net.IntegrationTest.Gherkin
{"g_V_sackXassignX_byXageX_sack", new
List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>>
{(g,p) =>g.V().Sack(Operator.Assign).By("age").Sack<object>()}},
{"g_withSackXBigInteger_TEN_powX1000X_assignX_V_localXoutXknowsX_barrierXnormSackXX_inXknowsX_barrier_sack",
new List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>>
{(g,p) =>g.WithSack(p["xx1"],
Operator.Assign).V().Local<object>(__.Out("knows").Barrier(Barrier.NormSack)).In("knows").Barrier().Sack<object>()}},
{"g_withSackX2X_V_sackXdivX_byXconstantX4_0XX_sack", new
List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>>
{(g,p)
=>g.WithSack(2).V().Sack(Operator.Div).By(__.Constant<object>(p["xx1"])).Sack<object>()}},
+ {"g_V_sackXassignX_byXageX_byXnameX_sack", new
List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>>
{(g,p) =>g.V().Sack(Operator.Assign).By("age").By("name").Sack<object>()}},
{"g_V_sideEffectXidentityX", new
List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>>
{(g,p) =>g.V().SideEffect(__.Identity())}},
{"g_V_sideEffectXidentity_valuesXnameXX", new
List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>>
{(g,p) =>g.V().SideEffect(__.Identity().Values<object>("name"))}},
{"g_V_sideEffectXpropertyXage_22X", new
List<Func<GraphTraversalSource, IDictionary<string, object>, ITraversal>>
{(g,p) =>g.AddV("person").Property("age", 21), (g,p)
=>g.V().SideEffect(__.Property("age", 22)), (g,p) =>g.V().Has("age", 21), (g,p)
=>g.V().Has("age", 22)}},
diff --git a/gremlin-go/driver/cucumber/gremlin.go
b/gremlin-go/driver/cucumber/gremlin.go
index 17ec0da51c..5e5564dc6d 100644
--- a/gremlin-go/driver/cucumber/gremlin.go
+++ b/gremlin-go/driver/cucumber/gremlin.go
@@ -1609,6 +1609,7 @@ var translationMap = map[string][]func(g
*gremlingo.GraphTraversalSource, p map[
"g_V_sackXassignX_byXageX_sack": {func(g *gremlingo.GraphTraversalSource,
p map[string]interface{}) *gremlingo.GraphTraversal {return
g.V().Sack(gremlingo.Operator.Assign).By("age").Sack()}},
"g_withSackXBigInteger_TEN_powX1000X_assignX_V_localXoutXknowsX_barrierXnormSackXX_inXknowsX_barrier_sack":
{func(g *gremlingo.GraphTraversalSource, p map[string]interface{})
*gremlingo.GraphTraversal {return g.WithSack(p["xx1"],
gremlingo.Operator.Assign).V().Local(gremlingo.T__.Out("knows").Barrier(gremlingo.Barrier.NormSack)).In("knows").Barrier().Sack()}},
"g_withSackX2X_V_sackXdivX_byXconstantX4_0XX_sack": {func(g
*gremlingo.GraphTraversalSource, p map[string]interface{})
*gremlingo.GraphTraversal {return
g.WithSack(2).V().Sack(gremlingo.Operator.Div).By(gremlingo.T__.Constant(p["xx1"])).Sack()}},
+ "g_V_sackXassignX_byXageX_byXnameX_sack": {func(g
*gremlingo.GraphTraversalSource, p map[string]interface{})
*gremlingo.GraphTraversal {return
g.V().Sack(gremlingo.Operator.Assign).By("age").By("name").Sack()}},
"g_V_sideEffectXidentityX": {func(g *gremlingo.GraphTraversalSource, p
map[string]interface{}) *gremlingo.GraphTraversal {return
g.V().SideEffect(gremlingo.T__.Identity())}},
"g_V_sideEffectXidentity_valuesXnameXX": {func(g
*gremlingo.GraphTraversalSource, p map[string]interface{})
*gremlingo.GraphTraversal {return
g.V().SideEffect(gremlingo.T__.Identity().Values("name"))}},
"g_V_sideEffectXpropertyXage_22X": {func(g
*gremlingo.GraphTraversalSource, p map[string]interface{})
*gremlingo.GraphTraversal {return g.AddV("person").Property("age", 21)}, func(g
*gremlingo.GraphTraversalSource, p map[string]interface{})
*gremlingo.GraphTraversal {return
g.V().SideEffect(gremlingo.T__.Property("age", 22))}, func(g
*gremlingo.GraphTraversalSource, p map[string]interface{})
*gremlingo.GraphTraversal {return g.V().Has("age", 21)}, func(g
*gremlingo.GraphTraversalSour [...]
diff --git
a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js
b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js
index c261f4f92d..074fb918ab 100644
---
a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js
+++
b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/gremlin.js
@@ -1639,6 +1639,7 @@ const gremlins = {
g_V_sackXassignX_byXageX_sack: [function({g}) { return
g.V().sack(Operator.assign).by("age").sack() }],
g_withSackXBigInteger_TEN_powX1000X_assignX_V_localXoutXknowsX_barrierXnormSackXX_inXknowsX_barrier_sack:
[function({g, xx1}) { return g.withSack(xx1,
Operator.assign).V().local(__.out("knows").barrier(Barrier.normSack)).in_("knows").barrier().sack()
}],
g_withSackX2X_V_sackXdivX_byXconstantX4_0XX_sack: [function({g, xx1}) {
return g.withSack(2).V().sack(Operator.div).by(__.constant(xx1)).sack() }],
+ g_V_sackXassignX_byXageX_byXnameX_sack: [function({g}) { return
g.V().sack(Operator.assign).by("age").by("name").sack() }],
g_V_sideEffectXidentityX: [function({g}) { return
g.V().sideEffect(__.identity()) }],
g_V_sideEffectXidentity_valuesXnameXX: [function({g}) { return
g.V().sideEffect(__.identity().values("name")) }],
g_V_sideEffectXpropertyXage_22X: [function({g}) { return
g.addV("person").property("age", 21) }, function({g}) { return
g.V().sideEffect(__.property("age", 22)) }, function({g}) { return
g.V().has("age", 21) }, function({g}) { return g.V().has("age", 22) }],
diff --git a/gremlin-python/src/main/python/radish/gremlin.py
b/gremlin-python/src/main/python/radish/gremlin.py
index 4985d650bc..e01425c3ae 100644
--- a/gremlin-python/src/main/python/radish/gremlin.py
+++ b/gremlin-python/src/main/python/radish/gremlin.py
@@ -1612,6 +1612,7 @@ world.gremlins = {
'g_V_sackXassignX_byXageX_sack': [(lambda
g:g.V().sack(Operator.assign).by('age').sack())],
'g_withSackXBigInteger_TEN_powX1000X_assignX_V_localXoutXknowsX_barrierXnormSackXX_inXknowsX_barrier_sack':
[(lambda g, xx1=None:g.with_sack(xx1,
Operator.assign).V().local(__.out('knows').barrier(Barrier.norm_sack)).in_('knows').barrier().sack())],
'g_withSackX2X_V_sackXdivX_byXconstantX4_0XX_sack': [(lambda g,
xx1=None:g.with_sack(2).V().sack(Operator.div).by(__.constant(xx1)).sack())],
+ 'g_V_sackXassignX_byXageX_byXnameX_sack': [(lambda
g:g.V().sack(Operator.assign).by('age').by('name').sack())],
'g_V_sideEffectXidentityX': [(lambda g:g.V().side_effect(__.identity()))],
'g_V_sideEffectXidentity_valuesXnameXX': [(lambda
g:g.V().side_effect(__.identity().values('name')))],
'g_V_sideEffectXpropertyXage_22X': [(lambda
g:g.add_v('person').property('age', 21)), (lambda
g:g.V().side_effect(__.property('age', 22))), (lambda g:g.V().has('age', 21)),
(lambda g:g.V().has('age', 22))],
diff --git
a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/GroupCount.feature
b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/GroupCount.feature
index b8533e4037..861ce5fbc4 100644
---
a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/GroupCount.feature
+++
b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/GroupCount.feature
@@ -243,4 +243,4 @@ Feature: Step - groupCount()
g.V().out("created").groupCount("x").by("name").by("age")
"""
When iterated to list
- Then the traversal will raise an error
\ No newline at end of file
+ Then the traversal will raise an error with message containing text of
"GroupCount step can only have one by modulator"
\ No newline at end of file
diff --git
a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/Sack.feature
b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/Sack.feature
index 315b8f9cca..14a099e867 100644
---
a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/Sack.feature
+++
b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/sideEffect/Sack.feature
@@ -140,4 +140,13 @@ Feature: Step - sack()
| d[0.5].d |
| d[0.5].d |
| d[0.5].d |
- | d[0.5].d |
\ No newline at end of file
+ | d[0.5].d |
+
+ Scenario: g_V_sackXassignX_byXageX_byXnameX_sack
+ Given the modern graph
+ And the traversal of
+ """
+ g.V().sack(assign).by("age").by("name").sack()
+ """
+ When iterated to list
+ Then the traversal will raise an error with message containing text of
"Sack step can only have one by modulator"
\ No newline at end of file
