Browse Source

Allow @Configuration classes to self-@ComponentScan

Prior to this change, a @Configuration classes that @ComponentScan
themselves would result in a ConflictingBeanDefinitionException.

For example:

    package com.foo.config;

    @Configuration
    @ComponentScan("com.foo");
    public class AppConfig {
        // ...
    }

This resulted in a ConflictingBeanDefinitionException that users have
typically worked around in the following fashion:

    package com.foo.config;

    @Configuration
    @ComponentScan(basePackages="com.foo",
        excludeFilters=@Filter(value=ANNOTATION_TYPE, type=Configuration.class);
    public class AppConfig {
        // ...
    }

This is obviously more verbose and cumbersome than would be desirable,
and furthermore potentially too constraining as it prohibits the ability
to include other legitimate @Configuration classes via scanning.

The exception was being thrown because of a logic problem in
ClassPathBeanDefinitionScanner.  The bean definition for AppConfig gets
registered once by the user (e.g. when constructing an
AnnotationConfigApplicationContext), then again when performing the
component scan for 'com.foo'. Prior to this change,
ClassPathBeanDefinitionScanner's #isCompatible returned false if the new
bean definition was anything other than an AnnotatedBeanDefinition.  The
intention of this check is really to see whether the new bean definition
is a *scanned* bean definition, i.e. the result of a component-scanning
operation.  If so, then it becomes safe to assume that the original bean
definition is the one that should be kept, as it is the one explicitly
registered by the user.

Therefore, the fix is as simple as narrowing the instanceof check from
AnnotatedBeanDefinition to its ScannedGenericBeanDefinition subtype.

Note that this commit partially reverts changes introduced in SPR-8307
that explicitly caught ConflictingBeanDefinitionExceptions when
processing recursive @ComponentScan definitions, and rethrew as a
"CircularComponentScanException.  With the changes in this commit,
such CBDEs will no longer occur, obviating the need for this check and
for this custom exception type altogether.

Issue: SPR-8808, SPR-8307
pull/7/head
Chris Beams 13 years ago
parent
commit
6991cd9cdf
  1. 32
      org.springframework.context/src/main/java/org/springframework/context/annotation/CircularComponentScanException.java
  2. 2
      org.springframework.context/src/main/java/org/springframework/context/annotation/ClassPathBeanDefinitionScanner.java
  3. 8
      org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java
  4. 9
      org.springframework.context/src/test/java/org/springframework/context/annotation/ComponentScanAnnotationRecursionTests.java
  5. 49
      org.springframework.context/src/test/java/org/springframework/context/annotation/spr8808/Spr8808Tests.java

32
org.springframework.context/src/main/java/org/springframework/context/annotation/CircularComponentScanException.java

@ -1,32 +0,0 @@ @@ -1,32 +0,0 @@
/*
* Copyright 2002-2011 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;
/**
* Exception thrown upon detection of circular {@link ComponentScan} use.
*
* @author Chris Beams
* @since 3.1
*/
@SuppressWarnings("serial")
class CircularComponentScanException extends IllegalStateException {
public CircularComponentScanException(String message, Exception cause) {
super(message, cause);
}
}

2
org.springframework.context/src/main/java/org/springframework/context/annotation/ClassPathBeanDefinitionScanner.java

@ -326,7 +326,7 @@ public class ClassPathBeanDefinitionScanner extends ClassPathScanningCandidateCo @@ -326,7 +326,7 @@ public class ClassPathBeanDefinitionScanner extends ClassPathScanningCandidateCo
* new definition to be skipped in favor of the existing definition
*/
protected boolean isCompatible(BeanDefinition newDefinition, BeanDefinition existingDefinition) {
return (!(existingDefinition instanceof AnnotatedBeanDefinition) || // explicitly registered overriding bean
return (!(existingDefinition instanceof ScannedGenericBeanDefinition) || // explicitly registered overriding bean
newDefinition.getSource().equals(existingDefinition.getSource()) || // scanned same file twice
newDefinition.equals(existingDefinition)); // scanned equivalent class twice
}

8
org.springframework.context/src/main/java/org/springframework/context/annotation/ConfigurationClassParser.java

@ -197,13 +197,7 @@ class ConfigurationClassParser { @@ -197,13 +197,7 @@ class ConfigurationClassParser {
// check the set of scanned definitions for any further config classes and parse recursively if necessary
for (BeanDefinitionHolder holder : scannedBeanDefinitions) {
if (ConfigurationClassUtils.checkConfigurationClassCandidate(holder.getBeanDefinition(), metadataReaderFactory)) {
try {
this.parse(holder.getBeanDefinition().getBeanClassName(), holder.getBeanName());
} catch (ConflictingBeanDefinitionException ex) {
throw new CircularComponentScanException(
"A conflicting bean definition was detected while processing @ComponentScan annotations. " +
"This usually indicates a circle between scanned packages.", ex);
}
this.parse(holder.getBeanDefinition().getBeanClassName(), holder.getBeanName());
}
}
}

9
org.springframework.context/src/test/java/org/springframework/context/annotation/ComponentScanAnnotationRecursionTests.java

@ -26,7 +26,7 @@ import org.springframework.context.annotation.componentscan.level2.Level2Config; @@ -26,7 +26,7 @@ import org.springframework.context.annotation.componentscan.level2.Level2Config;
import org.springframework.context.annotation.componentscan.level3.Level3Component;
/**
* Tests ensuring that configuration clasess marked with @ComponentScan
* Tests ensuring that configuration classes marked with @ComponentScan
* may be processed recursively
*
* @author Chris Beams
@ -50,11 +50,12 @@ public class ComponentScanAnnotationRecursionTests { @@ -50,11 +50,12 @@ public class ComponentScanAnnotationRecursionTests {
assertThat(ctx.getBean("level2Bean"), sameInstance(ctx.getBean("level2Bean")));
}
@Test(expected=CircularComponentScanException.class)
public void cycleDetection() {
public void evenCircularScansAreSupported() {
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
ctx.register(LeftConfig.class);
ctx.register(LeftConfig.class); // left scans right, and right scans left
ctx.refresh();
ctx.getBean("leftConfig"); // but this is handled gracefully
ctx.getBean("rightConfig"); // and beans from both packages are available
}
}

49
org.springframework.context/src/test/java/org/springframework/context/annotation/spr8808/Spr8808Tests.java

@ -0,0 +1,49 @@ @@ -0,0 +1,49 @@
/*
* Copyright 2002-2011 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.spr8808;
import org.junit.Test;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.ComponentScan;
import org.springframework.context.annotation.Configuration;
/**
* Tests cornering the bug in which @Configuration classes that @ComponentScan themselves
* would result in a ConflictingBeanDefinitionException.
*
* @author Chris Beams
* @since 3.1
*/
public class Spr8808Tests {
/**
* This test failed with ConflictingBeanDefinitionException prior to fixes for
* SPR-8808.
*/
@Test
public void repro() {
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
ctx.register(Config.class);
ctx.refresh();
}
}
@Configuration
@ComponentScan(basePackageClasses=Spr8808Tests.class) // scan *this* package
class Config {
}
Loading…
Cancel
Save