From 26c775eff8a4b27ac5a2a56a8792c37c2c8a582b Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 18 Nov 2024 11:25:29 -0800 Subject: [PATCH] Register `AutoConfigurations` using fully qualified class name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Update `AbstractApplicationContextRunner` and `Configurations` to allow registration of beans with a specific generated bean name. By default, no name is generated, however, `AutoConfigurations` has been updated to use bean names using the fully qualified class name. The update brings `ApplicationContextRunners` closer the behavior of a standard Spring Boot application where user `@Configuration` classes are usually registered with a simple name and auto-configurations are imported (via an `ImportSelector`) using a fully qualified name. Fixes gh-17963 Co-authored-by: Stéphane Nicoll Co-authored-by: Andy Wilkinson Co-authored-by: Dmytro Nosan --- .../autoconfigure/AutoConfigurations.java | 2 +- .../AutoConfigurationsTests.java | 6 ++ .../AbstractApplicationContextRunner.java | 22 +++++- .../example/duplicate/first/EmptyConfig.java | 29 ++++++++ .../example/duplicate/second/EmptyConfig.java | 29 ++++++++ ...AbstractApplicationContextRunnerTests.java | 54 +++++++++++++++ .../context/annotation/Configurations.java | 68 +++++++++++++------ .../annotation/ConfigurationsTests.java | 23 ++++++- 8 files changed, 207 insertions(+), 26 deletions(-) create mode 100644 spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/example/duplicate/first/EmptyConfig.java create mode 100644 spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/example/duplicate/second/EmptyConfig.java diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurations.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurations.java index 9c0b22bf9e7..bc38ee4768e 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurations.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/AutoConfigurations.java @@ -51,7 +51,7 @@ public class AutoConfigurations extends Configurations implements Ordered { } AutoConfigurations(UnaryOperator replacementMapper, Collection> classes) { - super(sorter(replacementMapper), classes); + super(sorter(replacementMapper), classes, Class::getName); this.replacementMapper = replacementMapper; } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/AutoConfigurationsTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/AutoConfigurationsTests.java index dcfb3da0d7f..5d3df4dc066 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/AutoConfigurationsTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/AutoConfigurationsTests.java @@ -54,6 +54,12 @@ class AutoConfigurationsTests { AutoConfigureA.class); } + @Test + void getBeanNameShouldUseClassName() { + Configurations configurations = AutoConfigurations.of(AutoConfigureA.class, AutoConfigureB.class); + assertThat(configurations.getBeanName(AutoConfigureA.class)).isEqualTo(AutoConfigureA.class.getName()); + } + private String replaceB(String className) { return (!AutoConfigureB.class.getName().equals(className)) ? className : AutoConfigureB2.class.getName(); } diff --git a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunner.java b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunner.java index 6a7bd667275..e6599e2f4ef 100644 --- a/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunner.java +++ b/spring-boot-project/spring-boot-test/src/main/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunner.java @@ -19,6 +19,7 @@ package org.springframework.boot.test.context.runner; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.function.BiConsumer; import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Supplier; @@ -27,6 +28,7 @@ import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.beans.factory.config.BeanDefinitionCustomizer; import org.springframework.beans.factory.config.ConfigurableListableBeanFactory; import org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory; +import org.springframework.beans.factory.support.BeanDefinitionRegistry; import org.springframework.beans.factory.support.BeanNameGenerator; import org.springframework.beans.factory.support.DefaultListableBeanFactory; import org.springframework.boot.context.annotation.Configurations; @@ -38,12 +40,14 @@ import org.springframework.boot.test.util.TestPropertyValues; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextInitializer; import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.AnnotatedBeanDefinitionReader; import org.springframework.context.annotation.AnnotationConfigRegistry; import org.springframework.context.support.GenericApplicationContext; import org.springframework.core.ResolvableType; import org.springframework.core.env.Environment; import org.springframework.core.io.DefaultResourceLoader; import org.springframework.util.Assert; +import org.springframework.util.CollectionUtils; /** * Utility design to run an {@link ApplicationContext} and provide AssertJ style @@ -439,15 +443,27 @@ public abstract class AbstractApplicationContextRunner registration.apply(context)); this.runnerConfiguration.initializers.forEach((initializer) -> initializer.initialize(context)); - Class[] classes = Configurations.getClasses(this.runnerConfiguration.configurations); - if (classes.length > 0) { - ((AnnotationConfigRegistry) context).register(classes); + if (!CollectionUtils.isEmpty(this.runnerConfiguration.configurations)) { + BiConsumer, String> registrar = getRegistrar(context); + for (Configurations configurations : Configurations.collate(this.runnerConfiguration.configurations)) { + for (Class beanClass : Configurations.getClasses(configurations)) { + String beanName = configurations.getBeanName(beanClass); + registrar.accept(beanClass, beanName); + } + } } if (refresh) { context.refresh(); } } + private BiConsumer, String> getRegistrar(C context) { + if (context instanceof BeanDefinitionRegistry registry) { + return new AnnotatedBeanDefinitionReader(registry, context.getEnvironment())::registerBean; + } + return (beanClass, beanName) -> ((AnnotationConfigRegistry) context).register(beanClass); + } + private void accept(ContextConsumer consumer, A context) { try { consumer.accept(context); diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/example/duplicate/first/EmptyConfig.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/example/duplicate/first/EmptyConfig.java new file mode 100644 index 00000000000..1af9ebd6c66 --- /dev/null +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/example/duplicate/first/EmptyConfig.java @@ -0,0 +1,29 @@ +/* + * Copyright 2012-2024 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.test.context.example.duplicate.first; + +import org.springframework.context.annotation.Configuration; + +/** + * Example configuration to showcase handing of duplicate class names. + * + * @author Stephane Nicoll + */ +@Configuration +public class EmptyConfig { + +} diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/example/duplicate/second/EmptyConfig.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/example/duplicate/second/EmptyConfig.java new file mode 100644 index 00000000000..54850a7ac4c --- /dev/null +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/example/duplicate/second/EmptyConfig.java @@ -0,0 +1,29 @@ +/* + * Copyright 2012-2024 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. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.boot.test.context.example.duplicate.second; + +import org.springframework.context.annotation.Configuration; + +/** + * Example configuration to showcase handing of duplicate class names. + * + * @author Stephane Nicoll + */ +@Configuration +public class EmptyConfig { + +} diff --git a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunnerTests.java b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunnerTests.java index d4a38ce5deb..44b1d44bc9d 100644 --- a/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunnerTests.java +++ b/spring-boot-project/spring-boot-test/src/test/java/org/springframework/boot/test/context/runner/AbstractApplicationContextRunnerTests.java @@ -17,6 +17,9 @@ package org.springframework.boot.test.context.runner; import java.io.IOException; +import java.util.Collection; +import java.util.List; +import java.util.Set; import java.util.UUID; import java.util.concurrent.atomic.AtomicBoolean; @@ -27,6 +30,8 @@ import org.springframework.beans.factory.BeanCurrentlyInCreationException; import org.springframework.beans.factory.BeanDefinitionStoreException; import org.springframework.beans.factory.ObjectProvider; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.beans.factory.support.BeanDefinitionOverrideException; +import org.springframework.boot.context.annotation.Configurations; import org.springframework.boot.context.annotation.UserConfigurations; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.context.properties.EnableConfigurationProperties; @@ -139,6 +144,38 @@ abstract class AbstractApplicationContextRunnerTests assertThat(context).hasBean("foo")); } + @Test + void runWithUserConfigurationsRegistersDefaultBeanName() { + get().withUserConfiguration(FooConfig.class) + .run((context) -> assertThat(context).hasBean("abstractApplicationContextRunnerTests.FooConfig")); + } + + @Test + void runWithUserConfigurationsWhenHasSameShortClassNamedRegistersWithoutBeanName() { + get() + .withUserConfiguration(org.springframework.boot.test.context.example.duplicate.first.EmptyConfig.class, + org.springframework.boot.test.context.example.duplicate.second.EmptyConfig.class) + .run((context) -> assertThat(context.getStartupFailure()) + .isInstanceOf(BeanDefinitionOverrideException.class)); + } + + @Test + void runFullyQualifiedNameConfigurationsRegistersFullyQualifiedBeanName() { + get().withConfiguration(FullyQualifiedNameConfigurations.of(FooConfig.class)) + .run((context) -> assertThat(context).hasBean(FooConfig.class.getName())); + } + + @Test + void runWithFullyQualifiedNameConfigurationsWhenHasSameShortClassNamedRegistersWithFullyQualifiedBeanName() { + get() + .withConfiguration(FullyQualifiedNameConfigurations.of( + org.springframework.boot.test.context.example.duplicate.first.EmptyConfig.class, + org.springframework.boot.test.context.example.duplicate.second.EmptyConfig.class)) + .run((context) -> assertThat(context) + .hasSingleBean(org.springframework.boot.test.context.example.duplicate.first.EmptyConfig.class) + .hasSingleBean(org.springframework.boot.test.context.example.duplicate.second.EmptyConfig.class)); + } + @Test void runWithUserNamedBeanShouldRegisterBean() { get().withBean("foo", String.class, () -> "foo").run((context) -> assertThat(context).hasBean("foo")); @@ -384,4 +421,21 @@ abstract class AbstractApplicationContextRunnerTests> classes) { + super(null, classes, Class::getName); + } + + @Override + protected Configurations merge(Set> mergedClasses) { + return new FullyQualifiedNameConfigurations(mergedClasses); + } + + static FullyQualifiedNameConfigurations of(Class... classes) { + return new FullyQualifiedNameConfigurations(List.of(classes)); + } + + } + } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/annotation/Configurations.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/annotation/Configurations.java index 8d86a545aad..b00121071f2 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/annotation/Configurations.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/annotation/Configurations.java @@ -25,6 +25,7 @@ import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; import java.util.Set; +import java.util.function.Function; import java.util.function.UnaryOperator; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -65,6 +66,8 @@ public abstract class Configurations { private final Set> classes; + private final Function, String> beanNameGenerator; + /** * Create a new {@link Configurations} instance. * @param classes the configuration classes @@ -74,20 +77,28 @@ public abstract class Configurations { Collection> sorted = sort(classes); this.sorter = null; this.classes = Collections.unmodifiableSet(new LinkedHashSet<>(sorted)); + this.beanNameGenerator = null; } /** * Create a new {@link Configurations} instance. * @param sorter a {@link UnaryOperator} used to sort the configurations * @param classes the configuration classes + * @param beanNameGenerator an optional function used to generate the bean name * @since 3.4.0 */ - protected Configurations(UnaryOperator>> sorter, Collection> classes) { - Assert.notNull(sorter, "Sorter must not be null"); + protected Configurations(UnaryOperator>> sorter, Collection> classes, + Function, String> beanNameGenerator) { Assert.notNull(classes, "Classes must not be null"); + sorter = (sorter != null) ? sorter : UnaryOperator.identity(); Collection> sorted = sorter.apply(classes); - this.sorter = sorter; + this.sorter = (sorter != null) ? sorter : UnaryOperator.identity(); this.classes = Collections.unmodifiableSet(new LinkedHashSet<>(sorted)); + this.beanNameGenerator = beanNameGenerator; + } + + protected final Set> getClasses() { + return this.classes; } /** @@ -95,17 +106,13 @@ public abstract class Configurations { * @param classes the classes to sort * @return a sorted set of classes * @deprecated since 3.4.0 for removal in 3.6.0 in favor of - * {@link #Configurations(UnaryOperator, Collection)} + * {@link #Configurations(UnaryOperator, Collection, Function)} */ @Deprecated(since = "3.4.0", forRemoval = true) protected Collection> sort(Collection> classes) { return classes; } - protected final Set> getClasses() { - return this.classes; - } - /** * Merge configurations from another source of the same type. * @param other the other {@link Configurations} (must be of the same type as this @@ -128,6 +135,17 @@ public abstract class Configurations { */ protected abstract Configurations merge(Set> mergedClasses); + /** + * Return the bean name that should be used for the given configuration class or + * {@code null} to use the default name. + * @param beanClass the bean class + * @return the bean name + * @since 3.4.0 + */ + public String getBeanName(Class beanClass) { + return (this.beanNameGenerator != null) ? this.beanNameGenerator.apply(beanClass) : null; + } + /** * Return the classes from all the specified configurations in the order that they * would be registered. @@ -145,30 +163,40 @@ public abstract class Configurations { * @return configuration classes in registration order */ public static Class[] getClasses(Collection configurations) { - List ordered = new ArrayList<>(configurations); - ordered.sort(COMPARATOR); - List collated = collate(ordered); + List collated = collate(configurations); LinkedHashSet> classes = collated.stream() .flatMap(Configurations::streamClasses) .collect(Collectors.toCollection(LinkedHashSet::new)); return ClassUtils.toClassArray(classes); } - private static Stream> streamClasses(Configurations configurations) { - return configurations.getClasses().stream(); - } - - private static List collate(List orderedConfigurations) { + /** + * Collate the given configuration by sorting and merging them. + * @param configurations the source configuration + * @return the collated configurations + * @since 3.4.0 + */ + public static List collate(Collection configurations) { LinkedList collated = new LinkedList<>(); - for (Configurations item : orderedConfigurations) { - if (collated.isEmpty() || collated.getLast().getClass() != item.getClass()) { - collated.add(item); + for (Configurations configuration : sortConfigurations(configurations)) { + if (collated.isEmpty() || collated.getLast().getClass() != configuration.getClass()) { + collated.add(configuration); } else { - collated.set(collated.size() - 1, collated.getLast().merge(item)); + collated.set(collated.size() - 1, collated.getLast().merge(configuration)); } } return collated; } + private static List sortConfigurations(Collection configurations) { + List sorted = new ArrayList<>(configurations); + sorted.sort(COMPARATOR); + return sorted; + } + + private static Stream> streamClasses(Configurations configurations) { + return configurations.getClasses().stream(); + } + } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/annotation/ConfigurationsTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/annotation/ConfigurationsTests.java index 23d50cd4c7f..715b85947ee 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/annotation/ConfigurationsTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/context/annotation/ConfigurationsTests.java @@ -23,7 +23,9 @@ import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Comparator; +import java.util.List; import java.util.Set; +import java.util.function.Function; import java.util.function.UnaryOperator; import org.junit.jupiter.api.Test; @@ -87,6 +89,18 @@ class ConfigurationsTests { OutputStream.class, String.class); } + @Test + void getBeanNameWhenNoFunctionReturnsNull() { + Configurations configurations = new TestConfigurations(Short.class); + assertThat(configurations.getBeanName(Short.class)).isNull(); + } + + @Test + void getBeanNameWhenFunctionReturnsBeanName() { + Configurations configurations = new TestConfigurations(Sorter.instance, List.of(Short.class), Class::getName); + assertThat(configurations.getBeanName(Short.class)).isEqualTo(Short.class.getName()); + } + @Order(Ordered.HIGHEST_PRECEDENCE) static class TestConfigurations extends Configurations { @@ -95,7 +109,12 @@ class ConfigurationsTests { } TestConfigurations(UnaryOperator>> sorter, Class... classes) { - super(sorter, Arrays.asList(classes)); + this(sorter, Arrays.asList(classes), null); + } + + TestConfigurations(UnaryOperator>> sorter, Collection> classes, + Function, String> beanNameGenerator) { + super(sorter, classes, beanNameGenerator); } TestConfigurations(Collection> classes) { @@ -117,7 +136,7 @@ class ConfigurationsTests { } protected TestSortedConfigurations(Collection> classes) { - super(Sorter.instance, classes); + super(Sorter.instance, classes, null); } @Override