This is an automated email from the ASF dual-hosted git repository. mikepersonick pushed a commit to branch TINKERPOP-2850 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit a17b40547ee163d5615c6f93ff194a7d16139f25 Author: Mike Personick <[email protected]> AuthorDate: Mon Jan 2 09:33:42 2023 -0700 Initial commit --- .../language/grammar/GenericLiteralVisitor.java | 9 + .../tinkerpop/gremlin/process/traversal/Merge.java | 4 +- .../traversal/dsl/graph/GraphTraversal.java | 2 - .../traversal/dsl/graph/GraphTraversalSource.java | 2 - .../process/traversal/step/map/MergeEdgeStep.java | 625 +++++++++------------ .../process/traversal/step/map/MergeStep.java | 299 ++++++++++ .../traversal/step/map/MergeVertexStep.java | 383 ++++--------- .../gremlin/util/iterator/IteratorUtils.java | 163 ++++-- .../Gherkin/CommonSteps.cs | 6 + .../main/javascript/gremlin-javascript/index.js | 1 + .../test/cucumber/feature-steps.js | 8 +- gremlin-language/src/main/antlr4/Gremlin.g4 | 3 + .../src/main/python/radish/feature_steps.py | 4 +- .../tinkerpop/gremlin/features/StepDefinition.java | 28 +- .../process/ProcessLimitedStandardSuite.java | 53 +- .../process/traversal/step/map/MergeEdgeTest.java | 54 ++ .../gremlin/test/features/map/MergeEdge.feature | 274 +++++---- .../gremlin/test/features/map/MergeVertex.feature | 101 +++- .../traversal/step/map/TinkerMergeEdgeStep.java | 115 ---- .../traversal/step/map/TinkerMergeVertexStep.java | 90 --- .../optimization/TinkerMergeEVStepStrategy.java | 61 -- .../gremlin/tinkergraph/structure/TinkerGraph.java | 4 +- .../tinkergraph/TinkerGraphFeatureTest.java | 4 +- 23 files changed, 1205 insertions(+), 1088 deletions(-) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/grammar/GenericLiteralVisitor.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/grammar/GenericLiteralVisitor.java index b3ae657bab..78d144616f 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/grammar/GenericLiteralVisitor.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/grammar/GenericLiteralVisitor.java @@ -21,6 +21,7 @@ package org.apache.tinkerpop.gremlin.language.grammar; import org.antlr.v4.runtime.tree.ParseTree; import org.antlr.v4.runtime.tree.TerminalNode; import org.apache.commons.text.StringEscapeUtils; +import org.apache.tinkerpop.gremlin.process.traversal.Merge; import org.apache.tinkerpop.gremlin.process.traversal.Pick; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; @@ -518,6 +519,14 @@ public class GenericLiteralVisitor extends DefaultGremlinBaseVisitor<Object> { return TraversalEnumParser.parseTraversalDirectionFromContext(ctx); } + /** + * {@inheritDoc} + */ + @Override + public Object visitTraversalMerge(final GremlinParser.TraversalMergeContext ctx) { + return TraversalEnumParser.parseTraversalEnumFromContext(Merge.class, ctx); + } + /** * {@inheritDoc} */ diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Merge.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Merge.java index 068dadd5cb..629217d74f 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Merge.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Merge.java @@ -42,5 +42,7 @@ public enum Merge { * * @since 3.6.0 */ - onMatch + onMatch, + + outV, inV } 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 6956b21bda..455709715c 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 @@ -1117,7 +1117,6 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { * @since 3.6.0 */ public default GraphTraversal<S, Vertex> mergeV(final Map<Object, Object> searchCreate) { - MergeVertexStep.validateMapInput(searchCreate, false); this.asAdmin().getBytecode().addStep(Symbols.mergeV, searchCreate); final MergeVertexStep<S> step = new MergeVertexStep<>(this.asAdmin(), false, searchCreate); return this.asAdmin().addStep(step); @@ -1161,7 +1160,6 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { */ public default GraphTraversal<S, Edge> mergeE(final Map<Object, Object> searchCreate) { // get a construction time exception if the Map is bad - MergeEdgeStep.validateMapInput(searchCreate, false); this.asAdmin().getBytecode().addStep(Symbols.mergeE, searchCreate); final MergeEdgeStep<S> step = new MergeEdgeStep(this.asAdmin(), false, searchCreate); return this.asAdmin().addStep(step); diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java index 696cd0b796..c12bf41e46 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java @@ -376,7 +376,6 @@ public class GraphTraversalSource implements TraversalSource { * @since 3.6.0 */ public GraphTraversal<Vertex, Vertex> mergeV(final Map<Object, Object> searchCreate) { - MergeVertexStep.validateMapInput(searchCreate, false); final GraphTraversalSource clone = this.clone(); clone.bytecode.addStep(GraphTraversal.Symbols.mergeV, searchCreate); final GraphTraversal.Admin<Vertex, Vertex> traversal = new DefaultGraphTraversal<>(clone); @@ -412,7 +411,6 @@ public class GraphTraversalSource implements TraversalSource { * @since 3.6.0 */ public GraphTraversal<Edge, Edge> mergeE(final Map<?, Object> searchCreate) { - MergeEdgeStep.validateMapInput(searchCreate, false); final GraphTraversalSource clone = this.clone(); clone.bytecode.addStep(GraphTraversal.Symbols.mergeE, searchCreate); final GraphTraversal.Admin<Edge, Edge> traversal = new DefaultGraphTraversal<>(clone); 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 9e364b0a38..eadc4f5285 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 @@ -18,11 +18,26 @@ */ package org.apache.tinkerpop.gremlin.process.traversal.step.map; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; + +import org.apache.commons.collections.CollectionUtils; import org.apache.tinkerpop.gremlin.process.traversal.Merge; 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.dsl.graph.__; 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.Mutating; @@ -33,6 +48,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.Event; import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.ListCallbackRegistry; import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.EventStrategy; import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement; +import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper; import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil; import org.apache.tinkerpop.gremlin.structure.Direction; import org.apache.tinkerpop.gremlin.structure.Edge; @@ -43,138 +59,109 @@ import org.apache.tinkerpop.gremlin.structure.Vertex; import org.apache.tinkerpop.gremlin.structure.util.Attachable; import org.apache.tinkerpop.gremlin.structure.util.CloseableIterator; import org.apache.tinkerpop.gremlin.structure.util.StringFactory; -import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph; -import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceVertex; import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Objects; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Stream; - /** * Implementation for the {@code mergeE()} step covering both the start step version and the one used mid-traversal. */ -public class MergeEdgeStep<S> extends FlatMapStep<S, Edge> implements Mutating<Event>, - TraversalOptionParent<Merge, S, Edge> { - - public static final Vertex PLACEHOLDER_VERTEX = new ReferenceVertex(Graph.Hidden.hide(MergeEdgeStep.class.getName())); - private final boolean isStart; - private boolean first = true; - private Traversal.Admin<S,Map<Object, Object>> searchCreateTraversal; - private Traversal.Admin<S, Map<Object, Object>> onCreateTraversal = null; - private Traversal.Admin<S, Map<String, Object>> onMatchTraversal = null; +public class MergeEdgeStep<S> extends MergeStep<S, Edge, Object> { - protected CallbackRegistry<Event> callbackRegistry; + private static final Set allowedTokens = new LinkedHashSet(Arrays.asList(T.id, T.label, Direction.IN, Direction.OUT)); - public MergeEdgeStep(final Traversal.Admin traversal, final boolean isStart) { - this(traversal, isStart, new IdentityTraversal<>()); + public static void validateMapInput(final Map map, final boolean ignoreTokens) { + MergeStep.validate(map, ignoreTokens, allowedTokens); } - public MergeEdgeStep(final Traversal.Admin traversal, final boolean isStart, final Map<Object, Object> searchCreate) { - this(traversal, isStart, new ConstantTraversal<>(searchCreate)); + private Traversal.Admin<S, Object> outVTraversal = null; + private Traversal.Admin<S, Object> inVTraversal = null; + + public MergeEdgeStep(final Traversal.Admin traversal, final boolean isStart) { + super(traversal, isStart); } - public MergeEdgeStep(final Traversal.Admin traversal, final boolean isStart, final Traversal.Admin<?,Map<Object, Object>> searchCreateTraversal) { - super(traversal); - this.isStart = isStart; - this.searchCreateTraversal = integrateChild(searchCreateTraversal); + public MergeEdgeStep(final Traversal.Admin traversal, final boolean isStart, final Map merge) { + super(traversal, isStart, merge); } - /** - * Gets the traversal that will be used to provide the {@code Map} that will be used to search for edges. - * This {@code Map} also will be used as the default data set to be used to create a edge if the search is not - * successful. - */ - public Traversal.Admin<S, Map<Object, Object>> getSearchCreateTraversal() { - return searchCreateTraversal; + public MergeEdgeStep(final Traversal.Admin traversal, final boolean isStart, final Traversal.Admin<S,Map> mergeTraversal) { + super(traversal, isStart, mergeTraversal); } /** - * Gets the traversal that will be used to provide the {@code Map} that will be the override to the one provided - * by the {@link #getSearchCreateTraversal()} for edge creation events. + * Gets the traversal that will be used to provide the {@code Map} that will be used to identify the Direction.OUT + * vertex during merge. */ - public Traversal.Admin<S, Map<Object, Object>> getOnCreateTraversal() { - return onCreateTraversal; + public Traversal.Admin<S, Object> getOutVTraversal() { + return outVTraversal; } /** - * Gets the traversal that will be used to provide the {@code Map} that will be used to modify edges that - * match the search criteria of {@link #getSearchCreateTraversal()}. + * Gets the traversal that will be used to provide the {@code Map} that will be used to identify the Direction.IN + * vertex during merge. */ - public Traversal.Admin<S, Map<String, Object>> getOnMatchTraversal() { - return onMatchTraversal; + public Traversal.Admin<S, Object> getInVTraversal() { + return inVTraversal; } - /** - * Determines if this is a start step. - */ - public boolean isStart() { - return isStart; + @Override + public void addChildOption(final Merge token, final Traversal.Admin<S, Object> traversalOption) { + if (token == Merge.outV) { + this.outVTraversal = this.integrateChild(traversalOption); + } else if (token == Merge.inV) { + this.inVTraversal = this.integrateChild(traversalOption); + } else { + super.addChildOption(token, traversalOption); + } } - /** - * Determine if this is the first pass through {@link #processNextStart()}. - */ - public boolean isFirst() { - return first; + @Override + public <S, C> List<Traversal.Admin<S, C>> getLocalChildren() { + final List<Traversal.Admin<S, C>> children = super.getLocalChildren(); + if (outVTraversal != null) children.add((Traversal.Admin<S, C>) outVTraversal); + if (inVTraversal != null) children.add((Traversal.Admin<S, C>) inVTraversal); + return children; } - public CallbackRegistry<Event> getCallbackRegistry() { - return callbackRegistry; + @Override + public int hashCode() { + int result = super.hashCode(); + if (outVTraversal != null) + result ^= outVTraversal.hashCode(); + if (inVTraversal != null) + result ^= inVTraversal.hashCode(); + return result; } @Override - public void addChildOption(final Merge token, final Traversal.Admin<S, Edge> traversalOption) { - if (token == Merge.onCreate) { - this.onCreateTraversal = this.integrateChild(traversalOption); - } else if (token == Merge.onMatch) { - this.onMatchTraversal = this.integrateChild(traversalOption); - } else { - throw new UnsupportedOperationException(String.format("Option %s for Merge is not supported", token.name())); - } + public void reset() { + super.reset(); + if (outVTraversal != null) outVTraversal.reset(); + if (inVTraversal != null) inVTraversal.reset(); } @Override - public <S, E> List<Traversal.Admin<S, E>> getLocalChildren() { - final List<Traversal.Admin<S, E>> children = new ArrayList<>(); - if (searchCreateTraversal != null) children.add((Traversal.Admin<S, E>) searchCreateTraversal); - if (onMatchTraversal != null) children.add((Traversal.Admin<S, E>) onMatchTraversal); - if (onCreateTraversal != null) children.add((Traversal.Admin<S, E>) onCreateTraversal); - return children; + public String toString() { + return StringFactory.stepString(this, mergeTraversal, onCreateTraversal, onMatchTraversal, outVTraversal, inVTraversal); } @Override - public void configure(final Object... keyValues) { - // this is a Mutating step but property() should not be folded into this step. The main issue here is that - // this method won't know what step called it - property() or with() or something else so it can't make the - // choice easily to throw an exception, write the keys/values to parameters, etc. It really is up to the - // caller to make sure it is handled properly at this point. this may best be left as a do-nothing method for - // now. + public void setTraversal(final Traversal.Admin<?, ?> parentTraversal) { + super.setTraversal(parentTraversal); + this.integrateChild(outVTraversal); + this.integrateChild(inVTraversal); } @Override - public Parameters getParameters() { - // merge doesn't take fold ups of property() calls. those need to get treated as regular old PropertyStep - // instances. not sure if this should support with() though.....none of the other Mutating steps do. - return null; + public MergeEdgeStep<S> clone() { + final MergeEdgeStep<S> clone = (MergeEdgeStep<S>) super.clone(); + clone.outVTraversal = outVTraversal != null ? outVTraversal.clone() : null; + clone.inVTraversal = inVTraversal != null ? inVTraversal.clone() : null; + return clone; } @Override - protected Traverser.Admin<Edge> processNextStart() { - // when it's a start step a traverser needs to be created to kick off the traversal. - if (isStart && first) { - first = false; - final TraverserGenerator generator = this.getTraversal().getTraverserGenerator(); - this.addStart(generator.generate(PLACEHOLDER_VERTEX, (Step) this, 1L)); - } - return super.processNextStart(); + protected Set getAllowedTokens() { + return allowedTokens; } /** @@ -183,202 +170,240 @@ public class MergeEdgeStep<S> extends FlatMapStep<S, Edge> implements Mutating<E * matching the list of edges. This implementation is only optimized for the {@link T#id} so any other usage * will simply be in-memory filtering which could be slow. */ - protected Stream<Edge> createSearchStream(final Map<Object,Object> search) { - final Graph graph = this.getTraversal().getGraph().get(); - - final Optional<Direction> directionUsedInLookup; - Stream<Edge> stream; - // prioritize lookup by id, then use vertices as starting point if possible - if (null == search) { - return Stream.empty(); - } else if (search.containsKey(T.id)) { - stream = IteratorUtils.stream(graph.edges(search.get(T.id))); - directionUsedInLookup = Optional.empty(); - } else if (search.containsKey(Direction.BOTH)) { - // filter self-edges with distinct() - stream = IteratorUtils.stream(graph.vertices(search.get(Direction.BOTH))).flatMap(v -> IteratorUtils.stream(v.edges(Direction.BOTH))).distinct(); - directionUsedInLookup = Optional.of(Direction.BOTH); - } else if (search.containsKey(Direction.OUT)) { - stream = IteratorUtils.stream(graph.vertices(search.get(Direction.OUT))).flatMap(v -> IteratorUtils.stream(v.edges(Direction.OUT))); - directionUsedInLookup = Optional.of(Direction.OUT); - } else if (search.containsKey(Direction.IN)) { - stream = IteratorUtils.stream(graph.vertices(search.get(Direction.IN))).flatMap(v -> IteratorUtils.stream(v.edges(Direction.IN))); - directionUsedInLookup = Optional.of(Direction.IN); + protected Iterator<Edge> searchEdges(final Map<?,?> search) { + final Graph graph = getGraph(); + + final Object edgeId = search.get(T.id); + final String edgeLabel = (String) search.get(T.label); + final Object fromId = search.get(Direction.OUT); + final Object toId = search.get(Direction.IN); + + GraphTraversal t; + if (edgeId != null) { + // if eid: + // g.E(edgeId).hasLabel(edgeLabel).where(__.outV().hasId(fromId)).where(__.inV().hasId(toId)); + t = graph.traversal().E(edgeId); + if (edgeLabel != null) + t = t.hasLabel(edgeLabel); + if (fromId != null) + t = t.where(__.outV().hasId(fromId)); + if (toId != null) + t = t.where(__.inV().hasId(toId)); + } else if (fromId != null) { + // else if fromId: + // g.V(fromId).outE(edgeLabel).where(__.inV().hasId(toId)); + t = graph.traversal().V(fromId); + if (edgeLabel != null) + t = t.outE(edgeLabel); + else + t = t.outE(); + if (toId != null) + t = t.where(__.inV().hasId(toId)); + } else if (toId != null) { + // else if toId: + // g.V(toId).inE(edgeLabel); + t = graph.traversal().V(toId); + if (edgeLabel != null) + t = t.inE(edgeLabel); + else + t = t.inE(); } else { - stream = IteratorUtils.stream(graph.edges()); - directionUsedInLookup = Optional.empty(); + // else: + t = graph.traversal().E(); + } + + // add property constraints + for (final Map.Entry<?,?> e : 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 t; + } + + protected Map<?,?> resolveVertices(final Map map, final Traverser.Admin<S> traverser) { + resolveVertex(Merge.outV, Direction.OUT, map, traverser, outVTraversal); + resolveVertex(Merge.inV, Direction.IN, map, traverser, inVTraversal); + return map; + } + + protected void resolveVertex(final Merge token, final Direction direction, final Map map, + final Traverser.Admin<S> traverser, final Traversal.Admin<S, Object> traversal) { + // no Direction specified in the map, nothing to resolve + if (!map.containsKey(direction)) + return; + + final Object value = map.get(direction); + if (Objects.equals(token, value)) { + if (traversal == null) { + throw new IllegalArgumentException(String.format( + "option(%s) must be specified if it is used for %s", token, direction)); + } + final Vertex vertex = resolveVertex(traverser, traversal); + if (vertex == null) + throw new IllegalArgumentException(String.format( + "Could not resolve vertex for option(%s)", token)); + map.put(direction, vertex.id()); + } else if (value instanceof Vertex) { + // flatten Vertex down to its id + map.put(direction, ((Vertex) value).id()); } + } - // in-memory filter is not going to be nice here. it will be up to graphs to optimize this step as they do - // for other Mutation steps - stream = stream.filter(e -> { - // try to match on all search criteria skipping T.id as it was handled above - return search.entrySet().stream().filter( - kv -> kv.getKey() != T.id && !(directionUsedInLookup.isPresent() && kv.getKey() == directionUsedInLookup.get())). - allMatch(kv -> { - if (kv.getKey() == T.label) { - return e.label().equals(kv.getValue()); - } else if (kv.getKey() instanceof Direction) { - final Direction direction = (Direction) kv.getKey(); - - // try to take advantage of string id conversions of the graph by doing a lookup rather - // than direct compare on id - final Iterator<Vertex> found = graph.vertices(kv.getValue()); - final Iterator<Vertex> dfound = e.vertices(direction); - final boolean matched = found.hasNext() && dfound.next().equals(found.next()); - CloseableIterator.closeIterator(found); - CloseableIterator.closeIterator(dfound); - return matched; - } else { - final Property<Object> vp = e.property(kv.getKey().toString()); - return vp.isPresent() && kv.getValue().equals(vp.value()); - } - }); - }); - - return stream; + /* + * outV/inV traversal can either provide a Map (which we then use to search for a vertex) or it can provide a + * Vertex directly. + */ + protected Vertex resolveVertex(final Traverser.Admin<S> traverser, final Traversal.Admin<S, Object> traversal) { + final Object o = TraversalUtil.apply(traverser, traversal); + if (o instanceof Vertex) + return (Vertex) o; + else if (o instanceof Map) { + final Map search = (Map) o; + final Vertex v = IteratorUtils.findFirst(MergeVertexStep.searchVertices(getGraph(), search)).get(); + return tryAttachVertex(v); + } + return null; } @Override protected Iterator<Edge> flatMap(final Traverser.Admin<S> traverser) { - final Map<Object,Object> searchCreate = TraversalUtil.apply(traverser, searchCreateTraversal); - - validateMapInput(searchCreate, false); + final Map unresolvedMergeMap = materializeMap(traverser, mergeTraversal); + validateMapInput(unresolvedMergeMap, false); - final Vertex outV = resolveVertex(traverser, searchCreate, Direction.OUT); - final Vertex inV = resolveVertex(traverser, searchCreate, Direction.IN); + /* + * Create a copy of the unresolved map and attempt to resolve any Vertex references. + */ + final Map mergeMap = resolveVertices(new LinkedHashMap<>(unresolvedMergeMap), traverser); - // need to copy searchCreate so that each traverser gets fresh search criteria if we use the traverser value - final Map<Object,Object> searchCreateCopy = null == searchCreate ? null : new HashMap<>(); - if (searchCreateCopy != null) { - searchCreateCopy.putAll(searchCreate); + Iterator<Edge> edges = searchEdges(mergeMap); - // out/in not specified in searchCreate so try to use the traverser. BOTH is not an accepted user input - // but helps with the search stream as it allows in/out to both be in the search stream. in other words, - // g.V().mergeE([label:'knows']) will end up traversing BOTH "knows" edges for each vertex - if (!searchCreateCopy.containsKey(Direction.OUT) && !searchCreateCopy.containsKey(Direction.IN) && - outV == inV && inV != PLACEHOLDER_VERTEX) { - searchCreateCopy.put(Direction.BOTH, outV); - } - } + if (onMatchTraversal != null) { - Stream<Edge> stream = createSearchStream(searchCreateCopy); - stream = stream.map(e -> { - // if no onMatch is defined then there is no update - return the edge unchanged - if (null == onMatchTraversal) return e; + edges = IteratorUtils.peek(edges, e -> { - // if this was a start step the traverser is initialized with placeholder edge, so override that with - // the matched Edge so that the option() traversal can operate on it properly - if (isStart) traverser.set((S) e); + // if this was a start step the traverser is initialized with placeholder edge, so override that with + // the matched Edge so that the option() traversal can operate on it properly + if (isStart) traverser.set((S) e); - // assume good input from GraphTraversal - folks might drop in a T here even though it is immutable - final Map<String, Object> onMatchMap = TraversalUtil.apply(traverser, onMatchTraversal); - validateMapInput(onMatchMap, true); + // assume good input from GraphTraversal - folks might drop in a T here even though it is immutable + final Map<String, ?> onMatchMap = materializeMap(traverser, onMatchTraversal); + validateMapInput(onMatchMap, true); - if (onMatchMap != null) { onMatchMap.forEach((key, value) -> { // trigger callbacks for eventing - in this case, it's a EdgePropertyChangedEvent. if there's no // registry/callbacks then just set the property if (this.callbackRegistry != null && !callbackRegistry.getCallbacks().isEmpty()) { - final EventStrategy eventStrategy = getTraversal().getStrategies().getStrategy(EventStrategy.class).get(); + final EventStrategy eventStrategy = + getTraversal().getStrategies().getStrategy(EventStrategy.class).get(); final Property<?> p = e.property(key); - final Property<Object> oldValue = p.isPresent() ? eventStrategy.detach(e.property(key)) : null; + final Property<Object> oldValue = + p.isPresent() ? eventStrategy.detach(e.property(key)) : null; final Event.EdgePropertyChangedEvent vpce = new Event.EdgePropertyChangedEvent(eventStrategy.detach(e), oldValue, value); this.callbackRegistry.getCallbacks().forEach(c -> c.accept(vpce)); } e.property(key, value); }); - } - return e; - }); + }); - // if the stream has something then there is a match (possibly updated) and is returned, otherwise a new - // edge is created - final Iterator<Edge> edges = stream.iterator(); + } + + /* + * Search produced results, and onMatch action will be triggered. + */ if (edges.hasNext()) { return edges; - } else { - final Edge edge; - - // if there is an onCreateTraversal then the search criteria is ignored for the creation as it is provided - // by way of the traversal which will return the Map - final boolean useOnCreate = onCreateTraversal != null; - final Map<Object,Object> onCreateMap = useOnCreate ? TraversalUtil.apply(traverser, onCreateTraversal) : searchCreateCopy; - - // searchCreate should have already been validated so only do it if it is overridden - if (useOnCreate) validateMapInput(onCreateMap, false); - - if (onCreateMap != null) { - // check if from/to were already determined by traverser/searchMatch, and if not, then at least ensure that - // the from/to is set in onCreateMap - if (outV == PLACEHOLDER_VERTEX && !onCreateMap.containsKey(Direction.OUT)) - throw new IllegalArgumentException("Out Vertex not specified - edge cannot be created"); - if (inV == PLACEHOLDER_VERTEX && !onCreateMap.containsKey(Direction.IN)) - throw new IllegalArgumentException("In Vertex not specified - edge cannot be created"); - - final List<Object> keyValues = new ArrayList<>(); - String label = Edge.DEFAULT_LABEL; - - // assume the to/from vertices from traverser/searchMatch are what we want, but then - // consider the override dropping in from onCreate - Vertex fromV = outV; - Vertex toV = inV; - - for (Map.Entry<Object, Object> entry : onCreateMap.entrySet()) { - if (entry.getKey() instanceof Direction) { - // only override if onCreate was specified otherwise stick to however traverser/searchMatch - // was resolved - if (useOnCreate && entry.getKey().equals(Direction.IN)) { - final Object o = searchCreateCopy.getOrDefault(Direction.IN, entry.getValue()); - toV = tryAttachVertex(o instanceof Vertex ? (Vertex) o : new ReferenceVertex(o)); - } else if (useOnCreate && entry.getKey().equals(Direction.OUT)) { - final Object o = searchCreateCopy.getOrDefault(Direction.OUT, entry.getValue()); - fromV = tryAttachVertex(o instanceof Vertex ? (Vertex) o : new ReferenceVertex(o)); - } - } else if (entry.getKey().equals(T.label)) { - label = (String) entry.getValue(); - } else { - keyValues.add(entry.getKey()); - keyValues.add(entry.getValue()); - } - } + } - edge = fromV.addEdge(label, toV, keyValues.toArray(new Object[keyValues.size()])); + // make sure we close the search traversal + CloseableIterator.closeIterator(edges); - // trigger callbacks for eventing - in this case, it's a VertexAddedEvent - if (this.callbackRegistry != null && !callbackRegistry.getCallbacks().isEmpty()) { - final EventStrategy eventStrategy = getTraversal().getStrategies().getStrategy(EventStrategy.class).get(); - final Event.EdgeAddedEvent vae = new Event.EdgeAddedEvent(eventStrategy.detach(edge)); - this.callbackRegistry.getCallbacks().forEach(c -> c.accept(vae)); - } + final Map onCreateMap = onCreateMap(traverser, unresolvedMergeMap, mergeMap); - return IteratorUtils.of(edge); - } else { - return Collections.emptyIterator(); - } + // check for from/to vertices, which must be specified for the create action + if (!onCreateMap.containsKey(Direction.OUT)) + throw new IllegalArgumentException("Out Vertex not specified - edge cannot be created"); + if (!onCreateMap.containsKey(Direction.IN)) + throw new IllegalArgumentException("In Vertex not specified - edge cannot be created"); + + final Vertex fromV = resolveVertex(onCreateMap.get(Direction.OUT)); + final Vertex toV = resolveVertex(onCreateMap.get(Direction.IN)); + final String label = (String) onCreateMap.getOrDefault(T.label, Edge.DEFAULT_LABEL); + + final List<Object> properties = new ArrayList<>(); + + // add property constraints + for (final Map.Entry e : ((Map<?,?>) onCreateMap).entrySet()) { + final Object k = e.getKey(); + if (k.equals(Direction.OUT) || k.equals(Direction.IN) || k.equals(T.label)) continue; + properties.add(k); + properties.add(e.getValue()); + } + + final Edge edge = fromV.addEdge(label, toV, properties.toArray()); + + // trigger callbacks for eventing - in this case, it's a VertexAddedEvent + if (this.callbackRegistry != null && !callbackRegistry.getCallbacks().isEmpty()) { + final EventStrategy eventStrategy = getTraversal().getStrategies().getStrategy(EventStrategy.class).get(); + final Event.EdgeAddedEvent vae = new Event.EdgeAddedEvent(eventStrategy.detach(edge)); + this.callbackRegistry.getCallbacks().forEach(c -> c.accept(vae)); } + + return IteratorUtils.of(edge); } - /** - * Little helper method that will resolve {@link Direction} map keys to a {@link Vertex} which is the currency - * of this step. Since this step can accept a {@link Vertex} as the traverser it uses that as a default value - * in the case where {@link Direction} is not specified in the {@code Map}. As a result the {@code Map} value - * overrides the traverser. Note that if this is a start step then the traverser will contain a - * {@link #PLACEHOLDER_VERTEX} which is basically just a dummy to use as a marker where it will be assumed a - * {@code Map} argument to the step will have the necessary {@link Vertex} to allow the step to do its work. If - * the {@link Direction} contains something other than a {@link Vertex} it will become the {@link T#id} to a - * fresh {@link ReferenceVertex}. + protected Map onCreateMap(final Traverser.Admin<S> traverser, final Map unresolvedMergeMap, final Map mergeMap) { + // no onCreateTraversal - use main mergeMap argument + if (onCreateTraversal == null) + return mergeMap; + + final Map onCreateMap = materializeMap(traverser, onCreateTraversal); + validateMapInput(onCreateMap, false); + + /* + * Now we need to merge the two maps - onCreate should inherit traits from mergeMap, and it is not allowed to + * override values for any keys. + */ + + /* + * We use the unresolved version here in case onCreateMap uses Merge tokens or Vertex objects for its values. + */ + validateNoOverrides(unresolvedMergeMap, onCreateMap); + + /* + * Use the resolved version here so that onCreateMap picks up fully resolved vertex arguments from the main + * merge argument and so we don't re-resolve them below. + */ + onCreateMap.putAll(mergeMap); + + /* + * Do any final vertex resolution, for example if Merge tokens were used in option(onCreate) but not in the main + * merge argument. + */ + resolveVertices(onCreateMap, traverser); + + return onCreateMap; + } + + /* + * Resolve the argument for Direction.IN/OUT into a proper Vertex. */ - protected Vertex resolveVertex(final Traverser.Admin<S> traverser, final Map<Object, Object> searchCreate, - final Direction direction) { - final Vertex traverserVertex = traverser.get() instanceof Vertex ? (Vertex) traverser.get() : PLACEHOLDER_VERTEX; - final Object o = searchCreate != null ? searchCreate.getOrDefault(direction, traverserVertex) : traverserVertex; - final Vertex v = o instanceof Vertex ? (Vertex) o : new ReferenceVertex(o); - if (v != PLACEHOLDER_VERTEX && v instanceof Attachable) { - return tryAttachVertex(v); - } else { - return v; + protected Vertex resolveVertex(final Object arg) { + if (arg instanceof Vertex) + return tryAttachVertex((Vertex) arg); + + final Iterator<Vertex> it = getGraph().vertices(arg); + try { + // otherwise use the arg as a vertex id + if (!it.hasNext()) + throw new IllegalArgumentException( + String.format("Vertex id %s could not be found and edge could not be created", arg)); + return it.next(); + } finally { + CloseableIterator.closeIterator(it); } } @@ -386,104 +411,16 @@ public class MergeEdgeStep<S> extends FlatMapStep<S, Edge> implements Mutating<E * Tries to attach a {@link Vertex} to its host {@link Graph} of the traversal. If the {@link Vertex} cannot be * found then an {@code IllegalArgumentException} is expected. */ - protected Vertex tryAttachVertex(final Vertex maybeAttachable) { - if (maybeAttachable instanceof Attachable) { + protected Vertex tryAttachVertex(final Vertex v) { + if (v instanceof Attachable) { try { - return ((Attachable<Vertex>) maybeAttachable).attach(Attachable.Method.get(this.getTraversal().getGraph().orElse(EmptyGraph.instance()))); + return ((Attachable<Vertex>) v).attach(Attachable.Method.get(getGraph())); } catch (IllegalStateException ise) { - throw new IllegalArgumentException(String.format("%s could not be found and edge could not be created", maybeAttachable)); + throw new IllegalArgumentException(String.format("%s could not be found and edge could not be created", v)); } } else { - return maybeAttachable; - } - } - - /** - * Validates input to any {@code Map} arguments to this step. For {@link Merge#onMatch} updates cannot be applied - * to immutable parts of an {@link Edge} (id, label, incident vertices) so those can be ignored in the validation. - */ - public static void validateMapInput(final Map<?,Object> m, final boolean ignoreTokens) { - if (null == m) return; - if (ignoreTokens) { - m.entrySet().stream().filter(e -> { - final Object k = e.getKey(); - return !(k instanceof String); - }).findFirst().map(e -> { - throw new IllegalArgumentException(String.format( - "option(onMatch) expects keys in Map to be of String - check: %s", - e.getKey())); - }); - } else { - m.entrySet().stream().filter(e -> { - final Object k = e.getKey(); - return k != T.id && k != T.label && k != Direction.OUT && k != Direction.IN && !(k instanceof String); - }).findFirst().map(e -> { - throw new IllegalArgumentException(String.format( - "mergeE() and option(onCreate) expects keys in Map to be of String, T.id, T.label, or any Direction except BOTH - check: %s", - e.getKey())); - }); - } - - if (!ignoreTokens) { - m.entrySet().stream().filter(e -> e.getKey() == T.label && !(e.getValue() instanceof String)).findFirst().map(e -> { - throw new IllegalArgumentException(String.format( - "mergeE() expects T.label value to be of String - found: %s", - e.getValue().getClass().getSimpleName())); - }); + return v; } } - @Override - public CallbackRegistry<Event> getMutatingCallbackRegistry() { - if (null == callbackRegistry) callbackRegistry = new ListCallbackRegistry<>(); - return callbackRegistry; - } - - @Override - public int hashCode() { - int result = super.hashCode(); - if (searchCreateTraversal != null) - result ^= searchCreateTraversal.hashCode(); - if (onCreateTraversal != null) - result ^= onCreateTraversal.hashCode(); - if (onMatchTraversal != null) - result ^= onMatchTraversal.hashCode(); - return result; - } - - @Override - public void reset() { - super.reset(); - first = true; - searchCreateTraversal.reset(); - if (onCreateTraversal != null) onCreateTraversal.reset(); - if (onMatchTraversal != null) onMatchTraversal.reset(); - } - - @Override - public Set<TraverserRequirement> getRequirements() { - return this.getSelfAndChildRequirements(); - } - - @Override - public String toString() { - return StringFactory.stepString(this, searchCreateTraversal, onCreateTraversal, onMatchTraversal); - } - - @Override - public void setTraversal(final Traversal.Admin<?, ?> parentTraversal) { - super.setTraversal(parentTraversal); - this.integrateChild(searchCreateTraversal); - this.integrateChild(onCreateTraversal); - this.integrateChild(onMatchTraversal); - } - - @Override - public MergeEdgeStep<S> clone() { - final MergeEdgeStep<S> clone = (MergeEdgeStep<S>) super.clone(); - clone.searchCreateTraversal = searchCreateTraversal.clone(); - clone.onCreateTraversal = onCreateTraversal != null ? onCreateTraversal.clone() : null; - clone.onMatchTraversal = onMatchTraversal != null ? onMatchTraversal.clone() : null; - return clone; - } } 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 new file mode 100644 index 0000000000..c3323023ad --- /dev/null +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeStep.java @@ -0,0 +1,299 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.tinkerpop.gremlin.process.traversal.step.map; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; + +import org.apache.tinkerpop.gremlin.process.traversal.Merge; +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.lambda.ConstantTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.lambda.IdentityTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.step.Mutating; +import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalOptionParent; +import org.apache.tinkerpop.gremlin.process.traversal.step.util.Parameters; +import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.CallbackRegistry; +import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.Event; +import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.ListCallbackRegistry; +import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement; +import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil; +import org.apache.tinkerpop.gremlin.structure.Direction; +import org.apache.tinkerpop.gremlin.structure.Edge; +import org.apache.tinkerpop.gremlin.structure.Graph; +import org.apache.tinkerpop.gremlin.structure.T; +import org.apache.tinkerpop.gremlin.structure.util.StringFactory; + +/** + * Abstract base class for for the {@code mergeV/E()} implementations. + */ +public abstract class MergeStep<S, E, C> extends FlatMapStep<S, E> + implements Mutating<Event>, TraversalOptionParent<Merge, S, C> { + + + protected final boolean isStart; + protected boolean first = true; + protected Traversal.Admin<S, Map> mergeTraversal; + protected Traversal.Admin<S, Map> onCreateTraversal = null; + protected Traversal.Admin<S, Map<String, ?>> onMatchTraversal = null; + + protected CallbackRegistry<Event> callbackRegistry; + + public MergeStep(final Traversal.Admin traversal, final boolean isStart) { + this(traversal, isStart, new IdentityTraversal<>()); + } + + public MergeStep(final Traversal.Admin traversal, final boolean isStart, final Map mergeMap) { + this(traversal, isStart, new ConstantTraversal<>(mergeMap)); + validate(mergeMap, false); + } + + public MergeStep(final Traversal.Admin traversal, final boolean isStart, + final Traversal.Admin mergeTraversal) { + super(traversal); + this.isStart = isStart; + this.mergeTraversal = integrateChild(mergeTraversal); + } + + /** + * Gets the traversal that will be used to provide the {@code Map} that will be used to search for elements. + * This {@code Map} also will be used as the default data set to be used to create the element if the search is not + * successful. + */ + public Traversal.Admin<S, Map> getMergeTraversal() { + return mergeTraversal; + } + + /** + * Gets the traversal that will be used to provide the {@code Map} that will be used to create elements that + * do not match the search criteria of {@link #getMergeTraversal()}. + */ + public Traversal.Admin<S, Map> getOnCreateTraversal() { + return onCreateTraversal; + } + + /** + * Gets the traversal that will be used to provide the {@code Map} that will be used to modify elements that + * match the search criteria of {@link #getMergeTraversal()}. + */ + public Traversal.Admin<S, Map<String, ?>> getOnMatchTraversal() { + return onMatchTraversal; + } + + /** + * Determines if this is a start step. + */ + public boolean isStart() { + return isStart; + } + + /** + * Determine if this is the first pass through {@link #processNextStart()}. + */ + public boolean isFirst() { + return first; + } + + public CallbackRegistry<Event> getCallbackRegistry() { + return callbackRegistry; + } + + @Override + public void addChildOption(final Merge token, final Traversal.Admin<S, C> traversalOption) { + if (token == Merge.onCreate) { + this.onCreateTraversal = this.integrateChild(traversalOption); + } else if (token == Merge.onMatch) { + this.onMatchTraversal = this.integrateChild(traversalOption); + } else { + throw new UnsupportedOperationException(String.format("Option %s for Merge is not supported", token.name())); + } + } + + @Override + public <S, C> List<Traversal.Admin<S, C>> getLocalChildren() { + final List<Traversal.Admin<S, C>> children = new ArrayList<>(); + if (mergeTraversal != null) children.add((Traversal.Admin<S, C>) mergeTraversal); + if (onMatchTraversal != null) children.add((Traversal.Admin<S, C>) onMatchTraversal); + if (onCreateTraversal != null) children.add((Traversal.Admin<S, C>) onCreateTraversal); + return children; + } + + @Override + public void configure(final Object... keyValues) { + // this is a Mutating step but property() should not be folded into this step. The main issue here is that + // this method won't know what step called it - property() or with() or something else so it can't make the + // choice easily to throw an exception, write the keys/values to parameters, etc. It really is up to the + // caller to make sure it is handled properly at this point. this may best be left as a do-nothing method for + // now. + } + + @Override + public Parameters getParameters() { + // merge doesn't take fold ups of property() calls. those need to get treated as regular old PropertyStep + // instances. not sure if this should support with() though.....none of the other Mutating steps do. + return null; + } + + @Override + protected Traverser.Admin<E> processNextStart() { + // when it's a start step a traverser needs to be created to kick off the traversal. + if (isStart && first) { + first = false; + generateTraverser(false); + } + return super.processNextStart(); + } + + private void generateTraverser(final Object o) { + final TraverserGenerator generator = this.getTraversal().getTraverserGenerator(); + this.addStart(generator.generate(o, (Step) this, 1L)); + } + + protected Graph getGraph() { + return this.getTraversal().getGraph().get(); + } + + @Override + public CallbackRegistry<Event> getMutatingCallbackRegistry() { + if (null == callbackRegistry) callbackRegistry = new ListCallbackRegistry<>(); + return callbackRegistry; + } + + @Override + public int hashCode() { + int result = super.hashCode(); + if (mergeTraversal != null) + result ^= mergeTraversal.hashCode(); + if (onCreateTraversal != null) + result ^= onCreateTraversal.hashCode(); + if (onMatchTraversal != null) + result ^= onMatchTraversal.hashCode(); + return result; + } + + @Override + public void reset() { + super.reset(); + first = true; + mergeTraversal.reset(); + if (onCreateTraversal != null) onCreateTraversal.reset(); + if (onMatchTraversal != null) onMatchTraversal.reset(); + } + + @Override + public Set<TraverserRequirement> getRequirements() { + return this.getSelfAndChildRequirements(); + } + + @Override + public String toString() { + return StringFactory.stepString(this, mergeTraversal, onCreateTraversal, onMatchTraversal); + } + + @Override + public void setTraversal(final Traversal.Admin<?, ?> parentTraversal) { + super.setTraversal(parentTraversal); + this.integrateChild(mergeTraversal); + this.integrateChild(onCreateTraversal); + this.integrateChild(onMatchTraversal); + } + + @Override + public MergeStep<S, E, C> clone() { + final MergeStep<S, E, C> clone = (MergeStep<S, E, C>) super.clone(); + clone.mergeTraversal = mergeTraversal.clone(); + clone.onCreateTraversal = onCreateTraversal != null ? onCreateTraversal.clone() : null; + clone.onMatchTraversal = onMatchTraversal != null ? onMatchTraversal.clone() : null; + return clone; + } + + protected void validate(final Map map, final boolean ignoreTokens) { + final Set allowedTokens = getAllowedTokens(); + validate(map, ignoreTokens, allowedTokens); + } + + protected static void validate(final Map map, final boolean ignoreTokens, final Set allowedTokens) { + if (null == map) return; + + ((Map<?,?>) map).entrySet().forEach(e -> { + final Object k = e.getKey(); + final Object v = e.getValue(); + + if (v == null) { + throw new IllegalArgumentException(String.format("merge() does not allow null Map values - check: %s", k)); + } + + 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 { + if (!(k instanceof String) && !allowedTokens.contains(k)) { + throw new IllegalArgumentException(String.format( + "merge() and option(onCreate) expects keys in Map to be either String or %s - check: %s", + allowedTokens, k)); + } + if (k == T.label && !(v instanceof String)) { + throw new IllegalArgumentException(String.format("merge() expects T.label value to be of String - found: %s", + v.getClass().getSimpleName())); + + } + 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)); + } + 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)); + } + } + }); + } + + protected void validateNoOverrides(final Map<?,?> mergeMap, final Map<?,?> onCreateMap) { + for (final Map.Entry e : onCreateMap.entrySet()) { + final Object k = e.getKey(); + final Object v = e.getValue(); + if (mergeMap.containsKey(k) && !Objects.equals(v, mergeMap.get(k))) { + throw new IllegalArgumentException(String.format( + "option(onCreate) cannot override values from merge() argument: (%s, %s)", k, v)); + } + } + } + + /** + * null Map == empty Map + */ + protected Map materializeMap(final Traverser.Admin<S> traverser, Traversal.Admin<S, ?> mapTraversal) { + final Map map = (Map) TraversalUtil.apply(traverser, mapTraversal); + return map == null ? new LinkedHashMap() : map; + } + + @Override + protected abstract Iterator<E> flatMap(final Traverser.Admin<S> traverser); + + protected abstract Set getAllowedTokens(); + +} 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 b7759590d6..bc4e98919a 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 @@ -18,160 +18,86 @@ */ package org.apache.tinkerpop.gremlin.process.traversal.step.map; -import org.apache.tinkerpop.gremlin.process.traversal.Merge; -import org.apache.tinkerpop.gremlin.process.traversal.Step; +import java.util.Arrays; +import java.util.Collections; +import java.util.Iterator; +import java.util.LinkedHashSet; +import java.util.Map; +import java.util.Objects; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; + 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.lambda.ConstantTraversal; -import org.apache.tinkerpop.gremlin.process.traversal.lambda.IdentityTraversal; -import org.apache.tinkerpop.gremlin.process.traversal.step.Mutating; -import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalOptionParent; -import org.apache.tinkerpop.gremlin.process.traversal.step.util.Parameters; -import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.CallbackRegistry; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.Event; -import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.ListCallbackRegistry; import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.EventStrategy; -import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement; import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil; -import org.apache.tinkerpop.gremlin.structure.Direction; -import org.apache.tinkerpop.gremlin.structure.Edge; import org.apache.tinkerpop.gremlin.structure.Graph; import org.apache.tinkerpop.gremlin.structure.Property; import org.apache.tinkerpop.gremlin.structure.T; import org.apache.tinkerpop.gremlin.structure.Vertex; -import org.apache.tinkerpop.gremlin.structure.VertexProperty; -import org.apache.tinkerpop.gremlin.structure.util.StringFactory; import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Set; -import java.util.stream.Stream; - /** * Implementation for the {@code mergeV()} step covering both the start step version and the one used mid-traversal. */ -public class MergeVertexStep<S> extends FlatMapStep<S, Vertex> implements Mutating<Event>, - TraversalOptionParent<Merge, S, Vertex> { - - private final boolean isStart; - private boolean first = true; - private Traversal.Admin<S,Map<Object, Object>> searchCreateTraversal; - private Traversal.Admin<S, Map<Object, Object>> onCreateTraversal = null; - private Traversal.Admin<S, Map<String, Object>> onMatchTraversal = null; +public class MergeVertexStep<S> extends MergeStep<S, Vertex, Map> { - protected CallbackRegistry<Event> callbackRegistry; + private static final Set allowedTokens = new LinkedHashSet(Arrays.asList(T.id, T.label)); - public MergeVertexStep(final Traversal.Admin traversal, final boolean isStart) { - this(traversal, isStart, new IdentityTraversal()); + public static void validateMapInput(final Map map, final boolean ignoreTokens) { + MergeStep.validate(map, ignoreTokens, allowedTokens); } - public MergeVertexStep(final Traversal.Admin traversal, final boolean isStart, final Map<Object, Object> searchCreate) { - this(traversal, isStart, new ConstantTraversal<>(searchCreate)); - } - public MergeVertexStep(final Traversal.Admin traversal, final boolean isStart, final Traversal.Admin<S,Map<Object, Object>> searchCreateTraversal) { - super(traversal); - this.isStart = isStart; - this.searchCreateTraversal = integrateChild(searchCreateTraversal); - } - - /** - * Gets the traversal that will be used to provide the {@code Map} that will be used to search for vertices. - * This {@code Map} also will be used as the default data set to be used to create a vertex if the search is not - * successful. - */ - public Traversal.Admin<S, Map<Object, Object>> getSearchCreateTraversal() { - return searchCreateTraversal; - } - - /** - * Gets the traversal that will be used to provide the {@code Map} that will be the override to the one provided - * by the {@link #getSearchCreateTraversal()} for vertex creation events. - */ - public Traversal.Admin<S, Map<Object, Object>> getOnCreateTraversal() { - return onCreateTraversal; - } - - /** - * Gets the traversal that will be used to provide the {@code Map} that will be used to modify vertices that - * match the search criteria of {@link #getSearchCreateTraversal()}. - */ - public Traversal.Admin<S, Map<String, Object>> getOnMatchTraversal() { - return onMatchTraversal; - } - - /** - * Determines if this is a start step. - */ - public boolean isStart() { - return isStart; + public MergeVertexStep(final Traversal.Admin traversal, final boolean isStart) { + super(traversal, isStart); } - /** - * Determine if this is the first pass through {@link #processNextStart()}. - */ - public boolean isFirst() { - return first; + public MergeVertexStep(final Traversal.Admin traversal, final boolean isStart, final Map merge) { + super(traversal, isStart, merge); } - public CallbackRegistry<Event> getCallbackRegistry() { - return callbackRegistry; + public MergeVertexStep(final Traversal.Admin traversal, final boolean isStart, final Traversal.Admin<S,Map> mergeTraversal) { + super(traversal, isStart, mergeTraversal); } @Override - public void addChildOption(final Merge token, final Traversal.Admin<S, Vertex> traversalOption) { - if (token == Merge.onCreate) { - this.onCreateTraversal = this.integrateChild(traversalOption); - } else if (token == Merge.onMatch) { - this.onMatchTraversal = this.integrateChild(traversalOption); - } else { - throw new UnsupportedOperationException(String.format("Option %s for Merge is not supported", token.name())); - } + public MergeVertexStep<S> clone() { + return (MergeVertexStep<S>) super.clone(); } @Override - public <S, E> List<Traversal.Admin<S, E>> getLocalChildren() { - final List<Traversal.Admin<S, E>> children = new ArrayList<>(); - if (searchCreateTraversal != null) children.add((Traversal.Admin<S, E>) searchCreateTraversal); - if (onMatchTraversal != null) children.add((Traversal.Admin<S, E>) onMatchTraversal); - if (onCreateTraversal != null) children.add((Traversal.Admin<S, E>) onCreateTraversal); - return children; + protected Set getAllowedTokens() { + return allowedTokens; } - @Override - public void configure(final Object... keyValues) { - // this is a Mutating step but property() should not be folded into this step. The main issue here is that - // this method won't know what step called it - property() or with() or something else so it can't make the - // choice easily to throw an exception, write the keys/values to parameters, etc. It really is up to the - // caller to make sure it is handled properly at this point. this may best be left as a do-nothing method for - // now. - } + public static Iterator<Vertex> searchVertices(final Graph graph, final Map search) { + if (search == null) + return Collections.emptyIterator(); - @Override - public Parameters getParameters() { - // merge doesn't take fold ups of property() calls. those need to get treated as regular old PropertyStep - // instances. not sure if this should support with() though.....none of the other Mutating steps do. - return null; - } + final Object id = search.get(T.id); + final String label = (String) search.get(T.label); - @Override - protected Traverser.Admin<Vertex> processNextStart() { - // when it's a start step a traverser needs to be created to kick off the traversal. - if (isStart && first) { - first = false; - generateTraverser(false); + GraphTraversal t; + if (id != null) + t = graph.traversal().V(id); + else + t = 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()); } - return super.processNextStart(); - } - private void generateTraverser(final Object o) { - final TraverserGenerator generator = this.getTraversal().getTraverserGenerator(); - this.addStart(generator.generate(o, (Step) this, 1L)); + // this should auto-close the underlying traversal + return t; } /** @@ -180,54 +106,31 @@ public class MergeVertexStep<S> extends FlatMapStep<S, Vertex> implements Mutati * matching the list of vertices. This implementation is only optimized for the {@link T#id} so any other usage * will simply be in-memory filtering which could be slow. */ - protected Stream<Vertex> createSearchStream(final Map<Object,Object> search) { - final Graph graph = this.getTraversal().getGraph().get(); - - Stream<Vertex> stream; - // prioritize lookup by id - if (null == search) - return Stream.empty(); - else if (search.containsKey(T.id)) - stream = IteratorUtils.stream(graph.vertices(search.get(T.id))); - else - stream = IteratorUtils.stream(graph.vertices()); - - // in-memory filter is not going to be nice here. it will be up to graphs to optimize this step as they do - // for other Mutation steps - stream = stream.filter(v -> { - // try to match on all search criteria skipping T.id as it was handled above - return search.entrySet().stream().filter(kv -> kv.getKey() != T.id).allMatch(kv -> { - if (kv.getKey() == T.label) { - return v.label().equals(kv.getValue()); - } else { - final VertexProperty<Object> vp = v.property(kv.getKey().toString()); - return vp.isPresent() && kv.getValue().equals(vp.value()); - } - }); - }); - - return stream; + protected Iterator<Vertex> searchVertices(final Map search) { + return searchVertices(getGraph(), search); } @Override protected Iterator<Vertex> flatMap(final Traverser.Admin<S> traverser) { - final Map<Object,Object> searchCreate = TraversalUtil.apply(traverser, searchCreateTraversal); - validateMapInput(searchCreate, false); + final Graph graph = getGraph(); - Stream<Vertex> stream = createSearchStream(searchCreate); - stream = stream.map(v -> { - // if no onMatch is defined then there is no update - return the vertex unchanged - if (null == onMatchTraversal) return v; + final Map mergeMap = materializeMap(traverser, mergeTraversal); + validateMapInput(mergeMap, false); - // if this was a start step the traverser is initialized with Boolean/false, so override that with - // the matched Vertex so that the option() traversal can operate on it properly - if (isStart) traverser.set((S) v); + Iterator<Vertex> vertices = searchVertices(mergeMap); - // assume good input from GraphTraversal - folks might drop in a T here even though it is immutable - final Map<String, Object> onMatchMap = TraversalUtil.apply(traverser, onMatchTraversal); - validateMapInput(onMatchMap, true); + if (onMatchTraversal != null) { + + vertices = IteratorUtils.peek(vertices, v -> { + + // if this was a start step the traverser is initialized with Boolean/false, so override that with + // the matched Vertex so that the option() traversal can operate on it properly + if (isStart) traverser.set((S) v); + + // assume good input from GraphTraversal - folks might drop in a T here even though it is immutable + final Map<String, Object> onMatchMap = materializeMap(traverser, onMatchTraversal); + validateMapInput(onMatchMap, true); - if (onMatchMap != null) { onMatchMap.forEach((key, value) -> { // trigger callbacks for eventing - in this case, it's a VertexPropertyChangedEvent. if there's no // registry/callbacks then just set the property @@ -240,152 +143,56 @@ public class MergeVertexStep<S> extends FlatMapStep<S, Vertex> implements Mutati } // try to detect proper cardinality for the key according to the graph - final Graph graph = this.getTraversal().getGraph().get(); v.property(graph.features().vertex().getCardinality(key), key, value); }); - } + }); - return v; - }); + } - // if the stream has something then there is a match (possibly updated) and is returned, otherwise a new - // vertex is created - final Iterator<Vertex> vertices = stream.iterator(); + /* + * Search produced results, and onMatch action will be triggered. + */ if (vertices.hasNext()) { return vertices; - } else { - final Vertex vertex; - - // if there is an onCreateTraversal then the search criteria is ignored for the creation as it is provided - // by way of the traversal which will return the Map - final boolean useOnCreate = onCreateTraversal != null; - final Map<Object,Object> onCreateMap = useOnCreate ? TraversalUtil.apply(traverser, onCreateTraversal) : searchCreate; - - // searchCreate should have already been validated so only do it if it is overridden - if (useOnCreate) validateMapInput(onCreateMap, false); - - // if onCreate is null then it's a do nothing - final List<Object> keyValues = new ArrayList<>(); - if (onCreateMap != null) { - for (Map.Entry<Object, Object> entry : onCreateMap.entrySet()) { - keyValues.add(entry.getKey()); - keyValues.add(entry.getValue()); - } - vertex = this.getTraversal().getGraph().get().addVertex(keyValues.toArray(new Object[keyValues.size()])); - - // trigger callbacks for eventing - in this case, it's a VertexAddedEvent - if (this.callbackRegistry != null && !callbackRegistry.getCallbacks().isEmpty()) { - final EventStrategy eventStrategy = getTraversal().getStrategies().getStrategy(EventStrategy.class).get(); - final Event.VertexAddedEvent vae = new Event.VertexAddedEvent(eventStrategy.detach(vertex)); - this.callbackRegistry.getCallbacks().forEach(c -> c.accept(vae)); - } - - return IteratorUtils.of(vertex); - } else { - return Collections.emptyIterator(); - } } - } - /** - * Validates input to any {@code Map} arguments to this step. For {@link Merge#onMatch} updates cannot be applied - * to immutable parts of an {@link Edge} (id, label, incident vertices) so those can be ignored in the validation. - */ - public static void validateMapInput(final Map<?,Object> m, final boolean ignoreTokens) { - if (null == m) return; - if (ignoreTokens) { - m.entrySet().stream().filter(e -> { - final Object k = e.getKey(); - return !(k instanceof String); - }).findFirst().map(e -> { - throw new IllegalArgumentException(String.format( - "option(onMatch) expects keys in Map to be of String - check: %s", - e.getKey())); - }); - } else { - m.entrySet().stream().filter(e -> { - final Object k = e.getKey(); - return k != T.id && k != T.label && !(k instanceof String); - }).findFirst().map(e -> { - throw new IllegalArgumentException(String.format( - "mergeV() and option(onCreate) expects keys in Map to be of String, T.id, T.label - check: %s", - e.getKey())); - }); - } + final Map<?,?> onCreateMap = onCreateMap(traverser, mergeMap); + + final Object[] flatArgs = onCreateMap.entrySet().stream() + .flatMap(e -> Stream.of(e.getKey(), e.getValue())).collect(Collectors.toList()).toArray(); - if (!ignoreTokens) { - if (m.containsKey(T.id)) { - if (null == m.get(T.id)) { - throw new IllegalArgumentException("Vertex id cannot be null"); - } - } - - if (m.containsKey(T.label)) { - final Object l = m.get(T.label); - if (null == l) { - throw new IllegalArgumentException("Vertex label cannot be null"); - } - - if (!(l instanceof String)) { - throw new IllegalArgumentException(String.format( - "mergeV() expects T.label value to be of String - found: %s", - l.getClass().getSimpleName())); - } - } + final Vertex vertex = graph.addVertex(flatArgs); + + // trigger callbacks for eventing - in this case, it's a VertexAddedEvent + if (this.callbackRegistry != null && !callbackRegistry.getCallbacks().isEmpty()) { + final EventStrategy eventStrategy = getTraversal().getStrategies().getStrategy(EventStrategy.class).get(); + final Event.VertexAddedEvent vae = new Event.VertexAddedEvent(eventStrategy.detach(vertex)); + this.callbackRegistry.getCallbacks().forEach(c -> c.accept(vae)); } - } - @Override - public CallbackRegistry<Event> getMutatingCallbackRegistry() { - if (null == callbackRegistry) callbackRegistry = new ListCallbackRegistry<>(); - return callbackRegistry; + return IteratorUtils.of(vertex); } - @Override - public int hashCode() { - int result = super.hashCode(); - if (searchCreateTraversal != null) - result ^= searchCreateTraversal.hashCode(); - if (onCreateTraversal != null) - result ^= onCreateTraversal.hashCode(); - if (onMatchTraversal != null) - result ^= onMatchTraversal.hashCode(); - return result; - } + protected Map onCreateMap(final Traverser.Admin<S> traverser, final Map mergeMap) { + // no onCreateTraversal - use main mergeMap argument + if (onCreateTraversal == null) + return mergeMap; - @Override - public void reset() { - super.reset(); - first = true; - searchCreateTraversal.reset(); - if (onCreateTraversal != null) onCreateTraversal.reset(); - if (onMatchTraversal != null) onMatchTraversal.reset(); - } + final Map onCreateMap = materializeMap(traverser, onCreateTraversal); + // null result from onCreateTraversal - use main mergeMap argument + if (onCreateMap == null) + return mergeMap; - @Override - public Set<TraverserRequirement> getRequirements() { - return this.getSelfAndChildRequirements(); - } + validateMapInput(onCreateMap, false); - @Override - public String toString() { - return StringFactory.stepString(this, searchCreateTraversal, onCreateTraversal, onMatchTraversal); - } + /* + * Now we need to merge the two maps - onCreate should inherit traits from mergeMap, and it is not allowed to + * override values for any keys. + */ + validateNoOverrides(mergeMap, onCreateMap); + onCreateMap.putAll(mergeMap); - @Override - public void setTraversal(final Traversal.Admin<?, ?> parentTraversal) { - super.setTraversal(parentTraversal); - this.integrateChild(searchCreateTraversal); - this.integrateChild(onCreateTraversal); - this.integrateChild(onMatchTraversal); + return onCreateMap; } - @Override - public MergeVertexStep<S> clone() { - final MergeVertexStep<S> clone = (MergeVertexStep<S>) super.clone(); - clone.searchCreateTraversal = searchCreateTraversal.clone(); - clone.onCreateTraversal = onCreateTraversal != null ? onCreateTraversal.clone() : null; - clone.onMatchTraversal = onMatchTraversal != null ? onMatchTraversal.clone() : null; - return clone; - } } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/iterator/IteratorUtils.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/iterator/IteratorUtils.java index 6e4a2939d4..11dfa83950 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/iterator/IteratorUtils.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/util/iterator/IteratorUtils.java @@ -30,6 +30,7 @@ import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.Spliterator; import java.util.Spliterators; @@ -49,20 +50,21 @@ public final class IteratorUtils { private IteratorUtils() { } - public static final <S> Iterator<S> of(final S a) { + public static <S> Iterator<S> of(final S a) { return new SingleIterator<>(a); } - public static final <S> Iterator<S> of(final S a, S b) { + public static <S> Iterator<S> of(final S a, S b) { return new DoubleIterator<>(a, b); } /////////////// - public static final <S extends Collection<T>, T> S fill(final Iterator<T> iterator, final S collection) { + public static <S extends Collection<T>, T> S fill(final Iterator<T> iterator, final S collection) { while (iterator.hasNext()) { collection.add(iterator.next()); } + CloseableIterator.closeIterator(iterator); return collection; } @@ -70,11 +72,13 @@ public final class IteratorUtils { while (iterator.hasNext()) { iterator.next(); } + CloseableIterator.closeIterator(iterator); } - public static final long count(final Iterator iterator) { + public static long count(final Iterator iterator) { long ix = 0; for (; iterator.hasNext(); ++ix) iterator.next(); + CloseableIterator.closeIterator(iterator); return ix; } @@ -97,7 +101,7 @@ public final class IteratorUtils { } public static <S> Iterator<S> limit(final Iterator<S> iterator, final int limit) { - return new Iterator<S>() { + return new CloseableIterator<S>() { private int count = 0; @Override @@ -116,39 +120,64 @@ public final class IteratorUtils { throw FastNoSuchElementException.instance(); return iterator.next(); } + + @Override + public void close() { + CloseableIterator.closeIterator(iterator); + } }; } /////////////////// public static <T> boolean allMatch(final Iterator<T> iterator, final Predicate<T> predicate) { - while (iterator.hasNext()) { - if (!predicate.test(iterator.next())) { - return false; + try { + while (iterator.hasNext()) { + if (!predicate.test(iterator.next())) { + return false; + } } + return true; + } finally { + CloseableIterator.closeIterator(iterator); } - - return true; } public static <T> boolean anyMatch(final Iterator<T> iterator, final Predicate<T> predicate) { - while (iterator.hasNext()) { - if (predicate.test(iterator.next())) { - return true; + try { + while (iterator.hasNext()) { + if (predicate.test(iterator.next())) { + return true; + } } + return false; + } finally { + CloseableIterator.closeIterator(iterator); } - - return false; } public static <T> boolean noneMatch(final Iterator<T> iterator, final Predicate<T> predicate) { - while (iterator.hasNext()) { - if (predicate.test(iterator.next())) { - return false; + try { + while (iterator.hasNext()) { + if (predicate.test(iterator.next())) { + return false; + } } + return true; + } finally { + CloseableIterator.closeIterator(iterator); } + } - return true; + public static <T> Optional<T> findFirst(final Iterator<T> iterator) { + try { + if (iterator.hasNext()) { + return Optional.ofNullable(iterator.next()); + } + return Optional.empty(); + } finally { + CloseableIterator.closeIterator(iterator); + } } /////////////////// @@ -163,6 +192,7 @@ public final class IteratorUtils { final S obj = iterator.next(); map.put(key.apply(obj), value.apply(obj)); } + CloseableIterator.closeIterator(iterator); return map; } @@ -172,6 +202,7 @@ public final class IteratorUtils { final S obj = iterator.next(); map.computeIfAbsent(groupBy.apply(obj), k -> new ArrayList<>()).add(obj); } + CloseableIterator.closeIterator(iterator); return map; } @@ -180,7 +211,7 @@ public final class IteratorUtils { while (iterator.hasNext()) { result = accumulator.apply(result, iterator.next()); } - + CloseableIterator.closeIterator(iterator); return result; } @@ -193,7 +224,7 @@ public final class IteratorUtils { while (iterator.hasNext()) { result = accumulator.apply(result, iterator.next()); } - + CloseableIterator.closeIterator(iterator); return result; } @@ -203,8 +234,8 @@ public final class IteratorUtils { /////////////// - public static final <S> Iterator<S> consume(final Iterator<S> iterator, final Consumer<S> consumer) { - return new Iterator<S>() { + public static <S> Iterator<S> consume(final Iterator<S> iterator, final Consumer<S> consumer) { + return new CloseableIterator<S>() { @Override public boolean hasNext() { return iterator.hasNext(); @@ -221,18 +252,23 @@ public final class IteratorUtils { consumer.accept(s); return s; } + + @Override + public void close() { + CloseableIterator.closeIterator(iterator); + } }; } - public static final <S> Iterable<S> consume(final Iterable<S> iterable, final Consumer<S> consumer) { + public static <S> Iterable<S> consume(final Iterable<S> iterable, final Consumer<S> consumer) { return () -> IteratorUtils.consume(iterable.iterator(), consumer); } /////////////// - public static final <S, E> Iterator<E> map(final Iterator<S> iterator, final Function<S, E> function) { - return new Iterator<E>() { + public static <S, E> Iterator<E> map(final Iterator<S> iterator, final Function<S, E> function) { + return new CloseableIterator<E>() { @Override public boolean hasNext() { return iterator.hasNext(); @@ -247,10 +283,15 @@ public final class IteratorUtils { public E next() { return function.apply(iterator.next()); } + + @Override + public void close() { + CloseableIterator.closeIterator(iterator); + } }; } - public static final <S, E> Iterable<E> map(final Iterable<S> iterable, final Function<S, E> function) { + public static <S, E> Iterable<E> map(final Iterable<S> iterable, final Function<S, E> function) { return () -> IteratorUtils.map(iterable.iterator(), function); } @@ -260,10 +301,40 @@ public final class IteratorUtils { /////////////// - public static final <S> Iterator<S> filter(final Iterator<S> iterator, final Predicate<S> predicate) { + public static <S> Iterator<S> peek(final Iterator<S> iterator, final Consumer<S> function) { + return new CloseableIterator<S>() { + @Override + public boolean hasNext() { + return iterator.hasNext(); + } + + @Override + public void remove() { + iterator.remove(); + } + @Override + public S next() { + final S next = iterator.next(); + function.accept(next); + return next; + } + + @Override + public void close() { + CloseableIterator.closeIterator(iterator); + } + }; + } + + public static <S> Iterable<S> peek(final Iterable<S> iterable, final Consumer<S> function) { + return () -> IteratorUtils.peek(iterable.iterator(), function); + } - return new Iterator<S>() { + /////////////// + + public static <S> Iterator<S> filter(final Iterator<S> iterator, final Predicate<S> predicate) { + return new CloseableIterator<S>() { S nextResult = null; @Override @@ -298,7 +369,12 @@ public final class IteratorUtils { } } - private final void advance() { + @Override + public void close() { + CloseableIterator.closeIterator(iterator); + } + + private void advance() { this.nextResult = null; while (iterator.hasNext()) { final S s = iterator.next(); @@ -311,14 +387,14 @@ public final class IteratorUtils { }; } - public static final <S> Iterable<S> filter(final Iterable<S> iterable, final Predicate<S> predicate) { + public static <S> Iterable<S> filter(final Iterable<S> iterable, final Predicate<S> predicate) { return () -> IteratorUtils.filter(iterable.iterator(), predicate); } /////////////////// - public static final <S, E> Iterator<E> flatMap(final Iterator<S> iterator, final Function<S, Iterator<E>> function) { - return new Iterator<E>() { + public static <S, E> Iterator<E> flatMap(final Iterator<S> iterator, final Function<S, Iterator<E>> function) { + return new CloseableIterator<E>() { private Iterator<E> currentIterator = Collections.emptyIterator(); @@ -348,13 +424,18 @@ public final class IteratorUtils { else throw FastNoSuchElementException.instance(); } + + @Override + public void close() { + CloseableIterator.closeIterator(iterator); + } }; } /////////////////// - public static final <S> Iterator<S> concat(final Iterator<S>... iterators) { + public static <S> Iterator<S> concat(final Iterator<S>... iterators) { final MultiIterator<S> iterator = new MultiIterator<>(); for (final Iterator<S> itty : iterators) { iterator.addIterator(itty); @@ -400,7 +481,7 @@ public final class IteratorUtils { } public static <T> Iterator<T> noRemove(final Iterator<T> iterator) { - return new Iterator<T>() { + return new CloseableIterator<T>() { @Override public boolean hasNext() { return iterator.hasNext(); @@ -415,11 +496,16 @@ public final class IteratorUtils { public T next() { return iterator.next(); } + + @Override + public void close() { + CloseableIterator.closeIterator(iterator); + } }; } public static <T> Iterator<T> removeOnNext(final Iterator<T> iterator) { - return new Iterator<T>() { + return new CloseableIterator<T>() { @Override public boolean hasNext() { return iterator.hasNext(); @@ -436,6 +522,11 @@ public final class IteratorUtils { iterator.remove(); return object; } + + @Override + public void close() { + CloseableIterator.closeIterator(iterator); + } }; } } diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/CommonSteps.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/CommonSteps.cs index a63b8d9a2f..60c2d03ad5 100644 --- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/CommonSteps.cs +++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/CommonSteps.cs @@ -60,6 +60,7 @@ namespace Gremlin.Net.IntegrationTest.Gherkin {@"vp\[(.+)\]", ToVertexProperty}, {@"d\[(.*)\]\.([bsilfdmn])", ToNumber}, {@"D\[(.+)\]", ToDirection}, + {@"M\[(.+)\]", ToMerge}, {@"v\[(.+)\]", ToVertex}, {@"v\[(.+)\]\.id", (x, graphName) => ToVertex(x, graphName).Id}, {@"v\[(.+)\]\.sid", (x, graphName) => ToVertex(x, graphName).Id!.ToString()}, @@ -368,6 +369,11 @@ namespace Gremlin.Net.IntegrationTest.Gherkin return Direction.GetByValue(enumName); } + private static object ToMerge(string enumName, string graphName) + { + return Merge.GetByValue(enumName); + } + private static object ToNumber(string stringNumber, string graphName) { return NumericParsers[stringNumber[stringNumber.Length - 1]]( diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/index.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/index.js index 71e60dfc4c..524bfe16fb 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/index.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/index.js @@ -67,6 +67,7 @@ module.exports = { cardinality: t.cardinality, column: t.column, direction: t.direction, + merge: t.merge, operator: t.operator, order: t.order, pick: t.pick, diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/feature-steps.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/feature-steps.js index 9dfae43f79..dbfa660ec4 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/feature-steps.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber/feature-steps.js @@ -40,6 +40,7 @@ const __ = graphTraversalModule.statics; const t = traversalModule.t; const P = traversalModule.P; const direction = traversalModule.direction; +const merge = traversalModule.merge; // Determines whether the feature maps (m[]), are deserialized as objects (true) or maps (false). // Use false for GraphSON3. @@ -61,7 +62,8 @@ const parsers = [ [ 'm\\[(.+)\\]', toMap ], [ 'c\\[(.+)\\]', toLambda ], [ 't\\[(.+)\\]', toT ], - [ 'D\\[(.+)\\]', toDirection ] + [ 'D\\[(.+)\\]', toDirection ], + [ 'M\\[(.+)\\]', toMerge ] ].map(x => [ new RegExp('^' + x[0] + '$'), x[1] ]); const ignoreReason = { @@ -357,6 +359,10 @@ function toDirection(value) { return direction[value.toLowerCase()]; } +function toDirection(value) { + return merge[value.toLowerCase()]; +} + function toArray(stringList) { if (stringList === '') { return new Array(0); diff --git a/gremlin-language/src/main/antlr4/Gremlin.g4 b/gremlin-language/src/main/antlr4/Gremlin.g4 index 7d9893e014..deff18b36c 100644 --- a/gremlin-language/src/main/antlr4/Gremlin.g4 +++ b/gremlin-language/src/main/antlr4/Gremlin.g4 @@ -896,6 +896,8 @@ traversalToken traversalMerge : 'onCreate' | 'Merge.onCreate' | 'onMatch' | 'Merge.onMatch' + | 'outV' | 'Merge.outV' + | 'inV' | 'Merge.inV' ; traversalOrder @@ -1381,6 +1383,7 @@ genericLiteral | traversalToken | traversalCardinality | traversalDirection + | traversalMerge | traversalPick | structureVertex | genericLiteralCollection diff --git a/gremlin-python/src/main/python/radish/feature_steps.py b/gremlin-python/src/main/python/radish/feature_steps.py index 4227f56366..9ad453bac7 100644 --- a/gremlin-python/src/main/python/radish/feature_steps.py +++ b/gremlin-python/src/main/python/radish/feature_steps.py @@ -24,7 +24,7 @@ from gremlin_python.structure.graph import Path, Vertex from gremlin_python.process.anonymous_traversal import traversal from gremlin_python.process.graph_traversal import __ from gremlin_python.process.traversal import Barrier, Cardinality, P, TextP, Pop, Scope, Column, Order, Direction, T, \ - Pick, Operator, IO, WithOptions + Pick, Operator, IO, WithOptions, Merge from radish import given, when, then, world from hamcrest import * @@ -238,6 +238,8 @@ def _convert(val, ctx): return T[val[2:-1]] elif isinstance(val, str) and re.match(r"^D\[.*\]$", val): # parse instance of Direction enum return direction[__alias_direction(val[2:-1])] + elif isinstance(val, str) and re.match(r"^M\[.*\]$", val): # parse instance of Merge enum + return Merge[val[2:-1]] elif isinstance(val, str) and re.match(r"^null$", val): # parse null to None return None elif isinstance(val, str) and re.match(r"^true$", val): # parse to boolean diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java index 24635eeaa6..d7ad76ad70 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/features/StepDefinition.java @@ -32,6 +32,7 @@ import org.antlr.v4.runtime.CommonTokenStream; import org.apache.tinkerpop.gremlin.language.grammar.GremlinAntlrToJava; import org.apache.tinkerpop.gremlin.language.grammar.GremlinLexer; import org.apache.tinkerpop.gremlin.language.grammar.GremlinParser; +import org.apache.tinkerpop.gremlin.process.traversal.Merge; import org.apache.tinkerpop.gremlin.process.traversal.Translator; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal; @@ -140,6 +141,7 @@ public final class StepDefinition { add(Pair.with(Pattern.compile("e\\[(.+)\\]\\.sid"), s -> world.convertIdToScript(getEdgeId(g, s), Edge.class))); add(Pair.with(Pattern.compile("t\\[(.*)\\]"), s -> String.format("T.%s", s))); add(Pair.with(Pattern.compile("D\\[(.*)\\]"), s -> String.format("Direction.%s", s))); + add(Pair.with(Pattern.compile("M\\[(.*)\\]"), s -> String.format("Merge.%s", s))); // the following force ignore conditions as they cannot be parsed by the grammar at this time. the grammar will // need to be modified to handle them or perhaps these tests stay relegated to the JVM in some way for certain @@ -216,6 +218,7 @@ public final class StepDefinition { add(Pair.with(Pattern.compile("t\\[(.*)\\]"), T::valueOf)); add(Pair.with(Pattern.compile("D\\[(.*)\\]"), Direction::valueOf)); + add(Pair.with(Pattern.compile("M\\[(.*)\\]"), Merge::valueOf)); add(Pair.with(Pattern.compile("c\\[(.*)\\]"), s -> { throw new AssumptionViolatedException("This test uses a lambda as a parameter which is not supported by gremlin-language"); @@ -277,8 +280,13 @@ public final class StepDefinition { @Given("the traversal of") public void theTraversalOf(final String docString) { - final String gremlin = tryUpdateDataFilePath(docString); - traversal = parseGremlin(applyParameters(gremlin)); + try { + final String gremlin = tryUpdateDataFilePath(docString); + traversal = parseGremlin(applyParameters(gremlin)); + } catch (Exception ex) { + ex.printStackTrace(); + error = ex; + } } @When("iterated to list") @@ -290,6 +298,15 @@ public final class StepDefinition { } } + @When("debug iterated to list") + public void debugIteratedToList() { + try { + result = traversal.toList(); + } catch (Exception ex) { + error = ex; + } + } + @When("iterated next") public void iteratedNext() { try { @@ -357,6 +374,13 @@ public final class StepDefinition { assertEquals(count.longValue(), ((GraphTraversal) parseGremlin(applyParameters(gremlin))).count().next()); } + @Then("debug the graph should return {int} for count of {string}") + public void debugTheGraphShouldReturnForCountOf(final Integer count, final String gremlin) { + assertThatNoErrorWasThrown(); + + assertEquals(count.longValue(), ((GraphTraversal) parseGremlin(applyParameters(gremlin))).count().next()); + } + @Then("the result should be empty") public void theResultShouldBeEmpty() { assertThatNoErrorWasThrown(); diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/ProcessLimitedStandardSuite.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/ProcessLimitedStandardSuite.java index c24f69bf8c..2b32695ff4 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/ProcessLimitedStandardSuite.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/ProcessLimitedStandardSuite.java @@ -26,6 +26,8 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.ComplexTest; import org.apache.tinkerpop.gremlin.process.traversal.step.OrderabilityTest; import org.apache.tinkerpop.gremlin.process.traversal.step.TernaryBooleanLogicsTest; import org.apache.tinkerpop.gremlin.process.traversal.step.map.MatchTest; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.MergeEdgeTest; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.MergeVertexTest; import org.apache.tinkerpop.gremlin.process.traversal.step.map.ProfileTest; import org.apache.tinkerpop.gremlin.process.traversal.step.map.WriteTest; import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.ExplainTest; @@ -59,31 +61,34 @@ public class ProcessLimitedStandardSuite extends AbstractGremlinSuite { */ private static final Class<?>[] allTests = new Class<?>[]{ - MatchTest.CountMatchTraversals.class, - MatchTest.GreedyMatchTraversals.class, - ProfileTest.Traversals.class, - WriteTest.Traversals.class, - ExplainTest.Traversals.class, - SideEffectTest.Traversals.class, - SubgraphTest.Traversals.class, - TreeTest.Traversals.class, + // MergeVertexTest.Traversals.class, + MergeEdgeTest.Traversals.class - // compliance - ComplexTest.Traversals.class, - CoreTraversalTest.class, - TraversalInterruptionTest.class, - - // creations - TranslationStrategyProcessTest.class, - - // decorations - ElementIdStrategyProcessTest.class, - EventStrategyProcessTest.class, - PartitionStrategyProcessTest.class, - - // optimizations - IncidentToAdjacentStrategyProcessTest.class, - EarlyLimitStrategyProcessTest.class, + // MatchTest.CountMatchTraversals.class, + // MatchTest.GreedyMatchTraversals.class, + // ProfileTest.Traversals.class, + // WriteTest.Traversals.class, + // ExplainTest.Traversals.class, + // SideEffectTest.Traversals.class, + // SubgraphTest.Traversals.class, + // TreeTest.Traversals.class, + // + // // compliance + // ComplexTest.Traversals.class, + // CoreTraversalTest.class, + // TraversalInterruptionTest.class, + // + // // creations + // TranslationStrategyProcessTest.class, + // + // // decorations + // ElementIdStrategyProcessTest.class, + // EventStrategyProcessTest.class, + // PartitionStrategyProcessTest.class, + // + // // optimizations + // IncidentToAdjacentStrategyProcessTest.class, + // EarlyLimitStrategyProcessTest.class, }; /** diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeTest.java index 15dffee512..aa8a58276f 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeTest.java @@ -62,6 +62,10 @@ public abstract class MergeEdgeTest extends AbstractGremlinTest { public abstract Traversal<Map<Object,Object>, Edge> get_g_injectXlabel_knows_out_marko_in_vadasX_mergeE(); + public abstract Traversal<Edge, Edge> get_g_mergeE_ifXxX_returnXxX_else_createXyX(); + + public abstract Traversal<Edge, Edge> get_g_mergeE_with_outV_inV_options(); +/* @Test @FeatureRequirementSet(FeatureRequirementSet.Package.SIMPLE) public void g_V_mergeEXlabel_self_weight_05X() { @@ -192,6 +196,43 @@ public abstract class MergeEdgeTest extends AbstractGremlinTest { assertEquals(1, IteratorUtils.count(g.V(100).out("knows").hasId(101))); } + @Test + @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_USER_SUPPLIED_IDS) + @FeatureRequirement(featureClass = Graph.Features.EdgeFeatures.class, feature = Graph.Features.EdgeFeatures.FEATURE_USER_SUPPLIED_IDS) + @FeatureRequirementSet(FeatureRequirementSet.Package.SIMPLE) + public void g_mergeE_ifXxX_returnXxX_else_createXyX() { + g.addV("person").property(T.id, 1) + .addV("person").property(T.id, 2) + .addV("person").property(T.id, 3) + .addV("person").property(T.id, 4).iterate(); + + final Traversal<Edge, Edge> traversal = get_g_mergeE_ifXxX_returnXxX_else_createXyX(); + printTraversalForm(traversal); + + assertEquals(1, IteratorUtils.count(traversal)); + assertEquals(4, IteratorUtils.count(g.V())); + assertEquals(1, IteratorUtils.count(g.E())); + assertEquals(0, IteratorUtils.count(g.E(101))); + assertEquals(1, IteratorUtils.count(g.E(201))); + assertEquals(0, IteratorUtils.count(g.V(1).out("knows").hasId(2))); + assertEquals(1, IteratorUtils.count(g.V(3).out("likes").hasId(4))); + } +*/ + @Test + @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_USER_SUPPLIED_IDS) + @FeatureRequirementSet(FeatureRequirementSet.Package.SIMPLE) + public void g_mergeE_with_outV_inV_options() { + g.addV("person").property(T.id, 1) + .addV("person").property(T.id, 2) + .iterate(); + + final Traversal<Edge, Edge> traversal = get_g_mergeE_with_outV_inV_options(); + printTraversalForm(traversal); + + assertEquals(1, IteratorUtils.count(traversal)); + assertEquals(1, IteratorUtils.count(g.V(1).out("knows").hasId(2))); + } + public static class Traversals extends MergeEdgeTest { @Override @@ -242,5 +283,18 @@ public abstract class MergeEdgeTest extends AbstractGremlinTest { public Traversal<Map<Object,Object>, Edge> get_g_injectXlabel_knows_out_marko_in_vadasX_mergeE() { return g.inject((Map<Object,Object>) asMap(T.label, "knows", Direction.IN, new ReferenceVertex(101), Direction.OUT, new ReferenceVertex(100))).mergeE(); } + + @Override + public Traversal<Edge, Edge> get_g_mergeE_ifXxX_returnXxX_else_createXyX() { + return g.mergeE(asMap(T.id, 101, T.label, "knows", Direction.OUT, 1, Direction.IN, 2)) + .option(Merge.onCreate, asMap(T.id, 201, T.label, "likes", Direction.OUT, 3, Direction.IN, 4)); + } + + @Override + public Traversal<Edge,Edge> get_g_mergeE_with_outV_inV_options() { + return g.mergeE(asMap(T.label, "knows", Direction.OUT, Merge.outV, Direction.IN, Merge.inV)) + .option(Merge.outV, asMap(T.id, 1)) + .option(Merge.inV, asMap(T.id, 2)); + } } } \ No newline at end of file diff --git a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeEdge.feature b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeEdge.feature index 821066ee75..35e7f5a4c1 100644 --- a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeEdge.feature +++ b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeEdge.feature @@ -23,10 +23,6 @@ Feature: Step - mergeE() # When the test name places a "1" following a "name" it just means that the test is using the id rather # than vertex reference. # - # g_V_mergeEXlabel_self_weight_05X - # - mergeE(Map) using the vertex of current traverser as reference - # - vertex already exists - # - results in a self edge # g_mergeEXlabel_knows_out_marko_in_vadasX # g_mergeEXlabel_knows_out_marko1_in_vadas1X # g_mergeEXlabel_knows_out_marko_in_vadasX_aliased_direction @@ -85,10 +81,6 @@ Feature: Step - mergeE() # - mergeE(Map) specifying out/in for vertices in the match/create with option(Map) # - vertices and edge already exist # - results in the existing edge getting a new property - # g_V_mapXmergeEXlabel_self_weight_05XX - # - mergeE(Map) using the vertex of current traverser as reference - testing child traversal - # - vertex already exists - # - results in a self edge # g_injectXlabel_knows_out_marko_in_vadas_label_self_out_vadas_in_vadasX # - mergeE() using the map of current traverser as reference # - vertices already exists @@ -97,7 +89,7 @@ Feature: Step - mergeE() # g_V_mergeEXnullX # - mergeE(null) and no option # - vertices already exists - # - results in no matched edges and no new edge + # - Directions not specified - error # g_mergeEXemptyX # - mergeE(empty) and no option # - vertex present and no edges @@ -118,18 +110,10 @@ Feature: Step - mergeE() # - mergeE(empty) and no option # - two vertices exist # - results in two new self edges, one for each vertex - # g_V_mergeEXlabel_knowsX_optionXonCreate_created_YX_optionXonMatch_created_NX_exists_updated - # - mergeE(Map) with onCreate(Map) and onMatch(Map) - # - two vertices, two "knows" edges and one "friends" edge - # - results in two returned "knows" edges where one has a property updated and the other a property added # g_V_mergeEXemptyX_optionXonCreate_nullX # - mergeE(empty) with onCreate(null) # - one existing vertex # - results in no new edges because onCreate() was null - # g_V_mergeEXlabel_selfX_optionXonCreate_emptyX - # - mergeE(Map) with onCreate(empty) - # - one existing vertex - # - results in one new edge with defaults because searchCreate is overridden # g_V_mergeEXlabel_selfX_optionXonMatch_nullX # - mergeE(Map) with onMatch(null) # - one existing vertex and one edge @@ -179,63 +163,21 @@ Feature: Step - mergeE() And the graph should return 0 for count of "g.E().properties()" And the graph should return 1 for count of "g.V()" - Scenario: g_V_mergeEXlabel_selfX_optionXonCreate_emptyX + Scenario: g_V_mergeEXemptyX_optionXonCreate_nullX Given the empty graph And the graph initializer of """ g.addV("person").property("name", "marko").property("age", 29) """ - And using the parameter xx1 defined as "m[{\"t[label]\": \"self\"}]" + And using the parameter xx1 defined as "m[{\"t[label]\": \"self\", \"D[OUT]\":\"M[outV]\", \"D[IN]\":\"M[inV]\"}]" And the traversal of """ - g.V().mergeE(xx1).option(Merge.onCreate,[:]) + g.V().as("v").mergeE(xx1).option(Merge.onCreate,null).option(Merge.outV,select("v")).option(Merge.inV,select("v")) """ When iterated to list Then the result should have a count of 1 And the graph should return 1 for count of "g.E()" And the graph should return 1 for count of "g.V()" - And the graph should return 0 for count of "g.E().hasLabel(\"self\")" - - Scenario: g_V_mergeEXemptyX_optionXonCreate_nullX - Given the empty graph - And the graph initializer of - """ - g.addV("person").property("name", "marko").property("age", 29) - """ - And the traversal of - """ - g.V().mergeE([:]).option(Merge.onCreate,null) - """ - When iterated to list - Then the result should be empty - And the graph should return 0 for count of "g.E()" - And the graph should return 1 for count of "g.V()" - - @UserSuppliedVertexIds - Scenario: g_V_mergeEXlabel_knowsX_optionXonCreate_created_YX_optionXonMatch_created_NX_exists_updated - Given the empty graph - And the graph initializer of - """ - g.addV("person").property(T.id, 100).property("name", "marko").as("a"). - addV("person").property(T.id, 101).property("name", "vadas").as("b"). - addE("knows").from("a").to("b").property("created","Y"). - addE("knows").from("b").to("a"). - addE("friends").from("b").to("a") - """ - And using the parameter xx1 defined as "m[{\"t[label]\": \"knows\"}]" - And using the parameter xx2 defined as "m[{\"t[label]\": \"knows\", \"D[OUT]\":\"v[100]\", \"D[IN]\":\"v[101]\",\"created\":\"Y\"}]" - And using the parameter xx3 defined as "m[{\"created\":\"N\"}]" - And the traversal of - """ - g.V().mergeE(xx1).option(Merge.onCreate,xx2).option(Merge.onMatch,xx3) - """ - When iterated to list - Then the result should have a count of 2 - And the graph should return 2 for count of "g.V()" - And the graph should return 3 for count of "g.E()" - And the graph should return 0 for count of "g.E().hasLabel(\"knows\").has(\"created\",\"Y\")" - And the graph should return 2 for count of "g.E().hasLabel(\"knows\").has(\"created\",\"N\")" - And the graph should return 1 for count of "g.E().hasLabel(\"friends\")" Scenario: g_mergeEXemptyX_exists Given the empty graph @@ -273,15 +215,17 @@ Feature: Step - mergeE() g.addV("person").property("name", "marko").property("age", 29). addV("person").property("name", "vadas").property("age", 27) """ + And using the parameter xx1 defined as "m[{\"t[label]\": \"self\", \"D[OUT]\":\"M[outV]\", \"D[IN]\":\"M[inV]\"}]" And the traversal of """ - g.V().mergeE([:]) + g.V().as("v").mergeE(xx1).option(Merge.outV,select("v")).option(Merge.inV,select("v")) """ When iterated to list Then the result should have a count of 2 And the graph should return 2 for count of "g.E()" And the graph should return 2 for count of "g.V()" + # null same as empty Scenario: g_mergeEXnullX Given the empty graph And the graph initializer of @@ -293,10 +237,9 @@ Feature: Step - mergeE() g.mergeE(null) """ When iterated to list - Then the result should be empty - And the graph should return 0 for count of "g.E()" - And the graph should return 1 for count of "g.V()" + Then the traversal will raise an error + # Directions not specified Scenario: g_V_mergeEXnullX Given the empty graph And the graph initializer of @@ -308,26 +251,7 @@ Feature: Step - mergeE() g.V().mergeE(null) """ When iterated to list - Then the result should be empty - And the graph should return 0 for count of "g.E()" - And the graph should return 1 for count of "g.V()" - - Scenario: g_V_mergeEXlabel_self_weight_05X - Given the empty graph - And the graph initializer of - """ - g.addV("person").property("name", "marko").property("age", 29) - """ - And using the parameter xx1 defined as "m[{\"t[label]\": \"self\", \"weight\":\"d[0.5].d\"}]" - And the traversal of - """ - g.V().mergeE(xx1) - """ - When iterated to list - Then the result should have a count of 1 - And the graph should return 1 for count of "g.E()" - And the graph should return 1 for count of "g.V()" - And the graph should return 1 for count of "g.V().has(\"person\",\"name\",\"marko\").as(\"a\").outE(\"self\").has(\"weight\",0.5).inV().where(eq(\"a\"))" + Then the traversal will raise an error @UserSuppliedVertexIds Scenario: g_mergeEXlabel_knows_out_marko_in_vadasX @@ -668,23 +592,6 @@ Feature: Step - mergeE() And the graph should return 1 for count of "g.E().hasLabel(\"knows\").has(\"created\",\"Y\")" And the graph should return 0 for count of "g.E().hasLabel(\"knows\").has(\"created\",\"N\")" - Scenario: g_V_mapXmergeEXlabel_self_weight_05XX - Given the empty graph - And the graph initializer of - """ - g.addV("person").property("name", "marko").property("age", 29) - """ - And using the parameter xx1 defined as "m[{\"t[label]\": \"self\", \"weight\":\"d[0.5].d\"}]" - And the traversal of - """ - g.V().map(__.mergeE(xx1)) - """ - When iterated to list - Then the result should have a count of 1 - And the graph should return 1 for count of "g.E()" - And the graph should return 1 for count of "g.V()" - And the graph should return 1 for count of "g.V().has(\"person\",\"name\",\"marko\").as(\"a\").outE(\"self\").has(\"weight\",0.5).inV().where(eq(\"a\"))" - @UserSuppliedVertexIds Scenario: g_mergeEXlabel_knows_out_marko_in_vadasX_aliased_direction Given the empty graph @@ -747,4 +654,163 @@ Feature: Step - mergeE() And the graph should return 2 for count of "g.V()" And the graph should return 0 for count of "g.E().hasLabel(\"knows\").has(\"created\",\"Y\")" And the graph should return 1 for count of "g.E().hasLabel(\"knows\").has(\"created\",\"N\")" - And the graph should return 0 for count of "g.E().hasLabel(\"knows\").has(\"weight\")" \ No newline at end of file + And the graph should return 0 for count of "g.E().hasLabel(\"knows\").has(\"weight\")" + + + @UserSuppliedVertexIds + Scenario: g_mergeE_with_outVinV_options_map + Given the empty graph + And the graph initializer of + """ + g.addV("person").property(T.id, 1).property("name", "a") + .addV("person").property(T.id, 2).property("name", "b") + """ + And using the parameter xx1 defined as "m[{ \"D[OUT]\": \"M[outV]\", \"D[IN]\": \"M[inV]\", \"t[label]\": \"knows\"}]" + And using the parameter xx2 defined as "m[{\"t[id]\": 1}]" + And using the parameter xx3 defined as "m[{\"t[id]\": 2}]" + And the traversal of + """ + g.mergeE(xx1).option(outV, xx2).option(inV, xx3) + """ + When iterated to list + Then the result should have a count of 1 + And the graph should return 2 for count of "g.V()" + And the graph should return 1 for count of "g.V(1).out(\"knows\").hasId(2)" + + @UserSuppliedVertexIds + Scenario: g_mergeE_with_outVinV_options_select + Given the empty graph + And the graph initializer of + """ + g.addV("person").property(T.id, 1).property("name", "a") + .addV("person").property(T.id, 2).property("name", "b") + """ + And using the parameter xx1 defined as "m[{ \"D[OUT]\": \"M[outV]\", \"D[IN]\": \"M[inV]\", \"t[label]\": \"knows\"}]" + And the traversal of + """ + g.V(1).as("x").V(2).as("y").mergeE(xx1).option(outV, select("x")).option(inV, select("y")) + """ + When iterated to list + Then the result should have a count of 1 + And the graph should return 2 for count of "g.V()" + And the graph should return 1 for count of "g.V(1).out(\"knows\").hasId(2)" + + # onCreate inherits from merge and can specify an eid + @UserSuppliedVertexIds + @UserSuppliedEdgeIds + Scenario: g_mergeE_with_eid_specified_and_inheritance + Given the empty graph + And the graph initializer of + """ + g.addV("person").property(T.id, 1).property("name", "a") + .addV("person").property(T.id, 2).property("name", "b") + """ + And using the parameter xx1 defined as "m[{\"D[OUT]\": \"v[1]\", \"D[IN]\": \"v[2]\", \"t[label]\": \"knows\"}]" + And using the parameter xx2 defined as "m[{\"t[id]\": 201}]" + And the traversal of + """ + g.mergeE(xx1).option(onCreate, xx2) + """ + When iterated to list + Then the result should have a count of 1 + And the graph should return 2 for count of "g.V()" + And the graph should return 1 for count of "g.E()" + And the graph should return 1 for count of "g.E(201)" + And the graph should return 1 for count of "g.V(1).out(\"knows\").hasId(2)" + + # onCreate inherits from merge and can specify an eid + @UserSuppliedVertexIds + @UserSuppliedEdgeIds + Scenario: g_mergeE_with_eid_specified_and_inheritance + Given the empty graph + And the graph initializer of + """ + g.addV("person").property(T.id, 1).property("name", "a") + .addV("person").property(T.id, 2).property("name", "b") + """ + And using the parameter xx1 defined as "m[{\"t[id]\": 201}]" + And using the parameter xx2 defined as "m[{\"D[OUT]\": \"v[1]\", \"D[IN]\": \"v[2]\", \"t[label]\": \"knows\"}]" + And the traversal of + """ + g.mergeE(xx1).option(onCreate, xx2) + """ + When iterated to list + Then the result should have a count of 1 + And the graph should return 2 for count of "g.V()" + And the graph should return 1 for count of "g.E()" + And the graph should return 1 for count of "g.E(201)" + And the graph should return 1 for count of "g.V(1).out(\"knows\").hasId(2)" + + # cannot override Direction.OUT in onCreate + @UserSuppliedVertexIds + Scenario: g_mergeE_outV_override_prohibited + Given the empty graph + And the graph initializer of + """ + g.addV("person").property(T.id, 1).property("name", "a") + .addV("person").property(T.id, 2).property("name", "b") + """ + And using the parameter xx1 defined as "m[{\"D[OUT]\" : \"v[1]\", \"D[IN]\" : \"v[2]\", \"t[label]\": \"knows\"}]" + And using the parameter xx2 defined as "m[{\"D[OUT]\" : \"v[2]\"}]" + And the traversal of + """ + g.mergeE(xx1).option(onCreate, xx2) + """ + When iterated to list + Then the traversal will raise an error + + # cannot override Direction.IN in onCreate + @UserSuppliedVertexIds + Scenario: g_mergeE_inV_override_prohibited + Given the empty graph + And the graph initializer of + """ + g.addV("person").property(T.id, 1).property("name", "a") + .addV("person").property(T.id, 2).property("name", "b") + """ + And using the parameter xx1 defined as "m[{\"D[OUT]\" : \"v[1]\", \"D[IN]\" : \"v[2]\", \"t[label]\": \"knows\"}]" + And using the parameter xx2 defined as "m[{\"D[IN]\" : \"v[1]\"}]" + And the traversal of + """ + g.mergeE(xx1).option(onCreate, xx2) + """ + When iterated to list + Then the traversal will raise an error + + # cannot override T.label in onCreate + @UserSuppliedVertexIds + Scenario: g_mergeE_label_override_prohibited + Given the empty graph + And the graph initializer of + """ + g.addV("person").property(T.id, 1).property("name", "a") + .addV("person").property(T.id, 2).property("name", "b") + """ + And using the parameter xx1 defined as "m[{\"D[OUT]\" : \"v[1]\", \"D[IN]\" : \"v[2]\", \"t[label]\": \"knows\"}]" + And using the parameter xx2 defined as "m[{\"t[label]\": \"likes\"}]" + And the traversal of + """ + g.mergeE(xx1).option(onCreate, xx2) + """ + When iterated to list + Then the traversal will raise an error + + # cannot override T.id in onCreate + @UserSuppliedVertexIds + @UserSuppliedEdgeIds + Scenario: g_mergeE_id_override_prohibited + Given the empty graph + And the graph initializer of + """ + g.addV("person").property(T.id, 1).property("name", "a") + .addV("person").property(T.id, 2).property("name", "b") + """ + And using the parameter xx1 defined as "m[{\"D[OUT]\" : \"v[1]\", \"D[IN]\" : \"v[2]\", \"t[label]\": \"knows\", \"t[id]\": \"101\"}]" + And using the parameter xx2 defined as "m[{\"t[id]\": \"201\"}]" + And the traversal of + """ + g.mergeE(xx1).option(onCreate, xx2) + """ + When iterated to list + Then the traversal will raise an error + diff --git a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeVertex.feature b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeVertex.feature index ee1bf1d017..c0283f8d3a 100644 --- a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeVertex.feature +++ b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/test/features/map/MergeVertex.feature @@ -139,9 +139,8 @@ Feature: Step - mergeV() And using the parameter xx1 defined as "m[{\"t[label]\": null, \"name\":\"marko\"}]" And the traversal of """ - g.mergeV(null).option(Merge.onCreate,xx1) + g.mergeV(xx1) """ - When iterated to list Then the traversal will raise an error Scenario: g_V_mergeVXnullX_optionXonCreate_label_null_name_markoX @@ -153,9 +152,8 @@ Feature: Step - mergeV() And using the parameter xx1 defined as "m[{\"t[label]\": null, \"name\":\"marko\"}]" And the traversal of """ - g.V().mergeV(null).option(Merge.onCreate,xx1) + g.V().mergeV(xx1) """ - When iterated to list Then the traversal will raise an error Scenario: g_mergeVXlabel_person_name_stephenX_optionXonCreate_nullX @@ -170,9 +168,10 @@ Feature: Step - mergeV() g.mergeV(xx1).option(Merge.onCreate, null) """ When iterated to list - Then the result should have a count of 0 - And the graph should return 1 for count of "g.V()" + Then the result should have a count of 1 + And the graph should return 2 for count of "g.V()" And the graph should return 1 for count of "g.V().has(\"person\",\"name\",\"marko\")" + And the graph should return 1 for count of "g.V().has(\"person\",\"name\",\"stephen\")" Scenario: g_V_mergeVXlabel_person_name_stephenX_optionXonCreate_nullX Given the empty graph @@ -186,9 +185,10 @@ Feature: Step - mergeV() g.V().mergeV(xx1).option(Merge.onCreate, null) """ When iterated to list - Then the result should have a count of 0 - And the graph should return 1 for count of "g.V()" + Then the result should have a count of 1 + And the graph should return 2 for count of "g.V()" And the graph should return 1 for count of "g.V().has(\"person\",\"name\",\"marko\")" + And the graph should return 1 for count of "g.V().has(\"person\",\"name\",\"stephen\")" Scenario: g_mergeVXnullX_optionXonCreate_emptyX Given the empty graph @@ -202,7 +202,7 @@ Feature: Step - mergeV() """ When iterated to list Then the result should have a count of 1 - And the graph should return 2 for count of "g.V()" + And the graph should return 1 for count of "g.V()" Scenario: g_V_mergeVXnullX_optionXonCreate_emptyX Given the empty graph @@ -216,7 +216,7 @@ Feature: Step - mergeV() """ When iterated to list Then the result should have a count of 1 - And the graph should return 2 for count of "g.V()" + And the graph should return 1 for count of "g.V()" Scenario: g_mergeVXemptyX_no_existing Given the empty graph @@ -280,7 +280,7 @@ Feature: Step - mergeV() g.mergeV(null) """ When iterated to list - Then the result should have a count of 0 + Then the result should have a count of 1 And the graph should return 1 for count of "g.V()" Scenario: g_V_mergeVXnullX @@ -294,7 +294,7 @@ Feature: Step - mergeV() g.V().mergeV(null) """ When iterated to list - Then the result should have a count of 0 + Then the result should have a count of 1 And the graph should return 1 for count of "g.V()" Scenario: g_mergeVXlabel_person_name_stephenX @@ -654,4 +654,79 @@ Feature: Step - mergeV() When iterated to list Then the result should have a count of 1 And the graph should return 1 for count of "g.V().has(\"person\",\"name\",\"marko\").has(\"age\", 19)" - And the graph should return 1 for count of "g.V().has(\"person\",\"name\",\"marko\").has(\"age\")" \ No newline at end of file + And the graph should return 1 for count of "g.V().has(\"person\",\"name\",\"marko\").has(\"age\")" + + + + # onCreate inheritance from merge + Scenario: g_mergeV_onCreate_inheritance_existing + Given the empty graph + And the graph initializer of + """ + g.addV("person").property("name", "mike").property(T.id, 1) + """ + And using the parameter xx1 defined as "m[{\"t[id]\": 1}]" + And using the parameter xx2 defined as "m[{\"t[label]\": \"person\", \"name\":\"mike\"}]" + And the traversal of + """ + g.mergeV(xx1).option(Merge.onCreate, xx2) + """ + When iterated to list + Then the result should have a count of 1 + And the graph should return 1 for count of "g.V()" + And the graph should return 1 for count of "g.V(1).has(\"person\",\"name\",\"mike\")" + + # onCreate inheritance from merge + Scenario: g_mergeV_onCreate_inheritance_new + Given the empty graph + And using the parameter xx1 defined as "m[{\"t[id]\": 1}]" + And using the parameter xx2 defined as "m[{\"t[label]\": \"person\", \"name\":\"mike\"}]" + And the traversal of + """ + g.mergeV(xx1).option(Merge.onCreate, xx2) + """ + When iterated to list + Then the result should have a count of 1 + And the graph should return 1 for count of "g.V()" + And the graph should return 1 for count of "g.V(1).has(\"person\",\"name\",\"mike\")" + + # onCreate inheritance from merge + Scenario: g_mergeV_onCreate_inheritance_new + Given the empty graph + And using the parameter xx1 defined as "m[{\"t[label]\": \"person\", \"name\":\"mike\"}]" + And using the parameter xx2 defined as "m[{\"t[id]\": 1}]" + And the traversal of + """ + g.mergeV(xx1).option(Merge.onCreate, xx2) + """ + When iterated to list + Then the result should have a count of 1 + And the graph should return 1 for count of "g.V()" + And the graph should return 1 for count of "g.V(1).has(\"person\",\"name\",\"mike\")" + + # cannot override T.label in onCreate + @UserSuppliedVertexIds + Scenario: g_mergeE_label_override_prohibited + Given the empty graph + And using the parameter xx1 defined as "m[{\"t[label]\": \"a\"}]" + And using the parameter xx2 defined as "m[{\"t[label]\": \"b\"}]" + And the traversal of + """ + g.mergeV(xx1).option(onCreate, xx2) + """ + When iterated to list + Then the traversal will raise an error + + # cannot override T.id in onCreate + @UserSuppliedVertexIds + Scenario: g_mergeE_id_override_prohibited + Given the empty graph + And using the parameter xx1 defined as "m[{\"t[id]\": 1}]" + And using the parameter xx2 defined as "m[{\"t[id]\": 2}]" + And the traversal of + """ + g.mergeV(xx1).option(onCreate, xx2) + """ + When iterated to list + Then the traversal will raise an error + diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/map/TinkerMergeEdgeStep.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/map/TinkerMergeEdgeStep.java deleted file mode 100644 index 482fea1214..0000000000 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/map/TinkerMergeEdgeStep.java +++ /dev/null @@ -1,115 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.step.map; - -import org.apache.tinkerpop.gremlin.process.traversal.Merge; -import org.apache.tinkerpop.gremlin.process.traversal.step.map.MergeEdgeStep; -import org.apache.tinkerpop.gremlin.structure.Direction; -import org.apache.tinkerpop.gremlin.structure.Edge; -import org.apache.tinkerpop.gremlin.structure.Property; -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.tinkergraph.structure.TinkerGraph; -import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerHelper; -import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; - -import java.util.Iterator; -import java.util.Map; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Stream; - -/** - * Optimizes {@code mergeE()} searches by attempting to use an index where possible. - */ -public class TinkerMergeEdgeStep<S> extends MergeEdgeStep<S> { - - public TinkerMergeEdgeStep(final MergeEdgeStep step) { - super(step.getTraversal(), step.isStart(), step.getSearchCreateTraversal()); - if (step.getOnMatchTraversal() != null) this.addChildOption(Merge.onMatch, step.getOnMatchTraversal()); - if (step.getOnCreateTraversal() != null) this.addChildOption(Merge.onCreate, step.getOnCreateTraversal()); - if (step.getCallbackRegistry() != null) this.callbackRegistry = step.getCallbackRegistry(); - } - - @Override - protected Stream<Edge> createSearchStream(final Map<Object, Object> search) { - final TinkerGraph graph = (TinkerGraph) this.getTraversal().getGraph().get(); - Optional<String> firstIndex = Optional.empty(); - - Stream<Edge> stream; - // prioritize lookup by id but otherwise attempt an index lookup - if (null == search) { - return Stream.empty(); - } else if (search.containsKey(T.id)) { - stream = IteratorUtils.stream(graph.edges(search.get(T.id))); - } else { - // look for the first index we can find - that's the lucky winner. may or may not be the most selective - final Set<String> indexedKeys = graph.getIndexedKeys(Edge.class); - firstIndex = search.keySet().stream(). - filter(k -> k instanceof String). - map(k -> (String) k). - filter(indexedKeys::contains).findFirst(); - - // use the index if possible otherwise just in memory filter - stream = firstIndex.map(s -> TinkerHelper.queryEdgeIndex(graph, s, search.get(s)).stream().map(e -> (Edge) e)). - orElseGet(() -> { - if (search.containsKey(Direction.BOTH)) { - // filter self-edges with distinct() - return IteratorUtils.stream(graph.vertices(search.get(Direction.BOTH))).flatMap(v -> IteratorUtils.stream(v.edges(Direction.BOTH))).distinct(); - } else if (search.containsKey(Direction.OUT)) { - return IteratorUtils.stream(graph.vertices(search.get(Direction.OUT))).flatMap(v -> IteratorUtils.stream(v.edges(Direction.OUT))); - } else if (search.containsKey(Direction.IN)) { - return IteratorUtils.stream(graph.vertices(search.get(Direction.IN))).flatMap(v -> IteratorUtils.stream(v.edges(Direction.IN))); - } else { - return IteratorUtils.stream(graph.edges()); - } - }); - } - - final Optional<String> indexUsed = firstIndex; - stream = stream.filter(e -> { - // try to match on all search criteria skipping T.id as it was handled above - return search.entrySet().stream().filter(kv -> { - final Object k = kv.getKey(); - return k != T.id && !(indexUsed.isPresent() && indexUsed.get().equals(k)); - }).allMatch(kv -> { - if (kv.getKey() == T.label) { - return e.label().equals(kv.getValue()); - } else if (kv.getKey() instanceof Direction) { - final Direction direction = (Direction) kv.getKey(); - - // try to take advantage of string id conversions of the graph by doing a lookup rather - // than direct compare on id - final Iterator<Vertex> found = graph.vertices(kv.getValue()); - final Iterator<Vertex> dfound = e.vertices(direction); - final boolean matched = found.hasNext() && dfound.next().equals(found.next()); - CloseableIterator.closeIterator(found); - CloseableIterator.closeIterator(dfound); - return matched; - } else { - final Property<Object> vp = e.property(kv.getKey().toString()); - return vp.isPresent() && kv.getValue().equals(vp.value()); - } - }); - }); - - return stream; - } -} diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/map/TinkerMergeVertexStep.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/map/TinkerMergeVertexStep.java deleted file mode 100644 index 9c3cc03c3a..0000000000 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/step/map/TinkerMergeVertexStep.java +++ /dev/null @@ -1,90 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.step.map; - -import org.apache.tinkerpop.gremlin.process.traversal.Merge; -import org.apache.tinkerpop.gremlin.process.traversal.Traversal; -import org.apache.tinkerpop.gremlin.process.traversal.step.map.MergeVertexStep; -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.VertexProperty; -import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerGraph; -import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerHelper; -import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; - -import java.util.Map; -import java.util.Optional; -import java.util.Set; -import java.util.stream.Stream; - -/** - * Optimizes {@code mergeV()} searches by attempting to use an index where possible. - */ -public class TinkerMergeVertexStep<S> extends MergeVertexStep<S> { - public TinkerMergeVertexStep(final MergeVertexStep step) { - super(step.getTraversal(), step.isStart(), step.getSearchCreateTraversal()); - if (step.getOnMatchTraversal() != null) this.addChildOption(Merge.onMatch, step.getOnMatchTraversal()); - if (step.getOnCreateTraversal() != null) this.addChildOption(Merge.onCreate, step.getOnCreateTraversal()); - if (step.getCallbackRegistry() != null) this.callbackRegistry = step.getCallbackRegistry(); - } - - @Override - protected Stream<Vertex> createSearchStream(final Map<Object, Object> search) { - final TinkerGraph graph = (TinkerGraph) this.getTraversal().getGraph().get(); - Optional<String> firstIndex = Optional.empty(); - - Stream<Vertex> stream; - // prioritize lookup by id but otherwise attempt an index lookup - if (null == search) { - return Stream.empty(); - } else if (search.containsKey(T.id)) { - stream = IteratorUtils.stream(graph.vertices(search.get(T.id))); - } else { - // look for the first index we can find - that's the lucky winner. may or may not be the most selective - final Set<String> indexedKeys = graph.getIndexedKeys(Vertex.class); - firstIndex = search.keySet().stream(). - filter(k -> k instanceof String). - map(k -> (String) k). - filter(indexedKeys::contains).findFirst(); - - // use the index if possible otherwise just in memory filter - stream = firstIndex.map(s -> TinkerHelper.queryVertexIndex(graph, s, search.get(s)).stream().map(v -> (Vertex) v)). - orElseGet(() -> IteratorUtils.stream(graph.vertices())); - } - - final Optional<String> indexUsed = firstIndex; - stream = stream.filter(v -> { - // try to match on all search criteria skipping T.id as it was handled above - return search.entrySet().stream().filter(kv -> { - final Object k = kv.getKey(); - return k != T.id && !(indexUsed.isPresent() && indexUsed.get().equals(k)); - }).allMatch(kv -> { - if (kv.getKey() == T.label) { - return v.label().equals(kv.getValue()); - } else { - final VertexProperty<Object> vp = v.property(kv.getKey().toString()); - return vp.isPresent() && kv.getValue().equals(vp.value()); - } - }); - }); - - return stream; - } -} diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerMergeEVStepStrategy.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerMergeEVStepStrategy.java deleted file mode 100644 index c4b1ea583d..0000000000 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/process/traversal/strategy/optimization/TinkerMergeEVStepStrategy.java +++ /dev/null @@ -1,61 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.strategy.optimization; - -import org.apache.tinkerpop.gremlin.process.traversal.Traversal; -import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; -import org.apache.tinkerpop.gremlin.process.traversal.step.map.MergeEdgeStep; -import org.apache.tinkerpop.gremlin.process.traversal.step.map.MergeVertexStep; -import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy; -import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper; -import org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.step.map.TinkerMergeEdgeStep; -import org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.step.map.TinkerMergeVertexStep; - -/** - * Optimizes {@code mergeV()} and {@code mergeE()} search lookups by using {@link TinkerMergeVertexStep} and - * {@link TinkerMergeEdgeStep} respectively. - */ -public final class TinkerMergeEVStepStrategy extends AbstractTraversalStrategy<TraversalStrategy.ProviderOptimizationStrategy> - implements TraversalStrategy.ProviderOptimizationStrategy { - - private static final TinkerMergeEVStepStrategy INSTANCE = new TinkerMergeEVStepStrategy(); - - private TinkerMergeEVStepStrategy() { - } - - @Override - public void apply(final Traversal.Admin<?, ?> traversal) { - if (TraversalHelper.onGraphComputer(traversal)) - return; - - for (final MergeVertexStep originalMergeVertexStep : TraversalHelper.getStepsOfClass(MergeVertexStep.class, traversal)) { - final TinkerMergeVertexStep tinkerMergeVertexStep = new TinkerMergeVertexStep(originalMergeVertexStep); - TraversalHelper.replaceStep(originalMergeVertexStep, tinkerMergeVertexStep, traversal); - } - - for (final MergeEdgeStep originalMergeEdgeStep : TraversalHelper.getStepsOfClass(MergeEdgeStep.class, traversal)) { - final TinkerMergeEdgeStep tinkerMergeEdgeStep = new TinkerMergeEdgeStep(originalMergeEdgeStep); - TraversalHelper.replaceStep(originalMergeEdgeStep, tinkerMergeEdgeStep, traversal); - } - } - - public static TinkerMergeEVStepStrategy instance() { - return INSTANCE; - } -} diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java index 465b9d7bb2..3a4651e721 100644 --- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java +++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraph.java @@ -39,7 +39,6 @@ import org.apache.tinkerpop.gremlin.tinkergraph.process.computer.TinkerGraphComp import org.apache.tinkerpop.gremlin.tinkergraph.process.computer.TinkerGraphComputerView; import org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.strategy.optimization.TinkerGraphCountStrategy; import org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.strategy.optimization.TinkerGraphStepStrategy; -import org.apache.tinkerpop.gremlin.tinkergraph.process.traversal.strategy.optimization.TinkerMergeEVStepStrategy; import org.apache.tinkerpop.gremlin.tinkergraph.services.TinkerServiceRegistry; import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils; @@ -77,8 +76,7 @@ public final class TinkerGraph implements Graph { static { TraversalStrategies.GlobalCache.registerStrategies(TinkerGraph.class, TraversalStrategies.GlobalCache.getStrategies(Graph.class).clone().addStrategies( TinkerGraphStepStrategy.instance(), - TinkerGraphCountStrategy.instance(), - TinkerMergeEVStepStrategy.instance())); + TinkerGraphCountStrategy.instance())); } private static final Configuration EMPTY_CONFIGURATION = new BaseConfiguration() {{ diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/TinkerGraphFeatureTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/TinkerGraphFeatureTest.java index da655d5437..73a7e1ebca 100644 --- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/TinkerGraphFeatureTest.java +++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/TinkerGraphFeatureTest.java @@ -33,7 +33,9 @@ import org.junit.runner.RunWith; tags = "not @RemoteOnly and not @GraphComputerOnly and not @AllowNullPropertyValues", glue = { "org.apache.tinkerpop.gremlin.features" }, objectFactory = TinkerGraphFeatureTest.TinkerGraphGuiceFactory.class, - features = { "classpath:/org/apache/tinkerpop/gremlin/test/features" }, + features = { "classpath:/org/apache/tinkerpop/gremlin/test/features/map/MergeEdge.feature" + //"classpath:/org/apache/tinkerpop/gremlin/test/features/map/MergeVertex.feature" + }, plugin = {"progress", "junit:target/cucumber.xml"}) public class TinkerGraphFeatureTest {
