Publish link-save events with the value being saved.

createPropertyReference(…) published BeforeLinkSaveEvent and AfterLinkSaveEvent with the property value captured before the submitted links were applied to the owning aggregate. For PUT on collection and map associations (which replace the value with a freshly created container) and for singular associations, that captured value was the previously linked one rather than the one being saved. Only the augmenting POST/PATCH case happened to reflect the new state, as it mutates the existing container in place.

This left listeners (e.g. methods annotated with @HandleBeforeLinkSave) inspecting the event's linked argument unable to see the value actually being persisted.

We now re-read the property after the links have been applied, so both events carry the value that is going to be saved, independent of the association's cardinality or the HTTP method used.

Closes GH-1600, GH-1419.

Author: odrotbohm

spring-data-rest-webmvc/src/main/java/org/springframework/data/rest/webmvc/RepositoryPropertyReferenceController.java

@@ -293,9 +293,14 @@ public ResponseEntity<?> createPropertyReference(
 						loadPropertyValue(prop.propertyType, source.getLinks().toList().get(0)));
 			}
 
-			publishEvent(new BeforeLinkSaveEvent(prop.accessor.getBean(), prop.propertyValue));
+			// Re-read the property after the mutation above so that the events carry the value actually being
+			// saved rather than the one captured before the assignment. The latter would be stale for PUT
+			// (which replaces collections/maps with a new container) and for singular references.
+			var linked = prop.accessor.getProperty(prop.property);
+
+			publishEvent(new BeforeLinkSaveEvent(prop.accessor.getBean(), linked));
 			var result = invoker.invokeSave(prop.accessor.getBean());
-			publishEvent(new AfterLinkSaveEvent(result, prop.propertyValue));
+			publishEvent(new AfterLinkSaveEvent(result, linked));
 
 			return null;
 		};

spring-data-rest-webmvc/src/test/java/org/springframework/data/rest/webmvc/RepositoryPropertyReferenceControllerUnitTests.java

@@ -15,19 +15,23 @@
  */
 package org.springframework.data.rest.webmvc;
 
+import static org.assertj.core.api.Assertions.*;
 import static org.mockito.ArgumentMatchers.*;
 import static org.mockito.Mockito.*;
 
 import java.util.ArrayList;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Optional;
 
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Mock;
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.springframework.context.ApplicationEventPublisher;
+import org.springframework.data.rest.core.event.BeforeLinkSaveEvent;
 import org.springframework.data.keyvalue.core.mapping.KeyValuePersistentEntity;
 import org.springframework.data.keyvalue.core.mapping.context.KeyValueMappingContext;
 import org.springframework.data.mapping.PersistentProperty;
@@ -90,9 +94,88 @@ void usesRepositoryInvokerToLookupRelatedInstance() throws Exception {
 		verify(invoker).invokeFindById("some-id");
 	}
 
+	@Test // Link-save events must carry the value actually being saved, not the pre-mutation one.
+	void publishesLinkSaveEventWithNewSingularReference() throws Exception {
+
+		KeyValuePersistentEntity<?, ?> entity = mappingContext.getRequiredPersistentEntity(Sample.class);
+
+		ResourceMappings mappings = new PersistentEntitiesResourceMappings(
+				new PersistentEntities(Collections.singleton(mappingContext)));
+		ResourceMetadata metadata = spy(mappings.getMetadataFor(Sample.class));
+		when(metadata.getSupportedHttpMethods()).thenReturn(AllSupportedHttpMethods.INSTANCE);
+
+		RepositoryPropertyReferenceController controller = new RepositoryPropertyReferenceController(repositories,
+				invokerFactory);
+		controller.setApplicationEventPublisher(publisher);
+
+		Sample owner = new Sample();
+		owner.reference = new Reference(); // the previously linked reference
+		Reference newReference = new Reference(); // the one being linked now
+
+		doReturn(invoker).when(invokerFactory).getInvokerFor(Reference.class);
+		doReturn(Optional.of(owner)).when(invoker).invokeFindById(4711);
+		doReturn(Optional.of(newReference)).when(invoker).invokeFindById("new-id");
+		doReturn(owner).when(invoker).invokeSave(any(Object.class));
+
+		RootResourceInformation information = new RootResourceInformation(metadata, entity, invoker);
+		CollectionModel<Object> request = CollectionModel.empty(Link.of("/reference/new-id"));
+
+		controller.createPropertyReference(information, HttpMethod.PUT, request, 4711, "reference");
+
+		assertThat(captureLinkSaveEvent().getLinked()).isSameAs(newReference);
+	}
+
+	@Test // Link-save events must carry the value actually being saved, not the pre-mutation one.
+	void publishesLinkSaveEventWithReplacedCollection() throws Exception {
+
+		KeyValuePersistentEntity<?, ?> entity = mappingContext.getRequiredPersistentEntity(Sample.class);
+
+		ResourceMappings mappings = new PersistentEntitiesResourceMappings(
+				new PersistentEntities(Collections.singleton(mappingContext)));
+		ResourceMetadata metadata = spy(mappings.getMetadataFor(Sample.class));
+		when(metadata.getSupportedHttpMethods()).thenReturn(AllSupportedHttpMethods.INSTANCE);
+
+		RepositoryPropertyReferenceController controller = new RepositoryPropertyReferenceController(repositories,
+				invokerFactory);
+		controller.setApplicationEventPublisher(publisher);
+
+		Sample owner = new Sample();
+		Reference oldReference = new Reference();
+		owner.references.add(oldReference); // the previously linked element
+		Reference newReference = new Reference(); // the one replacing it via PUT
+
+		doReturn(invoker).when(invokerFactory).getInvokerFor(Reference.class);
+		doReturn(Optional.of(owner)).when(invoker).invokeFindById(4711);
+		doReturn(Optional.of(newReference)).when(invoker).invokeFindById("new-id");
+		doReturn(owner).when(invoker).invokeSave(any(Object.class));
+
+		RootResourceInformation information = new RootResourceInformation(metadata, entity, invoker);
+		CollectionModel<Object> request = CollectionModel.empty(Link.of("/reference/new-id"));
+
+		controller.createPropertyReference(information, HttpMethod.PUT, request, 4711, "references");
+
+		@SuppressWarnings("unchecked")
+		Collection<Reference> linked = (Collection<Reference>) captureLinkSaveEvent().getLinked();
+
+		assertThat(linked).containsExactly(newReference).doesNotContain(oldReference);
+	}
+
+	private BeforeLinkSaveEvent captureLinkSaveEvent() {
+
+		ArgumentCaptor<Object> events = ArgumentCaptor.forClass(Object.class);
+		verify(publisher, atLeastOnce()).publishEvent(events.capture());
+
+		return events.getAllValues().stream() //
+				.filter(BeforeLinkSaveEvent.class::isInstance) //
+				.map(BeforeLinkSaveEvent.class::cast) //
+				.findFirst() //
+				.orElseThrow(() -> new AssertionError("No BeforeLinkSaveEvent published"));
+	}
+
 	@RestResource
 	static class Sample {
 		@org.springframework.data.annotation.Reference List<Reference> references = new ArrayList<Reference>();
+		@org.springframework.data.annotation.Reference Reference reference;
 	}
 
 	@RestResource