Detect Order on target class as well

Previously, the `@order` annotation was managed in an inconsistent way
when placed at the implementation level. For simple beans, it was
discovered properly but wasn't for beans requiring a proxy.

OrderComparator.SourceProvider now explicitly allows to return several
order sources; the default implementation returns not only the factory
method (if  any) but also the target class if it happens to be different
from the class of the bean.

Issue: SPR-12636

spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java

@@ -1459,14 +1459,27 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
 
 		@Override
 		public Object getOrderSource(Object obj) {
-			return getFactoryMethod(this.instancesToBeanNames.get(obj));
+			RootBeanDefinition beanDefinition = getRootBeanDefinition(this.instancesToBeanNames.get(obj));
+			if (beanDefinition == null) {
+				return null;
+			}
+			List<Object> sources = new ArrayList<Object>();
+			Method factoryMethod = beanDefinition.getResolvedFactoryMethod();
+			if (factoryMethod != null) {
+				sources.add(factoryMethod);
+			}
+			Class<?> targetType = beanDefinition.getTargetType();
+			if (targetType != null && !targetType.equals(obj.getClass())) {
+				sources.add(targetType);
+			}
+			return sources.toArray(new Object[sources.size()]);
 		}
 
-		private Method getFactoryMethod(String beanName) {
+		private RootBeanDefinition getRootBeanDefinition(String beanName) {
 			if (beanName != null && containsBeanDefinition(beanName)) {
 				BeanDefinition bd = getMergedBeanDefinition(beanName);
 				if (bd instanceof RootBeanDefinition) {
-					return ((RootBeanDefinition) bd).getResolvedFactoryMethod();
+					return (RootBeanDefinition) bd;
 				}
 			}
 			return null;

spring-context/src/test/java/org/springframework/context/annotation/Spr12636Tests.java

+/*
+ * Copyright 2002-2015 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.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.springframework.context.annotation;
+
+import java.util.List;
+
+import org.junit.After;
+import org.junit.Test;
+
+import org.springframework.aop.support.AopUtils;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.context.ConfigurableApplicationContext;
+import org.springframework.core.annotation.Order;
+import org.springframework.scheduling.annotation.Async;
+import org.springframework.scheduling.annotation.EnableAsync;
+import org.springframework.stereotype.Component;
+
+import static org.junit.Assert.*;
+
+/**
+ * @author Stephane Nicoll
+ */
+public class Spr12636Tests {
+
+	private ConfigurableApplicationContext context;
+
+	@After
+	public void closeContext() {
+		if (this.context != null) {
+			this.context.close();
+		}
+	}
+
+	@Test
+	public void orderOnImplementation() {
+		this.context = new AnnotationConfigApplicationContext(
+				UserServiceTwo.class, UserServiceOne.class, UserServiceCollector.class);
+		UserServiceCollector bean = this.context.getBean(UserServiceCollector.class);
+		assertSame(context.getBean("serviceOne", UserService.class), bean.userServices.get(0));
+		assertSame(context.getBean("serviceTwo", UserService.class), bean.userServices.get(1));
+
+	}
+
+	@Test
+	public void orderOnImplementationWithProxy() {
+		this.context = new AnnotationConfigApplicationContext(
+				UserServiceTwo.class, UserServiceOne.class, UserServiceCollector.class, AsyncConfig.class);
+
+		// Validate those beans are indeed wrapped by a proxy
+		UserService serviceOne = this.context.getBean("serviceOne", UserService.class);
+		UserService serviceTwo = this.context.getBean("serviceTwo", UserService.class);
+		assertTrue(AopUtils.isAopProxy(serviceOne));
+		assertTrue(AopUtils.isAopProxy(serviceTwo));
+
+		UserServiceCollector bean = this.context.getBean(UserServiceCollector.class);
+		assertSame(serviceOne, bean.userServices.get(0));
+		assertSame(serviceTwo, bean.userServices.get(1));
+	}
+
+	@Configuration
+	@EnableAsync
+	static class AsyncConfig {
+	}
+
+
+	@Component
+	static class UserServiceCollector {
+
+		public final List<UserService> userServices;
+
+		@Autowired
+		UserServiceCollector(List<UserService> userServices) {
+			this.userServices = userServices;
+		}
+	}
+
+	interface UserService {
+
+		void doIt();
+	}
+
+	@Component("serviceOne")
+	@Order(1)
+	static class UserServiceOne implements UserService {
+
+		@Async
+		@Override
+		public void doIt() {
+
+		}
+	}
+
+	@Component("serviceTwo")
+	@Order(2)
+	static class UserServiceTwo implements UserService {
+
+		@Async
+		@Override
+		public void doIt() {
+
+		}
+	}
+}

spring-core/src/main/java/org/springframework/core/OrderComparator.java

@@ -21,6 +21,8 @@ import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
 
+import org.springframework.util.ObjectUtils;
+
 /**
  * {@link Comparator} implementation for {@link Ordered} objects,
  * sorting by order value ascending (resp. by priority descending).
@@ -89,7 +91,19 @@ public class OrderComparator implements Comparator<Object> {
 	private int getOrder(Object obj, OrderSourceProvider sourceProvider) {
 		Integer order = null;
 		if (sourceProvider != null) {
-			order = findOrder(sourceProvider.getOrderSource(obj));
+			Object orderSource = sourceProvider.getOrderSource(obj);
+			if (orderSource != null && orderSource.getClass().isArray()) {
+				Object[] sources = ObjectUtils.toObjectArray(orderSource);
+				for (Object source : sources) {
+					order = findOrder(source);
+					if (order != null) {
+						break;
+					}
+				}
+			}
+			else {
+				order = findOrder(orderSource);
+			}
 		}
 		return (order != null ? order : getOrder(obj));
 	}
@@ -186,6 +200,7 @@ public class OrderComparator implements Comparator<Object> {
 		/**
 		 * Return an order source for the specified object, i.e. an object that
 		 * should be checked for an order value as a replacement to the given object.
+		 * <p>Can also be an array of order source objects.
 		 * <p>If the returned object does not indicate any order, the comparator
 		 * will fall back to checking the original object.
 		 * @param obj the object to find an order source for

spring-core/src/test/java/org/springframework/core/OrderComparatorTests.java

@@ -18,43 +18,90 @@ package org.springframework.core;
 
 import java.util.Comparator;
 
-import junit.framework.TestCase;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
 
 /**
  * Unit tests for the {@link OrderComparator} class.
  *
  * @author Rick Evans
+ * @author Stephane Nicoll
  */
-public final class OrderComparatorTests extends TestCase {
-
-	private Comparator comparator;
-
-
-	@Override
-	protected void setUp() throws Exception {
-		this.comparator = new OrderComparator();
-	}
+public final class OrderComparatorTests {
 
+	private final OrderComparator comparator = new OrderComparator();
 
-	public void testCompareOrderedInstancesBefore() throws Exception {
+	@Test
+	public void compareOrderedInstancesBefore() {
 		assertEquals(-1, this.comparator.compare(
 				new StubOrdered(100), new StubOrdered(2000)));
 	}
 
-	public void testCompareOrderedInstancesSame() throws Exception {
+	@Test
+	public void compareOrderedInstancesSame() {
 		assertEquals(0, this.comparator.compare(
 				new StubOrdered(100), new StubOrdered(100)));
 	}
 
-	public void testCompareOrderedInstancesAfter() throws Exception {
+	@Test
+	public void compareOrderedInstancesAfter() {
 		assertEquals(1, this.comparator.compare(
 				new StubOrdered(982300), new StubOrdered(100)));
 	}
 
-	public void testCompareTwoNonOrderedInstancesEndsUpAsSame() throws Exception {
+	@Test
+	public void compareTwoNonOrderedInstancesEndsUpAsSame() {
 		assertEquals(0, this.comparator.compare(new Object(), new Object()));
 	}
 
+	@Test
+	public void compareWithSimpleSourceProvider() {
+		Comparator<Object> customComparator = this.comparator.withSourceProvider(
+				new TestSourceProvider(5L, new StubOrdered(25)));
+		assertEquals(-1, customComparator.compare(new StubOrdered(10), 5L));
+	}
+
+	@Test
+	public void compareWithSourceProviderArray() {
+		Comparator<Object> customComparator = this.comparator.withSourceProvider(
+				new TestSourceProvider(5L, new Object[] {new StubOrdered(10), new StubOrdered(-25)}));
+		assertEquals(-1, customComparator.compare(5L, new Object()));
+	}
+
+	@Test
+	public void compareWithSourceProviderArrayNoMatch() {
+		Comparator<Object> customComparator = this.comparator.withSourceProvider(
+				new TestSourceProvider(5L, new Object[]{new Object(), new Object()}));
+		assertEquals(0, customComparator.compare(new Object(), 5L));
+	}
+
+	@Test
+	public void compareWithSourceProviderEmpty() {
+		Comparator<Object> customComparator = this.comparator.withSourceProvider(
+				new TestSourceProvider(50L, new Object()));
+		assertEquals(0, customComparator.compare(new Object(), 5L));
+	}
+
+
+	private static final class TestSourceProvider implements OrderComparator.OrderSourceProvider {
+
+		private final Object target;
+		private final Object orderSource;
+
+		public TestSourceProvider(Object target, Object orderSource) {
+			this.target = target;
+			this.orderSource = orderSource;
+		}
+
+		@Override
+		public Object getOrderSource(Object obj) {
+			if (target.equals(obj)) {
+				return orderSource;
+			}
+			return null;
+		}
+	}
 
 	private static final class StubOrdered implements Ordered {