spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.25k stars 37.98k forks source link

AOT/native support when registering beans with the Kotlin DSL #29555

Closed PedroAlvarado closed 1 month ago

PedroAlvarado commented 1 year ago

An early exception is thrown by the "bootBuildImage" Gradle task when attempting to create a native image for a WebFlux application using a functional router.

Exception:

Exception in thread "main" java.lang.IllegalStateException: No constructor or factory method candidate found for Root bean: class [org.springframework.web.reactive.function.server.RouterFunction]; 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:939)
    at org.springframework.beans.factory.support.RegisteredBean.resolveConstructorOrFactoryMethod(RegisteredBean.java:197)
    at org.springframework.beans.factory.aot.BeanDefinitionMethodGenerator.<init>(BeanDefinitionMethodGenerator.java:79)
    at org.springframework.beans.factory.aot.BeanDefinitionMethodGeneratorFactory.getBeanDefinitionMethodGenerator(BeanDefinitionMethodGeneratorFactory.java:102)
    at org.springframework.beans.factory.aot.BeanDefinitionMethodGeneratorFactory.getBeanDefinitionMethodGenerator(BeanDefinitionMethodGeneratorFactory.java:118)
    at org.springframework.beans.factory.aot.BeanRegistrationsAotProcessor.processAheadOfTime(BeanRegistrationsAotProcessor.java:45)
    at org.springframework.beans.factory.aot.BeanRegistrationsAotProcessor.processAheadOfTime(BeanRegistrationsAotProcessor.java:35)
    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:76)

Issue Observed in Spring Boot Version: >= 3.x.x Key Dependencies: Spring WebFlux Reproducer: https://github.com/PedroAlvarado/boot-issue-33318

Steps to Reproduce

  1. Checkout reproduce repo.
  2. Run the bootBuildImage task.
  3. Actual: A build failure.
  4. Expected: A native image is created.
snicoll commented 1 year ago

Thanks for the detailed report and the sample. FTR this should have been reported against the core Spring Framework as the exception you've shared doesn't contain anything Spring Boot-specifc (I've transferred it).

Unfortunately, this is something that we don't support at the moment. The AOT engine works on bean definitions and they need to define the necessary metadata so that we can translate this to generated code. The exception you're getting is because the DSL you use relies on an instance supplier (lambda style). There's no way for us to analyse the lambda and figure out what to write.

I think we should fail with a different exception though. We know it will never work and yet we attempt to locate the metadata. I've created #29556. For this particular issue at hand, we'll need to give it some thoughts.

PedroAlvarado commented 1 year ago

Thanks for the detailed report and the sample. FTR this should have been reported against the core Spring Framework as the exception you've shared doesn't contain anything Spring Boot-specifc (I've transferred it).

Understood.

Is it fair to say that in order to generate a native image in 6.0.0 it is necessary to completely abandon CoRouterFunctionDsl? If so, allow me to suggest for us to find a place in the documentation for folks to be warned about this limitation. Many are interested in taking advantage of native images and this can be important information when choosing between programming models.

What would be an alternate way to supply instances without lambdas that would provide the necessary metadata? In my use-case the routes are created at startup depending on application configuration parameters(application.yml).

Thank you for looking into this issue.

sdeleuze commented 1 year ago

That's indeed something we don't support yet, and I understand how it can be an issue for our users that were leveraging those functional constructs because they are naturally better supported on native via the static analysis and with less hints involved.

It is currently possible to make it work with the pretty ugly code below:

fun beans() = ApplicationContextInitializer<GenericApplicationContext> {
    it.registerBean(RouterFunction::class.java, { customizer ->
        customizer.beanClassName = "com.example.demo.DemoApplicationKt"
        customizer.factoryMethodName = "writeEndpoint"
    })
}

We could potentially update BeanDefinitionDsl in 6.0.x to support beanClassName and factoryMethodName parameters to support something like

fun beans() = beans {
    bean<RouterFunction<*>>(
        beanClassName =  "com.example.demo.DemoApplicationKt",
        factoryMethodName = "writeEndpoint")
}

Not amazing, but at least it provides a reasonable workaround, and I think it does not hurt to add those 2 parameters with default values since we have a use case here. Depending on the feedback I may create a related issue for that refinement.

The best way to support this use case would be IMO to allow using beans instance suppliers as they are, with inference mechanism disable which IMO could make sense with this kind of functional constructs where much less hints are needed, and where having full control on what is done without hint inference could be a feature and the desired behavior. I think this could be explored as part of https://github.com/spring-projects/spring-framework/issues/29553.

PedroAlvarado commented 1 year ago

I was able to verify the workaround. Thank you @sdeleuze.

Allow me to add that in our applications we make use of all the available DSLs and the only spring annotations we use are @SpringBootApplication and @SpringBootTest. In general, we would be in favor of writing more code in exchange for greater control. We have saved 100s of hours by taking advantage of the functional DSLs.

As one example, the security and routing requirements of our APIs are different between mobile/desktop(X-Auth-Tokens, routes, etc) and browser(cookies, csrf, etc) applications while the APIs themselves are practically the same. With the functional DSL we are able to conditionally accommodate all requirements in a cross-cutting way without having to break apart services or introduce gateways. At our early stage, these APIs save cloud resources, cognitive overhead and development time. Additionally, we are finding that the functional model facilitates onboarding of developers coming in from different ecosystem as the code (wiring) is explicit and IDE-assisted.

Anyhow, thank you for providing functional DSLs and I look forward to having first-class support of native images for them.

vladimirfx commented 1 year ago

We successfully migrated a few of our microservices (Bean DSL, functional routing) to spring-native - it works now in production. spring-native is practically deprecated but Spring Boot 3 is unable to run our services in native mode. So it looks like regression to me. The idiomatic programming model is more essential for us than benefits from native runtime so we start to 'denativate' our apps. Fortunately, native-build services are not so many...

Maybe raise some epic-type ticket for first-class functional support in AOT mode (bean definitions (#29556), routing, security, and so on)?

And clearly says these limitations in the documentation: want to go functional (Kotlin or Java) - AOT is not an option.

Sample bean:

    bean {
        val kafkaConsumerProps =
            Binder.get(env).bindOrCreate("kafka.consumer.properties", mapOf(String::class.java, String::class.java))
        val options = ReceiverOptions.create<String, ApiEvent>(kafkaConsumerProps as Map<String, Any>)
            .consumerProperty(
                ConsumerConfig.BOOTSTRAP_SERVERS_CONFIG,
                env.getRequiredProperty("kafka.bootstrap-servers")
            )
            .commitBatchSize(25)
            // 10 minutes of commit retry
            .maxCommitAttempts(1200)
            .withValueDeserializer(ApiEventDeserializer(ref()))
            .subscription(setOf("risk-engine.api.events"))

        KafkaCloudEventReceiver<ApiEvent>(options, provider(), ref())
    }

Produces error:

2022-11-25T08:12:44.270Z ERROR 1 --- [           main] o.s.b.d.LoggingFailureAnalysisReporter   : 

***************************
APPLICATION FAILED TO START
***************************

Description:

Parameter 0 of constructor in com.digitalfinance.riskengine.aggregation.kafka.KafkaCloudEventReceiver required a bean of type 'reactor.kafka.receiver.ReceiverOptions' that could not be found.

Action:

Consider defining a bean of type 'reactor.kafka.receiver.ReceiverOptions' in your configuration.
sdeleuze commented 1 year ago

Another workaround is to use @Bean to include the web router as mentioned in this comment.

fun main() {
    runApplication<AttendanceMainApp> {
        addInitializers(
            beans {
                bean<EmployeeService>()
                bean<EmployeeHandler>()
                bean<AttendanceService>()
                bean<AttendanceHandler>()
            }
        )
    }
}

@Configuration
class RouterConfiguration {

    @Bean
    fun router() = ::routes

}
sdeleuze commented 1 year ago

@vladimirfx I understand the pain, we are going to document this issue.

sdeleuze commented 1 year ago

https://stackoverflow.com/questions/21887358/reflection-type-inference-on-java-8-lambdas provides interesting background why instance supplier callback can't be supported in AOT code generation.

sdeleuze commented 1 year ago

In addition to the short term documentation of this limitation, there could be potentially way to tackle that middle term, by skipping AOT processing for some ApplicationContextInitializer or specific beans and just execute the one that require lambdas or method references at runtime.

It requires to be able to identify them (by class name, method name, annotation as suggested in #29484 or use cases like all SpringApplication#addInitializers usages in Boot). As pointed out by @wilkinsona, this kind of need is broader than ApplicationContextInitializer since it affects things like ContextCustomizer and ContextCustomizerFactory from the test framework as well.

Since there is a potential way to solve that issue, but that's not straightforward, I will create a distinct issue for fixing the documentation, restore the initial purpose of that issue (how to allow functional constructs in ApplicationContextInitializer for end users) and tentatively plan resolution for 6.1.0-M1.

vladimirfx commented 11 months ago

Any chance of seeing implementation in 6.1.x?

Use case: Our configuration code is mainly based on Kotlin Bean DSL. It's fantastically flexible modular and readable, so we want not to drop it to support AOT. But there are cases where AOT + native is very suitable because of resource consumption.

snicoll commented 11 months ago

With 6.1 RC around the corner, I am sorry to say that we won't have the time to craft the necessary API change for this. Keep in mind that using bean instance supplier really goes against the first principle of AOT of detecting components and their stereotype. As such, offering a support to suppress AOT processing on those (as we can't infer what's behind a lambda) will probably other side effects in terms of hints generation typically.

PedroAlvarado commented 7 months ago

@snicoll Allow me to raise that suppressing AOT processing is not being requested. While suppression may be a path, this ticket is not explicitly calling for it. By way of example, if the relevant DSLs need to change such that they allow additional context to be provided that explains what the lambdas are doing, that is a valid approach. This would be a cheap price to pay in exchange for AOT/native.

Anyhow, long way to ask for all options to be considered.

snicoll commented 7 months ago

Allow me to raise that suppressing AOT processing is not being requested.

I am perfectly aware of that and yes, we'll consider all the options. That's why the issue is still open.

snicoll commented 1 month ago

I've narrowed the scope of this issue to the use of the Kotin DSL although anybody writing beans with a lambda can make use of the option that has been added in #33243.

Since we have a DSL for Kotlin, we can do that on behalf of the user and that's what we'll do here.