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

Doc: Consistent @Profile declarations on overloaded @Bean methods [SPR-15266] #19831

Closed
spring-issuemaster opened this Issue Feb 18, 2017 · 7 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

spring-issuemaster commented Feb 18, 2017

Georg Herdt opened SPR-15266 and commented

When having bean factory methods with the same name but different arguments used for creating instance for different profiles the bean is not recognized for some profiles.
See example below:

@Configuration
public class SomeConfiguration {

    @Bean(name = "something")
    @Profile("A")
    public String valueForTest() {
        return "aaa";
    }

    @Bean
    public String secondValue() {
        return "bbb";
    }

    @Bean(name = "something")
    @Profile("B")
    @Autowired
    public String valueForTest(final String secondValue) {
        return secondValue;
    }
}

Corresponding test will fail:

@RunWith(SpringJUnit4ClassRunner.class)
@ActiveProfiles("A")
@ContextConfiguration(classes = { SomeConfiguration.class })
public class SomeConfigurationTest_ProfileA {

    @Autowired
    @Qualifier("something")
    private String valueForTest;

    @Autowired
    private String secondValue;

    @Test
    public void test() {
        Assert.assertEquals("aaa", valueForTest);
    }

}

When using profile "B" dependencies for bean named "something" will be created successfully. Providing string value "bbb".
For profile "A" it is not working properly.
Behaviour depends on used Spring version. For releases 4.0.0. up to 4.1.4 outcome will be "bbb".
I would expect "aaa".

From release 4.1.5 on (including 5.x.x releases) a NoSuchBeanDefinitionException is thrown.

See the attached gradle project to evaluate the behaviour.


Affects: 4.3.6, 5.0 M4

Attachments:

Issue Links:

  • #17341 Regression in 4.1.5: Alternative @Bean declarations with same primary bean name do not work anymore
  • #17292 Conditions on an overriding bean method effectively get ignored
  • #19074 Deterministic and JVM-independent @Bean registration order within Class-reflected configuration classes

Referenced from: commits 5d3249f, 6c370ed, 022aefd

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Feb 18, 2017

Juergen Hoeller commented

In general, metadata on @Bean methods should be consistent across overloaded methods. We do not take those into account as individual bean definitions but rather just register a single bean definition for that bean, choosing between several overloaded factory methods. Unfortunately, the behavior for inconsistent metadata is not well-defined here.

Also, @Autowired at the method level triggers an invocation of that method in the autowiring phase of the configuration class instance, which is not what you intended here. For autowiring of factory method arguments on construction of such a bean, no extra annotation is needed, analogous to Spring's resolution of overloaded constructors on a bean class.

So the general recommendation is to declare the same @Profile and other metadata on all overloaded factory methods for the same @Bean. We need to document that more clearly. Beyond that, the runtime behavior needs to produce a consistent outcome; I'll see what we can do here for 4.3.7, even if we might only fully address it for 5.0 eventually.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Apr 8, 2017

Juergen Hoeller commented

From a compatibility perspective, we need to leniently allow for conditions to be expressed on the first encounter of a method of a particular name, consistently applying to all factory methods of the same bean (with the same method name). As a consequence, the arrangement above - unintuitively - only registers bean "valueForTest" if the profile on the first processed method (as of 4.3.6 reliably in declaration order) is active, but that's at least consistent in the detection algorithm where we start processing at the concrete class level and make our way up through the class hierarchy, with the first encounter of a particular method overriding later occurrences.

Since it is tricky to enforce consistent declarations in a way that doesn't break existing valid arrangements, I'm rather turning this into a documentation task. As general advice, I'd recommend against overloaded @Bean methods to begin with, rather operating with optional factory method arguments. If there actually need to be overloaded factory methods, profile conditions should be consistently declared on all factory methods of the same name.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Apr 15, 2017

Sam Brannen commented

Juergen Hoeller, just to clarify, are you saying the following common configuration pattern is not actually supported?

@Configuration
public class DataSourceConfig {

    @Bean(name = "dataSource")
    @Profile("prod")
    public DataSource productionDataSource() {
        return // production data source
    }

    @Bean(name = "dataSource")
    @Profile("dev")
    public String developmentDataSource() {
        return // development data source
    }

}
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Apr 15, 2017

Juergen Hoeller commented

Distinct @Bean method names with a reference back to a common bean name work fine. Note that you couldn't even name the methods the same in your example since they both take no arguments; this is a hint at the method overloading problem.

The problem only comes in when the method names are the same: We consider this the very same definition then, just with overloaded "constructors". Any attempt to create that bean is going to resolve all factory methods of the same name in such a scenario, selecting among them depending on the availability of their required arguments. Individual overloaded methods can't be excluded through conditions, just distinctly named methods can (independent from their argument signature).

Aside from the documentation, I'm still trying to improve the error reporting in such a scenario. This turns out to be non-trivial though.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Apr 15, 2017

Sam Brannen commented

Thanks for the detailed clarification!

And yes, I'd recommend that the documentation be very explicit with regard to the two scenarios mentioned above -- in order not to confuse developers about the applicability of the supported scenario that I provided. ;-)

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Apr 15, 2017

Juergen Hoeller commented

Reopened for one more pass over the docs, differentiating between the @Bean-specified bean name scenario and actual overloaded methods of the same name.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

spring-issuemaster commented Apr 17, 2017

Sam Brannen commented

The revisions in your latest commit look much better.

Thanks, Juergen! (y)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.