spring-projects / spring-framework

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

Throw an exception for suspending factory methods #32719

Closed Hansanto closed 6 days ago

Hansanto commented 2 weeks ago

Hello

The @Bean seems to be called two times according to logs (below) in a suspend kotlin function.

This is my app code:

@SpringBootApplication
class SpringKaultApplication {

    @Bean
    suspend fun greeting(): String {
        println("Hello, World!")
        return "My test"
    }
}

fun main(args: Array<String>) {
    runApplication<SpringKaultApplication>(*args)
}

And the logs when I started the application:

2024-04-27T18:14:29.228+00:00  INFO 27824 --- [           main] o.e.s.SpringApplicationKt           : Starting SpringApplicationKt using Java 19.0.2
2024-04-27T18:14:29.230+00:00  INFO 27824 --- [           main] o.e.s.SpringApplicationKt           : No active profile set, falling back to 1 default profile: "default"
Hello, World!
Hello, World!
2024-04-27T18:14:30.566+00:00  INFO 27824 --- [           main] o.s.b.web.embedded.netty.NettyWebServer  : Netty started on port 8080
2024-04-27T18:14:30.572+00:00  INFO 27824 --- [           main] o.e.s.SpringApplicationKt           : Started SpringApplicationKt in 1.663 seconds (process running for 2.204)

Now, if I try without the suspend keyword like:

@SpringBootApplication
class SpringApplication {

    @Bean
    fun greeting(): String {
        println("Hello, World!")
        return "My test"
    }
}

fun main(args: Array<String>) {
    runApplication<SpringApplication>(*args)
}

And the logs:

2024-04-27T18:21:41.495+00:00  INFO 27660 --- [           main] o.e.s.SpringApplicationKt        : Starting SpringApplicationKt using Java 19.0.2
2024-04-27T18:21:41.497+00:00  INFO 27660 --- [           main] o.e.s.SpringApplicationKt        : No active profile set, falling back to 1 default profile: "default"
Hello, World!
2024-04-27T18:21:42.936+00:00  INFO 27660 --- [           main] o.s.b.web.embedded.netty.NettyWebServer  : Netty started on port 8080
2024-04-27T18:21:42.942+00:00  INFO 27660 --- [           main] o.e.s.SpringApplicationKt        : Started SpringApplicationKt in 1.755 seconds (process running for 2.226)

There are the other file to reproduces:

build.gradle.kts

import org.jetbrains.kotlin.gradle.tasks.KotlinCompile

plugins {
    id("org.springframework.boot") version "3.2.5"
    id("io.spring.dependency-management") version "1.1.4"
    kotlin("jvm") version "1.9.23"
    kotlin("plugin.spring") version "1.9.23"
}

group = "org.example"
version = "0.0.1-SNAPSHOT"

java {
    sourceCompatibility = JavaVersion.VERSION_17
}

repositories {
    mavenCentral()
}

dependencies {
    implementation("org.springframework.boot:spring-boot-starter-webflux")
    implementation("io.projectreactor.kotlin:reactor-kotlin-extensions")
    implementation("org.jetbrains.kotlin:kotlin-reflect")
    implementation("org.jetbrains.kotlinx:kotlinx-coroutines-reactor")
    testImplementation("org.springframework.boot:spring-boot-starter-test")
    testImplementation("io.projectreactor:reactor-test")
}

tasks.withType<KotlinCompile> {
    kotlinOptions {
        freeCompilerArgs += "-Xjsr305=strict"
        jvmTarget = "17"
    }
}

tasks.withType<Test> {
    useJUnitPlatform()
}

Version:

sdeleuze commented 2 weeks ago

Suspending functions should not be used with @Bean and for factory methods, Spring Framework 6.2+ is going to throw an exception for such use case (including when AOT is used).

FYI, in your repro, the message is printed 2 times because the decompiled generated bytecode is:

public class DemoSbWithValueClassApplication {
   @Bean
   @Nullable
   public Object greeting(@NotNull Continuation $completion) {
      return greeting$suspendImpl(this, $completion);
   }

   // $FF: synthetic method
   @Bean
   static Object greeting$suspendImpl(DemoSbWithValueClassApplication $this, Continuation $completion) {
      String var2 = "Hello, World!";
      System.out.println(var2);
      return "My test";
   }
}
Hansanto commented 2 weeks ago

I don't understand why it's a problem to instantiate a bean in a suspend method. Instead of throwing an exception, why not fix it to allow this feature?

In Kotlin, some libraries create a builder using a suspend method. Consequently, if an exception is thrown, you will block the possibility of using this type of builder.

I suggest creating the Bean using a coroutine attached to the main thread (or the dedicated thread where the Bean are instantiated). Of course, that implies checking if the function is a kotlin function, duplicated (one with the basic args and another with basic args & Continuation object (from coroutine)) and consequently load only matching Bean.

For example (that's not the best example, but maybe it will allow you to understand the need and how to do that in Kotlin):

import kotlin.properties.Delegates
import kotlinx.coroutines.delay

public class MyClient(
    public val arg1: Int,
    public val arg2: String,
) {

    public companion object {

        /**
         * ```kotlin
         * val client = MyClient {
         *   arg1 = 1
         *   // ...
         * }
         * ```
         */
        public suspend inline operator fun invoke(builder: Builder.() -> Unit): MyClient =
            Builder().apply(builder).build()
    }

    public class Builder {

        public var arg1: Int by Delegates.notNull()

        public lateinit var arg2: String

        public var shouldCallMyServer: Boolean = false

        public suspend fun build(): MyClient {
            return MyClient(
                arg1 = arg1,
                arg2 = arg2
            ).apply {
                // Init the client with suspending calls according to some builder properties
                if (shouldCallMyServer) {
                    // Simulate a suspending call
                    delay(1000)
                }
            }
        }
    }
}

With that, if I need to do something that needs I/O operation, according to the builder properties and to simplify the management of my object, I can do this in the builder.

Hansanto commented 2 weeks ago

After of course, you can wrap your Builder in a run blocking method. So we already have an option to do that without putting the suspend keyword on the function signature.

Like:


@SpringBootApplication
class SpringApplication {

    @Bean
    fun greeting(): MyClient {
        return runBlocking {
           MyClient { ... }
        }
    }
}
sdeleuze commented 6 days ago

I don't think suspending factory method conceptually make sense, they have side effects, and you indeed have the possibility to use runBlocking { } if you really need this, so Spring Framework 6.2+ will explicitly fail for such use case.