Mismatch between mechanism to check if nullChannel bean exists and mechanism to fetch that bean

[michaelwiles]

Found it in 5.1.X Though I'm sure it affects other versions as well.

Bug report

consider

registerNullChannel method in org.springframework.integration.config.DefaultConfiguringBeanFactoryPostProcessor

	private void registerNullChannel() {
		if (this.beanFactory.containsBean(IntegrationContextUtils.NULL_CHANNEL_BEAN_NAME)) {
			BeanDefinition nullChannelDefinition = null;
			if (this.beanFactory.containsBeanDefinition(IntegrationContextUtils.NULL_CHANNEL_BEAN_NAME)) {
				nullChannelDefinition =
						this.beanFactory.getBeanDefinition(IntegrationContextUtils.NULL_CHANNEL_BEAN_NAME);
			}
			else {
				BeanDefinitionRegistry parentBeanFactory =
						(BeanDefinitionRegistry) this.beanFactory.getParentBeanFactory();
				if (parentBeanFactory != null) {
					nullChannelDefinition =
							parentBeanFactory.getBeanDefinition(IntegrationContextUtils.NULL_CHANNEL_BEAN_NAME);
				}
			}

			if (nullChannelDefinition != null &&
					!NullChannel.class.getName().equals(nullChannelDefinition.getBeanClassName())) {
				throw new IllegalStateException("The bean name '" + IntegrationContextUtils.NULL_CHANNEL_BEAN_NAME
						+ "' is reserved.");
			}
		}
		else {
			this.registry.registerBeanDefinition(IntegrationContextUtils.NULL_CHANNEL_BEAN_NAME,
					new RootBeanDefinition(NullChannel.class));
		}
	}

So what it does is first ask the beanFactory if the bean exists - this.beanFactory.containsBean

If you look at the code for this method you'll notice that it goes all the way up the spring context heirarchy looking for the method.

And if this method returns true, the bean is requested.

When it does not find it in the current bean factory it only checks the immediate parent factory for the bean - it does not roll all the way up the heirarchy in the way that the containsBean method does.

So when your nullChannel bean is in not in the immediate parent (e.g. it's in the parent of the parent) - this method will fail.

I'll try see if I can create a test case - though I suspect it might be simpler for me to just create a pull request that rolls up the heirarchy looking for the factory that does contain the bean. So call containsBeanDefinition on each parent until a true is returned and then call getBeanDefinition on that beanfactory.

[artembilan]

I think this all really makes sense. We didn't anticipate in the past the long context hierarchy, so that's why we consulted only and immediate parent.
Although having a recursive logic in the beanFactory.containsBean(), I guess we indeed need to support the same for the containsBeanDefinition and getBeanDefinition.

Thank you for spotting that, @michaelwiles !

Feel free to raise a PR with the fix. However it might not be so short because many DefaultConfiguringBeanFactoryPostProcessor methods do exactly the same, therefore has to be addressed as well.

[michaelwiles]

Hmmm... it does in fact look like that in DefaultConfiguringBeanFactoryPostProcessor, registerNullChannel is the only one that extracts the bean via getBeanDefinition and thus the only one that needs the change.

This is because registeNullChannel validates the type of the existing bean - whereas all the other "register" methods do not validate the type of the existing bean.

All the others only check that the bean exists...

[artembilan]

Hm. Good catch!

Well, then all is good and it sounds like you are fully on board with an issue and we are more than happy to review a PR from you on the matter.

Thank you for your investigation!