[jira] [Updated] (TINKERPOP-3294) AddPropertyStep does not capture the old property for EventStrategy when cardinality is not specified

["Paul (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/TINKERPOP-3294?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Paul updated TINKERPOP-3294:
----------------------------
    Description: 
When EventStrategy is configured, AddPropertyStep calls captureRemovedProperty() to find the property being replaced, so it can be passed to vertexPropertyChanged() as the old value.

captureRemovedProperty() checks the {{cardinality}} field of the step. That field is only set when the cardinality is given explicitly in the traversal, e.g. {{{}property(single, "name", "x"){}}}. With {{property("name", "x")}} it is null, so neither the {{single}} nor the {{set}} branch is taken and the old value is always reported as empty.

The step already falls back to {{graph.features().vertex().getCardinality(key)}} when it writes the property (see TINKERPOP-2159), but that fallback is not used by captureRemovedProperty().

To reproduce, with TinkerGraph (default cardinality {{{}single{}}}):
{code:java}
MutationListener listener = ...; // records oldValue in vertexPropertyChanged
GraphTraversalSource g = traversal().withEmbedded(graph)
.withStrategies(EventStrategy.build().addListener(listener).create());

g.addV().property("name", "a").iterate();
g.V().property("name", "b").iterate();
// oldValue received by the listener is vp[empty], expected vp[name->a]
{code}
With {{property(single, "name", "b")}} the old value is reported correctly.

The fix is to compute the effective cardinality once and pass it to captureRemovedProperty(). On 3.8-dev the fallback is computed after the call to captureRemovedProperty(), so it also needs to be moved up. The code differs between the two branches, so I plan to open one PR for 3.7-dev and one for 3.8-dev.

  was:
When EventStrategy is configured, AddPropertyStep calls captureRemovedProperty() to find the property being replaced, so it can be passed to vertexPropertyChanged() as the old value.

captureRemovedProperty() checks the {{cardinality}} field of the step. That field is only set when the cardinality is given explicitly in the traversal, e.g. {{{}property(single, "name", "x"){}}}. With {{property("name", "x")}} it is null, so neither the {{single}} nor the {{set}} branch is taken and the old value is always reported as empty.

The step already falls back to {{graph.features().vertex().getCardinality(key)}} when it writes the property (see TINKERPOP-2159), but that fallback is not used by captureRemovedProperty().

To reproduce, with TinkerGraph (default cardinality {{{}single{}}}):
{code:java}
MutationListener listener = ...; // records oldValue in vertexPropertyChanged
GraphTraversalSource g = traversal().withEmbedded(graph)     .withStrategies(EventStrategy.build().addListener(listener).create());

g.addV().property("name", "a").iterate();
g.V().property("name", "b").iterate();
// oldValue received by the listener is vp[empty], expected vp[name->a]
{code}
With {{property(single, "name", "b")}} the old value is reported correctly.

The fix is to compute the effective cardinality once and pass it to captureRemovedProperty(). On 3.8-dev the fallback is computed after the call to captureRemovedProperty(), so it also needs to be moved up. The code differs between the two branches, so I plan to open one PR for 3.7-dev and one for 3.8-dev.


> AddPropertyStep does not capture the old property for EventStrategy when cardinality is not specified
> -----------------------------------------------------------------------------------------------------
>
>                 Key: TINKERPOP-3294
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-3294
>             Project: TinkerPop
>          Issue Type: Bug
>          Components: process
>    Affects Versions: 3.7.7, 3.8.2
>            Reporter: Paul
>            Priority: Minor
>             Fix For: 3.7.8, 3.8.3
>
>
> When EventStrategy is configured, AddPropertyStep calls captureRemovedProperty() to find the property being replaced, so it can be passed to vertexPropertyChanged() as the old value.
> captureRemovedProperty() checks the {{cardinality}} field of the step. That field is only set when the cardinality is given explicitly in the traversal, e.g. {{{}property(single, "name", "x"){}}}. With {{property("name", "x")}} it is null, so neither the {{single}} nor the {{set}} branch is taken and the old value is always reported as empty.
> The step already falls back to {{graph.features().vertex().getCardinality(key)}} when it writes the property (see TINKERPOP-2159), but that fallback is not used by captureRemovedProperty().
> To reproduce, with TinkerGraph (default cardinality {{{}single{}}}):
> {code:java}
> MutationListener listener = ...; // records oldValue in vertexPropertyChanged
> GraphTraversalSource g = traversal().withEmbedded(graph)
> .withStrategies(EventStrategy.build().addListener(listener).create());
> g.addV().property("name", "a").iterate();
> g.V().property("name", "b").iterate();
> // oldValue received by the listener is vp[empty], expected vp[name->a]
> {code}
> With {{property(single, "name", "b")}} the old value is reported correctly.
> The fix is to compute the effective cardinality once and pass it to captureRemovedProperty(). On 3.8-dev the fallback is computed after the call to captureRemovedProperty(), so it also needs to be moved up. The code differs between the two branches, so I plan to open one PR for 3.7-dev and one for 3.8-dev.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)