diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 7d9ea1c4c26..927efd0475a 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -31,9 +31,11 @@ This release also includes changes from <>. * Fixed bug in `GroovyTranslator` that surrounded `String` keys with parenthesis for `Map` when not necessary. * 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) 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 12d2639adfc..0bd44c98ca9 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 @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.LinkedHashSet; @@ -349,7 +350,7 @@ protected Map onCreateMap(final Traverser.Admin traverser, final Map unresolv final Map onCreateMap = materializeMap(traverser, onCreateTraversal); // null result from onCreateTraversal - use main mergeMap argument - if (onCreateMap == null) + if (onCreateMap == null || onCreateMap.size() == 0) return mergeMap; validateMapInput(onCreateMap, false); @@ -367,15 +368,17 @@ protected Map onCreateMap(final Traverser.Admin traverser, final Map unresolv * 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); + final Map combinedMap = new HashMap<>(onCreateMap.size() + mergeMap.size()); + combinedMap.putAll(onCreateMap); + combinedMap.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); + resolveVertices(combinedMap, traverser); - return onCreateMap; + return combinedMap; } /* 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 90177b6f215..70b4564434e 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 @@ -19,6 +19,7 @@ package org.apache.tinkerpop.gremlin.process.traversal.step.map; import java.util.Arrays; +import java.util.HashMap; import java.util.Iterator; import java.util.LinkedHashSet; import java.util.Map; @@ -192,19 +193,25 @@ protected Map onCreateMap(final Traverser.Admin traverser, final Map mergeMap final Map onCreateMap = materializeMap(traverser, onCreateTraversal); // null result from onCreateTraversal - use main mergeMap argument - if (onCreateMap == null) + if (onCreateMap == null || onCreateMap.size() == 0) return mergeMap; validateMapInput(onCreateMap, false); + if (mergeMap == null || mergeMap.size() == 0) + return onCreateMap; + /* * 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); - return onCreateMap; + final Map combinedMap = new HashMap<>(onCreateMap.size() + mergeMap.size()); + combinedMap.putAll(onCreateMap); + combinedMap.putAll(mergeMap); + + return combinedMap; } } diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeStepTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeStepTest.java index aa9f5bf466e..76d95b1b067 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeStepTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeStepTest.java @@ -18,14 +18,25 @@ */ package org.apache.tinkerpop.gremlin.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.TraversalSideEffects; +import org.apache.tinkerpop.gremlin.process.traversal.Traverser; import org.apache.tinkerpop.gremlin.structure.Direction; import org.apache.tinkerpop.gremlin.structure.T; import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceVertex; +import org.apache.tinkerpop.gremlin.util.function.TraverserSetSupplier; import org.apache.tinkerpop.gremlin.util.tools.CollectionFactory; import org.junit.Test; +import java.util.Collections; +import java.util.LinkedHashMap; import java.util.Map; +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + public class MergeEdgeStepTest { @Test @@ -100,4 +111,23 @@ public void shouldFailToValidateWithoutTokensBecauseOfWeirdKey() { new ReferenceVertex("weird"), 100000); MergeEdgeStep.validateMapInput(m, true); } + + @Test + public void shouldWorkWithImmutableMap() { + final Traversal.Admin traversal = mock(Traversal.Admin.class); + when(traversal.getTraverserSetSupplier()).thenReturn(TraverserSetSupplier.instance()); + final Traverser.Admin traverser = mock(Traverser.Admin.class); + when(traverser.split()).thenReturn(mock(Traverser.Admin.class)); + final Traversal.Admin onCreateTraversal = mock(Traversal.Admin.class); + when(onCreateTraversal.next()).thenReturn(Collections.unmodifiableMap(CollectionFactory.asMap("key1", "value1"))); + when(onCreateTraversal.getSideEffects()).thenReturn(mock(TraversalSideEffects.class)); + + final MergeEdgeStep step = new MergeEdgeStep<>(traversal, true); + step.addChildOption(Merge.onCreate, onCreateTraversal); + + final Map mergeMap = CollectionFactory.asMap("key2", "value2"); + final Map onCreateMap = step.onCreateMap(traverser, new LinkedHashMap<>(), mergeMap); + + assertEquals(CollectionFactory.asMap("key1", "value1", "key2", "value2"), onCreateMap); + } } diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeVertexStepTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeVertexStepTest.java index cafc72f1ce7..cc75190812c 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeVertexStepTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeVertexStepTest.java @@ -18,14 +18,24 @@ */ package org.apache.tinkerpop.gremlin.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.TraversalSideEffects; +import org.apache.tinkerpop.gremlin.process.traversal.Traverser; import org.apache.tinkerpop.gremlin.structure.Direction; import org.apache.tinkerpop.gremlin.structure.T; import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceVertex; +import org.apache.tinkerpop.gremlin.util.function.TraverserSetSupplier; import org.apache.tinkerpop.gremlin.util.tools.CollectionFactory; import org.junit.Test; +import java.util.Collections; import java.util.Map; +import static org.junit.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + public class MergeVertexStepTest { @Test @@ -75,4 +85,23 @@ public void shouldFailToValidateWithoutTokensBecauseOfWeirdKey() { new ReferenceVertex("weird"), 100000); MergeVertexStep.validateMapInput(m, true); } + + @Test + public void shouldWorkWithImmutableMap() { + final Traversal.Admin traversal = mock(Traversal.Admin.class); + when(traversal.getTraverserSetSupplier()).thenReturn(TraverserSetSupplier.instance()); + final Traverser.Admin traverser = mock(Traverser.Admin.class); + when(traverser.split()).thenReturn(mock(Traverser.Admin.class)); + final Traversal.Admin onCreateTraversal = mock(Traversal.Admin.class); + when(onCreateTraversal.next()).thenReturn(Collections.unmodifiableMap(CollectionFactory.asMap("key1", "value1"))); + when(onCreateTraversal.getSideEffects()).thenReturn(mock(TraversalSideEffects.class)); + + final MergeVertexStep step = new MergeVertexStep(traversal, true); + step.addChildOption(Merge.onCreate, onCreateTraversal); + + final Map mergeMap = CollectionFactory.asMap("key2", "value2"); + final Map onCreateMap = step.onCreateMap(traverser, mergeMap); + + assertEquals(CollectionFactory.asMap("key1", "value1", "key2", "value2"), onCreateMap); + } }