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

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

commit 8663ca7e6b70ab36f42936f73af3c8d9f2bedd37
Author: Stephen Mallette <[email protected]>
AuthorDate: Mon Oct 16 17:38:25 2023 -0400

    TINKERPOP-2811 Fixed bug with hasId() order for ElementIdStrategy
    
    Since HasContainer rolls up sequential has() steps checking the first item 
in the HasContainer only might not catch the id that needs to be replaced.
---
 CHANGELOG.asciidoc                                               | 1 +
 .../process/traversal/strategy/decoration/ElementIdStrategy.java | 7 ++++---
 .../strategy/decoration/ElementIdStrategyProcessTest.java        | 9 ++++++++-
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 8f77255c99..b227cd1865 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -32,6 +32,7 @@ This release also includes changes from <<release-3-5-8, 
3.5.8>>.
 * Added translator to the Go GLV.
 * Fixed bug with filtering for `group()` when the side-effect label was 
defined for it.
 * ProjectStep now throws exception when a duplicate key is provided in a query.
+* Fixed bug in `ElementIdStrategy` where the order of `hasId` was impacting 
proper filters.
 * Fixed bug in the Java driver configuration for serialization when reading 
settings from an `InputStream`.
 * Fixed bug in `DotNetTranslator` where `PartitionStrategy` usage was not 
translating properly when specifying the `readPartitions`.
 * Fixed bug in `PythonTranslator` where `Set` syntax was not being generated 
properly.
diff --git 
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ElementIdStrategy.java
 
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ElementIdStrategy.java
index 903ea3a8e9..65cda70de0 100644
--- 
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ElementIdStrategy.java
+++ 
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ElementIdStrategy.java
@@ -81,9 +81,10 @@ public final class ElementIdStrategy extends 
AbstractTraversalStrategy<Traversal
 
     @Override
     public void apply(final Traversal.Admin<?, ?> traversal) {
-        TraversalHelper.getStepsOfAssignableClass(HasStep.class, 
traversal).stream()
-                .filter(hasStep -> ((HasStep<?>) 
hasStep).getHasContainers().get(0).getKey().equals(T.id.getAccessor()))
-                .forEach(hasStep -> ((HasStep<?>) 
hasStep).getHasContainers().get(0).setKey(this.idPropertyKey));
+        TraversalHelper.getStepsOfAssignableClass(HasStep.class, 
traversal).stream().
+                forEach(hasStep -> ((HasStep<?>) 
hasStep).getHasContainers().stream().
+                            filter(container -> 
container.getKey().equals(T.id.getAccessor())).
+                                     forEach(container -> 
container.setKey(this.idPropertyKey)));
 
         if (traversal.getStartStep() instanceof GraphStep) {
             final GraphStep graphStep = (GraphStep) traversal.getStartStep();
diff --git 
a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ElementIdStrategyProcessTest.java
 
b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ElementIdStrategyProcessTest.java
index 69fa5cf437..7eb336f20e 100644
--- 
a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ElementIdStrategyProcessTest.java
+++ 
b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/ElementIdStrategyProcessTest.java
@@ -67,10 +67,14 @@ public class ElementIdStrategyProcessTest extends 
AbstractGremlinProcessTest {
     public void shouldGenerateDefaultIdOnGraphAddVWithGeneratedCustomId() 
throws Exception {
         final ElementIdStrategy strategy = 
ElementIdStrategy.build().idMaker(new ConstantSupplier<>("xxx")).create();
         final GraphTraversalSource sg = create(strategy);
-        final Vertex v = sg.addV().property("name", "stephen").next();
+        final Vertex v = sg.addV().property("name", 
"stephen").property("group", 10).next();
         assertEquals("stephen", v.value("name"));
         assertEquals("xxx", sg.V(v).id().next());
         assertEquals("xxx", sg.V("xxx").id().next());
+        assertEquals("xxx", sg.V().has(T.id, "xxx").id().next());
+        assertEquals("xxx", sg.V().hasId("xxx").id().next());
+        assertEquals("xxx", sg.V().has(T.id, "xxx").has("group", 
10).id().next());
+        assertEquals("xxx", sg.V().has("group", 10).has(T.id, 
"xxx").id().next());
     }
 
     @Test
@@ -200,6 +204,9 @@ public class ElementIdStrategyProcessTest extends 
AbstractGremlinProcessTest {
         assertEquals("some-id", e.value("name"));
         assertEquals("some-id", sg.E(e).id().next());
         assertEquals("some-id", sg.E("some-id").id().next());
+        assertEquals("some-id", sg.E().hasId("some-id").id().next());
+        assertEquals("some-id", sg.E().has(T.id, "some-id").has("test", 
"value").id().next());
+        assertEquals("some-id", sg.E().has("test", "value").has(T.id, 
"some-id").id().next());
     }
 
     private GraphTraversalSource create(final ElementIdStrategy strategy) {

Reply via email to