You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2024/10/16 14:41:29 UTC
(tinkerpop) branch master updated: TINKERPOP-3102 Prevent cardinality with any step besides mergeV
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/master by this push:
new 4869c95b13 TINKERPOP-3102 Prevent cardinality with any step besides mergeV
new 23247e8421 Merge branch '3.7-dev'
4869c95b13 is described below
commit 4869c95b13cbf64b9790a9ae3a1d07bb74d6ad8e
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Wed Oct 16 10:39:37 2024 -0400
TINKERPOP-3102 Prevent cardinality with any step besides mergeV
Folks were trying to use option() and Cardinality with mergeE which doesn't make sense. Now they get a clear error if they attempt that. CTR
---
CHANGELOG.asciidoc | 1 +
.../traversal/dsl/graph/GraphTraversal.java | 26 ++++++++++++++++++----
.../traversal/dsl/graph/GraphTraversalTest.java | 19 ++++++++++++++++
3 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 446a2ad893..4fc330c86b 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -30,6 +30,7 @@ This release also includes changes from <<release-3-6-8, 3.6.8>>.
* Deprecated public constructor for `SeedStrategy` in favor of builder pattern to be consistent with other strategies.
* Allowed specification of a customized Spark app name.
* Added getter method to `CoinStep` for its probability field.
+* Prevented specification of `Cardinality` to `option()` when not used in conjunction with `mergeV()`.
* Exposed a mechanism for providers to customize the assertion of error messages in feature tests.
* Attempted to detect JDK version for Gremlin Console to avoid problems with Java 17 if `neo4j-gremlin` is used.
* Fixed so that `TrimGlobalStep` and `TrimLocalStep` have the same character control handling as `Ltrim` and `Rtrim`
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
index f01afa8197..1d4bc0c663 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
@@ -3903,15 +3903,26 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> {
* @since 3.6.0
*/
public default <M, E2> GraphTraversal<S, E> option(final M token, final Map<Object, Object> m) {
+ final Step<?, ?> lastStep = this.asAdmin().getEndStep();
+
+ // CardinalityValueTraversal doesn't make sense for any prior step other than mergeV()
+ if (!(lastStep instanceof MergeVertexStep) && m != null) {
+ for (Object k : m.keySet()) {
+ final Object o = m.get(k);
+ if (o instanceof CardinalityValueTraversal)
+ throw new IllegalStateException("option() with the Cardinality argument can only be used following mergeV()");
+ }
+ }
+
this.asAdmin().getBytecode().addStep(Symbols.option, token, m);
((TraversalOptionParent<M, E, E2>) this.asAdmin().getEndStep()).addChildOption(token, (Traversal.Admin<E, E2>) new ConstantTraversal<>(m).asAdmin());
return this;
}
/**
- * This is a step modulator to a {@link TraversalOptionParent} like {@code choose()} or {@code mergeV()} where the
- * provided argument associated to the {@code token} is applied according to the semantics of the step. Please see
- * the documentation of such steps to understand the usage context.
+ * This is a step modulator to {@link #mergeV()} where the provided argument associated to the {@code token} is
+ * applied according to the semantics of the step. Please see the documentation of such steps to understand the
+ * usage context.
*
* @param m Provides a {@code Map} as the option which is the same as doing {@code constant(m)}.
* @return the traversal with the modulated step
@@ -3920,6 +3931,13 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> {
* @since 3.7.0
*/
public default <M, E2> GraphTraversal<S, E> option(final Merge merge, final Map<Object, Object> m, final VertexProperty.Cardinality cardinality) {
+ final Step<?, ?> lastStep = this.asAdmin().getEndStep();
+
+ // CardinalityValueTraversal doesn't make sense for any prior step other than mergeV()
+ if (!(lastStep instanceof MergeVertexStep)) {
+ throw new IllegalStateException("option() with the Cardinality argument can only be used following mergeV()");
+ }
+
this.asAdmin().getBytecode().addStep(Symbols.option, merge, m, cardinality);
// do explicit cardinality for every single pair in the map
for (Object k : m.keySet()) {
@@ -3927,7 +3945,7 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> {
if (!(o instanceof CardinalityValueTraversal))
m.put(k, new CardinalityValueTraversal(cardinality, o));
}
- ((TraversalOptionParent<M, E, E2>) this.asAdmin().getEndStep()).addChildOption((M) merge, (Traversal.Admin<E, E2>) new ConstantTraversal<>(m).asAdmin());
+ ((TraversalOptionParent<M, E, E2>) lastStep).addChildOption((M) merge, (Traversal.Admin<E, E2>) new ConstantTraversal<>(m).asAdmin());
return this;
}
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalTest.java
index 28936eaa98..32c81f4fc8 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalTest.java
@@ -19,6 +19,7 @@
package org.apache.tinkerpop.gremlin.process.traversal.dsl.graph;
import org.apache.tinkerpop.gremlin.process.traversal.Bytecode;
+import org.apache.tinkerpop.gremlin.process.traversal.Merge;
import org.apache.tinkerpop.gremlin.process.traversal.P;
import org.apache.tinkerpop.gremlin.process.traversal.Scope;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
@@ -33,8 +34,10 @@ import org.slf4j.LoggerFactory;
import java.lang.reflect.Method;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
+import java.util.Map;
import java.util.Random;
import java.util.Set;
import java.util.function.Consumer;
@@ -64,6 +67,22 @@ public class GraphTraversalTest {
g.addV().property(T.label, null);
}
+ @Test(expected = IllegalStateException.class)
+ public void shouldFailWhenUsingOptionAndCardinalityArgumentWithoutMergeV() {
+ final Map<Object, Object> m = new HashMap<Object,Object>() {{
+ put("k", 1);
+ }};
+ g.mergeE(new HashMap<>()).option(Merge.onMatch, m, VertexProperty.Cardinality.list);
+ }
+
+ @Test(expected = IllegalStateException.class)
+ public void shouldFailWhenUsingOptionAndCardinalityFunctionWithoutMergeV() {
+ final Map<Object, Object> m = new HashMap<Object,Object>() {{
+ put("k", VertexProperty.Cardinality.list(1));
+ }};
+ g.mergeE(new HashMap<>()).option(Merge.onMatch, m);
+ }
+
@Test(expected = IllegalArgumentException.class)
public void shouldFailPropertyWithCardinalityNullVertexId() {
g.addV().property(VertexProperty.Cardinality.single, T.id, null);