From 53b12aae0019ea7f86e37c426705cc0598251b06 Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Wed, 17 Jul 2024 12:55:15 +0100 Subject: [PATCH 1/4] Fix bson-kotlinx `encodeNullableSerializableValue` null handling Ensures that the deferredElement name is reset correctly. Test case ported to bson-kotlin JAVA-5524 --- .../bson/codecs/kotlin/DataClassCodecTest.kt | 16 +++++++++++++ .../bson/codecs/kotlin/samples/DataClasses.kt | 4 ++++ .../org/bson/codecs/kotlinx/BsonEncoder.kt | 2 ++ .../kotlinx/KotlinSerializerCodecTest.kt | 23 +++++++++++++++++++ .../codecs/kotlinx/samples/DataClasses.kt | 5 ++++ 5 files changed, 50 insertions(+) diff --git a/bson-kotlin/src/test/kotlin/org/bson/codecs/kotlin/DataClassCodecTest.kt b/bson-kotlin/src/test/kotlin/org/bson/codecs/kotlin/DataClassCodecTest.kt index 40abc3a9cfa..e3cfe530705 100644 --- a/bson-kotlin/src/test/kotlin/org/bson/codecs/kotlin/DataClassCodecTest.kt +++ b/bson-kotlin/src/test/kotlin/org/bson/codecs/kotlin/DataClassCodecTest.kt @@ -25,6 +25,7 @@ import org.bson.codecs.configuration.CodecConfigurationException import org.bson.codecs.configuration.CodecRegistries.fromProviders import org.bson.codecs.kotlin.samples.Box import org.bson.codecs.kotlin.samples.DataClassEmbedded +import org.bson.codecs.kotlin.samples.DataClassLastItemDefaultsToNull import org.bson.codecs.kotlin.samples.DataClassListOfDataClasses import org.bson.codecs.kotlin.samples.DataClassListOfListOfDataClasses import org.bson.codecs.kotlin.samples.DataClassListOfSealed @@ -51,6 +52,7 @@ import org.bson.codecs.kotlin.samples.DataClassWithEnum import org.bson.codecs.kotlin.samples.DataClassWithEnumMapKey import org.bson.codecs.kotlin.samples.DataClassWithFailingInit import org.bson.codecs.kotlin.samples.DataClassWithInvalidBsonRepresentation +import org.bson.codecs.kotlin.samples.DataClassWithListThatLastItemDefaultsToNull import org.bson.codecs.kotlin.samples.DataClassWithMutableList import org.bson.codecs.kotlin.samples.DataClassWithMutableMap import org.bson.codecs.kotlin.samples.DataClassWithMutableSet @@ -133,6 +135,20 @@ class DataClassCodecTest { assertDecodesTo(withStoredNulls, dataClass) } + @Test + fun testDataClassWithListThatLastItemDefaultsToNull() { + val expected = + """{ + | "elements": [{"required": "required"}, {"required": "required"}], + |}""" + .trimMargin() + + val dataClass = + DataClassWithListThatLastItemDefaultsToNull( + listOf(DataClassLastItemDefaultsToNull("required"), DataClassLastItemDefaultsToNull("required"))) + assertRoundTrips(expected, dataClass) + } + @Test fun testDataClassWithNullableGenericsNotNull() { val expected = diff --git a/bson-kotlin/src/test/kotlin/org/bson/codecs/kotlin/samples/DataClasses.kt b/bson-kotlin/src/test/kotlin/org/bson/codecs/kotlin/samples/DataClasses.kt index 5bc6e768ed8..aa2c8983b1d 100644 --- a/bson-kotlin/src/test/kotlin/org/bson/codecs/kotlin/samples/DataClasses.kt +++ b/bson-kotlin/src/test/kotlin/org/bson/codecs/kotlin/samples/DataClasses.kt @@ -57,6 +57,10 @@ data class DataClassWithDefaults( data class DataClassWithNulls(val boolean: Boolean?, val string: String?, val listSimple: List?) +data class DataClassWithListThatLastItemDefaultsToNull(val elements: List) + +data class DataClassLastItemDefaultsToNull(val required: String, val optional: String? = null) + data class DataClassSelfReferential( val name: String, val left: DataClassSelfReferential? = null, diff --git a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt index b3ae0c8cdf4..bb977a19880 100644 --- a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt +++ b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt @@ -156,6 +156,8 @@ internal class DefaultBsonEncoder( if (value != null || configuration.explicitNulls) { encodeName(it) super.encodeNullableSerializableValue(serializer, value) + } else { + deferredElementName = null } } ?: super.encodeNullableSerializableValue(serializer, value) diff --git a/bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodecTest.kt b/bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodecTest.kt index ed9e1bfb43a..05a0d3ffd7d 100644 --- a/bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodecTest.kt +++ b/bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodecTest.kt @@ -45,6 +45,7 @@ import org.bson.codecs.kotlinx.samples.DataClassContainsOpen import org.bson.codecs.kotlinx.samples.DataClassContainsValueClass import org.bson.codecs.kotlinx.samples.DataClassEmbedded import org.bson.codecs.kotlinx.samples.DataClassKey +import org.bson.codecs.kotlinx.samples.DataClassLastItemDefaultsToNull import org.bson.codecs.kotlinx.samples.DataClassListOfDataClasses import org.bson.codecs.kotlinx.samples.DataClassListOfListOfDataClasses import org.bson.codecs.kotlinx.samples.DataClassListOfSealed @@ -78,6 +79,7 @@ import org.bson.codecs.kotlinx.samples.DataClassWithEncodeDefault import org.bson.codecs.kotlinx.samples.DataClassWithEnum import org.bson.codecs.kotlinx.samples.DataClassWithEnumMapKey import org.bson.codecs.kotlinx.samples.DataClassWithFailingInit +import org.bson.codecs.kotlinx.samples.DataClassWithListThatLastItemDefaultsToNull import org.bson.codecs.kotlinx.samples.DataClassWithMutableList import org.bson.codecs.kotlinx.samples.DataClassWithMutableMap import org.bson.codecs.kotlinx.samples.DataClassWithMutableSet @@ -255,6 +257,27 @@ class KotlinSerializerCodecTest { assertRoundTrips(expectedNulls, dataClass, altConfiguration) } + @Test + fun testDataClassWithListThatLastItemDefaultsToNull() { + val expectedWithOutNulls = + """{ + | "elements": [{"required": "required"}, {"required": "required"}], + |}""" + .trimMargin() + + val dataClass = + DataClassWithListThatLastItemDefaultsToNull( + listOf(DataClassLastItemDefaultsToNull("required"), DataClassLastItemDefaultsToNull("required"))) + assertRoundTrips(expectedWithOutNulls, dataClass) + + val expectedWithNulls = + """{ + | "elements": [{"required": "required", "optional": null}, {"required": "required", "optional": null}], + |}""" + .trimMargin() + assertRoundTrips(expectedWithNulls, dataClass, BsonConfiguration(explicitNulls = true)) + } + @Test fun testDataClassWithNullableGenericsNotNull() { val expected = diff --git a/bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/samples/DataClasses.kt b/bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/samples/DataClasses.kt index 2511c7b0418..66907bff103 100644 --- a/bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/samples/DataClasses.kt +++ b/bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/samples/DataClasses.kt @@ -82,6 +82,11 @@ data class DataClassWithDefaults( @Serializable data class DataClassWithNulls(val boolean: Boolean?, val string: String?, val listSimple: List?) +@Serializable +data class DataClassWithListThatLastItemDefaultsToNull(val elements: List) + +@Serializable data class DataClassLastItemDefaultsToNull(val required: String, val optional: String? = null) + @Serializable data class DataClassSelfReferential( val name: String, From 62e6d0bb0cd296482776c90c6aee3711325f756f Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Wed, 24 Jul 2024 17:54:56 +0100 Subject: [PATCH 2/4] Cleanup deferredElementName use --- .../org/bson/codecs/kotlinx/BsonEncoder.kt | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt index bb977a19880..1a75b5540e5 100644 --- a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt +++ b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt @@ -117,6 +117,7 @@ internal class DefaultBsonEncoder( is StructureKind.CLASS -> { val elementName = descriptor.getElementName(index) if (descriptor.getElementDescriptor(index).isNullable) { + assert(deferredElementName == null) { -> "Overwriting an existing deferred name" } deferredElementName = elementName } else { encodeName(elementName) @@ -141,11 +142,10 @@ internal class DefaultBsonEncoder( override fun encodeSerializableValue(serializer: SerializationStrategy, value: T) { deferredElementName?.let { + deferredElementName = null if (value != null || configuration.explicitNulls) { encodeName(it) super.encodeSerializableValue(serializer, value) - } else { - deferredElementName = null } } ?: super.encodeSerializableValue(serializer, value) @@ -153,11 +153,10 @@ internal class DefaultBsonEncoder( override fun encodeNullableSerializableValue(serializer: SerializationStrategy, value: T?) { deferredElementName?.let { + deferredElementName = null if (value != null || configuration.explicitNulls) { encodeName(it) super.encodeNullableSerializableValue(serializer, value) - } else { - deferredElementName = null } } ?: super.encodeNullableSerializableValue(serializer, value) @@ -172,14 +171,7 @@ internal class DefaultBsonEncoder( override fun encodeDouble(value: Double) = writer.writeDouble(value) override fun encodeInt(value: Int) = writer.writeInt32(value) override fun encodeLong(value: Long) = writer.writeInt64(value) - override fun encodeNull() { - deferredElementName?.let { - if (configuration.explicitNulls) { - encodeName(it) - } - } - writer.writeNull() - } + override fun encodeNull() = writer.writeNull() override fun encodeString(value: String) { when (state) { @@ -208,7 +200,6 @@ internal class DefaultBsonEncoder( private fun encodeName(value: Any) { writer.writeName(value.toString()) - deferredElementName = null state = STATE.VALUE } From 081b41347c24f821ce1b0e4fb28e0c87c8dfce50 Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Wed, 24 Jul 2024 18:10:14 +0100 Subject: [PATCH 3/4] Encapsulate deferred element name handling --- .../org/bson/codecs/kotlinx/BsonEncoder.kt | 58 +++++++++++++------ 1 file changed, 39 insertions(+), 19 deletions(-) diff --git a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt index 1a75b5540e5..dd0392a0346 100644 --- a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt +++ b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt @@ -72,7 +72,7 @@ internal class DefaultBsonEncoder( private var isPolymorphic = false private var state = STATE.VALUE private var mapState = MapState() - private var deferredElementName: String? = null + private val deferredElementHandler: DeferredElementHandler = DeferredElementHandler() override fun shouldEncodeElementDefault(descriptor: SerialDescriptor, index: Int): Boolean = configuration.encodeDefaults @@ -117,8 +117,7 @@ internal class DefaultBsonEncoder( is StructureKind.CLASS -> { val elementName = descriptor.getElementName(index) if (descriptor.getElementDescriptor(index).isNullable) { - assert(deferredElementName == null) { -> "Overwriting an existing deferred name" } - deferredElementName = elementName + deferredElementHandler.set(elementName) } else { encodeName(elementName) } @@ -141,25 +140,25 @@ internal class DefaultBsonEncoder( } override fun encodeSerializableValue(serializer: SerializationStrategy, value: T) { - deferredElementName?.let { - deferredElementName = null - if (value != null || configuration.explicitNulls) { - encodeName(it) - super.encodeSerializableValue(serializer, value) - } - } - ?: super.encodeSerializableValue(serializer, value) + deferredElementHandler.with( + { + if (value != null || configuration.explicitNulls) { + encodeName(it) + super.encodeSerializableValue(serializer, value) + } + }, + { super.encodeSerializableValue(serializer, value) }) } override fun encodeNullableSerializableValue(serializer: SerializationStrategy, value: T?) { - deferredElementName?.let { - deferredElementName = null - if (value != null || configuration.explicitNulls) { - encodeName(it) - super.encodeNullableSerializableValue(serializer, value) - } - } - ?: super.encodeNullableSerializableValue(serializer, value) + deferredElementHandler.with( + { + if (value != null || configuration.explicitNulls) { + encodeName(it) + super.encodeNullableSerializableValue(serializer, value) + } + }, + { super.encodeNullableSerializableValue(serializer, value) }) } override fun encodeByte(value: Byte) = encodeInt(value.toInt()) @@ -222,4 +221,25 @@ internal class DefaultBsonEncoder( return getState() } } + + private class DeferredElementHandler { + private var deferredElementName: String? = null + + fun set(name: String) { + assert(deferredElementName == null) { -> "Overwriting an existing deferred name" } + deferredElementName = name + } + + fun with(actionWithDeferredElement: (String) -> Unit, actionWithoutDeferredElement: () -> Unit): Unit { + deferredElementName?.let { + reset() + actionWithDeferredElement(it) + } + ?: actionWithoutDeferredElement() + } + + private fun reset() { + deferredElementName = null + } + } } From ed561b5362ef9418da79e267032948817647f7bc Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Thu, 25 Jul 2024 10:13:50 +0100 Subject: [PATCH 4/4] Add comment regarding why value could be null --- .../src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt index dd0392a0346..75080254cdb 100644 --- a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt +++ b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt @@ -142,6 +142,8 @@ internal class DefaultBsonEncoder( override fun encodeSerializableValue(serializer: SerializationStrategy, value: T) { deferredElementHandler.with( { + // When using generics its possible for `value` to be null + // See: https://youtrack.jetbrains.com/issue/KT-66206 if (value != null || configuration.explicitNulls) { encodeName(it) super.encodeSerializableValue(serializer, value)