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.

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

Replace ApplicationContextAotInitializer with an AotApplicationContextInitializer interface #29157

Closed
philwebb opened this issue Sep 15, 2022 · 5 comments
Closed

Replace ApplicationContextAotInitializer with an AotApplicationContextInitializer interface #29157

philwebb opened this issue Sep 15, 2022 · 5 comments
Assignees
@snicoll
Labels
theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Milestone
6.0.0-RC1

Comments

@philwebb
Copy link
Member

philwebb commented Sep 15, 2022 •
edited

Spring Boot recently introduced AotApplicationContextInitializer to aid with AOT testing support. This class replaces the need to call org.springframework.context.aot.ApplicationContextAotInitializer from Spring Boot.

If we're the only consumer, then the Framework class could potentially be dropped.
@philwebb philwebb added the theme: aot An issue related to Ahead-of-time processing label Sep 15, 2022
@philwebb philwebb added the type: task A general task label Sep 15, 2022
@snicoll
Copy link
Member

snicoll commented Sep 15, 2022 •
edited

Thanks for the suggestion. This class is meant to be used by consumers of AOT, Spring Boot being one of them but we anticipate any custom framework built on top of Spring Framework to find it useful as well. I can see that the use in Spring Boot has been replaced by AotApplicationContextInitializer in Spring Boot.

Shouldn't this be the other way around, with AotApplicationContextInitializer being contributed here, rather than being a Spring Boot-specific class? Looking at the code it's not obvious to me why that is.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Sep 15, 2022
@philwebb
Copy link
Member Author

That's an alternative. I wanted to get the test support into Spring Boot without derailing the Spring Framework release. We need a way for the test code to plug in an ApplicationContextInitializer that acts as a signal that the convention based one should not be used. If we're happy for the one here to be a real ApplicationContextInitializer then we could rework things after the next Boot milestone.

@philwebb philwebb removed the status: waiting-for-feedback We need additional information before we can continue label Sep 15, 2022
@snicoll
Copy link
Member

snicoll commented Sep 21, 2022

We need a way for the test code to plug in an ApplicationContextInitializer that acts as a signal that the convention based one should not be used. I

I don't think I've understood that but I believe that general entry points should be provided by framework so that custom framework should build upon them the same way Spring Boot does. I've also created #29181 to move/refactor the current AotProcessor.

@philwebb philwebb changed the title Consider dropping ApplicationContextAotInitializer Replace ApplicationContextAotInitializer with an AotApplicationContextInitializer Sep 29, 2022
@philwebb philwebb changed the title Replace ApplicationContextAotInitializer with an AotApplicationContextInitializer Replace ApplicationContextAotInitializer with an AotApplicationContextInitializer interface Sep 29, 2022
philwebb added a commit that referenced this issue Sep 29, 2022
@philwebb
Replace AotApplicationContextInitializer with interface
ac13d26 
Replace the `ApplicationContextAotInitializer` class with an
`AotApplicationContextInitializer` interface so that its use can be
detected using a simple `instanceof` check. The existing functionality
has been moved to a factory method on the interface allowing:

	`new ApplicationContextAotInitializer()
		.initialize(context, names);`

To now be written as:

	`AotApplicationContextInitializer.forInitializerClasses(names)
		.initialize(context);`

See gh-29157
philwebb added a commit that referenced this issue Sep 29, 2022
@philwebb
Fix factory method return type
192bc1e 
See gh-29157
philwebb added a commit that referenced this issue Sep 29, 2022
@philwebb
Drop getAotInitializers method
86ad29a 
See gh-29157
@philwebb
Copy link
Member Author

I've pushed a new AotApplicationContextInitializer interface and deprecated the existing one. I've also refactored Spring Boot to use it. If you're happy with the new one we can probably delete ApplicationContextAotInitializer before RC1

philwebb added a commit that referenced this issue Sep 29, 2022
@philwebb
Remove unused import
f2568f2 
See gh-29157
@snicoll snicoll added type: enhancement A general enhancement and removed type: task A general task labels Sep 30, 2022
@snicoll snicoll added this to the 6.0.0-RC1 milestone Sep 30, 2022
@snicoll
Copy link
Member

snicoll commented Sep 30, 2022

Cool. I like it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@snicoll snicoll

Labels
theme: aot An issue related to Ahead-of-time processing type: enhancement A general enhancement
Projects
None yet
Milestone
6.0.0-RC1
Development

No branches or pull requests
2 participants
@snicoll @philwebb