When BeanDefinitionRegistry.getBeanDefinition()
is called it will throw NoSuchBeanDefinitionException
which will cause failure. To fix this, try-catch was introduced and the null check was removed since the method will never return null.
Relevant to: https://github.com/awspring/spring-cloud-aws/issues/101
Comment From: wilkinsona
Thanks for the PR. This problem has highlighted an interesting mismatch in the current code. We're using the current bean factory and all of its ancestors to get the names of any Validator
beans but then only using the current bean factory when retrieving each bean's definition.
I'm not sure that ignoring any beans that aren't known to the current bean factory is the right thing to do. Instead, I think we should address the mismatch by walking up the bean factory hierarchy using getParentBeanFactory()
until we find the definition. We may also want to use containsLocalBean()
to avoid the cost of an exception being thrown.
What do you think? If you agree, would you like to update your PR to this effect?
Comment From: MatejNedic
I agree that ignoring is not something that should be done. I would be more then happy to update PR! :)
Comment From: wilkinsona
Thank you, @MatejNedic. When you have time, and just in case you don't already know, you can update this PR by pushing another commit to your fix-validator-not-finding-bean-definition
branch or by force-pushing an amendment of the existing commit.
Comment From: wilkinsona
Thank you very much for making your first contribution to Spring Boot, @MatejNedic. While I was adding some tests to ValidationAutoConfigurationTests
, I realised that any parent bean factories are not important when determining if the auto-configured validator needs to be marked as primary. When a bean is being resolved, it's only the beans at the current level of the hierarchy that matter. If there's a single matching bean in the current context, it will be used irrespective of any matching bean, primary or otherwise, in a parent context. This meant that the fix could be simplified by ignoring the ancestor bean factories entirely. If you're interested, I applied that refinement and added a couple of tests in https://github.com/spring-projects/spring-boot/commit/cf0bd0f9590d237c735bb2bc7982b17267e23395.
Comment From: MatejNedic
@wilkinsona, thanks for responding I really appreciate the explanation! Have to say I am happy to learn something new which I did with this PR.