Properly emit domain events from calls to saveAll(…).

We now treat CrudRepository.saveAll(…) properly by unwrapping the given *Iterable*. This previously already worked for collections handed into the method but not for types only implementing Iterable directly (like Page or Window).

Fixes #3153.
Related tickets #2931, #2927.

Author: odrotbohm

src/main/java/org/springframework/data/repository/core/support/EventPublishingRepositoryProxyPostProcessor.java

@@ -54,7 +54,15 @@ public class EventPublishingRepositoryProxyPostProcessor implements RepositoryPr
 
 	private final ApplicationEventPublisher publisher;
 
+	/**
+	 * Creates a new {@link EventPublishingRepositoryProxyPostProcessor} for the given {@link ApplicationEventPublisher}.
+	 *
+	 * @param publisher must not be {@literal null}.
+	 */
 	public EventPublishingRepositoryProxyPostProcessor(ApplicationEventPublisher publisher) {
+
+		Assert.notNull(publisher, "Object must not be null");
+
 		this.publisher = publisher;
 	}
 
@@ -111,9 +119,9 @@ public Object invoke(MethodInvocation invocation) throws Throwable {
 				return result;
 			}
 
-			Object[] arguments = invocation.getArguments();
+			Iterable<?> arguments = asCollection(invocation.getArguments()[0], invocation.getMethod());
 
-			eventMethod.publishEventsFrom(arguments[0], publisher);
+			eventMethod.publishEventsFrom(arguments, publisher);
 
 			return result;
 		}
@@ -185,22 +193,18 @@ public static EventPublishingMethod of(Class<?> type) {
 		/**
 		 * Publishes all events in the given aggregate root using the given {@link ApplicationEventPublisher}.
 		 *
-		 * @param object can be {@literal null}.
+		 * @param aggregates can be {@literal null}.
 		 * @param publisher must not be {@literal null}.
 		 */
-		public void publishEventsFrom(@Nullable Object object, ApplicationEventPublisher publisher) {
+		public void publishEventsFrom(Iterable<?> aggregates, ApplicationEventPublisher publisher) {
 
-			if (object == null) {
-				return;
-			}
-
-			for (Object aggregateRoot : asCollection(object)) {
+			for (Object aggregateRoot : aggregates) {
 
 				if (!type.isInstance(aggregateRoot)) {
 					continue;
 				}
 
-				for (Object event : asCollection(ReflectionUtils.invokeMethod(publishingMethod, aggregateRoot))) {
+				for (Object event : asCollection(ReflectionUtils.invokeMethod(publishingMethod, aggregateRoot), null)) {
 					publisher.publishEvent(event);
 				}
 
@@ -269,25 +273,30 @@ private static Method getClearingMethod(AnnotationDetectionMethodCallback<?> cle
 			return method;
 		}
 
-		/**
-		 * Returns the given source object as collection, i.e. collections are returned as is, objects are turned into a
-		 * one-element collection, {@literal null} will become an empty collection.
-		 *
-		 * @param source can be {@literal null}.
-		 * @return
-		 */
-		@SuppressWarnings("unchecked")
-		private static Collection<Object> asCollection(@Nullable Object source) {
+	}
 
-			if (source == null) {
-				return Collections.emptyList();
-			}
+	/**
+	 * Returns the given source object as collection, i.e. collections are returned as is, objects are turned into a
+	 * one-element collection, {@literal null} will become an empty collection.
+	 *
+	 * @param source can be {@literal null}.
+	 * @return
+	 */
+	@SuppressWarnings("unchecked")
+	private static Iterable<Object> asCollection(@Nullable Object source, @Nullable Method method) {
 
-			if (Collection.class.isInstance(source)) {
-				return (Collection<Object>) source;
-			}
+		if (source == null) {
+			return Collections.emptyList();
+		}
 
-			return Collections.singletonList(source);
+		if (method != null && method.getName().startsWith("saveAll")) {
+			return (Iterable<Object>) source;
 		}
+
+		if (Collection.class.isInstance(source)) {
+			return (Collection<Object>) source;
+		}
+
+		return Collections.singletonList(source);
 	}
 }

src/test/java/org/springframework/data/repository/core/support/EventPublishingRepositoryProxyPostProcessorUnitTests.java

@@ -37,7 +37,6 @@
 import org.mockito.junit.jupiter.MockitoExtension;
 import org.mockito.junit.jupiter.MockitoSettings;
 import org.mockito.quality.Strictness;
-
 import org.springframework.aop.framework.ProxyFactory;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.data.domain.AfterDomainEventPublication;
@@ -68,19 +67,14 @@ void rejectsNullAggregateTypes() {
 		assertThatIllegalArgumentException().isThrownBy(() -> EventPublishingMethod.of(null));
 	}
 
-	@Test // DATACMNS-928
-	void publishingEventsForNullIsNoOp() {
-		EventPublishingMethod.of(OneEvent.class).publishEventsFrom(null, publisher);
-	}
-
 	@Test // DATACMNS-928
 	void exposesEventsExposedByEntityToPublisher() {
 
 		SomeEvent first = new SomeEvent();
 		SomeEvent second = new SomeEvent();
 		MultipleEvents entity = MultipleEvents.of(Arrays.asList(first, second));
 
-		EventPublishingMethod.of(MultipleEvents.class).publishEventsFrom(entity, publisher);
+		EventPublishingMethod.of(MultipleEvents.class).publishEventsFrom(Arrays.asList(entity), publisher);
 
 		verify(publisher).publishEvent(eq(first));
 		verify(publisher).publishEvent(eq(second));
@@ -92,7 +86,7 @@ void exposesSingleEventByEntityToPublisher() {
 		SomeEvent event = new SomeEvent();
 		OneEvent entity = OneEvent.of(event);
 
-		EventPublishingMethod.of(OneEvent.class).publishEventsFrom(entity, publisher);
+		EventPublishingMethod.of(OneEvent.class).publishEventsFrom(Arrays.asList(entity), publisher);
 
 		verify(publisher, times(1)).publishEvent(event);
 	}
@@ -102,7 +96,7 @@ void doesNotExposeNullEvent() {
 
 		OneEvent entity = OneEvent.of(null);
 
-		EventPublishingMethod.of(OneEvent.class).publishEventsFrom(entity, publisher);
+		EventPublishingMethod.of(OneEvent.class).publishEventsFrom(Arrays.asList(entity), publisher);
 
 		verify(publisher, times(0)).publishEvent(any());
 	}
@@ -194,7 +188,7 @@ void publishesEventsForCallToSaveWithIterable() throws Throwable {
 
 		SomeEvent event = new SomeEvent();
 		MultipleEvents sample = MultipleEvents.of(Collections.singletonList(event));
-		mockInvocation(invocation, SampleRepository.class.getMethod("saveAll", Iterable.class), sample);
+		mockInvocation(invocation, SampleRepository.class.getMethod("saveAll", Iterable.class), Arrays.asList(sample));
 
 		EventPublishingMethodInterceptor//
 				.of(EventPublishingMethod.of(MultipleEvents.class), publisher)//
@@ -208,7 +202,7 @@ void publishesEventsForCallToDeleteWithIterable() throws Throwable {
 
 		SomeEvent event = new SomeEvent();
 		MultipleEvents sample = MultipleEvents.of(Collections.singletonList(event));
-		mockInvocation(invocation, SampleRepository.class.getMethod("deleteAll", Iterable.class), sample);
+		mockInvocation(invocation, SampleRepository.class.getMethod("deleteAll", Iterable.class), Arrays.asList(sample));
 
 		EventPublishingMethodInterceptor//
 				.of(EventPublishingMethod.of(MultipleEvents.class), publisher)//
@@ -222,7 +216,8 @@ void publishesEventsForCallToDeleteInBatchWithIterable() throws Throwable {
 
 		SomeEvent event = new SomeEvent();
 		MultipleEvents sample = MultipleEvents.of(Collections.singletonList(event));
-		mockInvocation(invocation, SampleRepository.class.getMethod("deleteInBatch", Iterable.class), sample);
+		mockInvocation(invocation, SampleRepository.class.getMethod("deleteInBatch", Iterable.class),
+				Arrays.asList(sample));
 
 		EventPublishingMethodInterceptor//
 				.of(EventPublishingMethod.of(MultipleEvents.class), publisher)//
@@ -236,7 +231,8 @@ void publishesEventsForCallToDeleteAllInBatchWithIterable() throws Throwable {
 
 		SomeEvent event = new SomeEvent();
 		MultipleEvents sample = MultipleEvents.of(Collections.singletonList(event));
-		mockInvocation(invocation, SampleRepository.class.getMethod("deleteAllInBatch", Iterable.class), sample);
+		mockInvocation(invocation, SampleRepository.class.getMethod("deleteAllInBatch", Iterable.class),
+				Arrays.asList(sample));
 
 		EventPublishingMethodInterceptor//
 				.of(EventPublishingMethod.of(MultipleEvents.class), publisher)//
@@ -278,7 +274,7 @@ void clearsEventsEvenIfNoneWereExposedToPublish() {
 
 		EventsWithClearing entity = spy(EventsWithClearing.of(Collections.emptyList()));
 
-		EventPublishingMethod.of(EventsWithClearing.class).publishEventsFrom(entity, publisher);
+		EventPublishingMethod.of(EventsWithClearing.class).publishEventsFrom(Arrays.asList(entity), publisher);
 
 		verify(entity, times(1)).clearDomainEvents();
 	}
@@ -288,7 +284,7 @@ void clearsEventsIfThereWereSomeToBePublished() {
 
 		EventsWithClearing entity = spy(EventsWithClearing.of(Collections.singletonList(new SomeEvent())));
 
-		EventPublishingMethod.of(EventsWithClearing.class).publishEventsFrom(entity, publisher);
+		EventPublishingMethod.of(EventsWithClearing.class).publishEventsFrom(Arrays.asList(entity), publisher);
 
 		verify(entity, times(1)).clearDomainEvents();
 	}