Canceled @ConditionalOnMissingBean now also checks beans available via BeanFactory

[jkubrynski]

Currently when we've BeanFactory in our configuration and bean annotated with @ConditionalOnMissingBean both are created (ExampleBeanAndFactoryBeanConfiguration). This causes problem with autowiring/getting bean by type - NoUniqueBeanDefinitionException is thrown.

I've added support for such situation (it happens in our project - we've got MongoBeanFactory and MongoAutoConfiguration contains @bean returning Mongo instance.

In fact it's quite big change and causes problems, which I cannot overcome (for example with getting BeanFactory return type without initialization - to prevent eager loading).

Any comments/ideas are welcomed :)

[dsyer]

I think you mean "when we have a FactoryBean in our configuration"? In fact OnBeanCondition already tries to take this into account, but it has to at all cost prevent early instantiation of FactoryBeans (that is a huge problem, as you will have notices). Supposedly if you write your FactoryBean carefully it can be introspected to extract the object type without instantiating it - the generics support in Spring is very good now, so if the FactoryBean is a parametric type that might be enough. If that isn't possible, then you have to write the FactoryBean in such a way that instantiating the factory does not create the object (make it lazy). You will find that many if not all FactoryBeans in Spring already do that (e.g. they create the object in afterPropertiesSet()).

In short, I don't think we can do anything with this pull request. But please try to change your FactoryBean and see if that solves the problem for you.

[jkubrynski]

I'm using provided EmbeddedMongoFactoryBean and the problem is that MongoAutoConfiguration creates a Mongo bean as well as my factory bean. In fact there are two Mongo beans in Spring context. In my opinion it's a bug, because FactoryBean should be considered on the same rules like normal beans. To achieve that you have to check all available bean factories (since there is no method provided by Spring to get only bean factories for desired type?). Of course the best way would be to retrieve generic information and based on it decide it bean should be excluded or not. Is there any option to get Class object associated with bean by name?

[dsyer]

I guess BeanFactory.isTypeMatch() is the closest thing.

Anyway, BeanFactoryUtils.beanNamesForTypeIncludingAncestors() is already used in OnBeanCondition and it is supposed to do the best it can to find an accurate list of names without instantiating them. Your change is only going to duplicate that effort and quite possibly subvert it. I suggest you try some simple experiments with your FactoryBean and BeanFactoryUtils to see how it behaves or could be tweaked to be more friendly.

[jkubrynski]

Hmmm - can you take a look at my test? It shows where the problem is and fails in current code

[dsyer]

I included your test case (@Ignored with a comment) in that last commit. The basic problem is that FactoryBean is a bad idiom for @Configuration, even if we are sometimes forced to use it. If you either use XML for the FactoryBean or make your @Bean return an object of type Mongo (and call getObject() on the FactoryBean in that method), then the @ConditionalOnMissingBean will work as required.

[jkubrynski]

I meant testAnnotationOnMissingBeanConditionWithFactoryBean method - this one with ignore has already been there :)

I know that using FactoryBean is not the best option but currently, for example when using Mongo it works OK with bare Spring and causes problems in Spring Boot. In my opinion FactoryBeans should also be considered when checking beans availability in context.

The whole problem is that when you don't want to allow eager initialization (which of course if great) then you have to get generic type of BeanFactory via reflection. And currently BeanDefinition for such beanfactories doesn't contain bean class information. If I could solve this problem it would be great :)

[dsyer]

Look again. The one that is @Ignored is the one that you added, renamed to testOnMissingBeanConditionWithFactoryBean and using the existing FactoryBean definition instead of creating a new one. The one that was @Ignored is now running (since SPR-11069 is fixed). There is also a new test (testOnMissingBeanConditionWithFactoryBeanInXml) that actually proves that your use case is fine with XML.

[jkubrynski]

Ok - now I see. But I still think that this is a bug

[dsyer]

If every slightly surprising thing about Spring is a bug, then I guess you must be right. Maybe you could open an issue in the SPR JIRA highlighting the XML/Java conflict. There are other issues with FactoryBeans and type determination, e.g. https://jira.springsource.org/browse/SPR-11051 and https://jira.springsource.org/browse/SPR-11202, and I'm sure you can find others by searching JIRA.

[jkubrynski]

Ok, if you want we can close this PR, but I think the problem is deeper. For example using such configuration

    protected static class ExampleBeanAndFactoryBeanConfiguration {

        @Bean
        public ExampleBean exampleBean() {
            return new ExampleBean();
        }

        @Bean
        @ConditionalOnMissingBean(ExampleBean.class)
        public ExampleBean createExampleBean() {
            return new ExampleBean();
        }
    }

causes flickering test due to random order of beans initialization. Of course it could be just my incorrect approach to testing but I don't know if such situation won't happen in real application. If it's only applicable to beans in single @Configuration class then of course I've no problem with this :)

EDIT: In fact I've just solved that issue without instantiating FactoryBean beans. Closing this PR - I'll create a new one to avoid garbage. Thanks for help and explanations Dave :)