TINKERPOP-1650 * Updated setKeepLabels calls to make defensive copies of their input to avoid corruption. * Added a new test to PathRetractionStrategyTest for WhereStep.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/73a982c7 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/73a982c7 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/73a982c7 Branch: refs/heads/TINKERPOP-1784 Commit: 73a982c7c13bc00893f34c65beec6419a56c76bc Parents: a278edd Author: Ted Wilmes <[email protected]> Authored: Tue Oct 10 13:46:13 2017 -0500 Committer: Ted Wilmes <[email protected]> Committed: Tue Oct 10 13:46:13 2017 -0500 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../gremlin/process/traversal/step/PathProcessor.java | 2 +- .../process/traversal/step/filter/DedupGlobalStep.java | 4 ++-- .../gremlin/process/traversal/step/filter/PathFilterStep.java | 5 +++-- .../process/traversal/step/filter/WherePredicateStep.java | 4 ++-- .../process/traversal/step/filter/WhereTraversalStep.java | 2 +- .../gremlin/process/traversal/step/map/MatchStep.java | 4 ++-- .../gremlin/process/traversal/step/map/PathStep.java | 5 +++-- .../gremlin/process/traversal/step/map/SelectOneStep.java | 5 +++-- .../gremlin/process/traversal/step/map/SelectStep.java | 4 ++-- .../gremlin/process/traversal/step/map/TreeStep.java | 5 +++-- .../process/traversal/step/sideEffect/TreeSideEffectStep.java | 5 +++-- .../strategy/optimization/PathRetractionStrategy.java | 7 ++++--- .../strategy/optimization/PathRetractionStrategyTest.java | 6 +++++- 14 files changed, 35 insertions(+), 24 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/73a982c7/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index ec54cc2..2f6069d 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -43,6 +43,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima * Fixed a bug in `Neo4jGremlinPlugin` that prevented it from loading properly in the `GremlinPythonScriptEngine`. * Fixed a bug in `ComputerVerificationStrategy` where child traversals were being analyzed prior to compilation. * Fixed a bug that prevented Gremlin from ordering lists and streams made of mixed number types. +* Fixed a bug where `keepLabels` were being corrupted because a defensive copy was not being made when they were being set by `PathRetractionStrategy`. [[release-3-2-6]] === TinkerPop 3.2.6 (Release Date: August 21, 2017) http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/73a982c7/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PathProcessor.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PathProcessor.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PathProcessor.java index 0c8ed47..8a5843a 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PathProcessor.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/PathProcessor.java @@ -60,7 +60,7 @@ public interface PathProcessor { return max; } - public void setKeepLabels(final Set<String> labels); + public void setKeepLabels(final Set<String> keepLabels); public Set<String> getKeepLabels(); http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/73a982c7/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/DedupGlobalStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/DedupGlobalStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/DedupGlobalStep.java index 96bd0be..b4f70d9 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/DedupGlobalStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/DedupGlobalStep.java @@ -223,8 +223,8 @@ public final class DedupGlobalStep<S> extends FilterStep<S> implements Traversal } @Override - public void setKeepLabels(Set<String> labels) { - this.keepLabels = labels; + public void setKeepLabels(Set<String> keepLabels) { + this.keepLabels = new HashSet<>(keepLabels); } @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/73a982c7/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/PathFilterStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/PathFilterStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/PathFilterStep.java index 4fe5953..16e3f0f 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/PathFilterStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/PathFilterStep.java @@ -33,6 +33,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil; import org.apache.tinkerpop.gremlin.structure.util.StringFactory; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Set; @@ -123,8 +124,8 @@ public final class PathFilterStep<S> extends FilterStep<S> implements FromToModu } @Override - public void setKeepLabels(final Set<String> labels) { - this.keepLabels = labels; + public void setKeepLabels(final Set<String> keepLabels) { + this.keepLabels = new HashSet<>(keepLabels); } @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/73a982c7/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WherePredicateStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WherePredicateStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WherePredicateStep.java index 1b248af..240a4cc 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WherePredicateStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WherePredicateStep.java @@ -151,8 +151,8 @@ public final class WherePredicateStep<S> extends FilterStep<S> implements Scopin } @Override - public void setKeepLabels(final Set<String> labels) { - this.keepLabels = labels; + public void setKeepLabels(final Set<String> keepLabels) { + this.keepLabels = new HashSet<>(keepLabels); } @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/73a982c7/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WhereTraversalStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WhereTraversalStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WhereTraversalStep.java index 476ce11..384bbce 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WhereTraversalStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WhereTraversalStep.java @@ -139,7 +139,7 @@ public final class WhereTraversalStep<S> extends FilterStep<S> implements Traver @Override public void setKeepLabels(final Set<String> keepLabels) { - this.keepLabels = keepLabels; + this.keepLabels = new HashSet<>(keepLabels); } @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/73a982c7/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStep.java index 039c1c7..8e2207a 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MatchStep.java @@ -183,8 +183,8 @@ public final class MatchStep<S, E> extends ComputerAwareStep<S, Map<String, E>> } @Override - public void setKeepLabels(final Set<String> labels) { - this.keepLabels = new HashSet<>(labels); + public void setKeepLabels(final Set<String> keepLabels) { + this.keepLabels = new HashSet<>(keepLabels); if (null != this.dedupLabels) this.keepLabels.addAll(this.dedupLabels); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/73a982c7/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PathStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PathStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PathStep.java index 2c96261..b49a1e4 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PathStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/PathStep.java @@ -31,6 +31,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalRing; import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil; import org.apache.tinkerpop.gremlin.structure.util.StringFactory; +import java.util.HashSet; import java.util.List; import java.util.Set; @@ -108,8 +109,8 @@ public final class PathStep<S> extends MapStep<S, Path> implements TraversalPare } @Override - public void setKeepLabels(final Set<String> labels) { - this.keepLabels = labels; + public void setKeepLabels(final Set<String> keepLabels) { + this.keepLabels = new HashSet<>(keepLabels); } @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/73a982c7/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectOneStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectOneStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectOneStep.java index 34b8148..eb6f7c6 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectOneStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectOneStep.java @@ -31,6 +31,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil; import org.apache.tinkerpop.gremlin.structure.util.StringFactory; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Set; @@ -116,8 +117,8 @@ public final class SelectOneStep<S, E> extends MapStep<S, E> implements Traversa } @Override - public void setKeepLabels(final Set<String> labels) { - this.keepLabels = labels; + public void setKeepLabels(final Set<String> keepLabels) { + this.keepLabels = new HashSet<>(keepLabels); } @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/73a982c7/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectStep.java index 167fa47..875cf93 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/SelectStep.java @@ -142,8 +142,8 @@ public final class SelectStep<S, E> extends MapStep<S, Map<String, E>> implement } @Override - public void setKeepLabels(final Set<String> labels) { - this.keepLabels = labels; + public void setKeepLabels(final Set<String> keepLabels) { + this.keepLabels = new HashSet<>(keepLabels); } @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/73a982c7/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TreeStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TreeStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TreeStep.java index ac1fa07..41310aa 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TreeStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TreeStep.java @@ -33,6 +33,7 @@ import org.apache.tinkerpop.gremlin.structure.util.StringFactory; import org.apache.tinkerpop.gremlin.util.function.TreeSupplier; import java.io.Serializable; +import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.function.BinaryOperator; @@ -113,8 +114,8 @@ public final class TreeStep<S> extends ReducingBarrierStep<S, Tree> implements T } @Override - public void setKeepLabels(final Set<String> labels) { - this.keepLabels = labels; + public void setKeepLabels(final Set<String> keepLabels) { + this.keepLabels = new HashSet<>(keepLabels); } @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/73a982c7/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/TreeSideEffectStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/TreeSideEffectStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/TreeSideEffectStep.java index 15756d2..2d43ddc 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/TreeSideEffectStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/TreeSideEffectStep.java @@ -33,6 +33,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil; import org.apache.tinkerpop.gremlin.structure.util.StringFactory; import org.apache.tinkerpop.gremlin.util.function.TreeSupplier; +import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.function.Supplier; @@ -123,8 +124,8 @@ public final class TreeSideEffectStep<S> extends SideEffectStep<S> implements Si } @Override - public void setKeepLabels(final Set<String> labels) { - this.keepLabels = labels; + public void setKeepLabels(final Set<String> keepLabels) { + this.keepLabels = new HashSet<>(keepLabels); } @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/73a982c7/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java index 304161e..a079a1c 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategy.java @@ -120,7 +120,7 @@ public final class PathRetractionStrategy extends AbstractTraversalStrategy<Trav pathProcessor.getKeepLabels().addAll(((MatchStep) currentStep).getMatchEndLabels()); } else { if (pathProcessor.getKeepLabels() == null) - pathProcessor.setKeepLabels(new HashSet<>(keepLabels)); + pathProcessor.setKeepLabels(keepLabels); else pathProcessor.getKeepLabels().addAll(new HashSet<>(keepLabels)); } @@ -242,10 +242,11 @@ public final class PathRetractionStrategy extends AbstractTraversalStrategy<Trav } private void addLabels(final PathProcessor s, final Set<String> keepLabels) { + final Set<String> labelsCopy = new HashSet<>(keepLabels); if (null == s.getKeepLabels()) - s.setKeepLabels(new HashSet<>(keepLabels)); + s.setKeepLabels(labelsCopy); else - s.getKeepLabels().addAll(new HashSet<>(keepLabels)); + s.getKeepLabels().addAll(labelsCopy); } @Override http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/73a982c7/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategyTest.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategyTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategyTest.java index f42a914..67264ae 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategyTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/optimization/PathRetractionStrategyTest.java @@ -31,6 +31,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent; import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement; import org.apache.tinkerpop.gremlin.process.traversal.util.DefaultTraversalStrategies; +import org.apache.tinkerpop.gremlin.structure.T; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -210,7 +211,10 @@ public class PathRetractionStrategyTest { "[[[a, b]], [[a, b]], [[a, b]], [[[a, b]]], [[a, b]]]", null}, {__.V().as("a").out().where(neq("a")).program(labeledPathVertexProgram), PATH_RETRACTION_STRATEGY_DISABLED, null}, {__.V().as("a").out().where(neq("a")).program(pathVertexProgram).select("a"), PATH_RETRACTION_STRATEGY_DISABLED, null}, - {__.V().as("a").out().program(emptyRequirementsVertexProgram).select("a"), "[[]]", null} + {__.V().as("a").out().program(emptyRequirementsVertexProgram).select("a"), "[[]]", null}, + {__.V().as("a").out().as("b").where(__.as("b").in().count().is(eq(3)).or().where( + __.as("b").out("created").and().as("b").has(T.label, "person"))).select("a", "b"), + "[[a, b], [[[a, b]]], []]", null} }); } } \ No newline at end of file
