Kotlinx codec provider limit to data, value or sealed classes. #1147
Conversation
The kotlinx codec was picking up the double array as it was serializable. However, it should be limited to top level data, value or sealed kotlin classes. JAVA-5035
| return try { | ||
| create(kClass, kClass.serializer(), serializersModule, bsonConfiguration) | ||
| } catch (exception: SerializationException) { | ||
| return if (!(kClass.isData || kClass.isValue || kClass.isSealed)) { |
There was a problem hiding this comment.
Which of the assertions below test which of the tests here? It seems that at least one test is missing since there are three conditions here and two assertions.
There was a problem hiding this comment.
Ah that test ensures the two cases (double & CharSequence) that we knew were being picked up now return null.
All three types are already tested by the other tests.
There was a problem hiding this comment.
It seems like before this change this method was returning a codec for classes that were not data or value or sealed classes, and now it returns null for those. So I'm not clear how an assertion that it now returns null for all of these could already be in the code.
There was a problem hiding this comment.
Ah - KotlinSerializerCodecTest has existing tests for data, value and sealed classes which we want to continue to support.
The shouldReturnNullForSerializableButNotValueClassOrSealedOrDataClassTypes adds a test to ensure other serializable types (that we know about) fail.
There was a problem hiding this comment.
Oh, I'm dumb. Since it's a ! for all three then this is fine.
There was a problem hiding this comment.
Merged as is in 4.10.x branch
But added 28b9727 to the master branch to improve the readability of this check and the same for the DataclassCodec.
JAVA-5035