Affects: Spring 6.1.6
REST controllers are unable to return Kotlin value classes, either directly or wrapped in a ResponseEntity.
I'm using extensively value classes as a way of defining my domain functionally, but have found what I think it's a Spring MVC bug.
Minimal Proof of Concept:
@Serializable
@JvmInline
value class Foo(val value: String)
@GetMapping(
value = ["/foo"],
produces = [MediaType.TEXT_PLAIN_VALUE],
)
fun getFoo(): Foo = Foo("bar")
Expected result:
% curl localhost:8080/foo
bar
Actual result:
% curl localhost:8080/foo
{"type":"about:blank","title":"Internal Server Error","status":500,"detail":"Failed to write request","instance":"/foo"}
Workaround: just unwrap the inner value at the controller level before returning
@GetMapping(
value = ["/foo"],
produces = [MediaType.TEXT_PLAIN_VALUE],
)
fun getFoo(): String = Foo("bar").value
What's interesting to me is that I have no problem with value classes as input parameters, either in a RequestBody, a PathVariable, etc. They work seamlessly.
Comment From: snicoll
Minimal Proof of Concept:
Please move that to something we can actually run. It's too bare bone and anything that's missing is a guess on our end. Also, The actual result is pretty useless since we don't have the actual stacktrace that was thrown.
Comment From: sdeleuze
This looks a regression due to the fact that now all Kotlin function invocations are handled by Kotlin reflection to support various use cases (parameters with default values for example). For some reason, Kotlin reflective invocation via kotlin.reflect.KCallable#callBy return the wrapped value (Foo), not the unwrapped one (String). That should be fixed either on Spring or Kotlin side. I have asked a feedback from the Kotlin team.
Comment From: sdeleuze
After spending some time digging into this issue, I can say it is more nuanced and complex that what I thought originally.
As far as I can tell, inline value classes are handled correctly but are interpreted unwrapped since Kotlin reflection handle it that way by design, that's why when you want to treat @JvmInline value class Foo(val value: String) as String it fails. But without produces = [MediaType.TEXT_PLAIN_VALUE] with Jackson, it will serialize as "bar" as expected for the unwrapped serialization.
The use case that is really broken is the serialization by Kotlin Serialization (which is inline value class aware) where there is a mismatch between the Java type of the return value (String) and the type of the value to serialize (Foo). This should be fixed via #33016.
I made some try handling the Foo to String conversion on Spring side, but handling that for all use cases (plain return value, ResponseEntity<T> and suspending functions) on both WebMVC and WebFlux is very tricky and fragile. So for now, I think I am on the line of:
- Treating value classes as unwrapped for Java converters/codecs that are not inline value class aware (current behavior that we could document as part of this issue)
- Fix Kotlin Serialization use case via #33016
- Reconsider interpreting inline value classes as their unwraped type when we will rework Kotlin reflection support to avoid the wrapping/unwrapping via #21546
I will discuss that with the wider team and share the result of the discussion here.
Comment From: sdeleuze
@serandel The team agreed with my proposal, so I keep that issue in the backlog and create another documentation issue to document current behavior and will work on fixing the Kotlin Serialization use case via #33016. The behavior you hope will likely require #21546 which will take more time to be fixed.
Comment From: sdeleuze
Hum, based on the additional tests I did, it looks like we still have an inconsistency to fix between body, valueType and bodyType in AbstractMessageConverterMethodProcessor#writeWithMessageConverters
Comment From: serandel
Awesome work here, @sdeleuze, thanks a lot.
I didn't have the time till now, but I take it you already have a better PoC than anything I could extract from my original project, right? Just ping me if I can do anything to help.
Comment From: sdeleuze
That's ok, I have built a similar repro and I think I have found a way to fix this.