TINKERPOP-3056 Follow-on fix for mid-traversal mergeE/V

Prevents child traversals to option() from having an Element in it so that you can't mistakenly update the wrong thing CTR

CHANGELOG.asciidoc

@@ -29,13 +29,13 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 * Fixed bug in bytecode translation of `g.tx().commit()` and `g.tx().rollback()` in all languages.
 * Improved error message from `JavaTranslator` by including exception source.
 * Added missing `short` serialization (`gx:Int16`) to GraphSONV2 and GraphSONV3 in `gremlin-python`.
-* Added tests for error handling for GLV's if tx.commit() is called remotely for graphs without transactions support.
+* Added tests for error handling for GLV's if `tx.commit()`` is called remotely for graphs without transactions support.
 * Introduced multi-architecture AMD64/ARM64 docker images for gremlin-console.
 * Fixed bug in `JavaTranslator` where `has(String, null)` could call `has(String, Traversal)` to generate an error.
 * Fixed issue where server errors weren't being properly parsed when sending bytecode over HTTP.
-* Improved bulkset contains check for elements if all elements in bulkset are of the same type.
+* Improved `Bulkset` contains check for elements if all elements in `Bulkset` are of the same type.
 * Fixed bug in `EarlyLimitStrategy` which was too aggressive when promoting `limit()` before `map()`.
-* Fixed bug in mid-traversal `mergeE()` where mutations in `sideEffect()` were being applied to the current traverser rather than a `onMatch` edge.
+* Prevented mid-traversal `mergeE()` and `mergeV()` from operating on an incoming `Traverser` that contains an `Element`.
 * Improved performance of the application of `FilterRankingStrategy` for large traversals with deeply nested traversals by improving the cache operation.
 
 [[release-3-6-6]]

docs/src/reference/the-traversal.asciidoc

@@ -2515,6 +2515,13 @@ g.E().elementMap()
 <1> Create three dogs.
 <2> Stream the edge maps into `mergeE()` steps.
 
+WARNING: There is a bit of an inconsistency present when `mergeE()` is used as a start step versus when it is used
+mid-traversal. As a start step, `mergeE()` will promote the currently created or matched `Edge` to the child traversal,
+allowing you to directly update it like `option(onMatch, property('k', 'v').constant([:]))`. However, when `mergeE()` is
+used mid-traversal, the `Edge` is not promoted to the child traversal and the incoming traverser is used instead. Such
+behavior is essentially blocked to prevent accidental misuse and will result in an exception at execution time that will
+have a message like, "The incoming traverser for MergeEdgeStep cannot be an Element".
+
 The `mergeE` step can be combined with the `mergeV` step (or any other step producing a `Vertex`) using the
 `Merge.outV` and `Merge.inV` option modulators. These options can be used to "late-bind" the `OUT` and `IN`
 vertices in the main merge argument and in the `onCreate` argument:
@@ -2732,6 +2739,13 @@ g.V(400).valueMap().with(WithOptions.tokens)  <5>
 <4> Pixel exists now, so we will take this option.
 <5> The `updated` property has now been added.
 
+WARNING: There is a bit of an inconsistency present when `mergeV()` is used as a start step versus when it is used
+mid-traversal. As a start step, `mergeV()` will promote the currently created or matched `Vertex` to the child
+traversal, allowing you to directly update it like `option(onMatch, property('k', 'v').constant([:]))`. However, when
+`mergeV()` is used mid-traversal, the `Vertex` is not promoted to the child traversal and the incoming traverser is used
+instead. Such behavior is essentially blocked to prevent accidental misuse and will result in an exception at execution
+time that will have a message like, "The incoming traverser for MergeVertexStep cannot be an Element".
+
 *Additional References*
 
 link:++https://tinkerpop.apache.org/javadocs/x.y.z/core/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.html#mergeV()++[`mergeV()`],

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeEdgeStep.java

@@ -34,6 +34,7 @@
 import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
 import org.apache.tinkerpop.gremlin.process.traversal.lambda.ConstantTraversal;
+import org.apache.tinkerpop.gremlin.process.traversal.step.filter.LambdaFilterStep;
 import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.Event;
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.EventStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil;
@@ -274,8 +275,11 @@ protected Iterator<Edge> flatMap(final Traverser.Admin<S> traverser) {
             edges = IteratorUtils.peek(edges, e -> {
 
                 // override current traverser with the matched Edge so that the option() traversal can operate
-                // on it properly
-                traverser.set((S) e);
+                // on it properly. this should only work this way for the start step form to retain the original
+                // behavior for 3.6.0 where you might do g.inject(Map).mergeE() and want that Map to pass through.
+                // in 4.x this will be rectified such that the edge will always be promoted and you will be forced
+                // to select() the map if you did want the behavior.
+                if (isStart) traverser.set((S) e);
 
                 // assume good input from GraphTraversal - folks might drop in a T here even though it is immutable
                 final Map<String, ?> onMatchMap = materializeMap(traverser, onMatchTraversal);

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeStep.java

@@ -46,6 +46,7 @@
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil;
 import org.apache.tinkerpop.gremlin.structure.Direction;
+import org.apache.tinkerpop.gremlin.structure.Element;
 import org.apache.tinkerpop.gremlin.structure.Graph;
 import org.apache.tinkerpop.gremlin.structure.T;
 import org.apache.tinkerpop.gremlin.structure.Vertex;
@@ -142,6 +143,14 @@ public void addChildOption(final Merge token, final Traversal.Admin<S, C> traver
         if (token == Merge.onCreate) {
             this.onCreateTraversal = this.integrateChild(traversalOption);
         } else if (token == Merge.onMatch) {
+            // add a guard rail to ensure that the incoming object is not an Element. this will prevent
+            // a possibly inadvertent mutation of the graph if you did something like g.V().mergeE(). for
+            // 3.x we won't allow this behavior at all but in 4.x we will make it consistent like it will
+            // be in 4.x
+            if (!isStart && traversalOption != null && !(traversalOption instanceof ConstantTraversal)) {
+                traversalOption.addStep(0, new GuardRailStep<>(traversalOption, getClass().getSimpleName()));
+            }
+
             this.onMatchTraversal = this.integrateChild(traversalOption);
         } else {
             throw new UnsupportedOperationException(String.format("Option %s for Merge is not supported", token.name()));
@@ -374,4 +383,30 @@ protected GraphTraversal searchVerticesPropertyConstraints(GraphTraversal t, fin
 
     protected abstract Set getAllowedTokens();
 
+    /**
+     * Guard rail to ensure that the incoming object is not an {@link Element}.
+     */
+    public static class GuardRailStep<S, E> extends ScalarMapStep<S, E> {
+        private final String stepType;
+
+        public GuardRailStep(final Traversal.Admin traversal, final String stepType) {
+            super(traversal);
+            this.stepType = stepType;
+        }
+
+        @Override
+        protected E map(final Traverser.Admin<S> t) {
+            if (t.get() instanceof Element) {
+                throw new IllegalArgumentException(
+                        String.format("The incoming traverser for %s cannot be an Element", stepType));
+            }
+            return (E) t.get();
+        }
+
+        @Override
+        public String toString() {
+            return StringFactory.stepString(this);
+        }
+    }
+
 }

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/MergeVertexStep.java

@@ -93,8 +93,11 @@ protected Iterator<Vertex> flatMap(final Traverser.Admin<S> traverser) {
             // attach the onMatch properties
             vertices = IteratorUtils.peek(vertices, v -> {
 
-                // if this was a start step the traverser is initialized with Boolean/false, so override that with
-                // the matched Vertex so that the option() traversal can operate on it properly
+                // override current traverser with the matched Edge so that the option() traversal can operate
+                // on it properly. this should only work this way for the start step form to retain the original
+                // behavior for 3.6.0 where you might do g.inject(Map).mergeV() and want that Map to pass through.
+                // in 4.x this will be rectified such that the edge will always be promoted and you will be forced
+                // to select() the map if you did want the behavior.
                 if (isStart) traverser.set((S) v);
 
                 // assume good input from GraphTraversal - folks might drop in a T here even though it is immutable