Correctly order @AutoConfigureAfter values when sorting

Update `AutoConfigurationSorter` so that `getClassesRequestedAfter()` results are sorted to match the earlier name/order sorting. Prior to this commit the order of items added via `@AutoConfigureAfter` was in an undetermined order which could cause very subtle `@ConditionalOnBean` bugs. Thanks very much to Alexandre Baron for their help in diagnosing and reproducing this issue. Fixes gh-38904

Author: philwebb

spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurationSorter.java

@@ -20,11 +20,13 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashMap;
+import java.util.Comparator;
+import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.TreeSet;
 
 import org.springframework.core.type.AnnotationMetadata;
 import org.springframework.core.type.classreading.MetadataReader;
@@ -53,12 +55,14 @@ class AutoConfigurationSorter {
 }
 
 List<String> getInPriorityOrder(Collection<String> classNames) {
+// Initially sort alphabetically
+List<String> alphabeticallyOrderedClassNames = new ArrayList<>(classNames);
+Collections.sort(alphabeticallyOrderedClassNames);
+// Then sort by order
 AutoConfigurationClasses classes = new AutoConfigurationClasses(this.metadataReaderFactory,
-this.autoConfigurationMetadata, classNames);
+this.autoConfigurationMetadata, alphabeticallyOrderedClassNames);
 List<String> orderedClassNames = new ArrayList<>(classNames);
-// Initially sort alphabetically
 Collections.sort(orderedClassNames);
-// Then sort by order
 orderedClassNames.sort((o1, o2) -> {
 int i1 = classes.get(o1).getOrder();
 int i2 = classes.get(o2).getOrder();
@@ -87,7 +91,9 @@ private void doSortByAfterAnnotation(AutoConfigurationClasses classes, List<Stri
 current = toSort.remove(0);
 }
 processing.add(current);
-for (String after : classes.getClassesRequestedAfter(current)) {
+Set<String> afters = new TreeSet<>(Comparator.comparing(toSort::indexOf));
+afters.addAll(classes.getClassesRequestedAfter(current));
+for (String after : afters) {
 checkForCycles(processing, current, after);
 if (!sorted.contains(after) && toSort.contains(after)) {
 doSortByAfterAnnotation(classes, toSort, sorted, processing, after);
@@ -104,7 +110,7 @@ private void checkForCycles(Set<String> processing, String current, String after
 
 private static class AutoConfigurationClasses {
 
-private final Map<String, AutoConfigurationClass> classes = new HashMap<>();
+private final Map<String, AutoConfigurationClass> classes = new LinkedHashMap<>();
 
 AutoConfigurationClasses(MetadataReaderFactory metadataReaderFactory,
 AutoConfigurationMetadata autoConfigurationMetadata, Collection<String> classNames) {

spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/AutoConfigurationSorterTests.java

@@ -46,6 +46,7 @@
  * @author Phillip Webb
  * @author Andy Wilkinson
  * @author Moritz Halbritter
+ * @author Alexandre Baron
  */
 class AutoConfigurationSorterTests {
 
@@ -206,6 +207,17 @@ void useAnnotationWithNoDirectLinkAndCycle() throws Exception {
 .withMessageContaining("AutoConfigure cycle detected");
 }
 
+@Test // gh-38904
+void byBeforeAnnotationThenOrderAnnotation() {
+String oa = OrderAutoConfigureA.class.getName();
+String oa1 = OrderAutoConfigureASeedR1.class.getName();
+String oa2 = OrderAutoConfigureASeedY2.class.getName();
+String oa3 = OrderAutoConfigureASeedA3.class.getName();
+String oa4 = OrderAutoConfigureAutoConfigureASeedG4.class.getName();
+List<String> actual = this.sorter.getInPriorityOrder(Arrays.asList(oa4, oa3, oa2, oa1, oa));
+assertThat(actual).containsExactly(oa1, oa2, oa3, oa4, oa);
+}
+
 private AutoConfigurationMetadata getAutoConfigurationMetadata(String... classNames) throws Exception {
 Properties properties = new Properties();
 for (String className : classNames) {
@@ -348,6 +360,36 @@ static class AutoConfigureZ2 {
 
 }
 
+static class OrderAutoConfigureA {
+
+}
+
+// Use seeds in auto-configuration class names to mislead the sort by names done in
+// AutoConfigurationSorter class.
+@AutoConfigureBefore(OrderAutoConfigureA.class)
+@AutoConfigureOrder(1)
+static class OrderAutoConfigureASeedR1 {
+
+}
+
+@AutoConfigureBefore(OrderAutoConfigureA.class)
+@AutoConfigureOrder(2)
+static class OrderAutoConfigureASeedY2 {
+
+}
+
+@AutoConfigureBefore(OrderAutoConfigureA.class)
+@AutoConfigureOrder(3)
+static class OrderAutoConfigureASeedA3 {
+
+}
+
+@AutoConfigureBefore(OrderAutoConfigureA.class)
+@AutoConfigureOrder(4)
+static class OrderAutoConfigureAutoConfigureASeedG4 {
+
+}
+
 static class SkipCycleMetadataReaderFactory extends CachingMetadataReaderFactory {
 
 @Override