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

Provide guidelines for using Kotlin properties with proxies (@RequestScope and similar use cases) #32287

Closed
kasontey opened this issue Feb 17, 2024 · 5 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: kotlin An issue related to Kotlin support type: documentation A documentation task
Milestone

Comments

@kasontey
Copy link

Problem statement

Hi team,
I noticed a discrepancy in behavior when using the @RequestScope annotation.

Per the documentation I expect each request to produce a new bean with a fresh set of properties.

However, I observed two different outcomes, depending on the language of the model, Java vs Kotlin.

Example

Spring boot version: 3.2.2

I tested this by making repeated request to http://localhost:8080/uniqueRequest

When using a Java model, I noticed the messageId is appropriately assigned when chosen by chance and appropriately cleared/set to empty when not chosen by chance on each subsequent request.

When using a Kotlin model, I noticed the messageId is appropriately assigned when chosen by chance and the property is unexpectedly carried over when not chosen by chance on each subsequent request.

RequestContextConfig.kt

@Configuration
class RequestContextConfig {

    @Bean
    @RequestScope
    fun getRequestContext(): RequestContext {
        return RequestContext()
    }
}

HelloController.kt

@RestController
@RequestMapping("/uniqueRequest")
class HelloController {

    @Autowired
    private lateinit var requestContext: RequestContext

    @GetMapping()
    fun uniqueRequest(): HelloWorldResponse {

        // 1 in 2 chance that messageId is assigned
        if (Random.nextInt(0, 2) == 1) {
            requestContext.messageId = Random.nextInt().toString()
        }

        return HelloWorldResponse(requestContext.messageId ?: "empty");
    }
}

Here are the two models, which show the different behaviors

Kotlin

// Doesn't work as expected
open class RequestContext (var messageId:String? = null)

Java

// Works as expected
public class RequestContext {
    private String messageId = null;

    public String getMessageId() {
        return messageId;
    }

    public void setMessageId(String messageId) {
        this.messageId = messageId;
    }
}

Observed output

Here are the results when making subsequent calls with each model (Kotlin and Java)

"Random hit" and "random not hit" indicates whether the random if statement in the controller was evaluated to true. (This was verified with a breakpoint)

Kotlin

  1. (Random not hit) -> id: "empty"
  2. (Random hit) -> id: "1356740890"
  3. (Random not hit) -> id: "1356740890"
    • The id should be "empty"
  4. (Random hit) -> id: "-1070639845"
  5. (Random hit) -> id: "304745378"
  6. (Random not hit) -> id: "304745378"
    • The id should be "empty"

Java

  1. (Random not hit) -> id: "empty"
  2. (Random hit) -> id: "-997187451"
  3. (Random not hit) -> id: "empty"
  4. (Random hit) -> id: "-1800670903"
  5. (Random not hit) -> id: "empty"
  6. (Random hit) -> id: "-1120590571"
  7. (Random hit) -> id: "411956213"

Why would the property be cached/saved when the model is defined with Kotlin?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Feb 17, 2024
@sbrannen sbrannen changed the title Discrepancy in @RequestScope behavior when using Java vs Kotlin models. The expected behavior breaks in Kotlin. Discrepancy in @RequestScope behavior between Java and Kotlin models Feb 18, 2024
@sbrannen sbrannen added in: web Issues in web modules (web, webmvc, webflux, websocket) theme: kotlin An issue related to Kotlin support labels Feb 18, 2024
@sdeleuze sdeleuze self-assigned this Feb 19, 2024
@sdeleuze
Copy link
Contributor

This is due to Kotlin properties translating to final getters and setters which can't be proxied. In order to avoid this issue, I would recommend to remove RequestContextConfig and declare RequestContext as below in order to benefit from Kotlin all-open/kotlin-spring plugin behavior:

@RequestScope
@Component
class RequestContext (var messageId:String? = null)

I will check with the team if an error should be thrown for that use case instead of silently ignoring it (final seems to be checked at class level but not at method level).

@sbrannen
Copy link
Member

Please note that we log a message at DEBUG level in org.springframework.aop.framework.CglibAopProxy.doValidateClass(...) when a final method is detected.

If you set the log level for org.springframework.aop.framework to debug, you should see a log statement similar to the following.

DEBUG o.s.a.f.CglibAopProxy - Final method [public final java.lang.String com.example.RequestContext.getMessageId()] cannot get proxied via CGLIB: Calls to this method will NOT be routed to the target instance and might lead to NPEs against uninitialized fields in the proxy instance.

However, I concede that it's not intuitive to check debug log messages for org.springframework.aop.framework (from spring-aop) when using @RequestScope (from spring-web).

@sbrannen sbrannen added the in: core Issues in core modules (aop, beans, core, context, expression) label Feb 19, 2024
@snicoll snicoll removed the in: web Issues in web modules (web, webmvc, webflux, websocket) label Feb 19, 2024
@kasontey
Copy link
Author

Removing RequestContextConfig and modifying RequestContext as stated earlier fixed it.

In the future, an error might help others identify this issue sooner. Thanks for the quick responses!

@sdeleuze sdeleuze changed the title Discrepancy in @RequestScope behavior between Java and Kotlin models Provide guidelines for using Kotlin properties with proxies (@RequestScope and similar use cases) Feb 28, 2024
@sdeleuze sdeleuze added this to the 6.1.5 milestone Feb 28, 2024
@sdeleuze sdeleuze added type: documentation A documentation task and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Feb 28, 2024
@sdeleuze
Copy link
Contributor

After discussing this issue with the team, I can share that changing the behavior or differentiate valid use cases from obvious errors is much more complex and nuanced than it seems. As a consequence, I turn this issue into a documentation one where I will share the guidelines exposed above, and will delegate to #26729 some potential behavior changes.

@alshain
Copy link

alshain commented Aug 5, 2024

@sdeleuze Can you share some examples, where this is valid? Why not have the valid use cases opt-in explicitly and barring it by default?

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: kotlin An issue related to Kotlin support type: documentation A documentation task
Projects
None yet
Development

No branches or pull requests

6 participants