From dafd511284a6f5be921b175f7628af14f3add705 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 7 Jun 2021 20:40:20 -0700 Subject: [PATCH] Support DatabaseInitializerDetector ordering Update `DatabaseInitializationDependencyConfigurer` so that depends-on ordering is applied based on the `DatabaseInitializerDetector` order. Prior to this commit, if multiple DatabaseInitializer beans were detected the order in which they were initialized was not defined. See gh-26692 --- ...nitializerDatabaseInitializerDetector.java | 5 ++ ...aseInitializationDependencyConfigurer.java | 16 +++++-- .../DatabaseInitializerDetector.java | 8 +++- ...itializationDependencyConfigurerTests.java | 47 ++++++++++++++++++- 4 files changed, 70 insertions(+), 6 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/flyway/FlywayMigrationInitializerDatabaseInitializerDetector.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/flyway/FlywayMigrationInitializerDatabaseInitializerDetector.java index 4199b14b8e47..73690134dc8f 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/flyway/FlywayMigrationInitializerDatabaseInitializerDetector.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/flyway/FlywayMigrationInitializerDatabaseInitializerDetector.java @@ -34,4 +34,9 @@ protected Set> getDatabaseInitializerBeanTypes() { return Collections.singleton(FlywayMigrationInitializer.class); } + @Override + public int getOrder() { + return 1; + } + } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/sql/init/dependency/DatabaseInitializationDependencyConfigurer.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/sql/init/dependency/DatabaseInitializationDependencyConfigurer.java index 41196a28a099..4a70dd0e71b7 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/sql/init/dependency/DatabaseInitializationDependencyConfigurer.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/sql/init/dependency/DatabaseInitializationDependencyConfigurer.java @@ -98,12 +98,22 @@ public void postProcessBeanFactory(ConfigurableListableBeanFactory 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; + } for (String dependsOnInitializationBeanNames : detectDependsOnInitializationBeanNames(beanFactory)) { - BeanDefinition definition = getBeanDefinition(dependsOnInitializationBeanNames, beanFactory); - definition.setDependsOn(merge(definition.getDependsOn(), initializerBeanNames)); + BeanDefinition beanDefinition = getBeanDefinition(dependsOnInitializationBeanNames, beanFactory); + beanDefinition.setDependsOn(merge(beanDefinition.getDependsOn(), initializerBeanNames)); } } + private String[] merge(String[] source, String additional) { + return merge(source, (additional != null) ? Collections.singleton(additional) : Collections.emptySet()); + } + private String[] merge(String[] source, Set additional) { Set result = new LinkedHashSet<>((source != null) ? Arrays.asList(source) : Collections.emptySet()); result.addAll(additional); @@ -112,7 +122,7 @@ private String[] merge(String[] source, Set additional) { private Set detectInitializerBeanNames(ConfigurableListableBeanFactory beanFactory) { List detectors = getDetectors(beanFactory, DatabaseInitializerDetector.class); - Set beanNames = new HashSet<>(); + Set beanNames = new LinkedHashSet<>(); for (DatabaseInitializerDetector detector : detectors) { for (String beanName : detector.detect(beanFactory)) { BeanDefinition beanDefinition = beanFactory.getBeanDefinition(beanName); diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/sql/init/dependency/DatabaseInitializerDetector.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/sql/init/dependency/DatabaseInitializerDetector.java index 36ba4daee041..7d31de49c9bc 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/sql/init/dependency/DatabaseInitializerDetector.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/sql/init/dependency/DatabaseInitializerDetector.java @@ -21,6 +21,7 @@ import javax.sql.DataSource; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; +import org.springframework.core.Ordered; /** * Detects beans that initialize an SQL database. Implementations should be registered in @@ -30,7 +31,7 @@ * @author Andy Wilkinson * @since 2.5.0 */ -public interface DatabaseInitializerDetector { +public interface DatabaseInitializerDetector extends Ordered { /** * Detect beans defined in the given {@code beanFactory} that initialize a @@ -52,4 +53,9 @@ default void detectionComplete(ConfigurableListableBeanFactory beanFactory, Set dataSourceInitializerNames) { } + @Override + default int getOrder() { + return 0; + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/sql/init/dependency/DatabaseInitializationDependencyConfigurerTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/sql/init/dependency/DatabaseInitializationDependencyConfigurerTests.java index 7d40abd55235..c8fd26562731 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/sql/init/dependency/DatabaseInitializationDependencyConfigurerTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/sql/init/dependency/DatabaseInitializationDependencyConfigurerTests.java @@ -40,6 +40,7 @@ import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Configuration; import org.springframework.context.annotation.Import; +import org.springframework.core.Ordered; import org.springframework.core.env.ConfigurableEnvironment; import org.springframework.core.env.Environment; import org.springframework.mock.env.MockEnvironment; @@ -54,6 +55,7 @@ * Tests for {@link DatabaseInitializationDependencyConfigurer}. * * @author Andy Wilkinson + * @author Phillip Webb */ class DatabaseInitializationDependencyConfigurerTests { @@ -64,7 +66,8 @@ class DatabaseInitializationDependencyConfigurerTests { @BeforeEach void resetMocks() { - reset(MockDatabaseInitializerDetector.instance, MockedDependsOnDatabaseInitializationDetector.instance); + reset(MockDatabaseInitializerDetector.instance, OrderedMockDatabaseInitializerDetector.instance, + MockedDependsOnDatabaseInitializationDetector.instance); } @Test @@ -103,6 +106,30 @@ void whenDependenciesAreConfiguredThenBeansThatDependUponDatabaseInitializationD }); } + @Test + void whenDependenciesAreConfiguredDetectedDatabaseInitializersAreInitializedInCorrectOrder() { + BeanDefinition alpha = BeanDefinitionBuilder.genericBeanDefinition(String.class).getBeanDefinition(); + BeanDefinition bravo = BeanDefinitionBuilder.genericBeanDefinition(String.class).getBeanDefinition(); + BeanDefinition charlie = BeanDefinitionBuilder.genericBeanDefinition(String.class).getBeanDefinition(); + performDetection(Arrays.asList(MockDatabaseInitializerDetector.class, + OrderedMockDatabaseInitializerDetector.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(MockedDependsOnDatabaseInitializationDetector.instance.detect(context.getBeanFactory())) + .willReturn(Collections.singleton("charlie")); + context.registerBeanDefinition("alpha", alpha); + context.registerBeanDefinition("bravo", bravo); + context.registerBeanDefinition("charlie", charlie); + context.refresh(); + assertThat(charlie.getDependsOn()).containsExactly("alpha", "bravo"); + assertThat(bravo.getDependsOn()).containsExactly("alpha"); + assertThat(alpha.getDependsOn()).isNullOrEmpty(); + }); + } + private void performDetection(Collection> detectors, Consumer contextCallback) { DetectorSpringFactoriesClassLoader detectorSpringFactories = new DetectorSpringFactoriesClassLoader(this.temp); @@ -158,7 +185,7 @@ static class MockDatabaseInitializerDetector implements DatabaseInitializerDetec @Override public Set detect(ConfigurableListableBeanFactory beanFactory) { - return MockDatabaseInitializerDetector.instance.detect(beanFactory); + return instance.detect(beanFactory); } @Override @@ -169,6 +196,22 @@ public void detectionComplete(ConfigurableListableBeanFactory beanFactory, } + static class OrderedMockDatabaseInitializerDetector implements DatabaseInitializerDetector { + + private static DatabaseInitializerDetector instance = mock(DatabaseInitializerDetector.class); + + @Override + public Set detect(ConfigurableListableBeanFactory beanFactory) { + return instance.detect(beanFactory); + } + + @Override + public int getOrder() { + return Ordered.LOWEST_PRECEDENCE; + } + + } + static class MockedDependsOnDatabaseInitializationDetector implements DependsOnDatabaseInitializationDetector { private static DependsOnDatabaseInitializationDetector instance = mock(