diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index f3bbafa6463..b0f02d733ab 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -27,6 +27,8 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima This release also includes changes from prior 3.7.x releases. +* Fixed `AddPropertyStep` not passing the old property to `EventStrategy` listeners when the cardinality was not specified in `property()`. + [[release-3-8-2]] === TinkerPop 3.8.2 (Release Date: September 1, 2026) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java index 4223f98bd5c..691c11ad9f1 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java @@ -104,6 +104,10 @@ protected void sideEffect(final Traverser.Admin traverser) { "Property cardinality can only be set for a Vertex but the traversal encountered %s for key: %s", element.getClass().getSimpleName(), key)); + final VertexProperty.Cardinality effectiveCardinality = this.cardinality != null + ? this.cardinality + : element.graph().features().vertex().getCardinality(key); + final Optional optEventStrategy = getTraversal().getStrategies().getStrategy(EventStrategy.class); final boolean eventingIsConfigured = EventUtil.hasAnyCallbacks(callbackRegistry) && optEventStrategy.isPresent(); @@ -111,17 +115,13 @@ protected void sideEffect(final Traverser.Admin traverser) { // find property to remove. only need to capture the removedProperty if eventing is configured final Property removedProperty = eventingIsConfigured ? - captureRemovedProperty(element, key, value, es) : + captureRemovedProperty(element, key, value, effectiveCardinality, es) : VertexProperty.empty(); - final VertexProperty.Cardinality card = this.cardinality != null - ? this.cardinality - : element.graph().features().vertex().getCardinality(key); - // update property if (element instanceof Vertex) { - if (null != card) { - ((Vertex) element).property(card, key, value, vertexPropertyKeyValues); + if (null != effectiveCardinality) { + ((Vertex) element).property(effectiveCardinality, key, value, vertexPropertyKeyValues); } else if (vertexPropertyKeyValues.length > 0) { ((Vertex) element).property(key, value, vertexPropertyKeyValues); } else { @@ -140,11 +140,12 @@ protected void sideEffect(final Traverser.Admin traverser) { } private Property captureRemovedProperty(final Element element, final String key, final Object value, + final VertexProperty.Cardinality effectiveCardinality, final EventStrategy es){ Property removedProperty = VertexProperty.empty(); if (element instanceof Vertex) { - if (cardinality == VertexProperty.Cardinality.set) { + if (effectiveCardinality == VertexProperty.Cardinality.set) { final Iterator properties = element.properties(key); while (properties.hasNext()) { final Property property = properties.next(); @@ -153,7 +154,7 @@ private Property captureRemovedProperty(final Element element, final String key, break; } } - } else if (cardinality == VertexProperty.Cardinality.single) { + } else if (effectiveCardinality == VertexProperty.Cardinality.single) { removedProperty = element.property(key); } } else { diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/EventStrategyProcessTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/EventStrategyProcessTest.java index 0bad3387320..f5367917681 100644 --- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/EventStrategyProcessTest.java +++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/EventStrategyProcessTest.java @@ -57,6 +57,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.IsInstanceOf.instanceOf; import static org.junit.Assert.assertEquals; +import static org.junit.Assume.assumeTrue; /** * @author Stephen Mallette (http://stephen.genoprime.com) @@ -1051,6 +1052,39 @@ public void vertexPropertyChanged(final Vertex element, final VertexProperty old assertThat(triggered.get(), is(true)); } + @Test + @FeatureRequirementSet(FeatureRequirementSet.Package.VERTICES_ONLY) + public void shouldCaptureOldVertexPropertyWhenChangedWithDefaultCardinality() { + assumeTrue(graph.features().vertex().getCardinality("to-change") == VertexProperty.Cardinality.single); + + final AtomicBoolean triggered = new AtomicBoolean(false); + final Vertex v = graph.addVertex(); + v.property("to-change", "blah"); + + final MutationListener listener = new AbstractMutationListener() { + @Override + public void vertexPropertyChanged(final Vertex element, final VertexProperty oldValue, final Object setValue, final Object... vertexPropertyKeyValues) { + assertEquals("to-change", oldValue.key()); + assertEquals("blah", oldValue.value()); + assertEquals("dah", setValue); + triggered.set(true); + } + }; + final EventStrategy.Builder builder = EventStrategy.build().addListener(listener); + + if (graph.features().graph().supportsTransactions()) + builder.eventQueue(new EventStrategy.TransactionalEventQueue(graph)); + + final EventStrategy eventStrategy = builder.create(); + final GraphTraversalSource gts = create(eventStrategy); + + gts.V(v).property("to-change", "dah").iterate(); + tryCommit(graph); + + assertEquals(1, IteratorUtils.count(g.V(v).properties())); + assertThat(triggered.get(), is(true)); + } + @Test @FeatureRequirementSet(FeatureRequirementSet.Package.VERTICES_ONLY) public void shouldDetachVertexPropertyWhenNew() {