Fixed MergeV/MergeE steps to work when onCreate is immutable map

CHANGELOG.asciidoc

@@ -31,9 +31,11 @@ This release also includes changes from <<release-3-5-6, 3.5.6>>.
 * 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)
 

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<S> 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<S> 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<Object, Object> 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;
     }
 
     /*

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<S> 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<Object, Object> combinedMap = new HashMap<>(onCreateMap.size() + mergeMap.size());
+        combinedMap.putAll(onCreateMap);
+        combinedMap.putAll(mergeMap);
+
+        return combinedMap;
     }
 
 }

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);
+    }
 }

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);
+    }
 }