Unwrap Kotlin inline value classes return values

[serandel]

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.

[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.

[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.

[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 Kotlin Serialization support does not take null-safety into account #33016
• Reconsider interpreting inline value classes as their unwraped type when we will rework Kotlin reflection support to avoid the wrapping/unwrapping via Optimize Kotlin reflection runtime efficiency #21546

I will discuss that with the wider team and share the result of the discussion here.

[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.

[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
in in order to have a consistent behavior which would the outcome I would like to have with Spring Framework 6.2. I will share an update once #33016 is fixed.

[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.

[sdeleuze]

That's ok, I have built a similar repro and I think I have found a way to fix this.