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

@Bean method that declares @Autowired/@Qualifier use case no longer supported #33990

Closed
harmonic-ben opened this issue Nov 29, 2024 · 3 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: invalid An issue that we don't feel is valid

Comments

@harmonic-ben
Copy link

harmonic-ben commented Nov 29, 2024

Overview

The change to prevent @Autowired on a @Bean method prevents the following use case.

Before I go change all the code to reflect the new limitations I want to make sure this was an intended side effect.

Use Case

Create an annotation like:

@Target({ ElementType.FIELD, ElementType.METHOD, ElementType.PARAMETER})
@Retention(RetentionPolicy.RUNTIME)
@Inherited
@Documented
@Autowired
@Qualifier("transactionManager1") 
public @interface Tx1Manager { //Imagine we have 3 versions of this annotation

}

Use the annotation when setting up @Configuration.

public class FooConfig {
    @Bean
    @Tx1Manager
    PlatformTransactionManager tx1Manager() {
       ///... code omitted
    }
    
    @Bean
    @Tx2Manager
    PlatformTransactionManager tx2Manager() {
       ///... code omitted
    }
}

Use the annotation in a variety of contexts.

public class ConstructorInjected {
   
    public ConstuctorInjected(@Tx1Manager PlatformTransactionManager tx1Manager) {
    }
}

public class FieldInjected {
   @Tx1Manager 
   PlatformTransactionManager tx1Manager;

}

public class SetterInjected {
    PlatformTransactionManager tx1Manager;

    public setTtx1Manager(@Tx1Manager PlatformTransactionManager tx1Manager) {
    }
}

After this change I will remove @Autowired from the annotation class and selectively add to FieldInjected and SetterInjected use cases.

My question is:

Was the change worth it?

Or is it worth undoing the change to save me and others from having to rework our code bases?

I'll refactor everywhere if I need to, but it didn't look like #33051 discussed this side effect.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 29, 2024
@sbrannen sbrannen changed the title Fail fast if a @Bean method declares @Autowired qualifier usecase no longer supported Fail fast if a @Bean method declares @Autowired qualifier usecase no longer supported Nov 29, 2024
@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Nov 29, 2024
@sbrannen sbrannen self-assigned this Nov 29, 2024
@sbrannen sbrannen changed the title Fail fast if a @Bean method declares @Autowired qualifier usecase no longer supported @Bean method that declares @Autowired qualifier use case no longer supported Nov 29, 2024
@sbrannen
Copy link
Member

Hi @harmonic-ben,

@Autowired has never been supported (in production code [1]) on constructor or method parameters. Thus, in both 6.1.x and 6.2, the @Autowired meta-annotation is ignored for the ConstuctorInjected constructor as well as for the setTtx1Manager() method in SetterInjected. Furthermore, the latter actually fails on 6.1.x, since the setTtx1Manager() method itself needs to be annotated with @Autowired (or @Tx1Manager).

In other words, prior to 6.2, @Autowired was only adding value for your autowiring use cases when @Tx1Manager was applied directly on a field, setter method, or constructor.

In addition, meta-annotating @Bean methods with @Autowired results in those methods being invoked twice, which is undesirable.

In light of that, I would generally recommend removing @Autowired from your @Tx*Manager qualifier annotations.

In any case, we will brainstorm within the team and get back to you.

Cheers,

Sam


[1] @Autowired is only supported on constructor and method parameters for integration tests when using the SpringExtension for JUnit Jupiter.

@sbrannen sbrannen changed the title @Bean method that declares @Autowired qualifier use case no longer supported @Bean method that declares @Autowired/@Qualifier use case no longer supported Nov 29, 2024
@harmonic-ben
Copy link
Author

Cool I have enough guidance to know its probably not a great idea and will clean it up in our codebase. Will leave this open in case you decide something different.

@sbrannen
Copy link
Member

sbrannen commented Dec 3, 2024

Cool I have enough guidance to know its probably not a great idea and will clean it up in our codebase. Will leave this open in case you decide something different.

Thanks for the feedback, @harmonic-ben. 👍

In end the end, we have decided to leave things as-is, since the use of @Autowired as a meta-annotation on @Bean methods is semantically incorrect.

In light of that, I'm closing this issue.

Cheers,

Sam

@sbrannen sbrannen closed this as not planned Won't fix, can't repro, duplicate, stale Dec 3, 2024
@sbrannen sbrannen added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Dec 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

3 participants