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

Refine kotlinx.serialization support #26147

Closed
Aelerinya opened this issue Nov 24, 2020 · 6 comments
Closed

Refine kotlinx.serialization support #26147

Aelerinya opened this issue Nov 24, 2020 · 6 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Milestone

Comments

@Aelerinya
Copy link

Aelerinya commented Nov 24, 2020

Affects: 5.3.1

Bug

I'm using the kotlinx-serialization library. The library is properly set up, jackson is excluded from dependencies, and returning a class annotated with @Serializable works correctly.

I'm trying to return a simple Kotlin list from a controller. When accessing the route the server displays this error and nothing is returned:

import kotlinx.serialization.encodeToString
import kotlinx.serialization.json.Json
import org.springframework.web.bind.annotation.GetMapping
import org.springframework.web.bind.annotation.RestController

@RestController
class Controller {
    @GetMapping("/")
    fun notworking() = listOf(1, 2, 3)
}

Error message

2020-11-24 17:28:51.929  WARN 189362 --- [nio-8080-exec-2] .w.s.m.s.DefaultHandlerExceptionResolver : Resolved [org.springframework.http.converter.HttpMessageNotWritableException: No converter found for return value of type: class java.util.Arrays$ArrayList]
2020-11-24 17:28:51.945  WARN 189362 --- [nio-8080-exec-2] .w.s.m.s.DefaultHandlerExceptionResolver : Resolved [org.springframework.http.converter.HttpMessageNotWritableException: No converter found for return value of type: class java.util.LinkedHashMap]

What I expected

I expected the list to be serialized properly like this:

[1,2,3]

Workarounds

Those three workarounds work:

    @Serializable
    data class Wrapper(val content: List<Int>)
    @GetMapping("/")
    fun working() = Wrapper(listOf(1, 2, 3))
    @GetMapping("/", produces = ["application/json"])
    fun working() = Json.encodeToString(listOf(1, 2, 3))
    @GetMapping("/working")
    fun working() = Json.encodeToJsonElement(listOf(1, 2, 3))
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 24, 2020
@sdeleuze sdeleuze added this to the 5.3.2 milestone Nov 24, 2020
@sdeleuze sdeleuze self-assigned this Nov 24, 2020
@sdeleuze sdeleuze added in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 24, 2020
@sdeleuze
Copy link
Contributor

We should probably take advantage of GenericHttpMessageConverter here not just the boolean supports(Class<?> clazz) variant that does not carry generic type informations.

Let's also make sure it allows both Jackson and Kotlin serialization to be used together is a Boot app with actuator as discussed in spring-projects/spring-boot#24238.

@Aelerinya
Copy link
Author

I noticed also that error messages returned by the server too cannot be encoded when jackson is disabled, as described in this page

When a request in forbidden in a REST controller the server sends back this:

{"timestamp":"2020-11-25T22:29:58.573+00:00","status":403,"error":"Forbidden","message":"","path":"/user"}

But when kotlinx-serialization is enabled and jackson disabled, the server resolved the request with this error:

org.springframework.http.converter.HttpMessageNotWritableException: No converter found for return value of type: class java.util.LinkedHashMap

Maybe the documentation could be updated in the meantime to specify that disabling jackson as said prevents internal messages from being serialized ?

@sdeleuze
Copy link
Contributor

Yeah we probably need to advise (and maybe configure) both by default since Kotlin serialization is only for Kotlin classes annotated with @Serializable.

@sdeleuze sdeleuze changed the title kotlinx-serialization: Returning list from controller does not work Refine kotlinx.serialization behavior Nov 26, 2020
@sdeleuze sdeleuze changed the title Refine kotlinx.serialization behavior Refine kotlinx.serialization support Nov 26, 2020
sdeleuze added a commit to sdeleuze/spring-framework that referenced this issue Nov 26, 2020
This commit introduces the following changes:
 - Converters/codecs are now used based on generic type info.
 - On WebMvc and WebFlux, kotlinx.serialization is enabled along
   to Jackson because it only serialize Kotlin @serializable classes
   which is not enough for error or actuator endpoints in Boot as
   described on spring-projects/spring-boot#24238.

TODO: leverage Kotlin/kotlinx.serialization#1164 when fixed.

Closes spring-projectsgh-26147
@mickael-coquer
Copy link

Hi @sdeleuze ,

Migrating to Spring Boot 2.4.1, I encountered the following error while trying to deserialize JSON

org.springframework.http.converter.HttpMessageNotReadableException: Could not read JSON: Polymorphic serializer was not found for missing class discriminator ('null') ... nested exception is kotlinx.serialization.json.internal.JsonDecodingException: Polymorphic serializer was not found for missing class discriminator ('null')
I happen to have both Jackson and kotlinx-serialization in the classpath and Kotlin takes precedence over Jackson.

In my projet we're using Java interfaces implemented by Java classes.

I guess the issue comes from the KotlinSerializationJsonHttpMessageConverter.candRead method that returns true when the Type is a Java interface. But when the KotlinSerializationJsonHttpMessageConverter.read method is called then it fails because the concrete type is a Java class and not a Kotlin class.

This is not blocking for me as I can change the converters order or just remove kotlinx-serialization (it's a transitive dependency I don't actually need). But I thought you might want to be aware of this behavior.

@sdeleuze
Copy link
Contributor

@mickael-coquer Thanks for raising that, worth to fix IMO, could you please create a related issue?

@mickael-coquer
Copy link

Here you go: #26298

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants