Skip to content

Deprecate SpringFactoriesLoader#loadFactoryNames #27954

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.

Sign up for GitHub

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

Jump to bottom
Closed
bclozel opened this issue Jan 19, 2022 · 8 comments
Closed

Deprecate SpringFactoriesLoader#loadFactoryNames #27954

bclozel opened this issue Jan 19, 2022 · 8 comments
Assignees
@snicoll @jhoeller
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Milestone
6.0.0-M6

Comments

@bclozel
Copy link
Member

bclozel commented Jan 19, 2022 •
edited by philwebb
Loading

As seen in #27753, the SpringFactoriesLoader#loadFactoryNames (introduced in #15158 ) has been very useful over the years but is too permissive for our AOT and native work. Some existing use cases can use reflection in a way that's not predictable for AOT processors.

Here are three example use cases in Spring Boot:

Instead of Strings, the replacement should return a different type that allows for the actual type inspection and a Supplier to get an actual instance. Maybe something similar to ServiceLoader.Provider. This proposal will probably not solve the two use cases mentioned above, so we'll need to find proper replacements in Spring Boot or provide new features in Spring Framework to support them.
@bclozel bclozel added in: core Issues in core modules (aop, beans, core, context, expression) status: pending-design-work Needs design work before any code can be developed theme: aot An issue related to Ahead-of-time processing labels Jan 19, 2022
@bclozel bclozel added this to the 6.0.x milestone Jan 19, 2022
This was referenced Jan 19, 2022
Closed
Closed
@jhoeller jhoeller self-assigned this Jan 24, 2022
@jhoeller jhoeller modified the milestones: 6.0.x, 6.0.0-M3 Jan 24, 2022
@philwebb philwebb changed the title Deprecate SpringFactoriesLoader#loadFactoryNames Deprecate SpringFactoriesLoader#loadFactoryNames and provide replacement methods Feb 15, 2022
@philwebb philwebb mentioned this issue Feb 16, 2022
Closed
philwebb added a commit to philwebb/spring-framework that referenced this issue Feb 16, 2022
@philwebb @mbhave @wilkinsona
Allow flexible constructor arguments in factory implementations
0419528 
Update `SpringFactoriesLoader` so that factory implementation classes
can have a constructor with arguments that are resolved dynamically.

Arguments are resolved using a `ArgumentResolver` interface that is
passed to the `loadFactories` method. This strategy interface is
intentionally simple and only allows resolution based on the argument
type. A number of convenience methods are provided to allow resolvers
to be built. For example:

	ArgumentResolver.of(String.class, "tests")
			.and(Integer.class, 123);

Factory implementation classes must have a non-ambiguous constructor
in order to be instantiated. The `SpringFactoriesLoader` uses the same
algorithm as `BeanUtils.getResolvableConstructor`.

See spring-projectsgh-27954

Co-authored-by: Madhura Bhave <[email protected]>
Co-authored-by: Andy Wilkinson <[email protected]>
philwebb added a commit to philwebb/spring-framework that referenced this issue Feb 16, 2022
@philwebb @mbhave @wilkinsona
Provide more control over factory failure handling
db51ea0 
Add an additional `FactoryInstantiationFailureHandler` strategy
interface to `SpringFactoriesLoader` to allows instantiation
failures to be handled on a per-factory bases.

For example, to log trace messages for only factories that can't
be created the following can be used:

	FactoryInstantiationFailureHandler.logging(logger);

If no `FactoryInstantiationFailureHandler` instance is supplied
then `FactoryInstantiationFailureHandler.throwing()` is used
which provides back-compatible behavior by throwing an
`IllegalArgumentException`.

See spring-projectsgh-27954

Co-authored-by: Madhura Bhave <[email protected]>
Co-authored-by: Andy Wilkinson <[email protected]>
@snicoll snicoll added status: superseded An issue that has been superseded by another and removed status: pending-design-work Needs design work before any code can be developed labels Mar 15, 2022
@snicoll snicoll removed this from the 6.0.0-M3 milestone Mar 15, 2022
@snicoll
Copy link
Member

snicoll commented Mar 15, 2022

Actually the related issue doesn't deprecate the method so let's reopen this one.

@snicoll snicoll reopened this Mar 15, 2022
@snicoll snicoll self-assigned this Mar 15, 2022
@snicoll snicoll added this to the 6.0.0-M3 milestone Mar 15, 2022
@snicoll snicoll added type: enhancement A general enhancement and removed status: superseded An issue that has been superseded by another labels Mar 15, 2022
@snicoll snicoll changed the title Deprecate SpringFactoriesLoader#loadFactoryNames and provide replacement methods Deprecate SpringFactoriesLoader#loadFactoryNames Mar 15, 2022
@snicoll snicoll modified the milestones: 6.0.0-M3, 6.0.0-M4 Mar 16, 2022
@snicoll
Copy link
Member

snicoll commented Mar 16, 2022

Moving to the next milestone since we just introduce the infrastructure and we'd need to give some time for team to use it before deprecating loadFactoryByNames.

@jhoeller jhoeller modified the milestones: 6.0.0-M4, 6.0.0-M5 May 11, 2022
@snicoll
Copy link
Member

snicoll commented Jun 8, 2022 •
edited
Loading

"Blocked" by spring-projects/spring-boot#29699

@sbrannen
Copy link
Member

If we deprecate loadFactoryNames(), we should keep in mind that it is still actively used in spring-test in AbstractTestContextBootstrapper.getDefaultTestExecutionListenerClassNames().

getDefaultTestExecutionListenerClassNames() and getDefaultTestExecutionListenerClasses() were originally created to support custom exception handling since there was no equivalent of SpringFactoriesLoader.FailureHandler at the time.

Unfortunately, both of these methods are protected. Thus, a custom subclass of AbstractTestContextBootstrapper may rely on them.

@snicoll
Copy link
Member

snicoll commented Jun 20, 2022

Can you please create an issue to get rid of its use? 6.0 is the right time to break this contract if we have to.

@snicoll snicoll added the status: blocked An issue that's blocked on an external project change label Jun 20, 2022
@snicoll snicoll modified the milestones: 6.0.0-M5, 6.0.0-M6 Jun 20, 2022
@sbrannen sbrannen mentioned this issue Jun 20, 2022
Closed
@sbrannen
Copy link
Member

Can you please create an issue to get rid of its use? 6.0 is the right time to break this contract if we have to.

See:

sbrannen added a commit that referenced this issue Jun 24, 2022
@sbrannen
Stop using SpringFactoriesLoader.loadFactoryNames() in spring-test
d1b65f6 
Since SpringFactoriesLoader.loadFactoryNames() will be deprecated in
gh-27954, this commit removes the use of it in the spring-test module.

Specifically, this commit removes the protected
getDefaultTestExecutionListenerClasses() and
getDefaultTestExecutionListenerClassNames() methods from
AbstractTestContextBootstrapper and replaces them with a new protected
getDefaultTestExecutionListeners() method that makes use of new APIs
introduced in SpringFactoriesLoader for 6.0.

Third-party subclasses of AbstractTestContextBootstrapper that have
overridden or used getDefaultTestExecutionListenerClasses() or
getDefaultTestExecutionListenerClassNames() will therefore need to
migrate to getDefaultTestExecutionListeners() in Spring Framework 6.0.

Closes gh-28666
@sbrannen
Copy link
Member

sbrannen commented Jun 24, 2022 •
edited
Loading

This issue is no longer blocked by #28666.

Though I suppose it's still blocked by spring-projects/spring-boot#29699.

@sbrannen sbrannen added status: blocked An issue that's blocked on an external project change and removed status: blocked An issue that's blocked on an external project change labels Jun 24, 2022
@snicoll snicoll removed the status: blocked An issue that's blocked on an external project change label Jul 27, 2022
snicoll added a commit to spring-projects/spring-boot that referenced this issue Jul 27, 2022
@snicoll
Adapt to deprecations in Spring Framework
fdb1ee5 
See spring-projects/spring-framework#27954
See gh-29699
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@snicoll snicoll

@jhoeller jhoeller

Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Projects
None yet
Milestone
6.0.0-M6
Development

No branches or pull requests
4 participants
@bclozel @sbrannen @snicoll @jhoeller