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
- (Random not hit) -> id: "empty"
- (Random hit) -> id: "1356740890"
- (Random not hit) -> id: "1356740890"
- The id should be "empty"
- (Random hit) -> id: "-1070639845"
- (Random hit) -> id: "304745378"
- (Random not hit) -> id: "304745378"
- The id should be "empty"
Java
- (Random not hit) -> id: "empty"
- (Random hit) -> id: "-997187451"
- (Random not hit) -> id: "empty"
- (Random hit) -> id: "-1800670903"
- (Random not hit) -> id: "empty"
- (Random hit) -> id: "-1120590571"
- (Random hit) -> id: "411956213"
Why would the property be cached/saved when the model is defined with Kotlin?
Comment From: sdeleuze
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).
Comment From: sbrannen
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
).
Comment From: kasontey
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!
Comment From: sdeleuze
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.
Comment From: alshain
@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?