[
https://issues.apache.org/jira/browse/TINKERPOP-3304?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18136572#comment-18136572
]
ASF GitHub Bot commented on TINKERPOP-3304:
-------------------------------------------
codecov-commenter commented on PR #3759:
URL: https://github.com/apache/tinkerpop/pull/3759#issuecomment-6582203909
##
[Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/3759?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:x: Patch coverage is `88.23529%` with `6 lines` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 75.95%. Comparing base
([`4722890`](https://app.codecov.io/gh/apache/tinkerpop/commit/4722890ed831ba4f71c3916b5e6eafcf85439345?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`53c76cd`](https://app.codecov.io/gh/apache/tinkerpop/commit/53c76cdc5d4325e49b558a30820c305df84bf7d6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 181 commits behind head on 3.7-dev.
| [Files with missing
lines](https://app.codecov.io/gh/apache/tinkerpop/pull/3759?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Patch % | Lines |
|---|---|---|
|
[.../strategy/decoration/EventStrategyProcessTest.java](https://app.codecov.io/gh/apache/tinkerpop/pull/3759?src=pr&el=tree&filepath=gremlin-test%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Ftinkerpop%2Fgremlin%2Fprocess%2Ftraversal%2Fstrategy%2Fdecoration%2FEventStrategyProcessTest.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JlbWxpbi10ZXN0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9wcm9jZXNzL3RyYXZlcnNhbC9zdHJhdGVneS9kZWNvcmF0aW9uL0V2ZW50U3RyYXRlZ3lQcm9jZXNzVGVzdC5qYXZh)
| 91.11% | [2 Missing and 2 partials :warning:
](https://app.codecov.io/gh/apache/tinkerpop/pull/3759?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
|
[...n/process/traversal/step/util/event/EventUtil.java](https://app.codecov.io/gh/apache/tinkerpop/pull/3759?src=pr&el=tree&filepath=gremlin-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Ftinkerpop%2Fgremlin%2Fprocess%2Ftraversal%2Fstep%2Futil%2Fevent%2FEventUtil.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9wcm9jZXNzL3RyYXZlcnNhbC9zdGVwL3V0aWwvZXZlbnQvRXZlbnRVdGlsLmphdmE=)
| 66.66% | [1 Missing and 1 partial :warning:
](https://app.codecov.io/gh/apache/tinkerpop/pull/3759?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
|
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 3.7-dev #3759 +/- ##
=============================================
+ Coverage 75.49% 75.95% +0.45%
- Complexity 13161 13354 +193
=============================================
Files 1092 1067 -25
Lines 67208 62301 -4907
Branches 7391 7464 +73
=============================================
- Hits 50742 47322 -3420
+ Misses 13837 12528 -1309
+ Partials 2629 2451 -178
```
</details>
[:umbrella: View full report in Codecov by
Harness](https://app.codecov.io/gh/apache/tinkerpop/pull/3759?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it
here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests,
report on failures, and find test suite problems.
- :package: [JS Bundle
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save
yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
> 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.
> Pull request against 3.7-dev: https://github.com/apache/tinkerpop/pull/3759
> 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.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
