[
https://issues.apache.org/jira/browse/TINKERPOP-3304?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18136519#comment-18136519
]
ASF GitHub Bot commented on TINKERPOP-3304:
-------------------------------------------
paul-lartigau opened a new pull request, #3759:
URL: https://github.com/apache/tinkerpop/pull/3759
https://issues.apache.org/jira/browse/TINKERPOP-3304
`mergeV()`/`mergeE()` now report a keyed property (`KeyedProperty` /
`KeyedVertexProperty`) as the old value of a property that did not exist yet,
as `property()` does since 3.4.0 (TINKERPOP-1831), instead of `null`. The stale
`Event` Javadoc is aligned.
Tests: `shouldCaptureKeyOfNewEdgePropertyViaMergeE` and
`shouldCaptureKeyOfNewVertexPropertyViaMergeV` in `EventStrategyProcessTest`;
they fail without the fix.
Validated: targeted run of `EventStrategyProcessTest` through
`TinkerGraphProcessStandardTest` (`gremlin-core`, `gremlin-test`,
`tinkergraph-gremlin`) on JDK 11, not a full reactor build. 3.8-dev and master
have the same code, the change merges forward.
> mergeV() and mergeE() pass a null old value to MutationListener when the
> property did not exist yet
> ---------------------------------------------------------------------------------------------------
>
> Key: TINKERPOP-3304
> URL: https://issues.apache.org/jira/browse/TINKERPOP-3304
> Project: TinkerPop
> Issue Type: Bug
> Components: process
> Affects Versions: 3.7.7, 3.8.2
> Reporter: Paul
> Priority: Minor
>
> When EventStrategy is configured, {{MergeVertexStep}} and {{MergeEdgeStep}}
> (onMatch) report the properties they set through
> {{EventUtil.registerVertexPropertyChange()}} and
> {{EventUtil.registerEdgePropertyChange()}}. When the property does not exist
> yet, both pass {{null}} as the old value:
> {code:java}
> final Property<Object> oldValue = p.isPresent() ?
> eventStrategy.detach(edge.property(key)) : null;
> {code}
> {{AddPropertyStep}} does not. Since 3.4.0 (TINKERPOP-1831, see the 3.4.x
> upgrade documentation: "the event now raises with a {{KeyedVertexProperty}}
> or {{KeyedProperty}} instance, which only contains a property key and no
> value in them"), events raised from new mutations carry a keyed property
> instead of {{null}}, so a listener can tell which property was added. This is
> asserted by {{EventStrategyProcessTest}}
> ({{shouldDetachPropertyOfEdgeWhenNew}},
> {{shouldDetachVertexPropertyWhenNew}}). {{mergeV()}}/{{mergeE()}}, added in
> 3.6.0, still raise {{null}}: the key is lost, and a listener written for the
> documented contract fails with a {{NullPointerException}} on
> {{oldValue.key()}} or {{oldValue.isPresent()}}.
> To reproduce, with TinkerGraph:
> {code:java}
> MutationListener listener = ...; // records the oldValue received by
> edgePropertyChanged()
> GraphTraversalSource g = traversal().withEmbedded(graph)
> .withStrategies(EventStrategy.build().addListener(listener).create());
> Vertex v = graph.addVertex();
> v.addEdge("self", v);
> g.E().property("a", 1).iterate();
> // oldValue is a KeyedProperty, key() == "a"
> g.V(v).mergeE(Map.of(T.label, "self")).option(Merge.onMatch, Map.of("b",
> 2)).iterate();
> // oldValue is null: nothing tells the listener that "b" is the property that
> was set
> {code}
> The same happens with {{mergeV(...).option(Merge.onMatch, ...)}} and
> {{vertexPropertyChanged()}}. The repro above gives the same result on the
> current 3.7-dev and 3.8-dev branches; the code is identical in 3.7.3, 3.8.2
> and master.
> We hit this with a listener that records the history of property changes: the
> first modification of an edge made through {{mergeE()}} is never recorded.
> Expected: the same as {{property()}}, a {{KeyedProperty}} /
> {{KeyedVertexProperty}} when the property is new.
> A patch with tests against 3.7-dev is ready; a pull request will follow.
> Notes: the Javadoc of {{Event.EdgePropertyChangedEvent}} and
> {{Event.VertexPropertyChangedEvent}} still says that {{oldValue}} is {{null}}
> for a new property; it predates the 3.4.0 change and is updated in the fix. A
> listener that tests {{oldValue == null}} on merge events would receive a
> non-null, non-present property aft
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
