-
Notifications
You must be signed in to change notification settings - Fork 623
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
Fix incorrect behavior while deserializing maps to sealed classes #2052
Fix incorrect behavior while deserializing maps to sealed classes #2052
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for the PR. Yeah, I think that's the right way to solve the problem; it just needs more polishing.
Please also try to fix this problem for serializing to map, so this functionality won't be half-baked.
formats/json-tests/jvmTest/src/kotlinx/serialization/SealedClassSerializationFromJsonTest.kt
Outdated
Show resolved
Hide resolved
formats/properties/commonMain/src/kotlinx/serialization/properties/Properties.kt
Outdated
Show resolved
Hide resolved
formats/properties/commonMain/src/kotlinx/serialization/properties/Properties.kt
Outdated
Show resolved
Hide resolved
formats/properties/commonMain/src/kotlinx/serialization/properties/Properties.kt
Outdated
Show resolved
Hide resolved
Hey, @sandwwraith! Thanks for the comments Yeah, it definitely needs more polishing. I'll work on those things you mentioned and I'll let you know when it's ready for revision 👍 |
Hey, @sandwwraith! I think I managed to do everything you suggested, but I have a doubt concerning the serialization process. While it was possible to make it work by creating another method especifically for that, I think the best option would be using the built-in Do you think there's a better approach to solve this? |
It seems that JSON just ignores nullability: kotlinx.serialization/formats/json/commonMain/src/kotlinx/serialization/json/internal/Polymorphic.kt Line 26 in 93a06df
It makes sense, because AbstractPolymorphicSerializer does not support nullable values (they're handled mainly via |
I think it's done, @sandwwraith. |
formats/properties/commonMain/src/kotlinx/serialization/properties/Properties.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for your contribution!
Glad I could help, @sandwwraith! |
This is an initial attempt at fixing an incompatibility between the deserialization of sealed classes when using json vs map
fix: #2035
By reusing some of the stuff from StreamingJsonDecoder I was able to succesfully deserialize a map that has the same structure as the test json.
Am I in the right path?