You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by mi...@apache.org on 2023/04/21 15:13:54 UTC
[tinkerpop] 01/01: Fixing a couple mergeV/E issues.
This is an automated email from the ASF dual-hosted git repository.
mikepersonick pushed a commit to branch TINKERPOP-2931
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 1b37946b57f5e510b6d5be8dbef9b1a15221580b
Author: Mike Personick <mi...@supersonick.io>
AuthorDate: Fri Apr 21 09:13:05 2023 -0600
Fixing a couple mergeV/E issues.
Checking for illegal hidden keys.
Refactoring searchVertices to allow subclasses to override search criteria.
https://issues.apache.org/jira/browse/TINKERPOP-2931
---
.../process/traversal/step/map/MergeEdgeStep.java | 2 +-
.../process/traversal/step/map/MergeStep.java | 60 ++++++++++++++++++++--
.../traversal/step/map/MergeVertexStep.java | 40 ---------------
3 files changed, 57 insertions(+), 45 deletions(-)
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeStep.java
index 12d2639adf..dfd157f485 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeStep.java
@@ -388,7 +388,7 @@ public class MergeEdgeStep<S> extends MergeStep<S, Edge, Object> {
return (Vertex) o;
else if (o instanceof Map) {
final Map search = (Map) o;
- final Vertex v = IteratorUtils.findFirst(MergeVertexStep.searchVertices(getGraph(), search)).get();
+ final Vertex v = IteratorUtils.findFirst(searchVertices(search)).get();
return tryAttachVertex(v);
}
throw new IllegalArgumentException(
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeStep.java
index b1ca55732f..cfa53ca73c 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeStep.java
@@ -31,6 +31,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Step;
import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
import org.apache.tinkerpop.gremlin.process.traversal.TraverserGenerator;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.ConstantTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.lambda.IdentityTraversal;
import org.apache.tinkerpop.gremlin.process.traversal.step.Deleting;
@@ -45,6 +46,9 @@ import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil;
import org.apache.tinkerpop.gremlin.structure.Direction;
import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.tinkerpop.gremlin.structure.T;
+import org.apache.tinkerpop.gremlin.structure.Vertex;
+import org.apache.tinkerpop.gremlin.structure.util.CloseableIterator;
+import org.apache.tinkerpop.gremlin.structure.util.ElementHelper;
import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
/**
@@ -248,6 +252,8 @@ public abstract class MergeStep<S, E, C> extends FlatMapStep<S, E>
if (ignoreTokens) {
if (!(k instanceof String)) {
throw new IllegalArgumentException(String.format("option(onMatch) expects keys in Map to be of String - check: %s", k));
+ } else {
+ ElementHelper.validateProperty((String) k, v);
}
} else {
if (!(k instanceof String) && !allowedTokens.contains(k)) {
@@ -255,10 +261,14 @@ public abstract class MergeStep<S, E, C> extends FlatMapStep<S, E>
"%s() and option(onCreate) args expect keys in Map to be either String or %s - check: %s",
op, allowedTokens, k));
}
- if (k == T.label && !(v instanceof String)) {
- throw new IllegalArgumentException(String.format("%s() and option(onCreate) args expect T.label value to be of String - found: %s",
- op, v.getClass().getSimpleName()));
-
+ if (k == T.label) {
+ if (!(v instanceof String)) {
+ throw new IllegalArgumentException(String.format(
+ "%s() and option(onCreate) args expect T.label value to be of String - found: %s", op,
+ v.getClass().getSimpleName()));
+ } else {
+ ElementHelper.validateLabel((String) v);
+ }
}
if (k == Direction.OUT && v instanceof Merge && v != Merge.outV) {
throw new IllegalArgumentException(String.format("Only Merge.outV token may be used for Direction.OUT, found: %s", v));
@@ -266,6 +276,9 @@ public abstract class MergeStep<S, E, C> extends FlatMapStep<S, E>
if (k == Direction.IN && v instanceof Merge && v != Merge.inV) {
throw new IllegalArgumentException(String.format("Only Merge.inV token may be used for Direction.IN, found: %s", v));
}
+ if (k instanceof String) {
+ ElementHelper.validateProperty((String) k, v);
+ }
}
});
}
@@ -292,6 +305,45 @@ public abstract class MergeStep<S, E, C> extends FlatMapStep<S, E>
return map == null ? new LinkedHashMap() : map;
}
+ /**
+ * Translate the Map into a g.V() traversal against the supplied graph. Graph providers will presumably optimize
+ * this traversal to use whatever indices are present and appropriate for efficiency.
+ *
+ * Callers are responsible for closing this iterator when finished.
+ */
+ protected CloseableIterator<Vertex> searchVertices(final Map search) {
+ if (search == null)
+ return CloseableIterator.empty();
+
+ final Graph graph = getGraph();
+ final Object id = search.get(T.id);
+ final String label = (String) search.get(T.label);
+
+ GraphTraversal t = searchVerticesTraversal(graph, id);
+ t = searchVerticesLabelConstraint(t, label);
+ t = searchVerticesPropertyConstraints(t, search);
+
+ // this should auto-close the underlying traversal
+ return CloseableIterator.of(t);
+ }
+
+ protected GraphTraversal searchVerticesTraversal(final Graph graph, final Object id) {
+ return id != null ? graph.traversal().V(id) : graph.traversal().V();
+ }
+
+ protected GraphTraversal searchVerticesLabelConstraint(GraphTraversal t, final String label) {
+ return label != null ? t.hasLabel(label) : t;
+ }
+
+ protected GraphTraversal searchVerticesPropertyConstraints(GraphTraversal t, final Map search) {
+ for (final Map.Entry e : ((Map<?,?>) search).entrySet()) {
+ final Object k = e.getKey();
+ if (!(k instanceof String)) continue;
+ t = t.has((String) k, e.getValue());
+ }
+ return t;
+ }
+
@Override
protected abstract Iterator<E> flatMap(final Traverser.Admin<S> traverser);
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeVertexStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeVertexStep.java
index 90177b6f21..c4217786a3 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeVertexStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeVertexStep.java
@@ -73,46 +73,6 @@ public class MergeVertexStep<S> extends MergeStep<S, Vertex, Map> {
return allowedTokens;
}
- /**
- * Translate the Map into a g.V() traversal against the supplied graph. Graph providers will presumably optimize
- * this traversal to use whatever indices are present and appropriate for efficiency.
- *
- * Callers are responsible for closing this iterator when finished.
- */
- public static CloseableIterator<Vertex> searchVertices(final Graph graph, final Map search) {
- if (search == null)
- return CloseableIterator.empty();
-
- final Object id = search.get(T.id);
- final String label = (String) search.get(T.label);
-
- GraphTraversal t = id != null ? graph.traversal().V(id) : graph.traversal().V();
-
- if (label != null)
- t = t.hasLabel(label);
-
- // add property constraints
- for (final Map.Entry e : ((Map<?,?>) search).entrySet()) {
- final Object k = e.getKey();
- if (!(k instanceof String)) continue;
- t = t.has((String) k, e.getValue());
- }
-
- // this should auto-close the underlying traversal
- return CloseableIterator.of(t);
- }
-
- /**
- * Translate the Map into search criteria. Default implementation is to translate the Map into a g.V() traversal.
- * Graph providers will presumably optimize this traversal to use whatever indices are present and appropriate for
- * efficiency.
- *
- * Callers are responsible for closing this iterator when finished.
- */
- protected CloseableIterator<Vertex> searchVertices(final Map search) {
- return searchVertices(getGraph(), search);
- }
-
@Override
protected Iterator<Vertex> flatMap(final Traverser.Admin<S> traverser) {
final Graph graph = getGraph();