Skip to content

Commit 1aec6a6

Browse files
snicollsnicoll
committed
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
1 parent f20a624 commit 1aec6a6

File tree

4 files changed

+210
-18
lines changed

4 files changed

+210
-18
lines changed

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

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1459,14 +1459,27 @@ public FactoryAwareOrderSourceProvider(Map<Object, String> instancesToBeanNames)
14591459

14601460
@Override
14611461
public Object getOrderSource(Object obj) {
1462-
return getFactoryMethod(this.instancesToBeanNames.get(obj));
1462+
RootBeanDefinition beanDefinition = getRootBeanDefinition(this.instancesToBeanNames.get(obj));
1463+
if (beanDefinition == null) {
1464+
return null;
1465+
}
1466+
List<Object> sources = new ArrayList<Object>();
1467+
Method factoryMethod = beanDefinition.getResolvedFactoryMethod();
1468+
if (factoryMethod != null) {
1469+
sources.add(factoryMethod);
1470+
}
1471+
Class<?> targetType = beanDefinition.getTargetType();
1472+
if (targetType != null && !targetType.equals(obj.getClass())) {
1473+
sources.add(targetType);
1474+
}
1475+
return sources.toArray(new Object[sources.size()]);
14631476
}
14641477

1465-
private Method getFactoryMethod(String beanName) {
1478+
private RootBeanDefinition getRootBeanDefinition(String beanName) {
14661479
if (beanName != null && containsBeanDefinition(beanName)) {
14671480
BeanDefinition bd = getMergedBeanDefinition(beanName);
14681481
if (bd instanceof RootBeanDefinition) {
1469-
return ((RootBeanDefinition) bd).getResolvedFactoryMethod();
1482+
return (RootBeanDefinition) bd;
14701483
}
14711484
}
14721485
return null;

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

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
/*
2+
* Copyright 2002-2015 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.context.annotation;
18+
19+
import java.util.List;
20+
21+
import org.junit.After;
22+
import org.junit.Test;
23+
24+
import org.springframework.aop.support.AopUtils;
25+
import org.springframework.beans.factory.annotation.Autowired;
26+
import org.springframework.context.ConfigurableApplicationContext;
27+
import org.springframework.core.annotation.Order;
28+
import org.springframework.scheduling.annotation.Async;
29+
import org.springframework.scheduling.annotation.EnableAsync;
30+
import org.springframework.stereotype.Component;
31+
32+
import static org.junit.Assert.*;
33+
34+
/**
35+
* @author Stephane Nicoll
36+
*/
37+
public class Spr12636Tests {
38+
39+
private ConfigurableApplicationContext context;
40+
41+
@After
42+
public void closeContext() {
43+
if (this.context != null) {
44+
this.context.close();
45+
}
46+
}
47+
48+
@Test
49+
public void orderOnImplementation() {
50+
this.context = new AnnotationConfigApplicationContext(
51+
UserServiceTwo.class, UserServiceOne.class, UserServiceCollector.class);
52+
UserServiceCollector bean = this.context.getBean(UserServiceCollector.class);
53+
assertSame(context.getBean("serviceOne", UserService.class), bean.userServices.get(0));
54+
assertSame(context.getBean("serviceTwo", UserService.class), bean.userServices.get(1));
55+
56+
}
57+
58+
@Test
59+
public void orderOnImplementationWithProxy() {
60+
this.context = new AnnotationConfigApplicationContext(
61+
UserServiceTwo.class, UserServiceOne.class, UserServiceCollector.class, AsyncConfig.class);
62+
63+
// Validate those beans are indeed wrapped by a proxy
64+
UserService serviceOne = this.context.getBean("serviceOne", UserService.class);
65+
UserService serviceTwo = this.context.getBean("serviceTwo", UserService.class);
66+
assertTrue(AopUtils.isAopProxy(serviceOne));
67+
assertTrue(AopUtils.isAopProxy(serviceTwo));
68+
69+
UserServiceCollector bean = this.context.getBean(UserServiceCollector.class);
70+
assertSame(serviceOne, bean.userServices.get(0));
71+
assertSame(serviceTwo, bean.userServices.get(1));
72+
}
73+
74+
@Configuration
75+
@EnableAsync
76+
static class AsyncConfig {
77+
}
78+
79+
80+
@Component
81+
static class UserServiceCollector {
82+
83+
public final List<UserService> userServices;
84+
85+
@Autowired
86+
UserServiceCollector(List<UserService> userServices) {
87+
this.userServices = userServices;
88+
}
89+
}
90+
91+
interface UserService {
92+
93+
void doIt();
94+
}
95+
96+
@Component("serviceOne")
97+
@Order(1)
98+
static class UserServiceOne implements UserService {
99+
100+
@Async
101+
@Override
102+
public void doIt() {
103+
104+
}
105+
}
106+
107+
@Component("serviceTwo")
108+
@Order(2)
109+
static class UserServiceTwo implements UserService {
110+
111+
@Async
112+
@Override
113+
public void doIt() {
114+
115+
}
116+
}
117+
}

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@
2121
import java.util.Comparator;
2222
import java.util.List;
2323

24+
import org.springframework.util.ObjectUtils;
25+
2426
/**
2527
* {@link Comparator} implementation for {@link Ordered} objects,
2628
* sorting by order value ascending (resp. by priority descending).
@@ -89,7 +91,19 @@ else if (p2 && !p1) {
8991
private int getOrder(Object obj, OrderSourceProvider sourceProvider) {
9092
Integer order = null;
9193
if (sourceProvider != null) {
92-
order = findOrder(sourceProvider.getOrderSource(obj));
94+
Object orderSource = sourceProvider.getOrderSource(obj);
95+
if (orderSource != null && orderSource.getClass().isArray()) {
96+
Object[] sources = ObjectUtils.toObjectArray(orderSource);
97+
for (Object source : sources) {
98+
order = findOrder(source);
99+
if (order != null) {
100+
break;
101+
}
102+
}
103+
}
104+
else {
105+
order = findOrder(orderSource);
106+
}
93107
}
94108
return (order != null ? order : getOrder(obj));
95109
}
@@ -186,6 +200,7 @@ public static interface OrderSourceProvider {
186200
/**
187201
* Return an order source for the specified object, i.e. an object that
188202
* should be checked for an order value as a replacement to the given object.
203+
* <p>Can also be an array of order source objects.
189204
* <p>If the returned object does not indicate any order, the comparator
190205
* will fall back to checking the original object.
191206
* @param obj the object to find an order source for

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

Lines changed: 61 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,43 +18,90 @@
1818

1919
import java.util.Comparator;
2020

21-
import junit.framework.TestCase;
21+
import org.junit.Test;
22+
23+
import static org.junit.Assert.assertEquals;
2224

2325
/**
2426
* Unit tests for the {@link OrderComparator} class.
2527
*
2628
* @author Rick Evans
29+
* @author Stephane Nicoll
2730
*/
28-
public final class OrderComparatorTests extends TestCase {
29-
30-
private Comparator comparator;
31-
32-
33-
@Override
34-
protected void setUp() throws Exception {
35-
this.comparator = new OrderComparator();
36-
}
31+
public final class OrderComparatorTests {
3732

33+
private final OrderComparator comparator = new OrderComparator();
3834

39-
public void testCompareOrderedInstancesBefore() throws Exception {
35+
@Test
36+
public void compareOrderedInstancesBefore() {
4037
assertEquals(-1, this.comparator.compare(
4138
new StubOrdered(100), new StubOrdered(2000)));
4239
}
4340

44-
public void testCompareOrderedInstancesSame() throws Exception {
41+
@Test
42+
public void compareOrderedInstancesSame() {
4543
assertEquals(0, this.comparator.compare(
4644
new StubOrdered(100), new StubOrdered(100)));
4745
}
4846

49-
public void testCompareOrderedInstancesAfter() throws Exception {
47+
@Test
48+
public void compareOrderedInstancesAfter() {
5049
assertEquals(1, this.comparator.compare(
5150
new StubOrdered(982300), new StubOrdered(100)));
5251
}
5352

54-
public void testCompareTwoNonOrderedInstancesEndsUpAsSame() throws Exception {
53+
@Test
54+
public void compareTwoNonOrderedInstancesEndsUpAsSame() {
5555
assertEquals(0, this.comparator.compare(new Object(), new Object()));
5656
}
5757

58+
@Test
59+
public void compareWithSimpleSourceProvider() {
60+
Comparator<Object> customComparator = this.comparator.withSourceProvider(
61+
new TestSourceProvider(5L, new StubOrdered(25)));
62+
assertEquals(-1, customComparator.compare(new StubOrdered(10), 5L));
63+
}
64+
65+
@Test
66+
public void compareWithSourceProviderArray() {
67+
Comparator<Object> customComparator = this.comparator.withSourceProvider(
68+
new TestSourceProvider(5L, new Object[] {new StubOrdered(10), new StubOrdered(-25)}));
69+
assertEquals(-1, customComparator.compare(5L, new Object()));
70+
}
71+
72+
@Test
73+
public void compareWithSourceProviderArrayNoMatch() {
74+
Comparator<Object> customComparator = this.comparator.withSourceProvider(
75+
new TestSourceProvider(5L, new Object[]{new Object(), new Object()}));
76+
assertEquals(0, customComparator.compare(new Object(), 5L));
77+
}
78+
79+
@Test
80+
public void compareWithSourceProviderEmpty() {
81+
Comparator<Object> customComparator = this.comparator.withSourceProvider(
82+
new TestSourceProvider(50L, new Object()));
83+
assertEquals(0, customComparator.compare(new Object(), 5L));
84+
}
85+
86+
87+
private static final class TestSourceProvider implements OrderComparator.OrderSourceProvider {
88+
89+
private final Object target;
90+
private final Object orderSource;
91+
92+
public TestSourceProvider(Object target, Object orderSource) {
93+
this.target = target;
94+
this.orderSource = orderSource;
95+
}
96+
97+
@Override
98+
public Object getOrderSource(Object obj) {
99+
if (target.equals(obj)) {
100+
return orderSource;
101+
}
102+
return null;
103+
}
104+
}
58105

59106
private static final class StubOrdered implements Ordered {
60107

0 commit comments

Comments
 (0)