Preserve existing imported class over scanned configuration class

Closes gh-24643

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-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.
@@ -56,6 +56,8 @@ final class ConfigurationClass {
 	@Nullable
 	private String beanName;
 
+	private boolean scanned = false;
+
 	private final Set<ConfigurationClass> importedBy = new LinkedHashSet<>(1);
 
 	private final Set<BeanMethod> beanMethods = new LinkedHashSet<>();
@@ -73,7 +75,6 @@ final class ConfigurationClass {
 	 * Create a new {@link ConfigurationClass} with the given name.
 	 * @param metadataReader reader used to parse the underlying {@link Class}
 	 * @param beanName must not be {@code null}
-	 * @see ConfigurationClass#ConfigurationClass(Class, ConfigurationClass)
 	 */
 	ConfigurationClass(MetadataReader metadataReader, String beanName) {
 		Assert.notNull(beanName, "Bean name must not be null");
@@ -87,10 +88,10 @@ final class ConfigurationClass {
 	 * using the {@link Import} annotation or automatically processed as a nested
 	 * configuration class (if importedBy is not {@code null}).
 	 * @param metadataReader reader used to parse the underlying {@link Class}
-	 * @param importedBy the configuration class importing this one or {@code null}
+	 * @param importedBy the configuration class importing this one
 	 * @since 3.1.1
 	 */
-	ConfigurationClass(MetadataReader metadataReader, @Nullable ConfigurationClass importedBy) {
+	ConfigurationClass(MetadataReader metadataReader, ConfigurationClass importedBy) {
 		this.metadata = metadataReader.getAnnotationMetadata();
 		this.resource = metadataReader.getResource();
 		this.importedBy.add(importedBy);
@@ -100,7 +101,6 @@ final class ConfigurationClass {
 	 * Create a new {@link ConfigurationClass} with the given name.
 	 * @param clazz the underlying {@link Class} to represent
 	 * @param beanName name of the {@code @Configuration} class bean
-	 * @see ConfigurationClass#ConfigurationClass(Class, ConfigurationClass)
 	 */
 	ConfigurationClass(Class<?> clazz, String beanName) {
 		Assert.notNull(beanName, "Bean name must not be null");
@@ -114,10 +114,10 @@ final class ConfigurationClass {
 	 * using the {@link Import} annotation or automatically processed as a nested
 	 * configuration class (if imported is {@code true}).
 	 * @param clazz the underlying {@link Class} to represent
-	 * @param importedBy the configuration class importing this one (or {@code null})
+	 * @param importedBy the configuration class importing this one
 	 * @since 3.1.1
 	 */
-	ConfigurationClass(Class<?> clazz, @Nullable ConfigurationClass importedBy) {
+	ConfigurationClass(Class<?> clazz, ConfigurationClass importedBy) {
 		this.metadata = AnnotationMetadata.introspect(clazz);
 		this.resource = new DescriptiveResource(clazz.getName());
 		this.importedBy.add(importedBy);
@@ -127,13 +127,14 @@ final class ConfigurationClass {
 	 * Create a new {@link ConfigurationClass} with the given name.
 	 * @param metadata the metadata for the underlying class to represent
 	 * @param beanName name of the {@code @Configuration} class bean
-	 * @see ConfigurationClass#ConfigurationClass(Class, ConfigurationClass)
+	 * @param scanned whether the underlying class has been registered through a scan
 	 */
-	ConfigurationClass(AnnotationMetadata metadata, String beanName) {
+	ConfigurationClass(AnnotationMetadata metadata, String beanName, boolean scanned) {
 		Assert.notNull(beanName, "Bean name must not be null");
 		this.metadata = metadata;
 		this.resource = new DescriptiveResource(metadata.getClassName());
 		this.beanName = beanName;
+		this.scanned = scanned;
 	}
 
 
@@ -149,22 +150,30 @@ final class ConfigurationClass {
 		return ClassUtils.getShortName(getMetadata().getClassName());
 	}
 
-	void setBeanName(String beanName) {
+	void setBeanName(@Nullable String beanName) {
 		this.beanName = beanName;
 	}
 
 	@Nullable
-	public String getBeanName() {
+	String getBeanName() {
 		return this.beanName;
 	}
 
+	/**
+	 * Return whether this configuration class has been registered through a scan.
+	 * @since 6.2
+	 */
+	boolean isScanned() {
+		return this.scanned;
+	}
+
 	/**
 	 * Return whether this configuration class was registered via @{@link Import} or
 	 * automatically registered due to being nested within another configuration class.
 	 * @since 3.1.1
 	 * @see #getImportedBy()
 	 */
-	public boolean isImported() {
+	boolean isImported() {
 		return !this.importedBy.isEmpty();
 	}
 
@@ -198,6 +207,10 @@ final class ConfigurationClass {
 		this.importedResources.put(importedResource, readerClass);
 	}
 
+	Map<String, Class<? extends BeanDefinitionReader>> getImportedResources() {
+		return this.importedResources;
+	}
+
 	void addImportBeanDefinitionRegistrar(ImportBeanDefinitionRegistrar registrar, AnnotationMetadata importingClassMetadata) {
 		this.importBeanDefinitionRegistrars.put(registrar, importingClassMetadata);
 	}
@@ -206,10 +219,6 @@ final class ConfigurationClass {
 		return this.importBeanDefinitionRegistrars;
 	}
 
-	Map<String, Class<? extends BeanDefinitionReader>> getImportedResources() {
-		return this.importedResources;
-	}
-
 	void validate(ProblemReporter problemReporter) {
 		Map<String, Object> attributes = this.metadata.getAnnotationAttributes(Configuration.class.getName());
 

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-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.
@@ -162,7 +162,7 @@ class ConfigurationClassParser {
 			BeanDefinition bd = holder.getBeanDefinition();
 			try {
 				if (bd instanceof AnnotatedBeanDefinition annotatedBeanDef) {
-					parse(annotatedBeanDef.getMetadata(), holder.getBeanName());
+					parse(annotatedBeanDef, holder.getBeanName());
 				}
 				else if (bd instanceof AbstractBeanDefinition abstractBeanDef && abstractBeanDef.hasBeanClass()) {
 					parse(abstractBeanDef.getBeanClass(), holder.getBeanName());
@@ -183,31 +183,33 @@ class ConfigurationClassParser {
 		this.deferredImportSelectorHandler.process();
 	}
 
-	protected final void parse(@Nullable String className, String beanName) throws IOException {
-		Assert.notNull(className, "No bean class name for configuration class bean definition");
-		MetadataReader reader = this.metadataReaderFactory.getMetadataReader(className);
-		processConfigurationClass(new ConfigurationClass(reader, beanName), DEFAULT_EXCLUSION_FILTER);
+	private void parse(AnnotatedBeanDefinition beanDef, String beanName) {
+		processConfigurationClass(
+				new ConfigurationClass(beanDef.getMetadata(), beanName, (beanDef instanceof ScannedGenericBeanDefinition)),
+				DEFAULT_EXCLUSION_FILTER);
 	}
 
-	protected final void parse(Class<?> clazz, String beanName) throws IOException {
+	private void parse(Class<?> clazz, String beanName) {
 		processConfigurationClass(new ConfigurationClass(clazz, beanName), DEFAULT_EXCLUSION_FILTER);
 	}
 
-	protected final void parse(AnnotationMetadata metadata, String beanName) throws IOException {
-		processConfigurationClass(new ConfigurationClass(metadata, beanName), DEFAULT_EXCLUSION_FILTER);
+	final void parse(@Nullable String className, String beanName) throws IOException {
+		Assert.notNull(className, "No bean class name for configuration class bean definition");
+		MetadataReader reader = this.metadataReaderFactory.getMetadataReader(className);
+		processConfigurationClass(new ConfigurationClass(reader, beanName), DEFAULT_EXCLUSION_FILTER);
 	}
 
 	/**
 	 * Validate each {@link ConfigurationClass} object.
 	 * @see ConfigurationClass#validate
 	 */
-	public void validate() {
+	void validate() {
 		for (ConfigurationClass configClass : this.configurationClasses.keySet()) {
 			configClass.validate(this.problemReporter);
 		}
 	}
 
-	public Set<ConfigurationClass> getConfigurationClasses() {
+	Set<ConfigurationClass> getConfigurationClasses() {
 		return this.configurationClasses.keySet();
 	}
 
@@ -216,7 +218,12 @@ class ConfigurationClassParser {
 				Collections.emptyList());
 	}
 
-	protected void processConfigurationClass(ConfigurationClass configClass, Predicate<String> filter) throws IOException {
+	ImportRegistry getImportRegistry() {
+		return this.importStack;
+	}
+
+
+	protected void processConfigurationClass(ConfigurationClass configClass, Predicate<String> filter) {
 		if (this.conditionEvaluator.shouldSkip(configClass.getMetadata(), ConfigurationPhase.PARSE_CONFIGURATION)) {
 			return;
 		}
@@ -230,6 +237,14 @@ class ConfigurationClassParser {
 				// Otherwise ignore new imported config class; existing non-imported class overrides it.
 				return;
 			}
+			else if (configClass.isScanned()) {
+				String beanName = configClass.getBeanName();
+				if (beanName != null) {
+					this.registry.removeBeanDefinition(beanName);
+				}
+				// An implicitly scanned bean definition should not override an explicit import.
+				return;
+			}
 			else {
 				// Explicit bean definition found, probably replacing an import.
 				// Let's remove the old one and go with the new one.
@@ -563,11 +578,6 @@ class ConfigurationClassParser {
 		return false;
 	}
 
-	ImportRegistry getImportRegistry() {
-		return this.importStack;
-	}
-
-
 	/**
 	 * Factory method to obtain a {@link SourceClass} from a {@link ConfigurationClass}.
 	 */
@@ -636,7 +646,7 @@ class ConfigurationClassParser {
 
 		private final MultiValueMap<String, AnnotationMetadata> imports = new LinkedMultiValueMap<>();
 
-		public void registerImport(AnnotationMetadata importingClass, String importedClass) {
+		void registerImport(AnnotationMetadata importingClass, String importedClass) {
 			this.imports.add(importedClass, importingClass);
 		}
 
@@ -691,7 +701,7 @@ class ConfigurationClassParser {
 		 * @param configClass the source configuration class
 		 * @param importSelector the selector to handle
 		 */
-		public void handle(ConfigurationClass configClass, DeferredImportSelector importSelector) {
+		void handle(ConfigurationClass configClass, DeferredImportSelector importSelector) {
 			DeferredImportSelectorHolder holder = new DeferredImportSelectorHolder(configClass, importSelector);
 			if (this.deferredImportSelectors == null) {
 				DeferredImportSelectorGroupingHandler handler = new DeferredImportSelectorGroupingHandler();
@@ -703,7 +713,7 @@ class ConfigurationClassParser {
 			}
 		}
 
-		public void process() {
+		void process() {
 			List<DeferredImportSelectorHolder> deferredImports = this.deferredImportSelectors;
 			this.deferredImportSelectors = null;
 			try {
@@ -727,7 +737,7 @@ class ConfigurationClassParser {
 
 		private final Map<AnnotationMetadata, ConfigurationClass> configurationClasses = new HashMap<>();
 
-		public void register(DeferredImportSelectorHolder deferredImport) {
+		void register(DeferredImportSelectorHolder deferredImport) {
 			Class<? extends Group> group = deferredImport.getImportSelector().getImportGroup();
 			DeferredImportSelectorGrouping grouping = this.groupings.computeIfAbsent(
 					(group != null ? group : deferredImport),
@@ -737,7 +747,7 @@ class ConfigurationClassParser {
 					deferredImport.getConfigurationClass());
 		}
 
-		public void processGroupImports() {
+		void processGroupImports() {
 			for (DeferredImportSelectorGrouping grouping : this.groupings.values()) {
 				Predicate<String> exclusionFilter = grouping.getCandidateFilter();
 				grouping.getImports().forEach(entry -> {
@@ -775,16 +785,16 @@ class ConfigurationClassParser {
 
 		private final DeferredImportSelector importSelector;
 
-		public DeferredImportSelectorHolder(ConfigurationClass configClass, DeferredImportSelector selector) {
+		DeferredImportSelectorHolder(ConfigurationClass configClass, DeferredImportSelector selector) {
 			this.configurationClass = configClass;
 			this.importSelector = selector;
 		}
 
-		public ConfigurationClass getConfigurationClass() {
+		ConfigurationClass getConfigurationClass() {
 			return this.configurationClass;
 		}
 
-		public DeferredImportSelector getImportSelector() {
+		DeferredImportSelector getImportSelector() {
 			return this.importSelector;
 		}
 	}
@@ -800,7 +810,7 @@ class ConfigurationClassParser {
 			this.group = group;
 		}
 
-		public void add(DeferredImportSelectorHolder deferredImport) {
+		void add(DeferredImportSelectorHolder deferredImport) {
 			this.deferredImports.add(deferredImport);
 		}
 
@@ -808,7 +818,7 @@ class ConfigurationClassParser {
 		 * Return the imports defined by the group.
 		 * @return each import with its associated configuration class
 		 */
-		public Iterable<Group.Entry> getImports() {
+		Iterable<Group.Entry> getImports() {
 			for (DeferredImportSelectorHolder deferredImport : this.deferredImports) {
 				this.group.process(deferredImport.getConfigurationClass().getMetadata(),
 						deferredImport.getImportSelector());
@@ -816,7 +826,7 @@ class ConfigurationClassParser {
 			return this.group.selectImports();
 		}
 
-		public Predicate<String> getCandidateFilter() {
+		Predicate<String> getCandidateFilter() {
 			Predicate<String> mergedFilter = DEFAULT_EXCLUSION_FILTER;
 			for (DeferredImportSelectorHolder deferredImport : this.deferredImports) {
 				Predicate<String> selectorFilter = deferredImport.getImportSelector().getExclusionFilter();

spring-context/src/test/java/org/springframework/context/annotation/componentscan/ordered/SiblingImportingConfigA.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2002-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.context.annotation.componentscan.ordered;
+
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Import;
+
+/**
+ * @author Vladislav Kisel
+ */
+@Configuration
+@Import(SiblingImportingConfigB.class)
+public class SiblingImportingConfigA {
+
+	@Bean(name = "a-imports-b")
+	String bean() {
+		return "valueFromA";
+	}
+
+}

spring-context/src/test/java/org/springframework/context/annotation/componentscan/ordered/SiblingImportingConfigB.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2002-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.context.annotation.componentscan.ordered;
+
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+/**
+ * @author Vladislav Kisel
+ */
+@Configuration
+public class SiblingImportingConfigB {
+
+	@Bean(name = "a-imports-b")
+	String bean() {
+		return "valueFromB";
+	}
+
+}

spring-context/src/test/java/org/springframework/context/annotation/componentscan/ordered/SiblingReversedImportingConfigA.java

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2002-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.context.annotation.componentscan.ordered;
+
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+
+/**
+ * @author Vladislav Kisel
+ */
+@Configuration
+public class SiblingReversedImportingConfigA {
+
+	@Bean(name = "b-imports-a")
+	String bean() {
+		return "valueFromAR";
+	}
+
+}

spring-context/src/test/java/org/springframework/context/annotation/componentscan/ordered/SiblingReversedImportingConfigB.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright 2002-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.context.annotation.componentscan.ordered;
+
+import org.springframework.context.annotation.Bean;
+import org.springframework.context.annotation.Configuration;
+import org.springframework.context.annotation.Import;
+
+/**
+ * @author Vladislav Kisel
+ */
+@Configuration
+@Import(SiblingReversedImportingConfigA.class)
+public class SiblingReversedImportingConfigB {
+
+	@Bean(name = "b-imports-a")
+	String bean() {
+		return "valueFromBR";
+	}
+
+}

spring-context/src/test/java/org/springframework/context/annotation/configuration/ImportTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2023 the original author or authors.
+ * Copyright 2002-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.
@@ -28,6 +28,8 @@ import org.springframework.context.annotation.Configuration;
 import org.springframework.context.annotation.ConfigurationClassPostProcessor;
 import org.springframework.context.annotation.DependsOn;
 import org.springframework.context.annotation.Import;
+import org.springframework.context.annotation.componentscan.ordered.SiblingImportingConfigA;
+import org.springframework.context.annotation.componentscan.ordered.SiblingImportingConfigB;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -362,6 +364,8 @@ class ImportTests {
 	@Import(A.class)
 	static class B { }
 
+	// ------------------------------------------------------------------------
+
 	@Test
 	void testProcessImports() {
 		int configClasses = 2;
@@ -369,4 +373,20 @@ class ImportTests {
 		assertBeanDefinitionCount((configClasses + beansInClasses), ConfigurationWithImportAnnotation.class);
 	}
 
+	/**
+	 * An imported config must override a scanned one, thus bean definitions
+	 * from the imported class is overridden by its importer.
+	 */
+	@Test  // gh-24643
+	void importedConfigOverridesScanned() {
+		AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
+		ctx.scan(SiblingImportingConfigA.class.getPackage().getName());
+		ctx.refresh();
+
+		assertThat(ctx.getBean("a-imports-b")).isEqualTo("valueFromA");
+		assertThat(ctx.getBean("b-imports-a")).isEqualTo("valueFromBR");
+		assertThat(ctx.getBeansOfType(SiblingImportingConfigA.class)).hasSize(1);
+		assertThat(ctx.getBeansOfType(SiblingImportingConfigB.class)).hasSize(1);
+	}
+
 }