Merge branch '3.6-dev' into valentyn/TINKERPOP-2926

apache · Apr 21, 2023 · 8349c5d · 8349c5d
2 parents 4f48680 + 975c69c
commit 8349c5d
Show file tree

Hide file tree

Showing 31 changed files with 714 additions and 172 deletions.
diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml
@@ -21,7 +21,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        language: [ 'go', 'java', 'javascript', 'python' ]
+        language: [ 'go', 'javascript', 'python' ]
         # CodeQL supports [ 'cpp', 'csharp', 'go', 'java', 'javascript', 'python', 'ruby' ]
         # Use only 'java' to analyze code written in Java, Kotlin or both
         # Use only 'javascript' to analyze code written in JavaScript, TypeScript or both
@@ -97,3 +97,34 @@ jobs:
         uses: github/codeql-action/analyze@v2
         with:
           category: "/language:csharp"
+
+  # Java is run as a separate job as the autobuild for Java is prone to random timeout failures
+  analyze_java:
+    name: Analyze Java
+    runs-on: ubuntu-latest
+    permissions:
+      actions: read
+      contents: read
+      security-events: write
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v3
+
+      # Initializes the CodeQL tools for scanning.
+      - name: Initialize CodeQL
+        uses: github/codeql-action/init@v2
+        with:
+          languages: 'java'
+
+      - run: |
+          echo "Build Java"
+          mvn clean package -B -V -e \
+          -pl -:gremlin-javascript,-:gremlin-dotnet,-:gremlin-dotnet-source,-:gremlin-dotnet-tests,-:gremlin-go,-:gremlin-python,-:gremlin-language,-:gremlint \
+          -Dfindbugs.skip -Dcheckstyle.skip -Dpmd.skip=true -Dspotbugs.skip -Denforcer.skip -Dmaven.javadoc.skip \
+          -DskipTests -Dmaven.test.skip.exec -Dlicense.skip=true -Drat.skip=true -Dspotless.check.skip=true
+
+      - name: Perform CodeQL Analysis
+        uses: github/codeql-action/analyze@v2
+        with:
+          category: "/language:java"
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
@@ -32,6 +32,9 @@ This release also includes changes from <<release-3-5-6, 3.5.6>>.
 * Added support to the grammar allowing `List` and `Map` key declarations for `Map` entries.
 * Improved performance of comparison (equals) between not compatible types and nulls.
 * Fixed MergeV/MergeE steps to work when onCreate is immutable map.
+* Introduced `Writing` and `Deleting` marker interfaces to identify whether a step can perform write or delete or both on Graph.
+* Added static map capturing possible Traversal steps that shall be added to traversal for a given operator
+
 
 [[release-3-6-2]]
 === TinkerPop 3.6.2 (Release Date: January 16, 2023)
@@ -304,11 +307,15 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 * Bumped to Groovy 2.5.21.
 * Fixed bug in parsing of `math()` expressions where variables were not being identified if they contained a text associated with a function.
 * Refactored `FilterRankingStrategy` to improve performance for deeply nested traversals.
+* Refactored strategy application to improve performance by avoiding some excessive recursion.
+* Added `Traversal.lock()` to provide an explicit way to finalize a traversal object.
+* Changed `Traversal.getGraph()` to get its `Graph` object from itself or, if not available, its parent.
 * Added `AuthInfoProvider` interface and `NewDynamicAuth()` to gremlin-go for dynamic authentication support.
+* Fixed bug where `hasId()` unrolls ids in Java arrays to put into `P.within` but not ids in lists, this also aligned behavior of start-step and mid-traversal hasId().
 * Bumped to `snakeyaml` 2.0 to fix security vulnerability.
 * Bumped to Apache `commons-configuration` 2.9.0 to fix security vulnerability.
 * Improved `count` step optimization for negative values in input for 'eq' comparison.
-* Fixed performance issue when using SampleGlobalStep with a traverser that has either a LABELED_PATH or PATH requirement.
+* Fixed performance issue when using `SampleGlobalStep` with a traverser that has either a `LABELED_PATH` or `PATH` requirement.
 
 [[release-3-5-5]]
 === TinkerPop 3.5.5 (Release Date: January 16, 2023)

diff --git a/docs/src/upgrade/release-3.5.x.asciidoc b/docs/src/upgrade/release-3.5.x.asciidoc
@@ -31,6 +31,23 @@ complete list of all the modifications that are part of this release.
 === Upgrading for Users
 
 
+=== Upgrading for Providers
+
+==== Graph System Providers
+
+===== TraversalStrategy Expectations
+
+Given some important performance improvements in this version, providers may need to make alterations to their
+`TraversalStrategy` implementations as certain assumptions about the data available to a strategy may have changed.
+If a `TraversalStrategy` implementation requires access to the `Graph` implementation, side-effects, and similar data
+stored on a `Traversal`, it is best to get that information from the root of the traversal hierarchy rather than from
+the current traversal that the strategy is executing on as the strategy application process no longer take the
+expensive step to propagate that data throughout the traversal in between each strategy application. Use the
+`TraversalHelper.getRootTraversal()` helper function to get to the root traversal. Note also that
+`Traversal.getGraph()` will traverse through the parent traversals now when trying to find a `Graph`.
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-2855[TINKERPOP-2855],
+link:https://issues.apache.org/jira/browse/TINKERPOP-2888[TINKERPOP-2888]
 
 == TinkerPop 3.5.5
 
@@ -104,6 +121,7 @@ more likely now to note blocking behavior when a connection cannot be obtained.
 See: link:https://issues.apache.org/jira/browse/TINKERPOP-2813[TINKERPOP-2813]
 
 ==== Added User Agent to Gremlin drivers
+
 Previously, a server does not distinguish amongst the different types of clients connecting to it. We have now added
 user agent to web socket handshake in all the drivers, each with their own configuration to enable or disable user agent.
 User agent is enabled by default for all drivers.
@@ -117,6 +135,7 @@ User agent is enabled by default for all drivers.
 See: link:https://issues.apache.org/jira/browse/TINKERPOP-2480[TINKERPOP-2480]
 
 ==== Update to SSL Handshake Timeout Configuration
+
 Previously, the java driver relies on the default 10 second SSL handshake timeout defined by Netty. We have removed
 the default SSL handshake timeout. The SSL handshake timeout will instead be capped by setting `connectionSetupTimeoutMillis`.
 

diff --git a/...n/java/org/apache/tinkerpop/gremlin/process/remote/traversal/AbstractRemoteTraversal.java b/...n/java/org/apache/tinkerpop/gremlin/process/remote/traversal/AbstractRemoteTraversal.java
@@ -127,6 +127,11 @@ public boolean isLocked() {
         throw new UnsupportedOperationException("Remote traversals do not support this method");
     }
 
+    @Override
+    public void lock() {
+        throw new UnsupportedOperationException("Remote traversals do not support this method");
+    }
+
     @Override
     public Optional<Graph> getGraph() {
         throw new UnsupportedOperationException("Remote traversals do not support this method");

diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
@@ -543,7 +543,12 @@ public default boolean isRoot() {
         public boolean isLocked();
 
         /**
-         * Gets the {@link Graph} instance associated to this {@link Traversal}.
+         * Lock the traversal and perform any final adjustments to it after strategy application.
+         */
+        public void lock();
+
+        /**
+         * Gets the {@link Graph} instance associated directly to this {@link Traversal} or through its parent.
          */
         public Optional<Graph> getGraph();
 

diff --git a/...rc/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java b/...rc/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java
@@ -1795,39 +1795,35 @@ public default GraphTraversal<S, E> hasLabel(final P<String> predicate) {
     public default GraphTraversal<S, E> hasId(final Object id, final Object... otherIds) {
         if (id instanceof P) {
             return this.hasId((P) id);
-        } else {
-            Object[] ids;
-            if (id instanceof Object[]) {
-                ids = (Object[]) id;
-            } else {
-                ids = new Object[] {id};
-            }
-            int size = ids.length;
-            int capacity = size;
+        }
+        else {
+            this.asAdmin().getBytecode().addStep(Symbols.hasId, id, otherIds);
 
+            //using ArrayList given P.within() turns all arguments into lists
+            final List<Object> ids = new ArrayList<>();
+            if (id instanceof Object[]) {
+                Collections.addAll(ids, (Object[]) id);
+            } else if (id instanceof Collection) {
+                // as ids are unrolled when it's in array, they should also be unrolled when it's a list.
+                // this also aligns with behavior of hasId() when it's pushed down to g.V() (TINKERPOP-2863)
+                ids.addAll((Collection<?>) id);
+            } else
+                ids.add(id);
+
+            // unrolling ids from lists works cleaner with Collection too, as otherwise they will need to
+            // be turned into array first
             if (otherIds != null) {
                 for (final Object i : otherIds) {
-                    if (i != null && i.getClass().isArray()) {
-                        final Object[] tmp = (Object[]) i;
-                        int newLength = size + tmp.length;
-                        if (capacity < newLength) {
-                            ids = Arrays.copyOf(ids, capacity = size + tmp.length);
-                        }
-                        System.arraycopy(tmp, 0, ids, size, tmp.length);
-                        size = newLength;
-                    } else {
-                        if (capacity == size) {
-                            ids = Arrays.copyOf(ids, capacity = size * 2);
-                        }
-                        ids[size++] = i;
-                    }
-                }
-                if (capacity > size) {
-                    ids = Arrays.copyOf(ids, size);
+                    if (id instanceof Object[]) {
+                        Collections.addAll(ids, (Object[]) i);
+                    } else if (i instanceof Collection) {
+                        ids.addAll((Collection<?>) i);
+                    } else
+                        ids.add(i);
                 }
             }
-            this.asAdmin().getBytecode().addStep(Symbols.hasId, ids);
-            return TraversalHelper.addHasContainer(this.asAdmin(), new HasContainer(T.id.getAccessor(), ids.length == 1 ? P.eq(ids[0]) : P.within(ids)));
+
+            return TraversalHelper.addHasContainer(this.asAdmin(), new HasContainer(T.id.getAccessor(), ids.size() == 1 ? P.eq(ids.get(0)) : P.within(ids)));
         }
     }
 
@@ -2561,14 +2557,14 @@ public default GraphTraversal<S, E> property(final Object key, final Object valu
             return this.property(null, key, value, keyValues);
         }
     }
-    
+
     /**
      * When a {@link Map} is supplied then each of the key/value pairs in the map will
      * be added as property.  This method is the long-hand version of looping through the 
      * {@link #property(Object, Object, Object...)} method for each key/value pair supplied.
      * <p/>
      * If a {@link Map} is not supplied then an exception is thrown.
-     * <p /> 
+     * <p />
      * This method is effectively calls {@link #property(VertexProperty.Cardinality, Object, Object, Object...)}
      * as {@code property(null, key, value, keyValues}.
      *

diff --git a/...n/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/AbstractLambdaTraversal.java b/...n/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/AbstractLambdaTraversal.java
@@ -31,6 +31,7 @@
 import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement;
 import org.apache.tinkerpop.gremlin.process.traversal.util.EmptyTraversalSideEffects;
 import org.apache.tinkerpop.gremlin.process.traversal.util.EmptyTraversalStrategies;
+import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
 import org.apache.tinkerpop.gremlin.structure.Graph;
 
 import java.util.Collections;
@@ -170,6 +171,11 @@ public boolean isLocked() {
         return null == this.bypassTraversal || this.bypassTraversal.isLocked();
     }
 
+    @Override
+    public void lock() {
+       if (this.bypassTraversal != null) bypassTraversal.lock();
+    }
+
     /**
      * Implementations of this class can never be a root-level traversal as they are specialized implementations
      * intended to be child traversals by design.

diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Deleting.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Deleting.java
@@ -0,0 +1,30 @@
+/*
+ * 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;
+
+import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.Event;
+
+/**
+ * A marker interface for steps that deletes the graph data.
+ *
+ * @author Phanindhra (https://github.com/phanindhra876)
+ */
+public interface Deleting<E extends Event> extends Mutating<E> {
+}
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Writing.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Writing.java
@@ -0,0 +1,30 @@
+/*
+ * 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;
+
+import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.Event;
+
+/**
+ * A marker interface for steps that writes the graph data.
+ *
+ * @author Phanindhra (https://github.com/phanindhra876)
+ */
+public interface Writing<E extends Event> extends Mutating<E> {
+}
diff --git a/...re/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/DropStep.java b/...re/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/DropStep.java
@@ -20,7 +20,7 @@
 
 import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
 import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
-import org.apache.tinkerpop.gremlin.process.traversal.step.Mutating;
+import org.apache.tinkerpop.gremlin.process.traversal.step.Deleting;
 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;
@@ -31,13 +31,12 @@
 import org.apache.tinkerpop.gremlin.structure.Property;
 import org.apache.tinkerpop.gremlin.structure.Vertex;
 import org.apache.tinkerpop.gremlin.structure.VertexProperty;
-import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedFactory;
 
 /**
  * @author Marko A. Rodriguez (http://markorodriguez.com)
  * @author Stephen Mallette (http://stephen.genoprime.com)
  */
-public class DropStep<S> extends FilterStep<S> implements Mutating<Event> {
+public class DropStep<S> extends FilterStep<S> implements Deleting<Event> {
 
     private CallbackRegistry<Event> callbackRegistry;
 

diff --git a/...c/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeStartStep.java b/...c/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeStartStep.java
@@ -24,9 +24,9 @@
 import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
 import org.apache.tinkerpop.gremlin.process.traversal.TraverserGenerator;
 import org.apache.tinkerpop.gremlin.process.traversal.step.FromToModulating;
-import org.apache.tinkerpop.gremlin.process.traversal.step.Mutating;
 import org.apache.tinkerpop.gremlin.process.traversal.step.Scoping;
 import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
+import org.apache.tinkerpop.gremlin.process.traversal.step.Writing;
 import org.apache.tinkerpop.gremlin.process.traversal.step.util.AbstractStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.util.Parameters;
 import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.CallbackRegistry;
@@ -40,18 +40,16 @@
 import org.apache.tinkerpop.gremlin.structure.Vertex;
 import org.apache.tinkerpop.gremlin.structure.util.Attachable;
 import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
-import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedFactory;
 import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph;
 
-import java.util.Collections;
 import java.util.List;
 import java.util.Set;
 
 /**
  * @author Marko A. Rodriguez (http://markorodriguez.com)
  */
 public class AddEdgeStartStep extends AbstractStep<Edge, Edge>
-        implements Mutating<Event.EdgeAddedEvent>, TraversalParent, Scoping, FromToModulating {
+        implements Writing<Event.EdgeAddedEvent>, TraversalParent, Scoping, FromToModulating {
 
     private static final String FROM = Graph.Hidden.hide("from");
     private static final String TO = Graph.Hidden.hide("to");

diff --git a/...re/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeStep.java b/...re/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeStep.java
@@ -21,9 +21,9 @@
 import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
 import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
 import org.apache.tinkerpop.gremlin.process.traversal.step.FromToModulating;
-import org.apache.tinkerpop.gremlin.process.traversal.step.Mutating;
 import org.apache.tinkerpop.gremlin.process.traversal.step.Scoping;
 import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
+import org.apache.tinkerpop.gremlin.process.traversal.step.Writing;
 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;
@@ -46,7 +46,7 @@
  * @author Stephen Mallette (http://stephen.genoprime.com)
  */
 public class AddEdgeStep<S> extends ScalarMapStep<S, Edge>
-        implements Mutating<Event.EdgeAddedEvent>, TraversalParent, Scoping, FromToModulating {
+        implements Writing<Event.EdgeAddedEvent>, TraversalParent, Scoping, FromToModulating {
 
     private static final String FROM = Graph.Hidden.hide("from");
     private static final String TO = Graph.Hidden.hide("to");

diff --git a/...main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java b/...main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java
@@ -22,9 +22,9 @@
 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.step.Mutating;
 import org.apache.tinkerpop.gremlin.process.traversal.step.Scoping;
 import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent;
+import org.apache.tinkerpop.gremlin.process.traversal.step.Writing;
 import org.apache.tinkerpop.gremlin.process.traversal.step.util.AbstractStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.util.Parameters;
 import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.CallbackRegistry;
@@ -45,7 +45,7 @@
  * @author Stephen Mallette (http://stephen.genoprime.com)
  */
 public class AddVertexStartStep extends AbstractStep<Vertex, Vertex>
-        implements Mutating<Event.VertexAddedEvent>, TraversalParent, Scoping {
+        implements Writing<Event.VertexAddedEvent>, TraversalParent, Scoping {
 
     private Parameters parameters = new Parameters();
     private boolean first = true;