-
Notifications
You must be signed in to change notification settings - Fork 38.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AnnotationBeanNameGenerator
issues warning about explicitly aliased value
attribute
#34317
Comments
AnnotationBeanNameGenerator
issuing warnings about value()
declaration explicitly aliasingAnnotationBeanNameGenerator
issuing warnings about value()
declaration explicitly aliasing
AnnotationBeanNameGenerator
issuing warnings about value()
declaration explicitly aliasingAnnotationBeanNameGenerator
issues warning about value()
declaration explicitly aliasing
AnnotationBeanNameGenerator
issues warning about value()
declaration explicitly aliasingAnnotationBeanNameGenerator
issues warning about explicitly aliased value
attribute
This raises some interesting questions, so thanks for bringing this to our attention! 👍
We can definitely improve the wording, but before we do that we should decide what to do about the status quo in terms of actual behavior.
Yes and no. Let me explain... With the current behavior, we need such a warning, because that But if we were to no longer use such an explicitly aliased I put together the following to demonstrate the status quo. @Retention(RetentionPolicy.RUNTIME)
@interface SomeAnnotation {
String attribute() default "";
} @Retention(RetentionPolicy.RUNTIME)
@Component
@SomeAnnotation
@interface MyComponent {
@AliasFor(annotation = SomeAnnotation.class, attribute = "attribute")
String value() default "";
} @MyComponent("enigma")
class ConcreteComponent {
} @SpringJUnitConfig
class ComponentNameTests {
@Test
void annotationValues() {
MergedAnnotations mergedAnnotations = MergedAnnotations.from(ConcreteComponent.class);
SomeAnnotation someAnnotation = mergedAnnotations.get(SomeAnnotation.class).synthesize();
Component component = mergedAnnotations.get(Component.class).synthesize();
// @Component(value = "")
assertThat(component.value()).isBlank();
// @MyComponent(value = "enigma") -> @SomeAnnotation(attribute = "enigma")
assertThat(someAnnotation.attribute()).isEqualTo("enigma");
}
@Test
void beanName(ApplicationContext context) {
// "enigma" comes from @MyComponent("enigma");
// whereas, AnnotationBeanNameGenerator would generate "concreteComponent".
assertThat(context.getBeanNamesForType(ConcreteComponent.class))
.containsExactly("enigma");
}
@Configuration
@ComponentScan(includeFilters =
@Filter(type = FilterType.ASSIGNABLE_TYPE, classes = ConcreteComponent.class))
static class Config {
}
} The above tests pass which means that the Thus, with the status quo, that log message is intended to warn you that Historically, a Now that we've clarified the status quo, let's look at our options.
We can do We could consider doing We could consider doing Note, however, that If anyone would like to share their thoughts on the matter, feel free to do so here. In any case, we will discuss this within the Framework team. |
UpdateFor Spring Framework 6.2.x and 6.1.x, we will use this GitHub issue to improve the log message to clarify the following different scenarios (effectively option
For Spring Framework 7 (see #34346), we will stop using a |
In a local POC, the following warning is now generated for the
@odrotbohm and @jhoeller, WDYT? |
…name Prior to this commit, if a String 'value' attribute of an annotation was annotated with @AliasFor and explicitly configured to alias an attribute other than @Component.value, the value was still used as the @Component name, but the warning message that was logged stated that the 'value' attribute should be annotated with @AliasFor(annotation=Component.class). However, it is not possible to annotate an annotation attribute twice with @AliasFor. To address that, this commit revises the logic in AnnotationBeanNameGenerator so that it issues a log message similar to the following in such scenarios. WARN o.s.c.a.AnnotationBeanNameGenerator - Although the 'value' attribute in @example.MyStereotype declares @AliasFor for an attribute other than @Component's 'value' attribute, the value is still used as the @Component name based on convention. As of Spring Framework 7.0, such a 'value' attribute will no longer be used as the @Component name. See gh-34346 Closes gh-34317 (cherry picked from commit 17a94fb)
Prior to this commit, if a custom stereotype annotation was meta-annotated with @Component and declared a local String `value` attribute that was explicitly configured (via @AliasFor) as an override for an attribute other than @Component.value, the local `value` attribute was still used as a convention-based override for @Component.value. Consequently, a local `value` attribute was used as a custom @Component name, even when that is clearly not the intent. To address that, this commit revises the logic in AnnotationBeanNameGenerator so that a `value` attribute which is explicitly aliased to something other than @Component.value is no longer used as an explicit @Component name. See gh-34317 Closes gh-34346
Assume an annotation that declares a
value()
attribute explicitly aliasing an attribute other than@Component(value = "…")
.If that annotation gets used with a set value (
@MyAnnotation("foo")
), I now see a warning:This warning should not be issued if the
value()
attribute is explicitly aliased to some attribute other than@Component
'svalue
.Also, the phrasing of "convention-based stereotype names" in combination with
@Component
led me to believe it's about the bean name stereotypically being derived from the class name (which made it particularly puzzling as the warning only appears with a value set, which would not trigger that convention). It turns out this term is actually referring to the default aliasing convention of the annotation attributes (the@MyAnnotation.value()
conventionally being considered a redeclaration of@Component.value()
). I wonder if the message's wording could be adapted to clarify this.The text was updated successfully, but these errors were encountered: