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

Remove usage of bean definition instance supplier since not compatible with AOT #33763

Closed
sdeleuze opened this issue Jan 11, 2023 · 5 comments
Closed
Assignees
Labels
theme: aot An issue related to Ahead-of-time processing type: bug A general bug
Milestone

Comments

@sdeleuze
Copy link
Contributor

sdeleuze commented Jan 11, 2023

Bean definitions with instance supplier callbacks can't be supported by design on AOT as explained in spring-projects/spring-framework#29555. Spring Framework is going to document this and throw an exception during AOT processing of such bean via spring-projects/spring-framework#29556 instead of silently ignoring the instance supplier when a default constructor on the bean class is found.

While working on changing that behavior, I noticed a few Spring Boot beans are defining instance suppliers (there may be other ones, this is just on a simple app):

  • org.springframework.boot.context.properties.BoundConfigurationProperties
  • org.springframework.boot.web.server.WebServerFactoryCustomizerBeanPostProcessor
  • org.springframework.boot.web.server.ErrorPageRegistrarBeanPostProcessor
    It currently works because the instance supplier references the bean class default constructor and because Spring Framework silently ignores them, but we would like to report proper error to the user when that's the case.

Could you please update Spring Boot codebase to remove the few usages of bean instance suppliers? I will also check the other project portfolio via the AOT smoke tests and reach the related teams.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 11, 2023
@mhalbritter mhalbritter added type: bug A general bug theme: aot An issue related to Ahead-of-time processing and removed status: waiting-for-triage An issue we've not yet triaged labels Jan 11, 2023
@mhalbritter mhalbritter added this to the 3.0.x milestone Jan 11, 2023
@sdeleuze
Copy link
Contributor Author

org.springframework.boot.context.properties.ConfigurationPropertiesBinder is using a customized instance supplier here, not sure if that's currently handled on AOT side or just silently ignored for now.

@mhalbritter
Copy link
Contributor

mhalbritter commented Jan 17, 2023

I found these places where we use an instance supplier:

  • org.springframework.boot.autoconfigure.webservices.WebServicesAutoConfiguration.WsdlDefinitionBeanFactoryPostProcessor#registerBeans
  • org.springframework.boot.context.properties.BoundConfigurationProperties#register
  • org.springframework.boot.autoconfigure.web.reactive.ReactiveWebServerFactoryAutoConfiguration.BeanPostProcessorsRegistrar#registerSyntheticBeanIfMissing
  • org.springframework.boot.autoconfigure.web.servlet.ServletWebServerFactoryAutoConfiguration.BeanPostProcessorsRegistrar#registerSyntheticBeanIfMissing
  • org.springframework.boot.context.properties.ConfigurationPropertiesBinder#register

And usages which are not used in AOT:

  • org.springframework.boot.autoconfigure.SharedMetadataReaderFactoryContextInitializer.CachingMetadataReaderFactoryPostProcessor#register
  • org.springframework.boot.context.properties.ConfigurationPropertiesBeanRegistrar#createBeanDefinition

Not 100% sure that i found them all.

@mhalbritter
Copy link
Contributor

mhalbritter commented Jan 17, 2023

@mhalbritter mhalbritter added the for: team-meeting An issue we'd like to discuss as a team to make progress label Jan 17, 2023
@sdeleuze
Copy link
Contributor Author

For @ConfigurationProperties, I am not familiar with that part, but maybe this use case is already handled by ConfigurationPropertiesBeanRegistrationAotProcessor so no change is needed on that part? We can maybe do some experiment with a Boot version on top of spring-projects/spring-framework@d9101ab and check whats works and what does not.

@mhalbritter
Copy link
Contributor

mhalbritter commented Jan 18, 2023

Indeed, there are no changes needed in ConfigurationPropertiesBeanRegistrar. The SharedMetadataReaderFactoryContextInitializer doesn't need changes, too.

@mhalbritter mhalbritter removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Jan 18, 2023
@wilkinsona wilkinsona modified the milestones: 3.0.x, 3.0.2 Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: aot An issue related to Ahead-of-time processing type: bug A general bug
Projects
None yet
Development

No branches or pull requests

5 participants