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

Explicit BeanDefinition#targetType is not honoured in AOT scenarios #30689

Closed
snicoll opened this issue Jun 19, 2023 · 2 comments
Closed

Explicit BeanDefinition#targetType is not honoured in AOT scenarios #30689

snicoll opened this issue Jun 19, 2023 · 2 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: bug A general bug
Milestone

Comments

@snicoll
Copy link
Member

snicoll commented Jun 19, 2023

Courtesy of @wilkinsona https://github.com/snicoll-scratches/lost-target-type

To reproduce:

./gradlew bootJar
java -Dspring.aot.enabled=true -jar build/libs/lost-target-type-0.0.1-SNAPSHOT.jar

The use case is registering a bean definition on the fly with the factory method of another bean definition, but perhaps this can be generalized.

The bean definition looks like this:

public static BeanDefinition getTwoBeanDefinition() {
  Class<?> beanType = LostTargetTypeApplication.Two.class;
  RootBeanDefinition beanDefinition = new RootBeanDefinition(beanType);
  beanDefinition.setInstanceSupplier(getTwoInstanceSupplier());
  return beanDefinition;
}

Its instance supplier refers to a method that's going to produce a bean of type One. One and Two are unrelated and the factory method produces an instance that implements them both.

It looks like the bean definition's bean class is ignored in favor of the bean factory method. The following works:

public static BeanDefinition getTwoBeanDefinition() {
  Class<?> beanType = LostTargetTypeApplication.Two.class;
  RootBeanDefinition beanDefinition = new RootBeanDefinition(beanType);
  beanDefinition.setInstanceSupplier(getTwoInstanceSupplier());
  beanDefinition.setTargetType(beanType);
  return beanDefinition;
}

We should review this use case and see whether AOT has to generate a more precise definition or if the context should fallback first on the bean class.

@snicoll snicoll added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 19, 2023
@snicoll snicoll changed the title beanType ignored in favor of factory method Explicit BeanDefinition#targetType is not honoured in AOT scenarios Jun 20, 2023
@snicoll snicoll 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 or decided on labels Jun 20, 2023
@snicoll snicoll self-assigned this Jun 20, 2023
@snicoll snicoll added this to the 6.0.x milestone Jun 20, 2023
@snicoll
Copy link
Member Author

snicoll commented Jun 20, 2023

It turns out that we should do our best to generate the necessary code to restore the target type if it has been explicitly set on the BeanDefinition.

@snicoll
Copy link
Member Author

snicoll commented Jun 21, 2023

So it turns out that the previous arrangement was a bit odd where generated code would set the beanClass attribute for a non-generic resolved type, and the targetType otherwise (i.e. for resolved type with generics).

We're now more consistent, only applying the beanClass if it is set in the original bean definition and setting the targetType consistently as it is suitable for scenarios where the type has been determined upfront. This review of type handling also led to deprecate a constructor that was added for the previous arrangement, see #30704.

@jhoeller jhoeller added the in: core Issues in core modules (aop, beans, core, context, expression) label Jun 21, 2023
@jhoeller jhoeller modified the milestones: 6.0.x, 6.0.11 Jun 21, 2023
mdeinum pushed a commit to mdeinum/spring-framework that referenced this issue Jun 29, 2023
Previously, a bean definition that is optimized AOT could have
different metadata based on whether its resolved type had a generic or
not. This is due to RootBeanDefinition taking either a Class or a
ResolvableType doing fundamentally different things. While the former
sets the bean class which is to little use with an instance supplier,
the latter specifies the target type of the bean.

This commit sets the target type of the bean, using the existing
setter methods that take either a class or a ResolvableType and set the
same attribute consistently.

Closes spring-projectsgh-30689
jzheaux added a commit to spring-projects/spring-security that referenced this issue Jun 29, 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: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants