[
https://issues.apache.org/jira/browse/TINKERPOP-3304?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Paul updated TINKERPOP-3304:
----------------------------
Description:
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
was:
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 code is identical on 3.7.3, 3.8.2 and the
current 3.8-dev and master branches.
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 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 after the fix.
> 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.3, 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)
