Sort multiple @Autowired methods on same bean class via ASM

Closes gh-30359

(cherry picked from commit 7e6612a)

Author: jhoeller

spring-beans/src/main/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessor.java

@@ -17,6 +17,7 @@
 package org.springframework.beans.factory.annotation;
 
 import java.beans.PropertyDescriptor;
+import java.io.IOException;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Constructor;
@@ -62,6 +63,10 @@
 import org.springframework.core.annotation.AnnotationUtils;
 import org.springframework.core.annotation.MergedAnnotation;
 import org.springframework.core.annotation.MergedAnnotations;
+import org.springframework.core.type.AnnotationMetadata;
+import org.springframework.core.type.MethodMetadata;
+import org.springframework.core.type.classreading.MetadataReaderFactory;
+import org.springframework.core.type.classreading.SimpleMetadataReaderFactory;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
@@ -144,6 +149,9 @@ public class AutowiredAnnotationBeanPostProcessor implements SmartInstantiationA
 	@Nullable
 	private ConfigurableListableBeanFactory beanFactory;
 
+	@Nullable
+	private MetadataReaderFactory metadataReaderFactory;
+
 	private final Set<String> lookupMethodsChecked = Collections.newSetFromMap(new ConcurrentHashMap<>(256));
 
 	private final Map<Class<?>, Constructor<?>[]> candidateConstructorsCache = new ConcurrentHashMap<>(256);
@@ -238,6 +246,7 @@ public void setBeanFactory(BeanFactory beanFactory) {
 					"AutowiredAnnotationBeanPostProcessor requires a ConfigurableListableBeanFactory: " + beanFactory);
 		}
 		this.beanFactory = (ConfigurableListableBeanFactory) beanFactory;
+		this.metadataReaderFactory = new SimpleMetadataReaderFactory(this.beanFactory.getBeanClassLoader());
 	}
 
 
@@ -463,12 +472,11 @@ private InjectionMetadata buildAutowiringMetadata(Class<?> clazz) {
 			return InjectionMetadata.EMPTY;
 		}
 
-		List<InjectionMetadata.InjectedElement> elements = new ArrayList<>();
+		final List<InjectionMetadata.InjectedElement> elements = new ArrayList<>();
 		Class<?> targetClass = clazz;
 
 		do {
-			final List<InjectionMetadata.InjectedElement> currElements = new ArrayList<>();
-
+			final List<InjectionMetadata.InjectedElement> fieldElements = new ArrayList<>();
 			ReflectionUtils.doWithLocalFields(targetClass, field -> {
 				MergedAnnotation<?> ann = findAutowiredAnnotation(field);
 				if (ann != null) {
@@ -479,10 +487,11 @@ private InjectionMetadata buildAutowiringMetadata(Class<?> clazz) {
 						return;
 					}
 					boolean required = determineRequiredStatus(ann);
-					currElements.add(new AutowiredFieldElement(field, required));
+					fieldElements.add(new AutowiredFieldElement(field, required));
 				}
 			});
 
+			final List<InjectionMetadata.InjectedElement> methodElements = new ArrayList<>();
 			ReflectionUtils.doWithLocalMethods(targetClass, method -> {
 				Method bridgedMethod = BridgeMethodResolver.findBridgedMethod(method);
 				if (!BridgeMethodResolver.isVisibilityBridgeMethodPair(method, bridgedMethod)) {
@@ -504,11 +513,12 @@ private InjectionMetadata buildAutowiringMetadata(Class<?> clazz) {
 					}
 					boolean required = determineRequiredStatus(ann);
 					PropertyDescriptor pd = BeanUtils.findPropertyForMethod(bridgedMethod, clazz);
-					currElements.add(new AutowiredMethodElement(method, required, pd));
+					methodElements.add(new AutowiredMethodElement(method, required, pd));
 				}
 			});
 
-			elements.addAll(0, currElements);
+			elements.addAll(0, sortMethodElements(methodElements, targetClass));
+			elements.addAll(0, fieldElements);
 			targetClass = targetClass.getSuperclass();
 		}
 		while (targetClass != null && targetClass != Object.class);
@@ -573,6 +583,47 @@ protected <T> Map<String, T> findAutowireCandidates(Class<T> type) throws BeansE
 		return BeanFactoryUtils.beansOfTypeIncludingAncestors(this.beanFactory, type);
 	}
 
+	/**
+	 * Sort the method elements via ASM for deterministic declaration order if possible.
+	 */
+	private List<InjectionMetadata.InjectedElement> sortMethodElements(
+			List<InjectionMetadata.InjectedElement> methodElements, Class<?> targetClass) {
+
+		if (this.metadataReaderFactory != null && methodElements.size() > 1) {
+			// Try reading the class file via ASM for deterministic declaration order...
+			// Unfortunately, the JVM's standard reflection returns methods in arbitrary
+			// order, even between different runs of the same application on the same JVM.
+			try {
+				AnnotationMetadata asm =
+						this.metadataReaderFactory.getMetadataReader(targetClass.getName()).getAnnotationMetadata();
+				Set<MethodMetadata> asmMethods = asm.getAnnotatedMethods(Autowired.class.getName());
+				if (asmMethods.size() >= methodElements.size()) {
+					List<InjectionMetadata.InjectedElement> candidateMethods = new ArrayList<>(methodElements);
+					List<InjectionMetadata.InjectedElement> selectedMethods = new ArrayList<>(asmMethods.size());
+					for (MethodMetadata asmMethod : asmMethods) {
+						for (Iterator<InjectionMetadata.InjectedElement> it = candidateMethods.iterator(); it.hasNext();) {
+							InjectionMetadata.InjectedElement element = it.next();
+							if (element.getMember().getName().equals(asmMethod.getMethodName())) {
+								selectedMethods.add(element);
+								it.remove();
+								break;
+							}
+						}
+					}
+					if (selectedMethods.size() == methodElements.size()) {
+						// All reflection-detected methods found in ASM method set -> proceed
+						return selectedMethods;
+					}
+				}
+			}
+			catch (IOException ex) {
+				logger.debug("Failed to read class file via ASM for determining @Autowired method order", ex);
+				// No worries, let's continue with the reflection metadata we started with...
+			}
+		}
+		return methodElements;
+	}
+
 	/**
 	 * Register the specified bean as dependent on the autowired beans.
 	 */

spring-beans/src/test/java/org/springframework/beans/factory/annotation/AutowiredAnnotationBeanPostProcessorTests.java

@@ -73,6 +73,7 @@
 import org.springframework.core.annotation.Order;
 import org.springframework.core.testfixture.io.SerializationTestUtils;
 import org.springframework.lang.Nullable;
+import org.springframework.util.Assert;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.ReflectionUtils;
 
@@ -2620,13 +2621,12 @@ public static class ResourceInjectionBean {
 		@Autowired(required = false)
 		private TestBean testBean;
 
-		private TestBean testBean2;
+		TestBean testBean2;
 
 		@Autowired
 		public void setTestBean2(TestBean testBean2) {
-			if (this.testBean2 != null) {
-				throw new IllegalStateException("Already called");
-			}
+			Assert.state(this.testBean != null, "Wrong initialization order");
+			Assert.state(this.testBean2 == null, "Already called");
 			this.testBean2 = testBean2;
 		}
 
@@ -2661,7 +2661,7 @@ public NonPublicResourceInjectionBean() {
 		@Required
 		@SuppressWarnings("deprecation")
 		public void setTestBean2(TestBean testBean2) {
-			super.setTestBean2(testBean2);
+			this.testBean2 = testBean2;
 		}
 
 		@Autowired
@@ -2677,6 +2677,7 @@ private void inject(ITestBean testBean4) {
 
 		@Autowired
 		protected void initBeanFactory(BeanFactory beanFactory) {
+			Assert.state(this.baseInjected, "Wrong initialization order");
 			this.beanFactory = beanFactory;
 		}
 
@@ -4100,9 +4101,7 @@ public static abstract class Foo<T extends Runnable, RT extends Callable<T>> {
 		private RT obj;
 
 		protected void setObj(RT obj) {
-			if (this.obj != null) {
-				throw new IllegalStateException("Already called");
-			}
+			Assert.state(this.obj == null, "Already called");
 			this.obj = obj;
 		}
 	}

spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassBeanDefinitionReader.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -424,6 +424,7 @@ public ConfigurationClassBeanDefinition(
 
 		public ConfigurationClassBeanDefinition(RootBeanDefinition original,
 				ConfigurationClass configClass, MethodMetadata beanMethodMetadata, String derivedBeanName) {
+
 			super(original);
 			this.annotationMetadata = configClass.getMetadata();
 			this.factoryMethodMetadata = beanMethodMetadata;

spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2022 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -409,11 +409,14 @@ private Set<MethodMetadata> retrieveBeanMethodMetadata(SourceClass sourceClass)
 						this.metadataReaderFactory.getMetadataReader(original.getClassName()).getAnnotationMetadata();
 				Set<MethodMetadata> asmMethods = asm.getAnnotatedMethods(Bean.class.getName());
 				if (asmMethods.size() >= beanMethods.size()) {
+					Set<MethodMetadata> candidateMethods = new LinkedHashSet<>(beanMethods);
 					Set<MethodMetadata> selectedMethods = new LinkedHashSet<>(asmMethods.size());
 					for (MethodMetadata asmMethod : asmMethods) {
-						for (MethodMetadata beanMethod : beanMethods) {
+						for (Iterator<MethodMetadata> it = candidateMethods.iterator(); it.hasNext();) {
+							MethodMetadata beanMethod = it.next();
 							if (beanMethod.getMethodName().equals(asmMethod.getMethodName())) {
 								selectedMethods.add(beanMethod);
+								it.remove();
 								break;
 							}
 						}