You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2023/06/16 14:36:45 UTC

[tinkerpop] branch master updated: TINKERPOP-2858 Prevented CME when removing all labels CTR

This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git


The following commit(s) were added to refs/heads/master by this push:
     new 21fbcb9955 TINKERPOP-2858 Prevented CME when removing all labels CTR
21fbcb9955 is described below

commit 21fbcb9955977ce3c907208d02230b93e134ba51
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Fri Jun 16 10:35:22 2023 -0400

    TINKERPOP-2858 Prevented CME when removing all labels CTR
---
 CHANGELOG.asciidoc                                 |  1 +
 .../strategy/decoration/VertexProgramStrategy.java |  2 +-
 .../tinkerpop/gremlin/process/traversal/Step.java  |  5 +++
 .../process/traversal/step/util/AbstractStep.java  |  5 +++
 .../process/traversal/step/util/EmptyStep.java     | 40 +++++++---------------
 .../strategy/decoration/ConnectiveStrategy.java    |  2 +-
 .../gremlin/process/traversal/TraversalTest.java   | 37 ++++++--------------
 7 files changed, 36 insertions(+), 56 deletions(-)

diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 3298cd92a3..6dbbd224ee 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -41,6 +41,7 @@ This release also includes changes from <<release-3-6-XXX, 3.6.XXX>>.
 * Added `SubmitWithOptions()` methods to `Client` and `DriverRemoteConnection` in Go GLV to pass `RequestOptions` to the server.
 * Changed default behavior for returning properties on graph elements for OLTP queries so that properties are now returned.
 * Detachment is no longer performed in `TraverserIterator`.
+* Prevented `ConcurentModificationException` when removing all labels from a `Step`.
 * Added `materializeProperties` request option to control properties serialization.
 * Modified serializers in to handle serialization and deserialization of properties.
 * Added functional properties to the graph structure components for .NET, GO and Python.
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/strategy/decoration/VertexProgramStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/strategy/decoration/VertexProgramStrategy.java
index 0ca3dc00f1..7d920ab2f7 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/strategy/decoration/VertexProgramStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/strategy/decoration/VertexProgramStrategy.java
@@ -82,7 +82,7 @@ public final class VertexProgramStrategy extends AbstractTraversalStrategy<Trave
         while (!(currentStep instanceof EmptyStep)) {
             if (currentStep instanceof VertexComputing && !(currentStep instanceof ProgramVertexProgramStep)) {  // todo: is there a general solution?
                 currentLabels.addAll(currentStep.getLabels());
-                currentStep.getLabels().forEach(currentStep::removeLabel);
+                currentStep.clearLabels();
             } else {
                 currentLabels.forEach(currentStep::addLabel);
                 currentLabels.clear();
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Step.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Step.java
index 99fdd918e0..818d15b0bf 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Step.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Step.java
@@ -146,6 +146,11 @@ public interface Step<S, E> extends Iterator<Traverser.Admin<E>>, Serializable,
      */
     public void removeLabel(final String label);
 
+    /**
+     * Removes all labels on the step.
+     */
+    public void clearLabels();
+
     /**
      * Get the unique id of the step.
      * These ids can change when strategies are applied and anonymous traversals are embedded in the parent traversal.
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/AbstractStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/AbstractStep.java
index 89dbc0670d..d4545ead5c 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/AbstractStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/AbstractStep.java
@@ -75,6 +75,11 @@ public abstract class AbstractStep<S, E> implements Step<S, E> {
         this.labels.remove(label);
     }
 
+    @Override
+    public void clearLabels() {
+        this.labels.clear();
+    }
+
     @Override
     public Set<String> getLabels() {
         return Collections.unmodifiableSet(this.labels);
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/EmptyStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/EmptyStep.java
index 704cce3b06..01e745e6d2 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/EmptyStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/EmptyStep.java
@@ -41,13 +41,10 @@ public final class EmptyStep<S, E> implements Step<S, E>, TraversalParent {
         return INSTANCE;
     }
 
-    private EmptyStep() {
-    }
+    private EmptyStep() { }
 
     @Override
-    public void addStarts(final Iterator<Traverser.Admin<S>> starts) {
-
-    }
+    public void addStarts(final Iterator<Traverser.Admin<S>> starts) { }
 
     @Override
     public boolean hasStarts() {
@@ -55,19 +52,13 @@ public final class EmptyStep<S, E> implements Step<S, E>, TraversalParent {
     }
 
     @Override
-    public void addStart(final Traverser.Admin<S> start) {
-
-    }
+    public void addStart(final Traverser.Admin<S> start) { }
 
     @Override
-    public void setPreviousStep(final Step<?, S> step) {
-
-    }
+    public void setPreviousStep(final Step<?, S> step) { }
 
     @Override
-    public void reset() {
-
-    }
+    public void reset() { }
 
     @Override
     public Step<?, S> getPreviousStep() {
@@ -75,9 +66,7 @@ public final class EmptyStep<S, E> implements Step<S, E>, TraversalParent {
     }
 
     @Override
-    public void setNextStep(final Step<E, ?> step) {
-
-    }
+    public void setNextStep(final Step<E, ?> step) { }
 
     @Override
     public Step<E, ?> getNextStep() {
@@ -90,9 +79,7 @@ public final class EmptyStep<S, E> implements Step<S, E>, TraversalParent {
     }
 
     @Override
-    public void setTraversal(final Traversal.Admin<?, ?> traversal) {
-
-    }
+    public void setTraversal(final Traversal.Admin<?, ?> traversal) { }
 
     @Override
     @SuppressWarnings("CloneDoesntCallSuperClone")
@@ -106,19 +93,16 @@ public final class EmptyStep<S, E> implements Step<S, E>, TraversalParent {
     }
 
     @Override
-    public void addLabel(final String label) {
-
-    }
+    public void addLabel(final String label) { }
 
     @Override
-    public void removeLabel(final String label) {
-
-    }
+    public void removeLabel(final String label) { }
 
     @Override
-    public void setId(final String id) {
+    public void clearLabels() { }
 
-    }
+    @Override
+    public void setId(final String id) { }
 
     @Override
     public String getId() {
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ConnectiveStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ConnectiveStrategy.java
index 3deab6dbaf..d314b4d65c 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ConnectiveStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ConnectiveStrategy.java
@@ -95,7 +95,7 @@ public final class ConnectiveStrategy extends AbstractTraversalStrategy<Traversa
                     }
                     i++;
                     currentStep.addLocalChild(connectiveTraversal = connectiveTraversal(connectiveTraversal, currentStep));
-                    currentStep.getLabels().forEach(currentStep::removeLabel);
+                    currentStep.clearLabels();
                     while (i < steps.size()) {
                         final Step nextStep = steps.get(i);
                         if (legalCurrentStep(nextStep)) {
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java
index b7c352e23a..494e4c979a 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java
@@ -169,14 +169,10 @@ public class TraversalTest {
         }
 
         @Override
-        public void addStarts(final Iterator starts) {
-
-        }
+        public void addStarts(final Iterator starts) { }
 
         @Override
-        public void addStart(final Traverser.Admin start) {
-
-        }
+        public void addStart(final Traverser.Admin start) { }
 
         @Override
         public boolean hasStarts() {
@@ -184,9 +180,7 @@ public class TraversalTest {
         }
 
         @Override
-        public void setPreviousStep(final Step step) {
-
-        }
+        public void setPreviousStep(final Step step) { }
 
         @Override
         public Step getPreviousStep() {
@@ -194,9 +188,7 @@ public class TraversalTest {
         }
 
         @Override
-        public void setNextStep(final Step step) {
-
-        }
+        public void setNextStep(final Step step) { }
 
         @Override
         public Step getNextStep() {
@@ -209,14 +201,10 @@ public class TraversalTest {
         }
 
         @Override
-        public void setTraversal(final Traversal.Admin traversal) {
-
-        }
+        public void setTraversal(final Traversal.Admin traversal) { }
 
         @Override
-        public void reset() {
-
-        }
+        public void reset() { }
 
         @Override
         public Step clone() {
@@ -229,19 +217,16 @@ public class TraversalTest {
         }
 
         @Override
-        public void addLabel(final String label) {
-
-        }
+        public void addLabel(final String label) { }
 
         @Override
-        public void removeLabel(final String label) {
-
-        }
+        public void removeLabel(final String label) { }
 
         @Override
-        public void setId(final String id) {
+        public void clearLabels() { }
 
-        }
+        @Override
+        public void setId(final String id) { }
 
         @Override
         public String getId() {