Skip to content
This repository has been archived by the owner on Feb 23, 2023. It is now read-only.

Add suport for ManagedList and ManagedSet #1483

Closed
abuijze opened this issue Feb 7, 2022 · 11 comments
Closed

Add suport for ManagedList and ManagedSet #1483

abuijze opened this issue Feb 7, 2022 · 11 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@abuijze
Copy link

abuijze commented Feb 7, 2022

In one of my beans, I rely on a list of other bean instances to be injected. The items in the list can be of any type, so it's practically defined as a List<Object>.
A BeanDefinitionRegistryPostProcessor generates the BeanDefinition for that bean. I use a ManageList containing RuntimeBeanNameReferences (but tried other BeanReference implementations too) to refer to the beans that I need to be injected.

Without Spring AOT, this works perfectly. The injected value is indeed a list of actual bean instances.
When running Spring AOT, however, the injected value is a List of RuntimeBeanNameReference.

In the ContextBootstrapInitializer, the AOT plugin generates the following code for the bean:

    BeanDefinitionRegistrar.of("MessageHandlerConfigurer$$Axon", MessageHandlerConfigurer.class).withConstructor(List.class)
        .instanceSupplier((instanceContext) -> instanceContext.create(beanFactory, (attributes) -> new MessageHandlerConfigurer(attributes.get(0)))).customize((bd) -> bd.getConstructorArgumentValues().addIndexedArgumentValue(0, List.of(new RuntimeBeanReference("myEventHandler"), new RuntimeBeanReference("mySecondEventHandler")))).register(beanFactory);

The relevant part:

.customize((bd) -> bd.getConstructorArgumentValues()
                     .addIndexedArgumentValue(0, 
                                              List.of(new RuntimeBeanReference("myEventHandler"), 
                                                      new RuntimeBeanReference("mySecondEventHandler"))))

When I change that manually to

    BeanDefinitionRegistrar.of("MessageHandlerConfigurer$$Axon", MessageHandlerConfigurer.class).withConstructor(List.class)
                           .instanceSupplier((instanceContext) -> instanceContext.create(beanFactory, (attributes) -> new MessageHandlerConfigurer(attributes.get(0))))
                           .customize((bd) -> {
                             var list = new ManagedList<>();
                             list.addAll(List.of(new RuntimeBeanReference("myEventHandler"), new RuntimeBeanReference("mySecondEventHandler")));
                             bd.getConstructorArgumentValues().addIndexedArgumentValue(0, list);
                                                        }).register(beanFactory);

Everything works as expected.

Do I need to configure something differently, or should the generated ContextBootstrapInitializer use a ManageList as a container for collections that contain a BeanReference?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 7, 2022
@snicoll
Copy link
Contributor

snicoll commented Feb 8, 2022

@abuijze thanks for the report and the sample. AOT doesn't support ManagedList yet, I'll see what I can do.

@snicoll snicoll added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 8, 2022
@snicoll snicoll added this to the 0.11.3 milestone Feb 8, 2022
@snicoll snicoll self-assigned this Feb 8, 2022
@snicoll snicoll changed the title List of BeanReferences not resolved when using Spring AOT Preserve collection specific types if possible Feb 8, 2022
@snicoll snicoll closed this as completed in 9dd2342 Feb 8, 2022
@snicoll
Copy link
Contributor

snicoll commented Feb 8, 2022

@abuijze if you have the cycles to give Spring Native 0.11.3-SNAPSHOT a try, I would really appreciate it.

@abuijze
Copy link
Author

abuijze commented Feb 10, 2022

The generated AOT file now generates a compiler error (on OpenJDK 11.0.12)

[ERROR] Failed to execute goal org.springframework.experimental:spring-aot-maven-plugin:0.11.3-SNAPSHOT:generate (generate) on project native-axon-demo: Build failed during Spring AOT code generation: Unable to execute mojo: Compilation failure:
[ERROR] /native-axon-demo/target/generated-runtime-sources/spring-aot/src/main/java/org/springframework/aot/ContextBootstrapInitializer.java:[398,112] incompatible types: inference variable R has incompatible bounds
[ERROR]     lower bounds: org.springframework.beans.factory.config.ConstructorArgumentValues.ValueHolder,java.util.Collection<T>,java.lang.Object
[ERROR]     lower bounds: org.springframework.beans.factory.support.ManagedList<E>
[ERROR] /native-axon-demo/target/generated-runtime-sources/spring-aot/src/main/java/org/springframework/aot/ContextBootstrapInitializer.java:[404,160] incompatible types: inference variable R has incompatible bounds
[ERROR]     lower bounds: org.springframework.beans.factory.config.ConstructorArgumentValues.ValueHolder,java.util.Collection<T>,java.lang.Object
[ERROR]     lower bounds: org.springframework.beans.factory.support.ManagedList<E>
[ERROR] /native-axon-demo/target/generated-runtime-sources/spring-aot/src/main/java/org/springframework/aot/ContextBootstrapInitializer.java:[410,110] incompatible types: inference variable R has incompatible bounds
[ERROR]     lower bounds: org.springframework.beans.factory.config.ConstructorArgumentValues.ValueHolder,java.util.Collection<T>,java.lang.Object
[ERROR]     lower bounds: org.springframework.beans.factory.support.ManagedList<E>
[ERROR] -> [Help 1]
[

@abuijze
Copy link
Author

abuijze commented Feb 10, 2022

fyi, the code it generates is:

    BeanDefinitionRegistrar.of("MessageHandlerConfigurer$$Axon$$COMMAND", MessageHandlerConfigurer.class).withConstructor(MessageHandlerConfigurer.Type.class, List.class)
        .instanceSupplier((instanceContext) -> instanceContext.create(beanFactory, (attributes) -> new MessageHandlerConfigurer(attributes.get(0), attributes.get(1)))).customize((bd) -> {
      ConstructorArgumentValues argumentValues = bd.getConstructorArgumentValues();
      argumentValues.addIndexedArgumentValue(0, "COMMAND");
      argumentValues.addIndexedArgumentValue(1, Stream.of(new RuntimeBeanReference("myCommandHandler")).collect(Collectors.toCollection(ManagedList::new)));
    }).register(beanFactory);

Which looks correct to me...

@snicoll
Copy link
Contributor

snicoll commented Feb 10, 2022

Ah damn. That works on Java 17 but not 11 I suspect.

@snicoll snicoll reopened this Feb 10, 2022
@abuijze
Copy link
Author

abuijze commented Feb 10, 2022

Running it on OpenJDK 17.0.2 gives me the same compiler error.

@snicoll
Copy link
Contributor

snicoll commented Feb 10, 2022

🤦

@snicoll
Copy link
Contributor

snicoll commented Feb 10, 2022

The code is correct but the compiler is unable to infer the type unless the value is assigned unfortunately. This is due to the overloaded addIndexedArgumentValue that takes a ValueHolder.

snicoll added a commit that referenced this issue Feb 10, 2022
@snicoll
Copy link
Contributor

snicoll commented Feb 10, 2022

The data-elasticsearch sample also failed as a result. I've reverted for now and will research another solution for this.

@abuijze
Copy link
Author

abuijze commented Feb 10, 2022

What if you explicitly use the public void addIndexedArgumentValue(int index, @Nullable Object value, String type) method? Looks like null is the default used for type. Adding that should avoid confusion with the overloaded methods.

@snicoll
Copy link
Contributor

snicoll commented Feb 10, 2022

Nope. There are many other places where such an inference can go wrong. That code is generic and not specific to this particular feature.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: enhancement A general enhancement
Development

No branches or pull requests

3 participants