DATACMNS-1067 - Polishing.

Revert change to only invoke cleanup method if events have been exposed. We now again invoke the cleanup method for every aggregate. Changed the publication of events from the aggregate instances that were handed into the method to the ones the save method returns as the save call might return different object instances.

Cleanups in the unit tests. Moved newly introduced methods to the bottom of the test case class. Extracted method to set up mock method invocation.

Original pull request: #216.

Author: odrotbohm

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

@@ -95,9 +95,7 @@ public Object invoke(MethodInvocation invocation) throws Throwable {
 				return result;
 			}
 
-			for (Object argument : invocation.getArguments()) {
-				eventMethod.publishEventsFrom(argument, publisher);
-			}
+			eventMethod.publishEventsFrom(result, publisher);
 
 			return result;
 		}
@@ -166,12 +164,12 @@ public void publishEventsFrom(Object object, ApplicationEventPublisher publisher
 			}
 
 			for (Object aggregateRoot : asCollection(object)) {
-				Collection<Object> events = asCollection(ReflectionUtils.invokeMethod(publishingMethod, aggregateRoot));
-				for (Object event : events) {
+
+				for (Object event : asCollection(ReflectionUtils.invokeMethod(publishingMethod, aggregateRoot))) {
 					publisher.publishEvent(event);
 				}
 
-				if (clearingMethod != null && !events.isEmpty()) {
+				if (clearingMethod != null) {
 					ReflectionUtils.invokeMethod(clearingMethod, aggregateRoot);
 				}
 			}

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

@@ -105,40 +105,6 @@ public void doesNotExposeNullEvent() {
 		verify(publisher, times(0)).publishEvent(any());
 	}
 
-	@Test // DATACMNS-1067
-	public void clearEventsDoesNotExposedByEntity() {
-
-		EventsWithClearing entity = spy(EventsWithClearing.of(Collections.emptyList()));
-
-		EventPublishingMethod.of(EventsWithClearing.class).publishEventsFrom(entity, publisher);
-
-		verify(entity, times(0)).clearDomainEvents();
-	}
-
-	@Test // DATACMNS-1067
-	public void clearEventsExposedByEntity() {
-
-		EventsWithClearing entity = spy(EventsWithClearing.of(Collections.singletonList(new SomeEvent())));
-
-		EventPublishingMethod.of(EventsWithClearing.class).publishEventsFrom(entity, publisher);
-
-		verify(entity, times(1)).clearDomainEvents();
-	}
-
-	@Test // DATACMNS-1067
-	public void clearEventsExposedByEntities() {
-
-		EventsWithClearing firstEntity = spy(EventsWithClearing.of(Collections.emptyList()));
-		EventsWithClearing secondEntity = spy(EventsWithClearing.of(Collections.singletonList(new SomeEvent())));
-
-		Collection<EventsWithClearing> entities = Arrays.asList(firstEntity, secondEntity);
-
-		EventPublishingMethod.of(EventsWithClearing.class).publishEventsFrom(entities, publisher);
-
-		verify(firstEntity, times(0)).clearDomainEvents();
-		verify(secondEntity, times(1)).clearDomainEvents();
-	}
-
 	@Test // DATACMNS-928
 	public void doesNotCreatePublishingMethodIfNoAnnotationDetected() {
 		assertThat(EventPublishingMethod.of(Object.class), is(nullValue()));
@@ -147,11 +113,9 @@ public void doesNotCreatePublishingMethodIfNoAnnotationDetected() {
 	@Test // DATACMNS-928
 	public void interceptsSaveMethod() throws Throwable {
 
-		doReturn(SampleRepository.class.getMethod("save", Object.class)).when(invocation).getMethod();
-
 		SomeEvent event = new SomeEvent();
-		MultipleEvents sample = MultipleEvents.of(Arrays.asList(event));
-		doReturn(new Object[] { sample }).when(invocation).getArguments();
+		MultipleEvents sample = MultipleEvents.of(Collections.singletonList(event));
+		mockInvocation(invocation, SampleRepository.class.getMethod("save", Object.class), sample);
 
 		EventPublishingMethodInterceptor//
 				.of(EventPublishingMethod.of(MultipleEvents.class), publisher)//
@@ -202,10 +166,8 @@ public void doesNotAddAdviceIfDomainTypeDoesNotExposeEvents() {
 	public void publishesEventsForCallToSaveWithIterable() throws Throwable {
 
 		SomeEvent event = new SomeEvent();
-		MultipleEvents sample = MultipleEvents.of(Arrays.asList(event));
-		doReturn(new Object[] { Arrays.asList(sample) }).when(invocation).getArguments();
-
-		doReturn(SampleRepository.class.getMethod("save", Iterable.class)).when(invocation).getMethod();
+		MultipleEvents sample = MultipleEvents.of(Collections.singletonList(event));
+		mockInvocation(invocation, SampleRepository.class.getMethod("save", Iterable.class), sample);
 
 		EventPublishingMethodInterceptor//
 				.of(EventPublishingMethod.of(MultipleEvents.class), publisher)//
@@ -234,12 +196,9 @@ public void publishesEventsAfterSaveInvocation() throws Throwable {
 	@Test // DATACMNS-1113
 	public void invokesEventsForMethodsThatStartsWithSave() throws Throwable {
 
-		Method method = SampleRepository.class.getMethod("saveAndFlush", MultipleEvents.class);
-		doReturn(method).when(invocation).getMethod();
-
 		SomeEvent event = new SomeEvent();
 		MultipleEvents sample = MultipleEvents.of(Collections.singletonList(event));
-		doReturn(new Object[] { sample }).when(invocation).getArguments();
+		mockInvocation(invocation, SampleRepository.class.getMethod("saveAndFlush", MultipleEvents.class), sample);
 
 		EventPublishingMethodInterceptor//
 				.of(EventPublishingMethod.of(MultipleEvents.class), publisher)//
@@ -248,6 +207,48 @@ public void invokesEventsForMethodsThatStartsWithSave() throws Throwable {
 		verify(publisher).publishEvent(event);
 	}
 
+	@Test // DATACMNS-1067
+	public void clearsEventsEvenIfNoneWereExposedToPublish() {
+
+		EventsWithClearing entity = spy(EventsWithClearing.of(Collections.emptyList()));
+
+		EventPublishingMethod.of(EventsWithClearing.class).publishEventsFrom(entity, publisher);
+
+		verify(entity, times(1)).clearDomainEvents();
+	}
+
+	@Test // DATACMNS-1067
+	public void clearsEventsIfThereWereSomeToBePublished() {
+
+		EventsWithClearing entity = spy(EventsWithClearing.of(Collections.singletonList(new SomeEvent())));
+
+		EventPublishingMethod.of(EventsWithClearing.class).publishEventsFrom(entity, publisher);
+
+		verify(entity, times(1)).clearDomainEvents();
+	}
+
+	@Test // DATACMNS-1067
+	public void clearsEventsForOperationOnMutlipleAggregates() {
+
+		EventsWithClearing firstEntity = spy(EventsWithClearing.of(Collections.emptyList()));
+		EventsWithClearing secondEntity = spy(EventsWithClearing.of(Collections.singletonList(new SomeEvent())));
+
+		Collection<EventsWithClearing> entities = Arrays.asList(firstEntity, secondEntity);
+
+		EventPublishingMethod.of(EventsWithClearing.class).publishEventsFrom(entities, publisher);
+
+		verify(firstEntity, times(1)).clearDomainEvents();
+		verify(secondEntity, times(1)).clearDomainEvents();
+	}
+
+	private static void mockInvocation(MethodInvocation invocation, Method method, Object parameterAndReturnValue)
+			throws Throwable {
+
+		doReturn(method).when(invocation).getMethod();
+		doReturn(new Object[] { parameterAndReturnValue }).when(invocation).getArguments();
+		doReturn(parameterAndReturnValue).when(invocation).proceed();
+	}
+
 	@Value(staticConstructor = "of")
 	static class MultipleEvents {
 		@Getter(onMethod = @__(@DomainEvents)) Collection<? extends Object> events;