Improve documentation of how @ConditionalOnBean evaluates all of its attributes

[bcalmac]

Consider the following (simplified) auto-configuration from a starter project:

@Configuration
public class MyStarterConfiguration {

    @Bean
    @ConditionalOnBean(annotation = EnableSomething.class)
    @ConditionalOnMissingBean(Integer.class)
    public Integer myBean() {
        return 10;
    }

    @Bean
    public String myOtherBean(Integer myBean) {
        return myBean.toString();
    }
}

Assuming that the application is annotated with @EnableSomething and no Integer bean is defined, you'd expect that myBean above would be instantiated. Indeed this works fine in 1.5.x. However, in 2.x the auto-configuration fails as below:

The following candidates were found but could not be injected:
	- Bean method 'myBean' in 'MyStarterConfiguration' not loaded because @ConditionalOnBean (types: java.lang.Integer; SearchStrategy: all) did not find any beans of type java.lang.Integer

Instead of just looking for a bean annotated with @EnableSomething, it looks for a bean of type Integer. Is there a different pattern for using @EnableXXX style annotations in 2.x?

Here is a sample project: https://github.com/bcalmac/conditional-on-annotation. mvn test should fail and it would work if you change the parent to 1.5.x.

[bcalmac]

This also fixes it, but I find it ugly:

    @ConditionalOnBean(value = Object.class, annotation = EnableSomething.class)

[philwebb]

I think we could update this line to not attempt to deduce types in annotations have been defined.

I'm curious as to why this works in 1.5 because the code looks very similar. I'm also a little worried that if we make this change it might break existing applications that depend on the behavior.

i.e. given:

@Bean
@ConditionalOnBean(annotation = EnableSomething.class)
public MyBean myBean() {
   ...
}

Is it more reasonable to expect that this means you must have an existing bean of MyBean that has an annotation @EnableSomething, or should it mean any bean with @EnableSomething.

Either way, we should have a test for this and make sure the documentation is clear.

[bcalmac]

Also consider the case when @ConditionalOnBean(annotation = EnableSomething.class) is on a configuration class. That works with the meaning "this annotation on any bean" even in 2.x.

[philwebb]

@bcalmac That one kind of makes sense since there's no way to deduce the bean type (since there's not return).

[bcalmac]

Right. And isn't this a bit inconsistent? Developers would naturally expect to be able to move @ConditionalOn... from class level to method level, depending on whether they want it applied to all beans or just specific beans in that configuration.

Regarding the inference of the bean type, I think the 1.5 logic is most consistent:

• IF the annotation has no attributes as in @ConditionalOnBean THEN this is internally interpreted as @ConditionalOnBean(MyBean.class) based on the return type (if possible, only at method level).
• OTHERWISE no assumptions are made and @ConditionalOnBean(annotation=EnableSomething.class) would not check the bean type because it wasn't requested.

Looking beyond @ConditionalOnBean, would it make sense to add a @ConditionalOnAnnotation annotation, targeted specifically for the @EnableSomething use case?

[philwebb]

Testing this in 1.5 (https://github.com/philwebb/spring-boot/tree/gh-15177), it appears that #5279 is probably responsible for the change in behavior.

[wilkinsona]

When @ConditionalOnBean is used on a @Bean factory method, the javadoc notes that "the bean class defaults to the return type of the factory method".

Taking the example above:

@Bean
@ConditionalOnBean(annotation = EnableSomething.class)
@ConditionalOnMissingBean(Integer.class)
public Integer myBean() {
    return 10;
}

The @ConditionalOnBean annotation is on a @Bean method so, due to the default described above, it's the equivalent of the following:

@Bean
@ConditionalOnBean(value = Integer.class, annotation = EnableSomething.class)
@ConditionalOnMissingBean(Integer.class)
public Integer myBean() {
    return 10;
}

We made a conscious decision in #5279 to switch to a logical AND for @ConditionalOnBean and the behaviour described here is consistent with that. The condition is looking for a bean that is an Integer and that is annotated with @EnableSomething.

I think we could update this line to not attempt to deduce types in annotations have been defined.

I don't think we should do this. Special-casing one of the attributes will make it quite a bit harder to understand and to document how the condition will behave. I think it's easier to document and understand if all of the attributes are ANDed together as they currently are.

@bcalmac If you find the @ConditionalOnBean(value = Object.class, annotation = EnableSomething.class) to be ugly, you can do this instead if you prefer:

@Configuration
@ConditionalOnBean(annotation = EnableSomething.class)
class SomethingEnabledConfiguration {

    @Bean
    @ConditionalOnMissingBean(Integer.class)
    public Integer myBean() {
        return 10;
    }

}

[bcalmac]

To me, the explanation doesn't logically justify the observed change in behavior:

1. Even in 1.5 the bean type would default to the return type. No change there, so I wouldn't expect a change in behavior.

2. Make @ConditionalOnBean with multiple beans an AND rather than an OR #5279 deals with the case when multiple bean types are specified and changes the logic from OR to AND. We're not talking about multiple types here, so again I wouldn't expect a change in behavior.

3. The documentation specifies that there's an OR between bean type and bean name. It doesn't say anything about the annotation, but one would expect to follow the same logic: type OR name OR annotation.

Conditional that only matches when beans of the specified classes and/or with the specified names are already contained in the BeanFactory

Regarding your suggestion to move the annotation at class level, that wouldn't be equivalent. As in my original example, there are other beans in the configuration for which that annotation should not be applied.

[wilkinsona]

Even in 1.5 the bean type would default to the return type. No change there, so I wouldn't expect a change in behavior.

That's true, and that particular aspect of the behaviour has not changed.

#5279 deals with the case when multiple bean types are specified and changes the logic from OR to AND. We're not talking about multiple types here, so again I wouldn't expect a change in behavior.

It's broader than that. The issue's title is perhaps misleading, but if you look at the commit that closed the issue it's switching from a logical OR of all of the annotation's attributes to a logical AND.

The documentation specifies that there's an OR between bean type and bean name

It says "and/or" not "or". Regardless, I think it could be made clearer. What it's really trying to say is that you can specify the name or the type, you don't have to specify both. It isn't making a statement about how all of the attributes are evaluated. We should update the javadoc to add such a statement.

As in my original example, there are other beans in the configuration for which that annotation should not be applied.

That's precisely why I didn't include myOtherBean. The @Configuration class I proposed above was intended as a replacement only for the current myBean definition.

[bcalmac]

Thanks for the clarification. Do you think the combination "type AND annotation" is useful in practice? Any example in a real project?

Would it make sense to have a separate @ConditionalOnAnnotation annotation to cater directly for the @EnableSomething use case?

[bcalmac]

This may need to be updated as well, but the javadoc for annotation clearly specifies that the annotations are checked against beans in the application context, not against the classes specified by the value attribute:

	/**
	 * The annotation type decorating a bean that should be checked. The condition matches
	 * when all of the annotations specified are defined on beans in the
	 * {@link ApplicationContext}.
	 * @return the class-level annotation types to check
	 */
	Class<? extends Annotation>[] annotation() default {};

[wilkinsona]

Would it make sense to have a separate @ConditionalOnAnnotation annotation to cater directly for the @EnableSomething use case?

I don't think so. It's already possible to do what you want with @ConditionalOnBean as shown above and I think it would be a step backwards to deliberately provide two annotations that can achieve the same thing. We'd also end up adding an on missing annotation equivalent which would further increase the amount of duplication.

I think the best solution here is to carefully review and update the javadoc and reference documentation to make the current behaviour clearer.