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

AOT-generated code no longer sets bean class for beans created from a @Bean method #31242

Closed
itineric opened this issue Sep 15, 2023 · 8 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: aot An issue related to Ahead-of-time processing type: regression A bug that is also a regression
Milestone

Comments

@itineric
Copy link

itineric commented Sep 15, 2023

When using spring native with AOT processing, source generation for bean with version 6.0.10 produces:

  public static BeanDefinition getMyBeanBeanDefinition() {
    Class<?> beanType = MyBean.class;
    RootBeanDefinition beanDefinition = new RootBeanDefinition(beanType);
    beanDefinition.setInstanceSupplier(getMyBeanInstanceSupplier());
    return beanDefinition;
  }

Using version 6.0.11 produces:

  public static BeanDefinition getMyBeanBeanDefinition() {
    RootBeanDefinition beanDefinition = new RootBeanDefinition();
    beanDefinition.setTargetType(MyBean.class);
    beanDefinition.setInstanceSupplier(getMyBeanInstanceSupplier());
    return beanDefinition;
  }

Then, when entering a postProcessAfterInitialization method of a BeanFactoryPostProcessor, the bean definition seems uncomplete using 6.0.11. Did not figured out why yet, but stuff working with aot and native for month stops working when using 6.0.11 of spring-boot.

I used generated sources from an AOT processing performed with 6.0.10 to produce an artifact with version 6.0.11, These 2 lines make the behaviour change.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 15, 2023
@bclozel bclozel transferred this issue from spring-projects/spring-boot Sep 15, 2023
@snicoll
Copy link
Member

snicoll commented Sep 15, 2023

Thanks for the report. I am afraid we will need a bit more detail, as the two definitions above should be identical.

Please share a small sample that we can use to reproduce the problem. You can share it with us by pushing it to a separate GitHub repository.

@snicoll snicoll added status: waiting-for-feedback We need additional information before we can continue theme: aot An issue related to Ahead-of-time processing labels Sep 15, 2023
@itineric
Copy link
Author

itineric commented Sep 16, 2023

The repository: https://github.com/itineric/spring-framework-issue-31242

Started with the Hello world to reproduce it easilly.

Expected output is:

INFO 1609667 --- [           main] c.itineric.MyBeanFactoryPostProcessor    : name: myBean
INFO 1609667 --- [           main] c.itineric.MyBeanFactoryPostProcessor    : getBeanClassName: com.itineric.MyBean

And it is working with following dependency (and previous versions):

<dependency>
  <groupId>org.springframework.boot</groupId>
  <artifactId>spring-boot-starter-web</artifactId>
  <version>3.1.1</version>
</dependency>

When switching to version 3.1.2

<dependency>
  <groupId>org.springframework.boot</groupId>
  <artifactId>spring-boot-starter-web</artifactId>
  <version>3.1.2</version>
</dependency>

output changes to (bean class name is now null)

INFO 1611385 --- [           main] c.itineric.MyBeanFactoryPostProcessor    : name: myBean
INFO 1611385 --- [           main] c.itineric.MyBeanFactoryPostProcessor    : getBeanClassName: null

More precisely, the fault is with the spring-boot version that switches from 6.0.10 to 6.0.11.

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

snicoll commented Sep 17, 2023

Thanks for the feedback. I've edited your comment to remove what you've already stated in the original description.

We've now established that a BeanFactoryPostProcessor is expecting beanClassName to be set and that is not correct. If you had a bean with a generic type with 3.1.1 the generated code would only set the resolvable type, not the bean class so you would end up in the same situation. In general, there are other situations outside of AOT where this field might not have been set so expecting all bean definitions to have it is not correct I am afraid.

If you need to access the beanClass, beanDefinition.getResolvableType().toClass() would give you that information.

Can you share a bit more about the actual BeanFactoryPostProcessor? Is it your own code or a third party? Why does it need the class name?

@jhoeller wondering if we could change things a bit in RBD to set the beanClassName as well if it is null or is it weird?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Sep 17, 2023
@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 17, 2023
@itineric
Copy link
Author

itineric commented Sep 18, 2023

Something odd related to this: the AOT generated code is not the same when the @Bean is retrieved from a method instance a @Configuration class and when it is a @Component.

With @Bean on a method myBean():

public static BeanDefinition getMyBeanBeanDefinition() {
    RootBeanDefinition beanDefinition = new RootBeanDefinition();
    beanDefinition.setTargetType(MyBean.class);
    beanDefinition.setInstanceSupplier(getMyBeanInstanceSupplier());
    return beanDefinition;
  }

With @Component on MyBean class:

public static BeanDefinition getMyBeanBeanDefinition() {
    RootBeanDefinition beanDefinition = new RootBeanDefinition(MyBean.class);
    beanDefinition.setInstanceSupplier(MyBean::new);
    return beanDefinition;
  }

@snicoll

This comment was marked as outdated.

@snicoll
Copy link
Member

snicoll commented Sep 18, 2023

OK now that I've actually looked at the code, I can see what's happening. Regarding:

  public static BeanDefinition getMyBeanBeanDefinition() {
    RootBeanDefinition beanDefinition = new RootBeanDefinition();
    beanDefinition.setTargetType(MyBean.class);
    beanDefinition.setInstanceSupplier(getMyBeanInstanceSupplier());
    return beanDefinition;
  }

This is generated if the bean definition we're inspecting actually does not have a beanClass. So this new RootBeanDefinition() means that MyBean.class has no beanClass associated with it. Ironically, we were setting it previously and that was working for you.

I think the thing that's checking for the beanClass might be missing a case. I'll have a look.

@snicoll snicoll changed the title AOT breaking change between 6.0.10 and 6.0.11 AOT-generated code no longer set bean class for beans created from a @Bean method Sep 18, 2023
@snicoll snicoll added type: bug A general bug type: regression A bug that is also a regression and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided type: bug A general bug labels Sep 18, 2023
@snicoll snicoll added this to the 6.0.13 milestone Sep 18, 2023
@snicoll snicoll self-assigned this Sep 18, 2023
@snicoll
Copy link
Member

snicoll commented Sep 18, 2023

It isn't, really. beans produced by a @Bean method don't have a bean class. They don't have a beanClassName either. They do have a getFactoryBeanName() however but that is no longer relevant once AOT has processed the bean as it is no longer produced that way.

We do restore the factoryMethodToIntrospect though, as we need to be able to collect the annotations on the original factory method if necessary. But not the metadata about the factoryMethodBeanName as we don't need it.

I can't say enough about RESTEasy but I think the code there is a bit fragile and should probably be updated.

snicoll added a commit to snicoll/spring-framework that referenced this issue Sep 18, 2023
@jhoeller jhoeller added the in: core Issues in core modules (aop, beans, core, context, expression) label Sep 29, 2023
@sbrannen sbrannen changed the title AOT-generated code no longer set bean class for beans created from a @Bean method AOT-generated code no longer sets bean class for beans created from a @Bean method Oct 12, 2023
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) theme: aot An issue related to Ahead-of-time processing type: regression A bug that is also a regression
Projects
None yet
Development

No branches or pull requests

5 participants