root
/
spring-framework
mirror of https://github.com/spring-projects/spring-framework.git
1
0
Fork 0
Code Issues Actions 6 Packages Projects Releases Wiki Activity

Ensure Parent ConfigurationClass loaded on overrides 

Previously ConfigurationClassParser could override a nested
@Configuration without consideration of @Bean's defined in parent
classes.

This commit ensures that if the original ConfigurationClass contains
additional bean definitions it is processed again.

Issue: SPR-10546
This commit is contained in:
Rob Winch 2013-05-13 09:55:34 -05:00 committed by unknown
parent 92bbd8103b
commit 940011e233
10 changed files with 447 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -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<String> knownSuperclasses = new LinkedHashSet<String>();
private final Set<ConfigurationClass> configurationClasses =
new LinkedHashSet<ConfigurationClass>();
private final Map<ConfigurationClass,ConfigurationClass> configurationClasses =
new LinkedHashMap<ConfigurationClass,ConfigurationClass>();
private final Stack<PropertySource<?>> propertySources =
new Stack<PropertySource<?>>();
@ -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<String> beanMethodNames = new HashSet<String>();
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<String, Class<? extends BeanDefinitionReader>> 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<ConfigurationClass> getConfigurationClasses() {
return this.configurationClasses;
return this.configurationClasses.keySet();
}
public Stack<PropertySource<?>> getPropertySources() {

33
spring-context/src/test/java/org/springframework/context/annotation/spr10546/ImportedConfig.java Normal file
View File

@ -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";
}
}

34
spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentConfig.java Normal file
View File

@ -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";
}
}

32
spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithComponentScanConfig.java Normal file
View File

@ -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 {
}

31
spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithImportConfig.java Normal file
View File

@ -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 {
}

31
spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithImportResourceConfig.java Normal file
View File

@ -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 {
}

29
spring-context/src/test/java/org/springframework/context/annotation/spr10546/ParentWithParentConfig.java Normal file
View File

@ -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 {
}

149
spring-context/src/test/java/org/springframework/context/annotation/spr10546/Spr10546Tests.java Normal file
View File

@ -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.
*
* <p>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"));
}
}

8
spring-context/src/test/java/org/springframework/context/annotation/spr10546/importedResource.xml Normal file
View File

@ -0,0 +1,8 @@
<?xml version="1.0" encoding="UTF-8"?>
<beans xmlns="http://www.springframework.org/schema/beans"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns:c="http://www.springframework.org/schema/c"
xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans.xsd">
<bean id="myBean" class="java.lang.String" c:_0="myBean"/>
</beans>

34
spring-context/src/test/java/org/springframework/context/annotation/spr10546/scanpackage/AEnclosingConfig.java Normal file
View File

@ -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 {}
}