Improve core container exception meta-data [SPR-13968]

[spring-projects-issues]

Stéphane Nicoll opened SPR-13968 and commented

Andy Wilkinson submitted two PRs to improve the meta-data of the core container exception. We should review those and further improve it if necessary. The PRs are #969 and #965

---

Affects: 4.2.4

Issue Links:

• Mis-proxying of Mockito mock and poor diagnostics for type mismatch on proxy injection [SPR-14478] #19047 Mis-proxying of Mockito mock and poor diagnostics for type mismatch on proxy injection
• Documentation for InjectionPoint argument on @Bean method [SPR-14797] #19363 Documentation for InjectionPoint argument on @Bean method
• Unique XML-specified constructor not accepted in case of unresolvable name [SPR-13987] #18559 Unique XML-specified constructor not accepted in case of unresolvable name
• Improve failure metadata that is available from BeanInstantiationException [SPR-14166] #18738 Improve failure metadata that is available from BeanInstantiationException

Referenced from: commits b6f6949, b6dd8a9

[spring-projects-issues]

Andy Wilkinson commented

I have another change that I'd like to see. I haven't submitted a pull request for it, as I'm really not sure how to go about it.

I’d like NoUniqueBeanDefinitionException to provide access to the definitions of the beans rather than only their names. I thought about doing it in Boot and using the names to look up the definitions in the bean factory, but I have no way of knowing what sort of search was done when trying to find the beans (whether or not ancestor factories were consulted). Looking at doing it in the core, adding the definitions to the exception isn’t straightforward. I can only get the definition from a DefaultListableBeanFactory and in a lot of places its a ListableBeanFactory that’s passed around. I could do an instanceof and cast, but then the behaviour will be inconsistent if there’s a custom bean factory involved.

[spring-projects-issues]

Juergen Hoeller commented

Andy Wilkinson, I'm trying to address the intent of your two pull requests in a slightly different way... rolling it into UnsatisfiedDependencyException itself or maybe one custom variant of it, but ideally not separate ones for field/method/constructor. I might change my mind still, but anyway, just a note that I'm on it :-)

Juergen

[spring-projects-issues]

Juergen Hoeller commented

Current outcome: a new UnsatisfiedDependencyException constructor taking an InjectionPoint descriptor, used by ConstructorResolver as well as AutowiredAnnotationBeanPostProcessor, delivering a consistent exception surface for all autowired injection points now, with no unnecessary wrapping anymore. And of course, allowing for programmatic access to the field/method/constructor through the new getInjectionPoint() method on UnsatisfiedDependencyException.

InjectionPoint is a base class of our existing DependencyDescriptor, factoring out the core artifact references. We don't make use of that class hierarchy relationship yet (since we always create our own InjectionPoint instances for exception purposes, just reusing MethodParameter handles), but technically, an UnsatisfiedDependencyException could be created directly from a DependencyDescriptor now.

[spring-projects-issues]

Andy Wilkinson commented

Juergen Hoeller, this looks great. Thank you.

Did you have a chance to consider my comment above, about provided access to the definitions of the beans involved in a NoUniqueBeanDefinitionException. Assuming you think that's something that could and should be made possible, shall I open an issue?

[spring-projects-issues]

Juergen Hoeller commented

Andy Wilkinson, I'm not keen on including entire BeanDefinition objects there since they contain quite a bit of weight... and also come from the beans.factory.config subpackage, so can't be easily linked into a {{beans.factory}-defined exception.

As for resolving such bean names yourself, note that ConfigurableBeanFactory.getMergedBeanDefinition actually traverses the hierarchy for you if necessary. Since beans in a child factory always override same-named beans in an ancestor, bean names can be reliably resolved that way. The only special case you should take into account is that there might not be a backing bean definition at all, e.g. in case of a manually registered singleton instance. So if getMergedBeanDefinition throws a NoSuchBeanDefinitionException, either just ignore it or try to reduce the introspection to BeanFactory.getType (and the like) which is nevertheless going to work.

Juergen

[spring-projects-issues]

Andy Wilkinson commented

Thanks, Juergen Hoeller. I've prototyped something based on getMergedBeanDefinition and it seems to work nicely. The only small fly in the ointment is where I retrieve the bean names from the NoUniqueBeanDefinitionException as there's no getter for them. Would it be possible to add one so that I can avoid doing this:

private String[] extractBeanNames(NoUniqueBeanDefinitionException cause) {
	if (cause.getMessage().indexOf("but found") > -1) {
		return StringUtils.commaDelimitedListToStringArray(cause.getMessage()
				.substring(cause.getMessage().lastIndexOf(":") + 1).trim());
	}
	return null;
}

[spring-projects-issues]

Juergen Hoeller commented

Ah, good catch. I'll add a corresponding getBeanNamesFound accessor for 4.3 then.

Juergen