Affects: 6.0.11
When I compose my own annotations which all use @ComponentScan
and I use those annotations on an application class, only the first one is used:
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.TYPE})
@ComponentScan(basePackageClasses = ModuleAClient.class)
public @interface ConnectedToModuleA {
}
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.TYPE})
@ComponentScan(basePackageClasses = ModuleBClient.class)
public @interface ConnectedToModuleB {
}
@ConnectedToModuleA // Only the ComponentScan from this annotation is picked
@ConnectedToModuleB // ComponentScan from this annotation is ignored
public class MyApplication {
public static void main(String[] args) {
SpringApplication.run(MyApplication.class, args);
}
}
Comment From: snicoll
Thanks for the report. This looks like a bug to me, and I think org.springframework.context.annotation.AnnotationConfigUtils#attributesForRepeatable
shouldn't call metadata.getAnnotationAttributes
but rather metadata.getAllAnnotationAttributes(ComponentScan.class.getName())
.
Edit: however doing so does not manage overrides so I am not sure if that's such a good idea.
@jhoeller, thoughts?
Comment From: sbrannen
however doing so does not manage overrides so I am not sure if that's such a good idea.
Right. We cannot switch to getAllAnnotationAttributes
as-is, since that would break numerous existing use cases.
However, we might be able to introduce a variant of getAllAnnotationAttributes
that omits the .map(MergedAnnotation::withNonMergedAttributes)
restriction.
If I recall correctly, that restriction was included in order to match the behavior when using the MergedAnnotation
API with the previous behavior, and prior to the MergedAnnotation
API we did not have a way to merge all annotation attributes when using ASM.
Of course, simply removing .map(MergedAnnotation::withNonMergedAttributes)
and processing the resulting MultiValueMap
may not work as needed, but it's worth investigating.
Comment From: sbrannen
I've pushed a "proof of concept" solution to the following feature branch.
https://github.com/spring-projects/spring-framework/compare/main...sbrannen:spring-framework:issues/gh-30941-repeatable-composed-ComponentScan-annotations
It introduces two default
getAllMergedAnnotationAttributes(...)
methods in AnnotatedTypeMetadata
, which allow the use case in this issue to pass.
In addition, the rest of the test suite passes; however, if we decide to introduce these methods we will of course need to introduce additional tests.
@jhoeller and @snicoll, thoughts?
Comment From: sbrannen
We need to keep in mind that @PropertySource
is also affected by this.
If we make the changes I proposed above, the following Javadoc for @PropertySource
may no longer be applicable.
... all such
@PropertySource
annotations need to be declared at the same level: either directly on the configuration class or as meta-annotations on the same custom annotation. Mixing direct annotations and meta-annotations is not recommended since direct annotations will effectively override meta-annotations.
Comment From: sbrannen
We need to keep in mind that
@PropertySource
is also affected by this.
In light of that, I've updated the title of this issue and pushed related tests to the aforementioned feature branch.
Comment From: sbrannen
After further analysis, it turns out that in addition to not finding multiple composed @ComponentScan
or @PropertySource
annotations, Spring also does not find multiple repeated @ComponentScan
or @PropertySource
annotations within an annotation hierarchy.
Both of these bugs stem from the fact that AnnotationConfigUtils.attributesForRepeatable(...)
invokes AnnotatedTypeMetadata.getAnnotationAttributes(...)
for both direct annotations and annotations in a container, and getAnnotationAttributes(...)
only finds the first such annotation.
In light of that, I have revised the title of this issue and opened #31041 to introduce explicit support for finding all repeatable annotations via the AnnotatedTypeMetadata
abstraction.
Comment From: ladzar
Thanks very much for taking the time to fix this issue.
Comment From: sbrannen
Thanks very much for taking the time to fix this issue.
You're welcome!