ConsumerBeanAutoConfiguration needs to be more cautious with ContextRefreshedEvent

[dsyer]

ConsumerBeanAutoConfiguration needs to be more cautious with ContextRefreshedEvent because it will get one from any child contexts that are created in addition to the context it was itself part of. It should check the source of the event, perhaps, and compare it to the current ApplicationContext. Or maybe we need a different trigger for the auto configuration to happen (e.g. it might be appropriate to put that feature in Spring Boot for now).

[jbrisbin]

That was intentional. Isn't that what we want? If you enable Reactor in a parent context, I'd think any child contexts created under that scope would want to share in the Environment created at the top. Otherwise, you'll create multiple Environments, which is not desirable.

[dsyer]

Hmm. It depends on the implementation and usage I guess, but as things stand I think it might be surprising and have nasty side effects if @EnableReactor is in a child context where the parent already has it - the child context would be processed twice, for instance, which might well be idempotent, but it's something that reactor-spring would have to be explicitly aware of.

[jbrisbin]

The idea is to make sure that only one Environment is created. If we add @EnableReactor to a child context, then we should see the reference to the Environment bean in the parent and not create another one in the child, right?

[dsyer]

Environment construction is handled in the ReactorBeanDefinitionRegistrar isn't it, so that's an orthogonal discussion potentially, unless I'm missing something? (BTW it looks to me like a child context will get a new instance of Environment because in the registrar you only check for existence of a bean with a specific name in the current registry, so maybe that's another bug?)

Only the current registry is checked for an existing ConsumerBeanAutoConfiguration as well, so it looks to me like you get one in the child as well as the parent. My original point was that the ContextRefreshedEvent is dispatched to the whole hierarchy, and this will therefore cause reprocessing of the consumers in the parent, unless the child context is somehow prevented from having its own ConsumerBeanAutoConfiguration (maybe it is and I'm just reading the code wrong - it just looks fragile in this way).

[jbrisbin]

Ok...I'll tweak some stuff and add some tests to ensure the environment meets the expectations. 1 Environment bean in the hierarchy but possibly multiple auto configuration beans processing consumers in multiple ApplicationContexts but all sharing the same Environment.

[garyrussell]

How will you avoid different Environment instances in sibling contexts (when there's none in the parent)?

[jbrisbin]

Looking into it a little bit, I'm wondering if I don't need to make the bean registrar a little smarter still and look for beans not just of a given name but interrogate the BeanDefinition and look at the class name as well. There's no type information anywhere, so I can't check subclasses, etc... but I should be able to tell if there's an Environment I can use.

Can I @Autowire the Environment and make it optional from a ImportBeanDefinitionRegistrar? Are those components injected as well? (If it's null, create one, etc...)

[dsyer]

@Autowiring into an @Imported registrar is probably a bad idea - it would be extremely likely to either fail or cause unwanted side effects since it would happen really early in the context lifecycle. It's always better to get stuff lazily if you can (e.g. with BeanFactoryUtils).

[jbrisbin]

Is there even a clean way to ensure that only one bean definition exists in a hierarchy from an @Imported registrar?

[dsyer]

It's a dangerous game if you so it in the imported registry post processor itself. Lazy is best, so your approach if registering a bean definition that checks later before it needs to use it seems useful. ListableBeanFactory.getBeanNamesOfType() (from memory) gets a lot of use.

The neatest way these days is maybe to use @EnableAutoConfiguration to ensure that all user-specified beans are already defined when your @Import happens. In other words, if you would rather leave that feature to Spring Boot, maybe that makes sense - we can play around with that model to see if it makes sense if you like.

[jbrisbin]

What about switching what I'm registering? e.g. the @EnableReactor would register an EnvironmentFactoryBean that lazily provided one by either creating a new one the first time or by re-using a found one?

[dsyer]

That's cunning. I like cunning. You can even generalize that to a FactoryBean and use it everywhere. Remember to use BeanFactoryUtils to do the search for an existing instance.