DATAREST-970 - AnnotationEventHandlerInvoker now considers order of event handler methods.

We now make sure that an @order annotation on annotated event handler methods are considered and the methods are invoked in the defined order.

Non-annotation-based event handlers don't suffer from the same problem as they're ApplicationListener instances directly so that the container will enforce the correct ordering in case @order is used or Ordered is implemented.

Some cleanup in EventHandlerMethod.

Original pull request: #248.

Author: odrotbohm

spring-data-rest-core/src/main/java/org/springframework/data/rest/core/event/AnnotatedEventHandlerInvoker.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2012-2014 the original author or authors.
+ * Copyright 2012-2017 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,9 +15,14 @@
  */
 package org.springframework.data.rest.core.event;
 
+import lombok.EqualsAndHashCode;
+import lombok.RequiredArgsConstructor;
+import lombok.ToString;
+
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 import org.slf4j.Logger;
@@ -26,6 +31,7 @@
 import org.springframework.beans.factory.config.BeanPostProcessor;
 import org.springframework.context.ApplicationEvent;
 import org.springframework.context.ApplicationListener;
+import org.springframework.core.annotation.AnnotationAwareOrderComparator;
 import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.data.rest.core.annotation.HandleAfterCreate;
 import org.springframework.data.rest.core.annotation.HandleAfterDelete;
@@ -165,37 +171,50 @@ private <T extends Annotation> void inspect(Object handler, Method method, Class
 			throw new IllegalStateException(String.format(PARAMETER_MISSING, method));
 		}
 
-		EventHandlerMethod handlerMethod = new EventHandlerMethod(parameterTypes[0], handler, method);
+		EventHandlerMethod handlerMethod = EventHandlerMethod.of(parameterTypes[0], handler, method);
 
 		if (LOG.isDebugEnabled()) {
 			LOG.debug("Annotated handler method found: {}", handlerMethod);
 		}
 
-		handlerMethods.add(eventType, handlerMethod);
+		List<EventHandlerMethod> events = handlerMethods.get(eventType);
+
+		if (events == null) {
+			events = new ArrayList<EventHandlerMethod>();
+		}
+
+		if (events.isEmpty()) {
+			handlerMethods.add(eventType, handlerMethod);
+			return;
+		}
+
+		events.add(handlerMethod);
+		Collections.sort(events);
+		handlerMethods.put(eventType, events);
 	}
 
-	static class EventHandlerMethod {
+	@ToString
+	@EqualsAndHashCode
+	@RequiredArgsConstructor
+	static class EventHandlerMethod implements Comparable<EventHandlerMethod> {
 
 		final Class<?> targetType;
 		final Method method;
 		final Object handler;
 
-		private EventHandlerMethod(Class<?> targetType, Object handler, Method method) {
-
-			this.targetType = targetType;
-			this.method = method;
-			this.handler = handler;
+		public static EventHandlerMethod of(Class<?> targetType, Object handler, Method method) {
 
-			ReflectionUtils.makeAccessible(this.method);
+			ReflectionUtils.makeAccessible(method);
+			return new EventHandlerMethod(targetType, method, handler);
 		}
 
-		/*
+		/* 
 		 * (non-Javadoc)
-		 * @see java.lang.Object#toString()
+		 * @see java.lang.Comparable#compareTo(java.lang.Object)
 		 */
 		@Override
-		public String toString() {
-			return String.format("EventHandlerMethod{ targetType=%s, method=%s, handler=%s }", targetType, method, handler);
+		public int compareTo(EventHandlerMethod o) {
+			return AnnotationAwareOrderComparator.INSTANCE.compare(this.method, o.method);
 		}
 	}
 }

spring-data-rest-core/src/test/java/org/springframework/data/rest/core/event/AnnotatedEventHandlerInvokerUnitTests.java

@@ -20,6 +20,7 @@
 
 import org.junit.Test;
 import org.springframework.aop.framework.ProxyFactory;
+import org.springframework.core.annotation.Order;
 import org.springframework.data.rest.core.annotation.HandleBeforeCreate;
 import org.springframework.data.rest.core.annotation.RepositoryEventHandler;
 import org.springframework.data.rest.core.domain.Person;
@@ -32,6 +33,7 @@
  *
  * @author Oliver Gierke
  * @author Fabian Trampusch
+ * @author Joseph Valerio
  */
 public class AnnotatedEventHandlerInvokerUnitTests {
 
@@ -65,6 +67,24 @@ public void invokesPrivateEventHandlerMethods() {
 		assertThat(sampleHandler.wasCalled, is(true));
 	}
 
+	@Test // DATAREST-970
+	public void invokesEventHandlerInOrderMethods() {
+
+		SampleOrderEventHandler1 orderHandler1 = new SampleOrderEventHandler1();
+		SampleOrderEventHandler2 orderHandler2 = new SampleOrderEventHandler2();
+
+		AnnotatedEventHandlerInvoker invoker = new AnnotatedEventHandlerInvoker();
+		invoker.postProcessAfterInitialization(orderHandler1, "orderHandler1");
+		invoker.postProcessAfterInitialization(orderHandler2, "orderHandler2");
+
+		invoker.onApplicationEvent(new BeforeCreateEvent(new Person("Dave", "Matthews")));
+
+		assertThat(orderHandler1.wasCalled, is(true));
+		assertThat(orderHandler2.wasCalled, is(true));
+
+		assertThat(orderHandler1.timestamp, is(greaterThan(orderHandler2.timestamp)));
+	}
+
 	@RepositoryEventHandler
 	static class Sample {
 
@@ -82,4 +102,32 @@ private void method(Person sample) {
 			wasCalled = true;
 		}
 	}
+
+	@RepositoryEventHandler
+	static class SampleOrderEventHandler1 {
+
+		boolean wasCalled = false;
+		long timestamp;
+
+		@Order(2)
+		@HandleBeforeCreate
+		private void method(Person sample) {
+			wasCalled = true;
+			timestamp = System.nanoTime();
+		}
+	}
+
+	@RepositoryEventHandler
+	static class SampleOrderEventHandler2 {
+
+		boolean wasCalled = false;
+		long timestamp;
+
+		@Order(1)
+		@HandleBeforeCreate
+		private void method(Person sample) {
+			wasCalled = true;
+			timestamp = System.nanoTime();
+		}
+	}
 }