Skip to content
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

Allow @Qualifier to be used in composed annotations with attribute overrides [SPR-14058] #18630

Closed
spring-projects-issues opened this issue Mar 15, 2016 · 5 comments
Assignees
Labels
in: core status: declined type: enhancement

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Mar 15, 2016

Sam Brannen opened SPR-14058 and commented

Status Quo

@Qualifier currently cannot be used as a meta-annotation in composed annotations with attribute overrides configured via @AliasFor.

The reason is that Spring's official autowiring support does not function properly if @Qualifier is declared as a meta-annotation with the qualifier set to an empty string (e.g., when the user chooses not to specify a qualifier when using the composed annotation).

Impetus

This feature would be beneficial for #18151 & #18628 and is, in general, an enabler for anyone who wishes to use @Qualifier with string-based qualifier values in composed annotations.

Deliverables

  • Allow @Qualifier to be used as a meta-annotation in composed annotations with explicit attribute overrides for string-based qualifiers.

Affects: 4.2 GA

Issue Links:

  • #18020 Use AnnotatedElementUtils instead of AnnotationUtils wherever feasible
  • #18629 Allow @Autowired to be declared on parameters
  • #18628 Introduce autowiring support for individual handler method parameters
  • #18151 Introduce support for JUnit 5 in the TestContext framework

Referenced from: commits 3c18a45, e83e3ec

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 15, 2016

Sam Brannen commented

FYI: I have introduced a failing test case.

See autowiredFieldResolutionIgnoresEmptyQualifierFromComposedQualifierAnnotation() in QualifierAnnotationAutowireContextTests.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 19, 2016

Juergen Hoeller commented

The problem here is that the algorithm identifies such a composed autowiring+qualifier annotation as a custom qualifier annotation, with @Qualifier just as a marker for such a custom qualifier type. As a consequence, it is trying to match the entire custom @SpringBean annotation as a qualifier type and doesn't treat the meta @Qualifier as an independent String-based qualifier.

Aside from the entire algorithm not supporting merged annotations yet, we also have a conceptual conflict between the existing arrangement for custom qualifier types and a to-be-introduced model for custom text-based qualifiers, since both use the same meta annotation marker. We could identify qualifier types which are also marked as autowiring annotations, treating them specifically, but at this point the entire qualifier matching algorithm does not know about autowiring annotations at all... and arguably should preserve that characteristic.

Also, at the moment, a custom qualifier type marked with @Autowired is the only way to have a combined autowiring+qualifier annotation. The difference is that the custom annotation itself is a qualifier type in such a scenario, not just as pass-through to a String-based qualifier. This is a quite nice arrangement in its own right, so we cannot simply handle that case in a special String-qualifier-oriented way. We'd have to identify that the custom annotation actually declares a pass-through attribute for @Qualifier's value and only match in String-based mode then.

So all in all, I wonder whether this is worth supporting in the first place. With method argument autowiring as in JUnit 5, having regular @Autowired and regular @Qualifier annotations as autowiring markers does not require a composed annotation to begin with, or am I missing something? In other words, #18628 and #18151 do not actually depend on this according to our recent discussions?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 19, 2016

Sam Brannen commented

Hi Juergen,

Thanks for the detailed analysis and thorough write-up!

I had also investigated a bit on my own and came to the conclusion that this feature would in the very least require a considerable rewrite of the qualifier lookup algorithm. But based on your input here, it actually seems like it's not even feasible.

Regarding our support for method injection in JUnit 5 tests, your analysis is correct: we already support all necessary use cases via @Autowired, @Qualifier, and @Value. The current implementation of the SpringExtension simply checks for the presence of any of these annotations and then delegates to the BeanFactory for the actual resolution of the dependency. So in that regard, #18628 and #18151 no longer depend on this feature. (I'll update those issues accordingly)

My original goal was to allow users to use a single annotation in JUnit 5 for autowiring dependencies from a Spring ApplicationContext (excluding @Value use cases). This was possible with my initial support for @SpringBean. For example, one could originally declare @SpringBean instead of @Autowired and @SpringBean("myQualifier") instead of @Qualifier("myQualifier"). I just thought it seemed cleaner and perhaps more intuitive with the single annotation approach. However, if it's as difficult as you say it is (or perhaps impossible) to allow @Qualifier to be used in such composed annotations, I would agree that it's not worth supporting.

Long story, short: feel free to resolve this issue as Won't Fix if you deem that appropriate.

Cheers!

Sam

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 19, 2016

Juergen Hoeller commented

Alright, let's resolve it as Won't Fix then.

The real goal is to get rid of the need for qualifiers in the first place :-) Seriously, in particular with generics-based matching and primary markers, there is hardly a need for qualifiers anymore. In recent years, I'm telling people that qualifiers are becoming a niche feature in the Spring world.

Juergen

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Mar 19, 2016

Sam Brannen commented

Agreed: case closed! ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core status: declined type: enhancement
Projects
None yet
Development

No branches or pull requests

2 participants