From 41d4cb5cbf608a3ef6b0585f675091c427dba5d7 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Fri, 14 Sep 2018 23:56:25 +0200 Subject: [PATCH] Ordered stream access on ObjectProvider with strong order guarantees Issue: SPR-17272 --- .../beans/factory/ObjectProvider.java | 41 ++++++------- .../support/DefaultListableBeanFactory.java | 57 +++++++++---------- .../support/StaticListableBeanFactory.java | 5 ++ ...wiredAnnotationBeanPostProcessorTests.java | 27 ++++++--- .../support/BeanFactoryGenericsTests.java | 28 +++++---- .../springframework/core/OrderComparator.java | 5 +- 6 files changed, 92 insertions(+), 71 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/ObjectProvider.java b/spring-beans/src/main/java/org/springframework/beans/factory/ObjectProvider.java index c13195c8537..166c06de486 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/ObjectProvider.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/ObjectProvider.java @@ -17,10 +17,8 @@ package org.springframework.beans.factory; import java.util.Iterator; -import java.util.List; import java.util.function.Consumer; import java.util.function.Supplier; -import java.util.stream.Collectors; import java.util.stream.Stream; import org.springframework.beans.BeansException; @@ -141,17 +139,7 @@ public interface ObjectProvider extends ObjectFactory, Iterable { } /** - * Return a sequential {@link Stream} over lazily resolved object instances, - * without specific ordering guarantees (but typically in registration order). - * @since 5.1 - * @see #iterator() - */ - default Stream stream() { - throw new UnsupportedOperationException("Multi-element access not supported"); - } - - /** - * Return an {@link Iterator} over lazily resolved object instances, + * Return an {@link Iterator} over all matching object instances, * without specific ordering guarantees (but typically in registration order). * @since 5.1 * @see #stream() @@ -162,17 +150,30 @@ public interface ObjectProvider extends ObjectFactory, Iterable { } /** - * Return a {@link List} with fully resolved object instances, - * potentially pre-ordered according to a common comparator. - *

In a common Spring application context, this will be ordered - * according to {@link org.springframework.core.Ordered} / - * {@link org.springframework.core.annotation.Order} conventions, + * Return a sequential {@link Stream} over all matching object instances, + * without specific ordering guarantees (but typically in registration order). + * @since 5.1 + * @see #iterator() + * @see #orderedStream() + */ + default Stream stream() { + throw new UnsupportedOperationException("Multi-element access not supported"); + } + + /** + * Return a sequential {@link Stream} over all matching object instances, + * pre-ordered according to the factory's common order comparator. + *

In a standard Spring application context, this will be ordered + * according to {@link org.springframework.core.Ordered} conventions, + * and in case of annotation-based configuration also considering the + * {@link org.springframework.core.annotation.Order} annotation, * analogous to multi-element injection points of list/array type. * @since 5.1 * @see #stream() + * @see org.springframework.core.OrderComparator */ - default List toList() { - return stream().collect(Collectors.toList()); + default Stream orderedStream() { + throw new UnsupportedOperationException("Multi-element access not supported"); } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java index 531d5c0dafc..86bbf183b25 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/DefaultListableBeanFactory.java @@ -40,7 +40,6 @@ import java.util.Map; import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.stream.Collectors; import java.util.stream.Stream; import javax.inject.Provider; @@ -388,7 +387,7 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto .filter(bean -> !(bean instanceof NullBean)); } @Override - public List toList() { + public Stream orderedStream() { String[] beanNames = getBeanNamesForType(requiredType); Map matchingBeans = new LinkedHashMap<>(beanNames.length); for (String beanName : beanNames) { @@ -397,12 +396,8 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto matchingBeans.put(beanName, (T) beanInstance); } } - List result = new ArrayList<>(matchingBeans.values()); - Comparator comparator = adaptDependencyComparator(matchingBeans); - if (comparator != null) { - result.sort(comparator); - } - return result; + Stream stream = matchingBeans.values().stream(); + return stream.sorted(adaptOrderComparator(matchingBeans)); } }; } @@ -1267,16 +1262,13 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto if (autowiredBeanNames != null) { autowiredBeanNames.addAll(matchingBeans.keySet()); } - Stream result = matchingBeans.keySet().stream() + Stream stream = matchingBeans.keySet().stream() .map(name -> descriptor.resolveCandidate(name, type, this)) .filter(bean -> !(bean instanceof NullBean)); - if (((StreamDependencyDescriptor) descriptor).isSorted()) { - Comparator comparator = adaptDependencyComparator(matchingBeans); - if (comparator != null) { - result = result.sorted(comparator); - } + if (((StreamDependencyDescriptor) descriptor).isOrdered()) { + stream = stream.sorted(adaptOrderComparator(matchingBeans)); } - return result; + return stream; } else if (type.isArray()) { Class componentType = type.getComponentType(); @@ -1375,6 +1367,13 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto } } + private Comparator adaptOrderComparator(Map matchingBeans) { + Comparator dependencyComparator = getDependencyComparator(); + OrderComparator comparator = (dependencyComparator instanceof OrderComparator ? + (OrderComparator) dependencyComparator : OrderComparator.INSTANCE); + return comparator.withSourceProvider(createFactoryAwareOrderSourceProvider(matchingBeans)); + } + private OrderComparator.OrderSourceProvider createFactoryAwareOrderSourceProvider(Map beans) { IdentityHashMap instancesToBeanNames = new IdentityHashMap<>(); beans.forEach((beanName, instance) -> instancesToBeanNames.put(instance, beanName)); @@ -1779,15 +1778,15 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto */ private static class StreamDependencyDescriptor extends DependencyDescriptor { - private final boolean sorted; + private final boolean ordered; - public StreamDependencyDescriptor(DependencyDescriptor original, boolean sorted) { + public StreamDependencyDescriptor(DependencyDescriptor original, boolean ordered) { super(original); - this.sorted = sorted; + this.ordered = ordered; } - public boolean isSorted() { - return this.sorted; + public boolean isOrdered() { + return this.ordered; } } @@ -1897,22 +1896,22 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto } } - @SuppressWarnings("unchecked") @Override public Stream stream() { - DependencyDescriptor descriptorToUse = new StreamDependencyDescriptor(this.descriptor, false); - Object result = doResolveDependency(descriptorToUse, this.beanName, null, null); - Assert.state(result instanceof Stream, "Stream expected"); - return (Stream) result; + return resolveStream(false); + } + + @Override + public Stream orderedStream() { + return resolveStream(true); } @SuppressWarnings("unchecked") - @Override - public List toList() { - DependencyDescriptor descriptorToUse = new StreamDependencyDescriptor(this.descriptor, true); + private Stream resolveStream(boolean ordered) { + DependencyDescriptor descriptorToUse = new StreamDependencyDescriptor(this.descriptor, ordered); Object result = doResolveDependency(descriptorToUse, this.beanName, null, null); Assert.state(result instanceof Stream, "Stream expected"); - return ((Stream) result).collect(Collectors.toList()); + return (Stream) result; } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/StaticListableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/StaticListableBeanFactory.java index 9162692889a..2ae91e7bdfd 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/StaticListableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/StaticListableBeanFactory.java @@ -35,6 +35,7 @@ import org.springframework.beans.factory.NoSuchBeanDefinitionException; import org.springframework.beans.factory.NoUniqueBeanDefinitionException; import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.SmartFactoryBean; +import org.springframework.core.OrderComparator; import org.springframework.core.ResolvableType; import org.springframework.core.annotation.AnnotationUtils; import org.springframework.lang.Nullable; @@ -245,6 +246,10 @@ public class StaticListableBeanFactory implements ListableBeanFactory { public Stream stream() { return Arrays.stream(getBeanNamesForType(requiredType)).map(name -> (T) getBean(name)); } + @Override + public Stream orderedStream() { + return stream().sorted(OrderComparator.INSTANCE); + } }; } diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java index 65d31fa1652..ad1736c9ecf 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java @@ -1289,15 +1289,13 @@ public class AutowiredAnnotationBeanPostProcessorTests { @Test public void testObjectProviderInjectionWithTargetPrimary() { - bf.setDependencyComparator(Comparators.comparable()); - bf.registerBeanDefinition("annotatedBean", new RootBeanDefinition(ObjectProviderInjectionBean.class)); - RootBeanDefinition tb1 = new RootBeanDefinition(TestBean.class); - tb1.getPropertyValues().add("name", "yours"); + RootBeanDefinition tb1 = new RootBeanDefinition(OrderedTestBean.class); + tb1.getPropertyValues().add("order", 1); tb1.setPrimary(true); bf.registerBeanDefinition("testBean1", tb1); - RootBeanDefinition tb2 = new RootBeanDefinition(TestBean.class); - tb2.getPropertyValues().add("name", "mine"); + RootBeanDefinition tb2 = new RootBeanDefinition(OrderedTestBean.class); + tb2.getPropertyValues().add("order", 0); tb2.setLazyInit(true); bf.registerBeanDefinition("testBean2", tb2); @@ -2885,7 +2883,7 @@ public class AutowiredAnnotationBeanPostProcessorTests { } public List sortedTestBeans() { - return this.testBeanProvider.toList(); + return this.testBeanProvider.orderedStream().collect(Collectors.toList()); } } @@ -2983,6 +2981,21 @@ public class AutowiredAnnotationBeanPostProcessorTests { } + public static class OrderedTestBean extends TestBean implements Ordered { + + private int order; + + public void setOrder(int order) { + this.order = order; + } + + @Override + public int getOrder() { + return this.order; + } + } + + public static class OrderedNestedTestBean extends NestedTestBean implements Ordered { private int order; diff --git a/spring-beans/src/test/java/org/springframework/beans/factory/support/BeanFactoryGenericsTests.java b/spring-beans/src/test/java/org/springframework/beans/factory/support/BeanFactoryGenericsTests.java index 3297bf711ff..4237a5da4d0 100644 --- a/spring-beans/src/test/java/org/springframework/beans/factory/support/BeanFactoryGenericsTests.java +++ b/spring-beans/src/test/java/org/springframework/beans/factory/support/BeanFactoryGenericsTests.java @@ -43,6 +43,7 @@ import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.config.TypedStringValue; import org.springframework.beans.factory.xml.XmlBeanDefinitionReader; import org.springframework.beans.propertyeditors.CustomNumberEditor; +import org.springframework.core.Ordered; import org.springframework.core.OverridingClassLoader; import org.springframework.core.ResolvableType; import org.springframework.core.io.ClassPathResource; @@ -53,7 +54,6 @@ import org.springframework.tests.sample.beans.GenericBean; import org.springframework.tests.sample.beans.GenericIntegerBean; import org.springframework.tests.sample.beans.GenericSetOfIntegerBean; import org.springframework.tests.sample.beans.TestBean; -import org.springframework.util.comparator.Comparators; import static org.junit.Assert.*; @@ -847,7 +847,6 @@ public class BeanFactoryGenericsTests { public void testGenericMatchingWithFullTypeDifferentiation() { DefaultListableBeanFactory bf = new DefaultListableBeanFactory(); bf.setAutowireCandidateResolver(new GenericTypeAwareAutowireCandidateResolver()); - bf.setDependencyComparator(Comparators.comparable()); bf.registerBeanDefinition("store1", new RootBeanDefinition(DoubleStore.class)); bf.registerBeanDefinition("store2", new RootBeanDefinition(FloatStore.class)); @@ -907,7 +906,7 @@ public class BeanFactoryGenericsTests { assertSame(bf.getBean("store1"), resolved.get(0)); assertSame(bf.getBean("store2"), resolved.get(1)); - resolved = numberStoreProvider.toList(); + resolved = numberStoreProvider.orderedStream().collect(Collectors.toList()); assertEquals(2, resolved.size()); assertSame(bf.getBean("store2"), resolved.get(0)); assertSame(bf.getBean("store1"), resolved.get(1)); @@ -923,7 +922,7 @@ public class BeanFactoryGenericsTests { assertEquals(1, resolved.size()); assertTrue(resolved.contains(bf.getBean("store1"))); - resolved = (List) doubleStoreProvider.toList(); + resolved = (List) doubleStoreProvider.orderedStream().collect(Collectors.toList()); assertEquals(1, resolved.size()); assertTrue(resolved.contains(bf.getBean("store1"))); @@ -938,7 +937,7 @@ public class BeanFactoryGenericsTests { assertEquals(1, resolved.size()); assertTrue(resolved.contains(bf.getBean("store2"))); - resolved = (List) floatStoreProvider.toList(); + resolved = (List) floatStoreProvider.orderedStream().collect(Collectors.toList()); assertEquals(1, resolved.size()); assertTrue(resolved.contains(bf.getBean("store2"))); } @@ -1006,20 +1005,25 @@ public class BeanFactoryGenericsTests { } - public static class NumberStore implements Comparable { + public static class NumberStore { + } + + + public static class DoubleStore extends NumberStore implements Ordered { @Override - public int compareTo(NumberStore other) { - return getClass().getName().compareTo(other.getClass().getName()) * -1; + public int getOrder() { + return 1; } } - public static class DoubleStore extends NumberStore { - } + public static class FloatStore extends NumberStore implements Ordered { - - public static class FloatStore extends NumberStore { + @Override + public int getOrder() { + return 0; + } } diff --git a/spring-core/src/main/java/org/springframework/core/OrderComparator.java b/spring-core/src/main/java/org/springframework/core/OrderComparator.java index 513e631481a..ea3332a082d 100644 --- a/spring-core/src/main/java/org/springframework/core/OrderComparator.java +++ b/spring-core/src/main/java/org/springframework/core/OrderComparator.java @@ -59,7 +59,7 @@ public class OrderComparator implements Comparator { * @return the adapted comparator * @since 4.1 */ - public Comparator withSourceProvider(final OrderSourceProvider sourceProvider) { + public Comparator withSourceProvider(OrderSourceProvider sourceProvider) { return (o1, o2) -> doCompare(o1, o2, sourceProvider); } @@ -78,10 +78,9 @@ public class OrderComparator implements Comparator { return 1; } - // Direct evaluation instead of Integer.compareTo to avoid unnecessary object creation. int i1 = getOrder(o1, sourceProvider); int i2 = getOrder(o2, sourceProvider); - return (i1 < i2) ? -1 : (i1 > i2) ? 1 : 0; + return Integer.compare(i1, i2); } /**