Use ExtendedBeanInfo on an as-needed basis only [SPR-9723]

[spring-projects-issues]

Chris Beams opened SPR-9723 and commented

Create an ExtendedBeanInfoFactory that inspects a given bean class, determining whether that bean class is a candidate for EBI's functionality. i.e. does the bean class have non-void returning setter methods?

If so, handle the class and return an ExtendedBeanInfo.

If not, return false from from ExtendedBeanInfo#supports(Class) and have CachedIntrospectionResults fall back to the default Introspector#getBeanInfo.

This will resolve a number of open issues that are essentially regressions due to the introduction of ExtendedBeanInfo. The issues with ExtendedBeanInfo should of course also be fixed, but getting EBI out of the execution path for classes that don't need it means better performance and lower risk of bugs in the future.

An open question is whether the ExtendedBeanInfoFactory should be discovered via the META-INF/spring.beanInfoFactories mechanism, or simply hard-coded. In either case, it will always be picked up. Hard coding it means explicitness, type safety, etc. Discovery via META-INF means that there is a prominent example of the mechanism that others can look to without digging into tests.

---

Issue Links:

• Introduce strategy for BeanInfo creation [SPR-9677] #14311 Introduce strategy for BeanInfo creation ("depends on")
• Support injection against setter methods with non-void return types [SPR-8079] #12734 Support injection against setter methods with non-void return types
• Overhaul non-void JavaBean write method support [SPR-10029] #14663 Overhaul non-void JavaBean write method support

Referenced from: commits spring-attic/spring-framework-issues@114ca5c

[spring-projects-issues]

Chris Beams commented

commit 5bcf68e25a0c5a4e33772cbbd5c8885b7dcfabaf
Author: Chris Beams <cbeams@vmware.com>
Commit: Chris Beams <cbeams@vmware.com>

    Use ExtendedBeanInfo on an as-needed basis only
    
    Prior to this change, CachedIntrospectionResults delegated to
    ExtendedBeanInfo by default in order to inspect JavaBean
    PropertyDescriptor information for bean classes.
    
    Originally introduced with SPR-8079, ExtendedBeanInfo was designed to
    go beyond the capabilities of the default JavaBeans Introspector in
    order to support non-void returning setter methods, principally to
    support use of builder-style APIs within Spring XML. This is a complex
    affair, and the non-trivial logic in ExtendedBeanInfo has led to various
    bugs including regressions for bean classes that do not declare
    non-void returning setters.
    
    This commit takes advantage of the new BeanInfoFactory mechanism
    introduced in SPR-9677 to take ExtendedBeanInfo out of the default code
    path for CachedIntrospectionResults. Now, the new
    ExtendedBeanInfoFactory class will be detected and instantiated (per its
    entry in the META-INF/spring.beanInfoFactories properties file shipped
    with the spring-beans jar). ExtendedBeanInfoFactory#supports is invoked
    for all bean classes in order to determine whether they are candidates
    for ExtendedBeanInfo introspection, i.e. whether they declare non-void
    returning setter methods.
    
    If a class does not declare any such non-standard setter methods (the
    99% case), then CachedIntrospectionResults will fall back to the
    default JavaBeans Introspector. While efforts have been made to fix any
    bugs with ExtendedBeanInfo, this change means that EBI will not pose
    any future risk for bean classes that do not declare non-standard
    setter methods, and also means greater efficiency in general.
    
    Issue: SPR-9723, SPR-9677, SPR-8079

[spring-projects-issues]

Chris Beams commented

A general note to watchers of ExtendedBeanInfo-related issues: #14663 is a major refactoring of ExtendedBeanInfo and overall support for non-void returning setter methods.

If you have submitted a reproduction project with this issue, we have run it through its paces against these new changes, but we would like to ask you to do the same against your actual applications.

Please consider updating your dev or test builds to work against 3.1.4.BUILD-SNAPSHOT and/or 3.2.0.BUILD-SNAPSHOT to verify, and we would appreciate any feedback, even if it's to let us know that all is well. So that we can consolidate feedback, please add your comments to #14663, and mention the original issue(s) that you were watching.

Thanks!

Note also that testing against 3.1.4 is preferable to 3.2.0 because ExtendedBeanInfo is always in the code path in the latter, while in 3.2.0 we've optimized things such that ExtendedBeanInfo is only in play for bean classes that have one or more non-void returning setter methods.

[spring-projects-issues]

Chris Beams commented

From (modified) backport commit to 3.1.x:

commit a3161632dd71920efb769b3a80555e89949b7695
Author: Chris Beams <cbeams@vmware.com>
Commit: Chris Beams <cbeams@vmware.com>

    Backport "Use ExtendedBeanInfo on an as-needed basis only"
    
    Prior to this change, CachedIntrospectionResults delegated to
    ExtendedBeanInfo by default in order to inspect JavaBean
    PropertyDescriptor information for bean classes.
    
    Originally introduced with SPR-8079, ExtendedBeanInfo was designed to
    go beyond the capabilities of the default JavaBeans Introspector in
    order to support non-void returning setter methods, principally to
    support use of builder-style APIs within Spring XML. This is a complex
    affair, and the non-trivial logic in ExtendedBeanInfo has led to various
    bugs including regressions for bean classes that do not declare
    non-void returning setters.
    
    Many of these issues were fixed when overhauling non-void JavaBean write
    method support in SPR-10029, however it appears that some extremely
    subtle issues may still remain. ExtendedBeanInfo was taken out of use
    completely in the 3.2.x line with SPR-9677 and the new BeanInfoFactory
    mechanism. This support was not backported to 3.1.x, however, in the
    interest of stability.
    
    This commit, then, inlines the conditional logic introduced by
    BeanInfoFactory directly into CachedIntrospectionResults. The net effect
    is that ExtendedBeanInfo is out of the code path entirely except for
    bean classes that actually contain 'candidate' methods, i.e. non-void
    and/or static write methods.
    
    This commit also includes the changes made in SPR-10115.
    
    Issue: SPR-9723, SPR-10029, SPR-9677, SPR-8079