Properties Format: Support wrapped sealed classes #2255
Conversation
| @@ -102,7 +103,7 @@ public sealed class Properties( | |||
| } | |||
|
|
|||
| final override fun <T> decodeSerializableValue(deserializer: DeserializationStrategy<T>): T { | |||
| val type = map["type"]?.toString() | |||
| val type = map[nested("type")]?.toString() | |||
There was a problem hiding this comment.
I would put this statement under the if condition below. Most types are not polymorphic, and here we will have an unnecessary creation of a nested key.
There was a problem hiding this comment.
Good idea, thank you, I changed it.
EdwarDDay
force-pushed
the
properties_format_support_wrapped_polymorphics
branch
from
e07f815
to
fda0392
Compare
|
Hi, thanks for the PR! Can you please elaborate on what 'wrapped' means? You used lists in test, but it seems change should work also with sealed classes contained in other classes, as in the test in here: https://github.com/Kotlin/kotlinx.serialization/pull/2262/files#diff-bec5521c216f61f12d3d730fecbe158051faf72330a89ec11de2fc6f90b85e16R54 Can you please also add this test to a test set and describe 'wrapped' in more detail in commit message? Thank you! |
|
Yeah, I can do this. Maybe "nested" would be a better wording then "wrapped". But I just wanted to express properties (I mean properties of a class) with a sealed class type - or in the case of lists or maps it would be the sealed class type as the type parameter. |
EdwarDDay
force-pushed
the
properties_format_support_wrapped_polymorphics
branch
from
fda0392
to
794989e
Compare
|
Next time, please open the PR against |
The properties format didn't support serialization of nested sealed classes before. This should be fixed with these changes.