Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide a variant of ListableBeanFactory.findAnnotationOnBean(String, Class<A>) that does not initialize factory beans #27796

Closed
wilkinsona opened this issue Dec 10, 2021 · 5 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Milestone

Comments

@wilkinsona
Copy link
Member

Affects: 5.3.x

DefaultListableBeanFactory's implementation of findAnnotationOnBean calls getType(String) which allows factory bean init. Assuming that it's intentional for findAnnotationOnBean to initialize factory beans, would it be possible to overload the method with a variant that does not do so? This would help to fix spring-projects/spring-boot#28977 where we're looking for an annotation in a bean factory post-processor.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 10, 2021
@sbrannen sbrannen added in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement labels Dec 10, 2021
@sbrannen sbrannen added this to the Triage Queue milestone Dec 10, 2021
@jhoeller jhoeller removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 10, 2021
@jhoeller jhoeller modified the milestones: Triage Queue, 5.3.14 Dec 10, 2021
@scantor
Copy link

scantor commented Jan 25, 2022

It would have been nice to have a more prominent documentation note that a patch release just broke API compatibility by adding a method to a public interface. This is a blatant violation of semantic versioning, so it also shouldn't have been done at all without a darn good (i.e. security) reason.

@jhoeller
Copy link
Contributor

We generally allow for adding methods to existing interfaces at any point (in a defensive form also in maintenance releases), provided that those interfaces are APIs which are designed as facades for our own implementations - in contrast to SPIs which are meant to be implemented by user code and/or third-party integrations. Even in the case of SPIs, we occasionally introduce new methods that then come declared as default methods, not forcing existing implementors to care for the new method.

We apply semantic versioning rules for source and binary compatibility with existing callers of our APIs, as well as with existing implementors of our SPIs. Not every public interface is the same in that respect. If we enforced 100% strictness even beyond the common intended use (never adding methods to APIs - or e.g. never changing a protected signature in some internal class hierarchy of ours), we would severely constrain our own ability to evolve, in particular in long-lived branches such as 5.3.x.

There are many open source projects treating their public surface selectively, we're not the odd ones out here. If you happened to build some custom ListableBeanFactory implementations or decorators, I apologize for the inconvenience, but even then you could easily declare the new method and still remain compatible with older versions of Spring (since the existence of the new method wouldn't hurt even then, as long as pre-existing types are used as argument types and return types).

@scantor
Copy link

scantor commented Jan 25, 2022

Is there any documentation or formalism regarding which APIs are in which category? I didn't run across it but I will check the documentation again.

@jhoeller
Copy link
Contributor

Not that I'm aware of. Usually the javadoc or simply the common usage model quite clearly suggests usage as an API (that application code is calling only) versus usage as an SPI (interfaces to be implemented by specific extensions/customizations), or the also common notion of callback interfaces (to be commonly implemented in application code as part of the usage model).

Good point, in any case: We should consider introducing some dedicated indications for API versus SPI characteristics. We use interfaces rather broadly for a range of different purposes, after all, and the intended treatment isn't as obvious as it should be.

@scantor
Copy link

scantor commented Jan 25, 2022

It would be appreciated, we can more easily make special note of any we (mis)use to watch for changes more carefully. My project has much more strict policies around versioning than most, so we get bitten a lot by these sorts of changes when we have to update dependencies for security fixes.

That's another side comment. I believe 5.3.14 was a security patch as well, which means it's not so "optional" for us to pick up that version. Maybe consider a stricter review of such changes in releases that address vulnerabilities as a general principle.

I appreciate the feedback and response in any event.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants