diff --git a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java index c5a47b891a6..50ade42f822 100644 --- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java +++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java @@ -23,9 +23,12 @@ import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.Stack; @@ -76,6 +79,7 @@ import static org.springframework.context.annotation.MetadataUtils.*; * * @author Chris Beams * @author Juergen Hoeller + * @author Rob Winch * @since 3.0 * @see ConfigurationClassBeanDefinitionReader */ @@ -89,8 +93,8 @@ class ConfigurationClassParser { private final Set knownSuperclasses = new LinkedHashSet(); - private final Set configurationClasses = - new LinkedHashSet(); + private final Map configurationClasses = + new LinkedHashMap(); private final Stack> propertySources = new Stack>(); @@ -157,13 +161,60 @@ class ConfigurationClassParser { } while (metadata != null); - if (this.configurationClasses.contains(configClass) && configClass.getBeanName() != null) { + if (getConfigurationClasses().contains(configClass) && configClass.getBeanName() != null) { // Explicit bean definition found, probably replacing an import. // Let's remove the old one and go with the new one. - this.configurationClasses.remove(configClass); + ConfigurationClass originalConfigClass = removeConfigurationClass(configClass); + + mergeFromOriginalConfig(originalConfigClass,configClass); } - this.configurationClasses.add(configClass); + addConfigurationClass(configClass); + } + + + /** + * Merges from the original {@link ConfigurationClass} to the new + * {@link ConfigurationClass}. This is necessary if parent classes have already been + * processed. + * + * @param originalConfigClass the original {@link ConfigurationClass} that may have + * additional metadata + * @param configClass the new {@link ConfigurationClass} that will have metadata added + * to it if necessary + */ + private void mergeFromOriginalConfig(ConfigurationClass originalConfigClass, + ConfigurationClass configClass) { + + Set beanMethodNames = new HashSet(); + for(BeanMethod beanMethod : configClass.getBeanMethods()) { + beanMethodNames.add(createBeanMethodName(beanMethod)); + } + + for(BeanMethod originalBeanMethod : originalConfigClass.getBeanMethods()) { + String originalBeanMethodName = createBeanMethodName(originalBeanMethod); + if(!beanMethodNames.contains(originalBeanMethodName)) { + configClass.addBeanMethod(new BeanMethod(originalBeanMethod.getMetadata(), configClass)); + } + } + for(Entry> originalImportedEntry : originalConfigClass.getImportedResources().entrySet()) { + if(!configClass.getImportedResources().containsKey(originalImportedEntry.getKey())) { + configClass.addImportedResource(originalImportedEntry.getKey(), originalImportedEntry.getValue()); + } + } + } + + /** + * Converts a {@link BeanMethod} into the fully qualified name of the Method + * + * @param beanMethod + * @return fully qualified name of the {@link BeanMethod} + */ + private String createBeanMethodName(BeanMethod beanMethod) { + String hashDelim = "#"; + String dClassName = beanMethod.getMetadata().getDeclaringClassName(); + String methodName = beanMethod.getMetadata().getMethodName(); + return dClassName + hashDelim + methodName; } /** @@ -251,6 +302,14 @@ class ConfigurationClassParser { return null; } + private void addConfigurationClass(ConfigurationClass configClass) { + this.configurationClasses.put(configClass,configClass); + } + + private ConfigurationClass removeConfigurationClass(ConfigurationClass configClass) { + return this.configurationClasses.remove(configClass); + } + /** * Register member (nested) classes that happen to be configuration classes themselves. * @param metadata the metadata representation of the containing class @@ -441,13 +500,13 @@ class ConfigurationClassParser { * @see ConfigurationClass#validate */ public void validate() { - for (ConfigurationClass configClass : this.configurationClasses) { + for (ConfigurationClass configClass : getConfigurationClasses()) { configClass.validate(this.problemReporter); } } public Set getConfigurationClasses() { - return this.configurationClasses; + return this.configurationClasses.keySet(); } public Stack> getPropertySources() { diff --git a/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ImportedConfig.java b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ImportedConfig.java new file mode 100644 index 00000000000..f20e79eb7e5 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ImportedConfig.java @@ -0,0 +1,33 @@ +/* + * Copyright 2002-2013 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 + * + * http://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.spr10546; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + + +/** + * + * @author Rob Winch + */ +@Configuration +public class ImportedConfig { + @Bean + public String myBean() { + return "myBean"; + } +} diff --git a/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentConfig.java b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentConfig.java new file mode 100644 index 00000000000..884622e22d9 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentConfig.java @@ -0,0 +1,34 @@ +/* + * Copyright 2002-2013 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 + * + * http://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.spr10546; + +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; + + +/** + * + * @author Rob Winch + */ +@Configuration +public class ParentConfig { + @Bean + public String myBean() { + return "myBean"; + } +} + diff --git a/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithComponentScanConfig.java b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithComponentScanConfig.java new file mode 100644 index 00000000000..bd6c7b4977a --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithComponentScanConfig.java @@ -0,0 +1,32 @@ +/* + * Copyright 2002-2013 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 + * + * http://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.spr10546; + +import org.springframework.context.annotation.ComponentScan; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.spr10546.scanpackage.AEnclosingConfig; + + +/** + * + * @author Rob Winch + */ +@Configuration +@ComponentScan(basePackageClasses=AEnclosingConfig.class) +public class ParentWithComponentScanConfig { + +} diff --git a/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithImportConfig.java b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithImportConfig.java new file mode 100644 index 00000000000..450d13fbad3 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithImportConfig.java @@ -0,0 +1,31 @@ +/* + * Copyright 2002-2013 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 + * + * http://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.spr10546; + +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; + + +/** + * + * @author Rob Winch + */ +@Configuration +@Import(ImportedConfig.class) +public class ParentWithImportConfig { + +} diff --git a/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithImportResourceConfig.java b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithImportResourceConfig.java new file mode 100644 index 00000000000..e05f4134eca --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithImportResourceConfig.java @@ -0,0 +1,31 @@ +/* + * Copyright 2002-2013 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 + * + * http://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.spr10546; + +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.ImportResource; + + +/** + * + * @author Rob Winch + */ +@Configuration +@ImportResource("classpath:org/springframework/context/annotation/spr10546/importedResource.xml") +public class ParentWithImportResourceConfig { + +} diff --git a/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithParentConfig.java b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithParentConfig.java new file mode 100644 index 00000000000..dcaaa3b5b2a --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithParentConfig.java @@ -0,0 +1,29 @@ +/* + * Copyright 2002-2013 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 + * + * http://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.spr10546; + +import org.springframework.context.annotation.Configuration; + + +/** + * + * @author Rob Winch + */ +@Configuration +public class ParentWithParentConfig extends ParentConfig { + +} diff --git a/spring-context/src/test/java/org/springframework/context/annotation/spr10546/Spr10546Tests.java b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/Spr10546Tests.java new file mode 100644 index 00000000000..59dac470a21 --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/Spr10546Tests.java @@ -0,0 +1,149 @@ +/* + * Copyright 2002-2013 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 + * + * http://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.spr10546; + +import org.junit.After; +import org.junit.Test; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.Import; +import org.springframework.context.annotation.spr10546.scanpackage.AEnclosingConfig; + +import static org.hamcrest.CoreMatchers.*; +import static org.junit.Assert.*; + + +/** + * + * @author Rob Winch + */ +public class Spr10546Tests { + private ConfigurableApplicationContext context; + + @After + public void closeContext() { + if(context != null) { + context.close(); + } + } + + // These fail prior to fixing SPR-10546 + + @Test + public void enclosingConfigFirstParentDefinesBean() { + assertLoadsMyBean(AEnclosingConfig.class,AEnclosingConfig.ChildConfig.class); + } + + /** + * Prior to fixing SPR-10546 this might have succeeded depending on the ordering the + * classes were picked up. If they are picked up in the same order as + * {@link #enclosingConfigFirstParentDefinesBean()} then it would fail. This test is + * mostly for illustration purposes, but doesn't hurt to continue using it. + * + *

We purposely use the {@link AEnclosingConfig} to make it alphabetically prior to the + * {@link AEnclosingConfig.ChildConfig} which encourages this to occur with the + * classpath scanning implementation being used by the author of this test. + */ + @Test + public void enclosingConfigFirstParentDefinesBeanWithScanning() { + AnnotationConfigApplicationContext ctx= new AnnotationConfigApplicationContext(); + context = ctx; + ctx.scan(AEnclosingConfig.class.getPackage().getName()); + ctx.refresh(); + assertThat(context.getBean("myBean",String.class), equalTo("myBean")); + } + + @Test + public void enclosingConfigFirstParentDefinesBeanWithImportResource() { + assertLoadsMyBean(AEnclosingWithImportResourceConfig.class,AEnclosingWithImportResourceConfig.ChildConfig.class); + } + + @Configuration + static class AEnclosingWithImportResourceConfig { + @Configuration + public static class ChildConfig extends ParentWithImportResourceConfig {} + } + + @Test + public void enclosingConfigFirstParentDefinesBeanWithComponentScan() { + assertLoadsMyBean(AEnclosingWithComponentScanConfig.class,AEnclosingWithComponentScanConfig.ChildConfig.class); + } + + @Configuration + static class AEnclosingWithComponentScanConfig { + @Configuration + public static class ChildConfig extends ParentWithComponentScanConfig {} + } + + @Test + public void enclosingConfigFirstParentWithParentDefinesBean() { + assertLoadsMyBean(AEnclosingWithGrandparentConfig.class,AEnclosingWithGrandparentConfig.ChildConfig.class); + } + + @Configuration + static class AEnclosingWithGrandparentConfig { + @Configuration + public static class ChildConfig extends ParentWithParentConfig {} + } + + @Test + public void importChildConfigThenChildConfig() { + assertLoadsMyBean(ImportChildConfig.class,ChildConfig.class); + } + + @Configuration + static class ChildConfig extends ParentConfig {} + + @Configuration + @Import(ChildConfig.class) + static class ImportChildConfig {} + + + // These worked prior, but validating they continue to work + + @Test + public void enclosingConfigFirstParentDefinesBeanWithImport() { + assertLoadsMyBean(AEnclosingWithImportConfig.class,AEnclosingWithImportConfig.ChildConfig.class); + } + + @Configuration + static class AEnclosingWithImportConfig { + @Configuration + public static class ChildConfig extends ParentWithImportConfig {} + } + + @Test + public void childConfigFirst() { + assertLoadsMyBean(AEnclosingConfig.ChildConfig.class, AEnclosingConfig.class); + } + + @Test + public void enclosingConfigOnly() { + assertLoadsMyBean(AEnclosingConfig.class); + } + + @Test + public void childConfigOnly() { + assertLoadsMyBean(AEnclosingConfig.ChildConfig.class); + } + + private void assertLoadsMyBean(Class... annotatedClasses) { + context = new AnnotationConfigApplicationContext(annotatedClasses); + assertThat(context.getBean("myBean",String.class), equalTo("myBean")); + } +} diff --git a/spring-context/src/test/java/org/springframework/context/annotation/spr10546/importedResource.xml b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/importedResource.xml new file mode 100644 index 00000000000..6ce3074b51f --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/importedResource.xml @@ -0,0 +1,8 @@ + + + + + diff --git a/spring-context/src/test/java/org/springframework/context/annotation/spr10546/scanpackage/AEnclosingConfig.java b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/scanpackage/AEnclosingConfig.java new file mode 100644 index 00000000000..3c8c091222f --- /dev/null +++ b/spring-context/src/test/java/org/springframework/context/annotation/spr10546/scanpackage/AEnclosingConfig.java @@ -0,0 +1,34 @@ +/* + * Copyright 2002-2013 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 + * + * http://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.spr10546.scanpackage; + +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.spr10546.ParentConfig; + + +/** + * Note the name of {@link AEnclosingConfig} is chosen to help ensure scanning picks up + * the enclosing configuration prior to {@link ChildConfig} to demonstrate this can happen + * with classpath scanning. + * + * @author Rob Winch + */ +@Configuration +public class AEnclosingConfig { + @Configuration + public static class ChildConfig extends ParentConfig {} +}