AOT processing fails for @ConfigurationProperties class without default constructor

[odrotbohm]

During AOT processing, the constructor detection for AOT fails for configuration properties classes that do not declare a default constructor. An example can be found in Spring RESTBucks for MomentsProperties.

Steps to reproduce:

$ git clone https://github.com/odrotbohm/spring-restbucks
$ cd spring-restbucks/server
$ git checkout hacking/aot-properties
$ ./mvnw -Paot

The output is the following stack trace:

Exception in thread "main" java.lang.IllegalStateException: No constructor or factory method candidate found for Root bean: class [org.springframework.modulith.moments.support.MomentsProperties]; scope=singleton; abstract=false; lazyInit=null; autowireMode=0; dependencyCheck=0; autowireCandidate=true; primary=false; factoryBeanName=null; factoryMethodName=null; initMethodNames=null; destroyMethodNames=null and argument types []
	at org.springframework.beans.factory.support.ConstructorResolver.resolveConstructorOrFactoryMethod(ConstructorResolver.java:993)
	at org.springframework.beans.factory.support.RegisteredBean.resolveConstructorOrFactoryMethod(RegisteredBean.java:212)
	at org.springframework.beans.factory.aot.BeanDefinitionMethodGenerator.<init>(BeanDefinitionMethodGenerator.java:86)
	at org.springframework.beans.factory.aot.BeanDefinitionMethodGeneratorFactory.getBeanDefinitionMethodGenerator(BeanDefinitionMethodGeneratorFactory.java:100)
	at org.springframework.beans.factory.aot.BeanDefinitionMethodGeneratorFactory.getBeanDefinitionMethodGenerator(BeanDefinitionMethodGeneratorFactory.java:115)
	at org.springframework.beans.factory.aot.BeanRegistrationsAotProcessor.processAheadOfTime(BeanRegistrationsAotProcessor.java:49)
	at org.springframework.beans.factory.aot.BeanRegistrationsAotProcessor.processAheadOfTime(BeanRegistrationsAotProcessor.java:37)
	at org.springframework.context.aot.BeanFactoryInitializationAotContributions.getContributions(BeanFactoryInitializationAotContributions.java:67)
	at org.springframework.context.aot.BeanFactoryInitializationAotContributions.<init>(BeanFactoryInitializationAotContributions.java:49)
	at org.springframework.context.aot.BeanFactoryInitializationAotContributions.<init>(BeanFactoryInitializationAotContributions.java:44)
	at org.springframework.context.aot.ApplicationContextAotGenerator.lambda$processAheadOfTime$0(ApplicationContextAotGenerator.java:58)
	at org.springframework.context.aot.ApplicationContextAotGenerator.withCglibClassHandler(ApplicationContextAotGenerator.java:67)
	at org.springframework.context.aot.ApplicationContextAotGenerator.processAheadOfTime(ApplicationContextAotGenerator.java:53)
	at org.springframework.context.aot.ContextAotProcessor.performAotProcessing(ContextAotProcessor.java:106)
	at org.springframework.context.aot.ContextAotProcessor.doProcess(ContextAotProcessor.java:84)
	at org.springframework.context.aot.ContextAotProcessor.doProcess(ContextAotProcessor.java:49)
	at org.springframework.context.aot.AbstractAotProcessor.process(AbstractAotProcessor.java:82)
	at org.springframework.boot.SpringApplicationAotProcessor.main(SpringApplicationAotProcessor.java:80)

ConstructorResolver.resolveConstructor(…) is given an empty list of value type, as the bean definition for a configuration properties class doesn't have any constructor arguments registered. It will then process the existing complex constructors and disregard them all as they do not match the empty value types.

The problem was originally reported here. To solve that I can work around the problem by adding a default constructor to MomentsProperties. The reproducer works if built with that change in place.

[sdeleuze]

Seems to be caused by the fact that org.springframework.beans.factory.support.ConstructorResolver#resolveConstructor invoked by BeanRegistrationsAotProcessor can't resolve the constructor to use because MomentsProperties has 2 non default constructors, the one to use is identified by @ConstructorBinding but this is a Spring Boot annotation Spring Framework knows nothing about.

Not sure yet how to fix this, should Spring Framework provide an extension point that Spring Boot would leverage?

[wilkinsona]

I think the constructor or factory method's only actually going to be used to determine the class name of the target:

ClassName target = codeFragments.getTarget(this.registeredBean, this.constructorOrFactoryMethod);

The implementation of getTarget just uses the declaring class of the Executable. I wonder if another code path could be introduced that makes use of the bean type from the RegisteredBean instead? Everything else would then rely on a custom code fragment taking care of generating the code to instantiate the bean.

[snicoll]

ConfigurationPropertiesBeanRegistrationCodeFragments reveals an interesting use case that generateInstanceSupplierCode is provided with the constructorOrFactoryMethod but doesn't need to use it as it has its own way of dealing with this.

Looking at other use of it, there's getTarget indeed which has special checks for FactoryBean, but the resolution of the default constructor is available on RegisteredBean directly via resolveConstructorOrFactoryMethod. I think we could change those methods so that they don't provide this argument, where the default implementation resolves it. This would give the opportunity to whoever needs another constructor to override the behaviour.

the only thing I am not too fan is that users would then have to override getTarget to avoid the default constructor resolution, which is far from obvious.

[snicoll]

This is surprisingly difficult. The code that uses the Executable uses it to register custom hints so moving it to a different place doesn't really work. The good news is that the change reveals the current arrangement is a bit awkward. I was hoping to backport the fix but it doesn't look like possible I am afraid.

[snicoll]

I have something that works but is a breaking change for anyone implementing BeanRegistrationCodeFragments. The good news is that the scope of things is a little bit more clear though, and most custom implementations don't care about the factory method as they know exactly what it should be.

@jhoeller wondering if that's acceptable to break the interface in 6.1. The alternative is to reintroduce the previous methods with a default implementation that returns null and the caller calls that first and uses it if the return value is non-null.

[snicoll]

Unfortunately, I had to go with the breaking change as keeping the original method meant that the executable had to be resolved before calling it and check if it was actually implemented by a custom code fragment. It's unfortunate but acceptable given the scope of this API.