kotlinx.serialization icon indicating copy to clipboard operation
kotlinx.serialization copied to clipboard

Instruct Kotlin serialization to always serialize a third party interface using a specific serializer

Open jntakpe opened this issue 3 years ago • 14 comments

What is your use-case and why do you need this feature?

I need to serialize using kotlinx.serialization a third party interface IApiError returned by a Spring controller advice :

@RestControllerAdvice
class SpringAdvice(private val type: SerializerType) {

    @ExceptionHandler(CustomException::class)
    fun handle(exception: CustomException): ResponseEntity<IApiError> {
     return ResponseEntity(IApiError.of(type, exception), 400) // will return an implementation of IApiError either serializable with Jackson or Kotlinx.serialization (annotated 
    }
}

Spring will execute later the following code that will fail because SerializersKt.serializer(type) will return a Polymorphic serializer and when the serializer is polymorphic Spring fails

//class KotlinSerializationJsonEncoder
//type being `IApiError` type 
//value being `KApiError` a Kotlin annotation with `@Serializable`
Json.Default.encodeToString(SerializersKt.serializer(type), value)

Describe the solution you'd like

I would like to have a way to instruct Kotlinx.serialization that everytime the interface IApiError needs to be serialized it should use a specific serializer, in this case the KApiError class serializer. For example, with Jackson it is possible to do the following :

SimpleAbstractTypeResolver().addMapping(IApiError::class.java, DefaultApiError::class.java)

I've tried declaring the serializer as polymorphic but Spring does not support Open polyphormism.

     serializersModule = SerializersModule {
            polymorphicDefaultSerializer(IApiError::class) {
                KApiError.serializer() as SerializationStrategy<IApiError>
            }
        }

Also I've tried declaring a custom serializer:

@Serializer(forClass = IApiError::class)
object IApiErrorSerializer: KSerializer<IApiError> { /// }

Thanks for your help 🙏

jntakpe avatar Oct 13 '22 07:10 jntakpe

Unfortunately, there isn't a straightforward way now to override serializer for third-party class globally. I wonder why Spring doesn't support open polymorphism though.

sandwwraith avatar Oct 13 '22 14:10 sandwwraith

Spring typical arrangement is to have both kotlinx.serialization and Jackson in the classpath, with kotlinx.serialization converter used first to evaluate if if should be used, if not, Jackson is used. So the behavior of Spring Support is to determine early a signal that shows that kotlinx.serialization should be used.

In our unit tests, you can see what use case we support or not. List with no information on the element type (open polymorphism) won't be serialized by kotlinx.serialization while List<T> will if T is annotated with @Serializable. Open polymorphism is the way to differentiate those use cases.

That's not exactly the ask from this issue to support third party interface, by in https://github.com/spring-projects/spring-framework/issues/28389 we have an issue about serialization of org.springframework.data.domain.Page<T> not working when T is annotated with @Serializable. Since Page<T> extends Iterable<T>, I am wondering if it could be recognize as a collection as List<T> to be supported. Please let me know if I should create a dedicated issue for that.

The ask from this issue could also be helpful in order to allow people to indicate : I know for sure how to serialize this interface with kotlinx.serialization, unless @sandwwraith has a better suggestion.

sdeleuze avatar Jan 03 '23 14:01 sdeleuze

@sandwwraith Any thoughts?

sdeleuze avatar Jan 16 '23 18:01 sdeleuze

I'm a bit confused because it seems we're discussing a bit different things. kotlinx.serialization indeed does not work well with polymorphism for List<T>, because it would require enumerating all possible Ts beforehand in SerializersModule, and that's not very convenient. However, the original issue was about the non-generic interface IApiError, for which the SerializersModule can easily be provided, and there shouldn't be any problems there.

sandwwraith avatar Jan 16 '23 19:01 sandwwraith

Ok I will double check tomorrow that the use case described here works as expected (as you I would expected it does) and if confirmed I will comment asking closing this issue and will create a dedicated one for the need described in https://github.com/Kotlin/kotlinx.serialization/issues/2060#issuecomment-1369808972 which is from my POV the real need for Spring use cases.

sdeleuze avatar Jan 16 '23 19:01 sdeleuze

Hello, @sdeleuze After checking the above comments, I found the reason why IterableSerializer is not supported.

Reference comment: #151 (comment)

Personally I think Iterable is a too broad interface to be serializable, because while it can be an usual list (and why don't use List instead), it also can be lazy and/or infinite stream of elements.

So it seems to be implemented as below (Reference): Screenshot from 2023-01-29 15-47-34 => After debugging SerializersModule.reflectiveOrContextual(), it is as follows.

[Order of method calls] SerializersModule.reflectiveOrContextual() -> Class<T>.constructSerializerForGivenTypeArgs() -> interfaceSerializer()

=> A PolymorphicSerializer is returned due to the Class<T>.constructSerializerForGivenTypeArgs() implementation.

In order to solve the serialization of org.springframework.data.domain.Page<T> mentioned in the #2060 (comment), I think the Spring module should provide a PageSerializer.

Please check the contents and review this spring-projects/spring-framework/pull/29761 again. 🙏

meloning avatar Jan 29 '23 07:01 meloning

@sandwwraith I had a deeper look and I think I am not confortable adding custom logic in Spring to prefer a specific contextual serializer and tend to agree with users ask in #1870 and #2026. I mean, it is not just about Spring, Ktor had to implement custom logic like https://github.com/ktorio/ktor/pull/2865/files.

In the discussion we had, you said

As contextual is meant to be 'fallback on missing default', not 'override the default'

But that's kind of contradict with ContextualSerializer KDoc:

Typical usage of ContextualSerializer would be ... or desire to override serialized class form in one dedicated output format.

And I agree with what the documentation says as it is a common use case.

I understand you don't want to change the existing behavior as it would break use cases, but I am still confused with it.

In https://github.com/Kotlin/kotlinx.serialization/issues/1870#issuecomment-1255169242, you said:

Whether we should provide a new function that prioritizes module over built-in is an open question.

If you are really not up to implement this proposal which sounds attractive to me, could we maybe discuss the introduction of such new function to resolve the need described in this issue "Instruct Kotlin serialization to always serialize a third party interface using a specific serializer"?

sdeleuze avatar Jan 30 '23 09:01 sdeleuze

Okay, I see — this problem indeed needs some design investigation from our side to determine whether a module-prioritizing function is enough for 'always serialize a third party interface using a specific serializer' or not

sandwwraith avatar Jan 30 '23 16:01 sandwwraith

hello. @sandwwraith I am very curious about the current progress regarding this comment.

could you please share? :pray:

meloning avatar Feb 17 '23 14:02 meloning

We've discussed the matter and cam to a conclusion that serializer function indeed should be changed, but only for interfaces: lookup will be first performed in the module (unless there's no explicit @Serializable(with)), and only after, unconditional PolymorphicSerializer will be returned. We'll implement this change in a next major release.

sandwwraith avatar Feb 17 '23 15:02 sandwwraith

Thank you for the explanation. is it okay if i understand the meaning of a next major release as version 1.5.0?

meloning avatar Feb 17 '23 16:02 meloning

No, because 1.5.0-RC is already released. It'll be 1.6.0

sandwwraith avatar Feb 20 '23 11:02 sandwwraith

@sandwwraith Is it available in 1.6.0-RC?

sdeleuze avatar Aug 18 '23 07:08 sdeleuze

@sdeleuze Unfortunately not. Given that now serializer resolution have to be also implemented in plugin due to #1348, we'll be able to sync changes in plugin and runtime only in Kotlin 2.0/serialization 1.7

sandwwraith avatar Aug 18 '23 13:08 sandwwraith