Configure initializer dependencies grouped by detector

Previously, database initializers were detected and were configured
with dependencies based on their detection order. For example, if
detectors a, b, and c detected initializers a1, b1, b2, and c1,
c1 would depend on b2, b2 on b1, and b1 on a1:

------     ------     ------     ------
| c1 | --> | b2 | --> | b1 | --> | a1 |
------     ------     ------     ------

This could cause a dependency cycle in certain situations, for
example because the user had already configured b1 to depend on b2.

This commit reduces the risk of a cycle being created by batching
the initializers by their detector, with dependencies being
configured between each batch rather than between every initializer.
In the example above, this results in c1 depending on b1 and b2,
and b1 and b2 depending on a1:

           ------
------     | b1 |     ------
| c1 | --> |    | --> | a1 |
------     | b2 |     ------
           ------

As b1 and b2 were detected by the same detector, no dependency
between those initializers is defined.

Closes gh-27131

Author: wilkinsona

spring-boot-project/spring-boot/src/main/java/org/springframework/boot/sql/init/dependency/DatabaseInitializationDependencyConfigurer.java

@@ -20,8 +20,10 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
+import java.util.LinkedHashMap;
 import java.util.LinkedHashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 
 import org.springframework.beans.factory.BeanFactory;
@@ -38,6 +40,7 @@
 import org.springframework.core.env.Environment;
 import org.springframework.core.io.support.SpringFactoriesLoader;
 import org.springframework.core.type.AnnotationMetadata;
+import org.springframework.util.CollectionUtils;
 import org.springframework.util.StringUtils;
 
 /**
@@ -100,48 +103,49 @@ public int getOrder() {
 
 		@Override
 		public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) {
-			Set<String> initializerBeanNames = detectInitializerBeanNames(beanFactory);
+			InitializerBeanNames initializerBeanNames = detectInitializerBeanNames(beanFactory);
 			if (initializerBeanNames.isEmpty()) {
 				return;
 			}
-			String previousInitializerBeanName = null;
-			for (String initializerBeanName : initializerBeanNames) {
-				BeanDefinition beanDefinition = getBeanDefinition(initializerBeanName, beanFactory);
-				beanDefinition.setDependsOn(merge(beanDefinition.getDependsOn(), previousInitializerBeanName));
-				previousInitializerBeanName = initializerBeanName;
+			Set<String> previousInitializerBeanNamesBatch = null;
+			for (Set<String> initializerBeanNamesBatch : initializerBeanNames.batchedBeanNames()) {
+				for (String initializerBeanName : initializerBeanNamesBatch) {
+					BeanDefinition beanDefinition = getBeanDefinition(initializerBeanName, beanFactory);
+					beanDefinition
+							.setDependsOn(merge(beanDefinition.getDependsOn(), previousInitializerBeanNamesBatch));
+				}
+				previousInitializerBeanNamesBatch = initializerBeanNamesBatch;
 			}
 			for (String dependsOnInitializationBeanNames : detectDependsOnInitializationBeanNames(beanFactory)) {
 				BeanDefinition beanDefinition = getBeanDefinition(dependsOnInitializationBeanNames, beanFactory);
-				beanDefinition.setDependsOn(merge(beanDefinition.getDependsOn(), initializerBeanNames));
+				beanDefinition.setDependsOn(merge(beanDefinition.getDependsOn(), initializerBeanNames.beanNames()));
 			}
 		}
 
-		private String[] merge(String[] source, String additional) {
-			return merge(source, (additional != null) ? Collections.singleton(additional) : Collections.emptySet());
-		}
-
 		private String[] merge(String[] source, Set<String> additional) {
+			if (CollectionUtils.isEmpty(additional)) {
+				return source;
+			}
 			Set<String> result = new LinkedHashSet<>((source != null) ? Arrays.asList(source) : Collections.emptySet());
 			result.addAll(additional);
 			return StringUtils.toStringArray(result);
 		}
 
-		private Set<String> detectInitializerBeanNames(ConfigurableListableBeanFactory beanFactory) {
+		private InitializerBeanNames detectInitializerBeanNames(ConfigurableListableBeanFactory beanFactory) {
 			List<DatabaseInitializerDetector> detectors = getDetectors(beanFactory, DatabaseInitializerDetector.class);
-			Set<String> beanNames = new LinkedHashSet<>();
+			InitializerBeanNames initializerBeanNames = new InitializerBeanNames();
 			for (DatabaseInitializerDetector detector : detectors) {
 				for (String beanName : detector.detect(beanFactory)) {
 					BeanDefinition beanDefinition = beanFactory.getBeanDefinition(beanName);
 					beanDefinition.setAttribute(DatabaseInitializerDetector.class.getName(),
 							detector.getClass().getName());
-					beanNames.add(beanName);
+					initializerBeanNames.detected(detector, beanName);
 				}
 			}
-			beanNames = Collections.unmodifiableSet(beanNames);
 			for (DatabaseInitializerDetector detector : detectors) {
-				detector.detectionComplete(beanFactory, beanNames);
+				detector.detectionComplete(beanFactory, initializerBeanNames.beanNames());
 			}
-			return beanNames;
+			return initializerBeanNames;
 		}
 
 		private Collection<String> detectDependsOnInitializationBeanNames(ConfigurableListableBeanFactory beanFactory) {
@@ -174,6 +178,31 @@ private static BeanDefinition getBeanDefinition(String beanName, ConfigurableLis
 			}
 		}
 
+		static class InitializerBeanNames {
+
+			private final Map<DatabaseInitializerDetector, Set<String>> byDetectorBeanNames = new LinkedHashMap<>();
+
+			private final Set<String> beanNames = new LinkedHashSet<>();
+
+			private void detected(DatabaseInitializerDetector detector, String beanName) {
+				this.byDetectorBeanNames.computeIfAbsent(detector, (key) -> new LinkedHashSet<>()).add(beanName);
+				this.beanNames.add(beanName);
+			}
+
+			private boolean isEmpty() {
+				return this.beanNames.isEmpty();
+			}
+
+			private Iterable<Set<String>> batchedBeanNames() {
+				return this.byDetectorBeanNames.values();
+			}
+
+			private Set<String> beanNames() {
+				return Collections.unmodifiableSet(this.beanNames);
+			}
+
+		}
+
 	}
 
 }

spring-boot-project/spring-boot/src/test/java/org/springframework/boot/sql/init/dependency/DatabaseInitializationDependencyConfigurerTests.java

@@ -26,6 +26,7 @@
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Properties;
@@ -71,7 +72,8 @@ class DatabaseInitializationDependencyConfigurerTests {
 
 	@BeforeEach
 	void resetMocks() {
-		reset(MockDatabaseInitializerDetector.instance, OrderedMockDatabaseInitializerDetector.instance,
+		reset(MockDatabaseInitializerDetector.instance, OrderedNearLowestMockDatabaseInitializerDetector.instance,
+				OrderedLowestMockDatabaseInitializerDetector.instance,
 				MockedDependsOnDatabaseInitializationDetector.instance);
 	}
 
@@ -94,8 +96,7 @@ void beanFactoryPostProcessorHasOrderAllowingSubsequentPostProcessorsToFineTuneD
 					context.refresh();
 					assertThat(DependsOnCaptor.dependsOn).hasEntrySatisfying("bravo",
 							(dependencies) -> assertThat(dependencies).containsExactly("alpha"));
-					assertThat(DependsOnCaptor.dependsOn).hasEntrySatisfying("alpha",
-							(dependencies) -> assertThat(dependencies).isEmpty());
+					assertThat(DependsOnCaptor.dependsOn).doesNotContainKey("alpha");
 				});
 	}
 
@@ -140,24 +141,34 @@ void whenDependenciesAreConfiguredThenBeansThatDependUponDatabaseInitializationD
 	@Test
 	void whenDependenciesAreConfiguredDetectedDatabaseInitializersAreInitializedInCorrectOrder() {
 		BeanDefinition alpha = BeanDefinitionBuilder.genericBeanDefinition(String.class).getBeanDefinition();
-		BeanDefinition bravo = BeanDefinitionBuilder.genericBeanDefinition(String.class).getBeanDefinition();
+		BeanDefinition bravo1 = BeanDefinitionBuilder.genericBeanDefinition(String.class).getBeanDefinition();
+		BeanDefinition bravo2 = BeanDefinitionBuilder.genericBeanDefinition(String.class).getBeanDefinition();
 		BeanDefinition charlie = BeanDefinitionBuilder.genericBeanDefinition(String.class).getBeanDefinition();
-		performDetection(Arrays.asList(MockDatabaseInitializerDetector.class,
-				OrderedMockDatabaseInitializerDetector.class, MockedDependsOnDatabaseInitializationDetector.class),
+		BeanDefinition delta = BeanDefinitionBuilder.genericBeanDefinition(String.class).getBeanDefinition();
+		performDetection(
+				Arrays.asList(MockDatabaseInitializerDetector.class, OrderedLowestMockDatabaseInitializerDetector.class,
+						OrderedNearLowestMockDatabaseInitializerDetector.class,
+						MockedDependsOnDatabaseInitializationDetector.class),
 				(context) -> {
 					given(MockDatabaseInitializerDetector.instance.detect(context.getBeanFactory()))
 							.willReturn(Collections.singleton("alpha"));
-					given(OrderedMockDatabaseInitializerDetector.instance.detect(context.getBeanFactory()))
-							.willReturn(Collections.singleton("bravo"));
+					given(OrderedNearLowestMockDatabaseInitializerDetector.instance.detect(context.getBeanFactory()))
+							.willReturn(new LinkedHashSet<>(Arrays.asList("bravo1", "bravo2")));
+					given(OrderedLowestMockDatabaseInitializerDetector.instance.detect(context.getBeanFactory()))
+							.willReturn(new LinkedHashSet<>(Arrays.asList("charlie")));
 					given(MockedDependsOnDatabaseInitializationDetector.instance.detect(context.getBeanFactory()))
-							.willReturn(Collections.singleton("charlie"));
+							.willReturn(Collections.singleton("delta"));
 					context.registerBeanDefinition("alpha", alpha);
-					context.registerBeanDefinition("bravo", bravo);
+					context.registerBeanDefinition("bravo1", bravo1);
+					context.registerBeanDefinition("bravo2", bravo2);
 					context.registerBeanDefinition("charlie", charlie);
+					context.registerBeanDefinition("delta", delta);
 					context.register(DependencyConfigurerConfiguration.class);
 					context.refresh();
-					assertThat(charlie.getDependsOn()).containsExactly("alpha", "bravo");
-					assertThat(bravo.getDependsOn()).containsExactly("alpha");
+					assertThat(delta.getDependsOn()).containsExactlyInAnyOrder("alpha", "bravo1", "bravo2", "charlie");
+					assertThat(charlie.getDependsOn()).containsExactly("bravo1", "bravo2");
+					assertThat(bravo1.getDependsOn()).containsExactly("alpha");
+					assertThat(bravo2.getDependsOn()).containsExactly("alpha");
 					assertThat(alpha.getDependsOn()).isNullOrEmpty();
 				});
 	}
@@ -227,7 +238,7 @@ public void detectionComplete(ConfigurableListableBeanFactory beanFactory,
 
 	}
 
-	static class OrderedMockDatabaseInitializerDetector implements DatabaseInitializerDetector {
+	static class OrderedLowestMockDatabaseInitializerDetector implements DatabaseInitializerDetector {
 
 		private static DatabaseInitializerDetector instance = mock(DatabaseInitializerDetector.class);
 
@@ -243,6 +254,22 @@ public int getOrder() {
 
 	}
 
+	static class OrderedNearLowestMockDatabaseInitializerDetector implements DatabaseInitializerDetector {
+
+		private static DatabaseInitializerDetector instance = mock(DatabaseInitializerDetector.class);
+
+		@Override
+		public Set<String> detect(ConfigurableListableBeanFactory beanFactory) {
+			return instance.detect(beanFactory);
+		}
+
+		@Override
+		public int getOrder() {
+			return Ordered.LOWEST_PRECEDENCE - 100;
+		}
+
+	}
+
 	static class MockedDependsOnDatabaseInitializationDetector implements DependsOnDatabaseInitializationDetector {
 
 		private static DependsOnDatabaseInitializationDetector instance = mock(