From 6a200ed7e2e51f8a77f972537bcb9080db01eafd Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Fri, 16 Jun 2023 15:48:52 +0100 Subject: [PATCH 1/3] Kotlinx serialization decoding optional ObjectId / BsonValues fails to hydrate The DefaultBsonDecoder required extra handling to ensure it did not double decode optional values and overwrite them with a null value. Tracking currentIndex and processed status allows the default decoder to correctly track if all elements have been processed and report the correct value when calling decodeElementIndex. JAVA-5031 --- .../org/bson/codecs/kotlinx/BsonDecoder.kt | 55 ++++++++++++------- .../kotlinx/KotlinSerializerCodecTest.kt | 47 ++++++++++++++++ .../codecs/kotlinx/samples/DataClasses.kt | 6 ++ driver-kotlin-sync/build.gradle.kts | 3 +- 4 files changed, 89 insertions(+), 22 deletions(-) diff --git a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonDecoder.kt b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonDecoder.kt index 14a6e9e22ab..25c39a122dc 100644 --- a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonDecoder.kt +++ b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonDecoder.kt @@ -23,7 +23,6 @@ import kotlinx.serialization.descriptors.PrimitiveKind import kotlinx.serialization.descriptors.SerialDescriptor import kotlinx.serialization.descriptors.SerialKind import kotlinx.serialization.descriptors.StructureKind -import kotlinx.serialization.descriptors.elementDescriptors import kotlinx.serialization.encoding.AbstractDecoder import kotlinx.serialization.encoding.CompositeDecoder import kotlinx.serialization.encoding.CompositeDecoder.Companion.DECODE_DONE @@ -61,45 +60,58 @@ internal open class DefaultBsonDecoder( internal val configuration: BsonConfiguration ) : BsonDecoder, AbstractDecoder() { + private data class ElementMetadata(val name: String, val nullable: Boolean, var processed: Boolean = false) + private var elementsMetadata: Array? = null + private var currentIndex: Int = UNKNOWN_INDEX + private var level = 0 + companion object { val validKeyKinds = setOf(PrimitiveKind.STRING, PrimitiveKind.CHAR, SerialKind.ENUM) val bsonValueCodec = BsonValueCodec() + const val UNKNOWN_INDEX = -10 } - private var elementsIsNullableIndexes: BooleanArray? = null - - private fun initElementNullsIndexes(descriptor: SerialDescriptor) { - if (elementsIsNullableIndexes != null) return - val elementIndexes = BooleanArray(descriptor.elementsCount) - descriptor.elementDescriptors.withIndex().forEach { - elementIndexes[it.index] = !descriptor.isElementOptional(it.index) && it.value.isNullable - } - elementsIsNullableIndexes = elementIndexes + private fun initElementMetadata(descriptor: SerialDescriptor) { + if (this.elementsMetadata != null) return + val elementsMetadata = + Array(descriptor.elementsCount) { + val elementDescriptor = descriptor.getElementDescriptor(it) + ElementMetadata( + elementDescriptor.serialName, elementDescriptor.isNullable && !descriptor.isElementOptional(it)) + } + this.elementsMetadata = elementsMetadata } - @Suppress("ReturnCount") override fun decodeElementIndex(descriptor: SerialDescriptor): Int { - initElementNullsIndexes(descriptor) + initElementMetadata(descriptor) + currentIndex = decodeElementIndexImpl(descriptor) + return currentIndex + } + @Suppress("ReturnCount", "ComplexMethod") + private fun decodeElementIndexImpl(descriptor: SerialDescriptor): Int { val name: String? = when (reader.state ?: error("State of reader may not be null.")) { AbstractBsonReader.State.NAME -> reader.readName() AbstractBsonReader.State.VALUE -> reader.currentName AbstractBsonReader.State.TYPE -> { - reader.readBsonType() - return decodeElementIndex(descriptor) + val type = reader.readBsonType() + if (type.isContainer) { + level++ + } + return decodeElementIndexImpl(descriptor) } AbstractBsonReader.State.END_OF_DOCUMENT, AbstractBsonReader.State.END_OF_ARRAY -> { - val isNullableIndexes = - elementsIsNullableIndexes ?: error("elementsIsNullableIndexes may not be null.") - val indexOfNullableElement = isNullableIndexes.indexOfFirst { it } + level-- + val elementMetadata = elementsMetadata ?: error("elementsMetadata may not be null.") - return if (indexOfNullableElement == -1) { + return if (level == 0) { DECODE_DONE } else { - isNullableIndexes[indexOfNullableElement] = false - indexOfNullableElement + currentIndex = elementMetadata.indexOfFirst { it.nullable && !it.processed } + if (currentIndex >= 0) elementMetadata[currentIndex].processed = true + return currentIndex } } else -> null @@ -109,7 +121,7 @@ internal open class DefaultBsonDecoder( val index = descriptor.getElementIndex(it) return if (index == UNKNOWN_NAME) { reader.skipValue() - decodeElementIndex(descriptor) + decodeElementIndexImpl(descriptor) } else { index } @@ -182,6 +194,7 @@ internal open class DefaultBsonDecoder( private inline fun readOrThrow(action: () -> T, bsonType: BsonType): T { return try { + elementsMetadata?.get(currentIndex)?.processed = true action() } catch (e: BsonInvalidOperationException) { throw BsonInvalidOperationException( 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 a54e0ee3818..cd12c68df81 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 @@ -73,6 +73,7 @@ import org.bson.codecs.kotlinx.samples.DataClassWithNestedParameterized import org.bson.codecs.kotlinx.samples.DataClassWithNestedParameterizedDataClass import org.bson.codecs.kotlinx.samples.DataClassWithNulls import org.bson.codecs.kotlinx.samples.DataClassWithObjectIdAndBsonDocument +import org.bson.codecs.kotlinx.samples.DataClassWithOptionalObjectIdAndBsonDocument import org.bson.codecs.kotlinx.samples.DataClassWithPair import org.bson.codecs.kotlinx.samples.DataClassWithParameterizedDataClass import org.bson.codecs.kotlinx.samples.DataClassWithRequired @@ -472,6 +473,52 @@ class KotlinSerializerCodecTest { assertRoundTrips(expected, dataClass) } + @Test + fun testDataClassWithOptionalObjectIdAndBsonDocument() { + val subDocument = + """{ + | "_id": 1, + | "arrayEmpty": [], + | "arraySimple": [{"${'$'}numberInt": "1"}, {"${'$'}numberInt": "2"}, {"${'$'}numberInt": "3"}], + | "arrayComplex": [{"a": {"${'$'}numberInt": "1"}}, {"a": {"${'$'}numberInt": "2"}}], + | "arrayMixedTypes": [{"${'$'}numberInt": "1"}, {"${'$'}numberInt": "2"}, true, + | [{"${'$'}numberInt": "1"}, {"${'$'}numberInt": "2"}, {"${'$'}numberInt": "3"}], + | {"a": {"${'$'}numberInt": "2"}}], + | "arrayComplexMixedTypes": [{"a": {"${'$'}numberInt": "1"}}, {"a": "a"}], + | "binary": {"${'$'}binary": {"base64": "S2Fma2Egcm9ja3Mh", "subType": "00"}}, + | "boolean": true, + | "code": {"${'$'}code": "int i = 0;"}, + | "codeWithScope": {"${'$'}code": "int x = y", "${'$'}scope": {"y": {"${'$'}numberInt": "1"}}}, + | "dateTime": {"${'$'}date": {"${'$'}numberLong": "1577836801000"}}, + | "decimal128": {"${'$'}numberDecimal": "1.0"}, + | "documentEmpty": {}, + | "document": {"a": {"${'$'}numberInt": "1"}}, + | "double": {"${'$'}numberDouble": "62.0"}, + | "int32": {"${'$'}numberInt": "42"}, + | "int64": {"${'$'}numberLong": "52"}, + | "maxKey": {"${'$'}maxKey": 1}, + | "minKey": {"${'$'}minKey": 1}, + | "null": null, + | "objectId": {"${'$'}oid": "5f3d1bbde0ca4d2829c91e1d"}, + | "regex": {"${'$'}regularExpression": {"pattern": "^test.*regex.*xyz$", "options": "i"}}, + | "string": "the fox ...", + | "symbol": {"${'$'}symbol": "ruby stuff"}, + | "timestamp": {"${'$'}timestamp": {"t": 305419896, "i": 5}}, + | "undefined": {"${'$'}undefined": true} + | }""" + .trimMargin() + val expected = """{"objectId": {"${'$'}oid": "111111111111111111111111"}, "bsonDocument": $subDocument}""" + + val dataClass = + DataClassWithOptionalObjectIdAndBsonDocument( + ObjectId("111111111111111111111111"), BsonDocument.parse(subDocument)) + assertRoundTrips(expected, dataClass) + + val expectedWithNulls = """{}""" + val dataClassWithNulls = DataClassWithOptionalObjectIdAndBsonDocument(null, null) + assertRoundTrips(expectedWithNulls, dataClassWithNulls) + } + @Test fun testDataClassSealed() { val expectedA = """{"a": "string"}""" 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 7d85fd9e739..c5ba953e7c2 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 @@ -167,6 +167,12 @@ data class DataClassWithObjectIdAndBsonDocument( @Contextual val bsonDocument: BsonDocument ) +@Serializable +data class DataClassWithOptionalObjectIdAndBsonDocument( + @Contextual val objectId: ObjectId?, + @Contextual val bsonDocument: BsonDocument? +) + @Serializable sealed class DataClassSealed @Serializable data class DataClassSealedA(val a: String) : DataClassSealed() diff --git a/driver-kotlin-sync/build.gradle.kts b/driver-kotlin-sync/build.gradle.kts index 80f4ebf14fd..c29d8c5e42e 100644 --- a/driver-kotlin-sync/build.gradle.kts +++ b/driver-kotlin-sync/build.gradle.kts @@ -19,6 +19,7 @@ import org.jetbrains.kotlin.gradle.tasks.KotlinCompile plugins { id("org.jetbrains.kotlin.jvm") `java-library` + kotlin("plugin.serialization") // Test based plugins id("com.diffplug.spotless") @@ -72,7 +73,7 @@ dependencies { integrationTestImplementation("org.jetbrains.kotlin:kotlin-test-junit") integrationTestImplementation(project(path = ":driver-sync")) integrationTestImplementation(project(path = ":driver-core")) - integrationTestImplementation(project(path = ":bson")) + integrationTestImplementation(project(path = ":bson-kotlinx")) } kotlin { explicitApi() } From 0db660771b4aead42a7f55825212b0da0957fd6b Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Mon, 19 Jun 2023 11:09:14 +0100 Subject: [PATCH 2/3] Updated BsonValues tests --- .../kotlinx/KotlinSerializerCodecTest.kt | 219 +++++++++++------- .../codecs/kotlinx/samples/DataClasses.kt | 79 ++++++- 2 files changed, 209 insertions(+), 89 deletions(-) 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 cd12c68df81..ed62e3176e4 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 @@ -27,9 +27,14 @@ import org.bson.BsonDocument import org.bson.BsonDocumentReader import org.bson.BsonDocumentWriter import org.bson.BsonInvalidOperationException +import org.bson.BsonMaxKey +import org.bson.BsonMinKey +import org.bson.BsonNull +import org.bson.BsonUndefined import org.bson.codecs.DecoderContext import org.bson.codecs.EncoderContext import org.bson.codecs.configuration.CodecConfigurationException +import org.bson.codecs.kotlinx.samples.DataClassBsonValues import org.bson.codecs.kotlinx.samples.DataClassContainsOpen import org.bson.codecs.kotlinx.samples.DataClassContainsValueClass import org.bson.codecs.kotlinx.samples.DataClassEmbedded @@ -43,6 +48,7 @@ import org.bson.codecs.kotlinx.samples.DataClassNestedParameterizedTypes import org.bson.codecs.kotlinx.samples.DataClassOpen import org.bson.codecs.kotlinx.samples.DataClassOpenA import org.bson.codecs.kotlinx.samples.DataClassOpenB +import org.bson.codecs.kotlinx.samples.DataClassOptionalBsonValues import org.bson.codecs.kotlinx.samples.DataClassParameterized import org.bson.codecs.kotlinx.samples.DataClassSealed import org.bson.codecs.kotlinx.samples.DataClassSealedA @@ -72,8 +78,6 @@ import org.bson.codecs.kotlinx.samples.DataClassWithMutableSet import org.bson.codecs.kotlinx.samples.DataClassWithNestedParameterized import org.bson.codecs.kotlinx.samples.DataClassWithNestedParameterizedDataClass import org.bson.codecs.kotlinx.samples.DataClassWithNulls -import org.bson.codecs.kotlinx.samples.DataClassWithObjectIdAndBsonDocument -import org.bson.codecs.kotlinx.samples.DataClassWithOptionalObjectIdAndBsonDocument import org.bson.codecs.kotlinx.samples.DataClassWithPair import org.bson.codecs.kotlinx.samples.DataClassWithParameterizedDataClass import org.bson.codecs.kotlinx.samples.DataClassWithRequired @@ -82,7 +86,6 @@ import org.bson.codecs.kotlinx.samples.DataClassWithSimpleValues import org.bson.codecs.kotlinx.samples.DataClassWithTriple import org.bson.codecs.kotlinx.samples.Key import org.bson.codecs.kotlinx.samples.ValueClass -import org.bson.types.ObjectId import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertThrows @@ -93,6 +96,41 @@ class KotlinSerializerCodecTest { private val altConfiguration = BsonConfiguration(encodeDefaults = false, classDiscriminator = "_t", explicitNulls = true) + private val allBsonTypesJson = + """{ + | "id": {"${'$'}oid": "111111111111111111111111"}, + | "arrayEmpty": [], + | "arraySimple": [{"${'$'}numberInt": "1"}, {"${'$'}numberInt": "2"}, {"${'$'}numberInt": "3"}], + | "arrayComplex": [{"a": {"${'$'}numberInt": "1"}}, {"a": {"${'$'}numberInt": "2"}}], + | "arrayMixedTypes": [{"${'$'}numberInt": "1"}, {"${'$'}numberInt": "2"}, true, + | [{"${'$'}numberInt": "1"}, {"${'$'}numberInt": "2"}, {"${'$'}numberInt": "3"}], + | {"a": {"${'$'}numberInt": "2"}}], + | "arrayComplexMixedTypes": [{"a": {"${'$'}numberInt": "1"}}, {"a": "a"}], + | "binary": {"${'$'}binary": {"base64": "S2Fma2Egcm9ja3Mh", "subType": "00"}}, + | "boolean": true, + | "code": {"${'$'}code": "int i = 0;"}, + | "codeWithScope": {"${'$'}code": "int x = y", "${'$'}scope": {"y": {"${'$'}numberInt": "1"}}}, + | "dateTime": {"${'$'}date": {"${'$'}numberLong": "1577836801000"}}, + | "decimal128": {"${'$'}numberDecimal": "1.0"}, + | "documentEmpty": {}, + | "document": {"a": {"${'$'}numberInt": "1"}}, + | "double": {"${'$'}numberDouble": "62.0"}, + | "int32": {"${'$'}numberInt": "42"}, + | "int64": {"${'$'}numberLong": "52"}, + | "maxKey": {"${'$'}maxKey": 1}, + | "minKey": {"${'$'}minKey": 1}, + | "null": null, + | "objectId": {"${'$'}oid": "211111111111111111111112"}, + | "regex": {"${'$'}regularExpression": {"pattern": "^test.*regex.*xyz$", "options": "i"}}, + | "string": "the fox ...", + | "symbol": {"${'$'}symbol": "ruby stuff"}, + | "timestamp": {"${'$'}timestamp": {"t": 305419896, "i": 5}}, + | "undefined": {"${'$'}undefined": true} + | }""" + .trimMargin() + + private val allBsonTypesDocument = BsonDocument.parse(allBsonTypesJson) + @Test fun testDataClassWithSimpleValues() { val expected = @@ -433,90 +471,105 @@ class KotlinSerializerCodecTest { } @Test - fun testDataClassWithObjectIdAndBsonDocument() { - val subDocument = - """{ - | "_id": 1, - | "arrayEmpty": [], - | "arraySimple": [{"${'$'}numberInt": "1"}, {"${'$'}numberInt": "2"}, {"${'$'}numberInt": "3"}], - | "arrayComplex": [{"a": {"${'$'}numberInt": "1"}}, {"a": {"${'$'}numberInt": "2"}}], - | "arrayMixedTypes": [{"${'$'}numberInt": "1"}, {"${'$'}numberInt": "2"}, true, - | [{"${'$'}numberInt": "1"}, {"${'$'}numberInt": "2"}, {"${'$'}numberInt": "3"}], - | {"a": {"${'$'}numberInt": "2"}}], - | "arrayComplexMixedTypes": [{"a": {"${'$'}numberInt": "1"}}, {"a": "a"}], - | "binary": {"${'$'}binary": {"base64": "S2Fma2Egcm9ja3Mh", "subType": "00"}}, - | "boolean": true, - | "code": {"${'$'}code": "int i = 0;"}, - | "codeWithScope": {"${'$'}code": "int x = y", "${'$'}scope": {"y": {"${'$'}numberInt": "1"}}}, - | "dateTime": {"${'$'}date": {"${'$'}numberLong": "1577836801000"}}, - | "decimal128": {"${'$'}numberDecimal": "1.0"}, - | "documentEmpty": {}, - | "document": {"a": {"${'$'}numberInt": "1"}}, - | "double": {"${'$'}numberDouble": "62.0"}, - | "int32": {"${'$'}numberInt": "42"}, - | "int64": {"${'$'}numberLong": "52"}, - | "maxKey": {"${'$'}maxKey": 1}, - | "minKey": {"${'$'}minKey": 1}, - | "null": null, - | "objectId": {"${'$'}oid": "5f3d1bbde0ca4d2829c91e1d"}, - | "regex": {"${'$'}regularExpression": {"pattern": "^test.*regex.*xyz$", "options": "i"}}, - | "string": "the fox ...", - | "symbol": {"${'$'}symbol": "ruby stuff"}, - | "timestamp": {"${'$'}timestamp": {"t": 305419896, "i": 5}}, - | "undefined": {"${'$'}undefined": true} - | }""" - .trimMargin() - val expected = """{"objectId": {"${'$'}oid": "111111111111111111111111"}, "bsonDocument": $subDocument}""" + fun testDataClassBsonValues() { val dataClass = - DataClassWithObjectIdAndBsonDocument(ObjectId("111111111111111111111111"), BsonDocument.parse(subDocument)) - assertRoundTrips(expected, dataClass) - } - - @Test - fun testDataClassWithOptionalObjectIdAndBsonDocument() { - val subDocument = - """{ - | "_id": 1, - | "arrayEmpty": [], - | "arraySimple": [{"${'$'}numberInt": "1"}, {"${'$'}numberInt": "2"}, {"${'$'}numberInt": "3"}], - | "arrayComplex": [{"a": {"${'$'}numberInt": "1"}}, {"a": {"${'$'}numberInt": "2"}}], - | "arrayMixedTypes": [{"${'$'}numberInt": "1"}, {"${'$'}numberInt": "2"}, true, - | [{"${'$'}numberInt": "1"}, {"${'$'}numberInt": "2"}, {"${'$'}numberInt": "3"}], - | {"a": {"${'$'}numberInt": "2"}}], - | "arrayComplexMixedTypes": [{"a": {"${'$'}numberInt": "1"}}, {"a": "a"}], - | "binary": {"${'$'}binary": {"base64": "S2Fma2Egcm9ja3Mh", "subType": "00"}}, - | "boolean": true, - | "code": {"${'$'}code": "int i = 0;"}, - | "codeWithScope": {"${'$'}code": "int x = y", "${'$'}scope": {"y": {"${'$'}numberInt": "1"}}}, - | "dateTime": {"${'$'}date": {"${'$'}numberLong": "1577836801000"}}, - | "decimal128": {"${'$'}numberDecimal": "1.0"}, - | "documentEmpty": {}, - | "document": {"a": {"${'$'}numberInt": "1"}}, - | "double": {"${'$'}numberDouble": "62.0"}, - | "int32": {"${'$'}numberInt": "42"}, - | "int64": {"${'$'}numberLong": "52"}, - | "maxKey": {"${'$'}maxKey": 1}, - | "minKey": {"${'$'}minKey": 1}, - | "null": null, - | "objectId": {"${'$'}oid": "5f3d1bbde0ca4d2829c91e1d"}, - | "regex": {"${'$'}regularExpression": {"pattern": "^test.*regex.*xyz$", "options": "i"}}, - | "string": "the fox ...", - | "symbol": {"${'$'}symbol": "ruby stuff"}, - | "timestamp": {"${'$'}timestamp": {"t": 305419896, "i": 5}}, - | "undefined": {"${'$'}undefined": true} - | }""" - .trimMargin() - val expected = """{"objectId": {"${'$'}oid": "111111111111111111111111"}, "bsonDocument": $subDocument}""" + DataClassBsonValues( + allBsonTypesDocument["id"]!!.asObjectId().value, + allBsonTypesDocument["arrayEmpty"]!!.asArray(), + allBsonTypesDocument["arraySimple"]!!.asArray(), + allBsonTypesDocument["arrayComplex"]!!.asArray(), + allBsonTypesDocument["arrayMixedTypes"]!!.asArray(), + allBsonTypesDocument["arrayComplexMixedTypes"]!!.asArray(), + allBsonTypesDocument["binary"]!!.asBinary(), + allBsonTypesDocument["boolean"]!!.asBoolean(), + allBsonTypesDocument["code"]!!.asJavaScript(), + allBsonTypesDocument["codeWithScope"]!!.asJavaScriptWithScope(), + allBsonTypesDocument["dateTime"]!!.asDateTime(), + allBsonTypesDocument["decimal128"]!!.asDecimal128(), + allBsonTypesDocument["documentEmpty"]!!.asDocument(), + allBsonTypesDocument["document"]!!.asDocument(), + allBsonTypesDocument["double"]!!.asDouble(), + allBsonTypesDocument["int32"]!!.asInt32(), + allBsonTypesDocument["int64"]!!.asInt64(), + allBsonTypesDocument["maxKey"]!! as BsonMaxKey, + allBsonTypesDocument["minKey"]!! as BsonMinKey, + allBsonTypesDocument["null"]!! as BsonNull, + allBsonTypesDocument["objectId"]!!.asObjectId(), + allBsonTypesDocument["regex"]!!.asRegularExpression(), + allBsonTypesDocument["string"]!!.asString(), + allBsonTypesDocument["symbol"]!!.asSymbol(), + allBsonTypesDocument["timestamp"]!!.asTimestamp(), + allBsonTypesDocument["undefined"]!! as BsonUndefined) + + assertRoundTrips(allBsonTypesJson, dataClass) + } + + @Test + fun testDataClassOptionalBsonValues() { val dataClass = - DataClassWithOptionalObjectIdAndBsonDocument( - ObjectId("111111111111111111111111"), BsonDocument.parse(subDocument)) - assertRoundTrips(expected, dataClass) - - val expectedWithNulls = """{}""" - val dataClassWithNulls = DataClassWithOptionalObjectIdAndBsonDocument(null, null) - assertRoundTrips(expectedWithNulls, dataClassWithNulls) + DataClassOptionalBsonValues( + allBsonTypesDocument["id"]!!.asObjectId().value, + allBsonTypesDocument["arrayEmpty"]!!.asArray(), + allBsonTypesDocument["arraySimple"]!!.asArray(), + allBsonTypesDocument["arrayComplex"]!!.asArray(), + allBsonTypesDocument["arrayMixedTypes"]!!.asArray(), + allBsonTypesDocument["arrayComplexMixedTypes"]!!.asArray(), + allBsonTypesDocument["binary"]!!.asBinary(), + allBsonTypesDocument["boolean"]!!.asBoolean(), + allBsonTypesDocument["code"]!!.asJavaScript(), + allBsonTypesDocument["codeWithScope"]!!.asJavaScriptWithScope(), + allBsonTypesDocument["dateTime"]!!.asDateTime(), + allBsonTypesDocument["decimal128"]!!.asDecimal128(), + allBsonTypesDocument["documentEmpty"]!!.asDocument(), + allBsonTypesDocument["document"]!!.asDocument(), + allBsonTypesDocument["double"]!!.asDouble(), + allBsonTypesDocument["int32"]!!.asInt32(), + allBsonTypesDocument["int64"]!!.asInt64(), + allBsonTypesDocument["maxKey"]!! as BsonMaxKey, + allBsonTypesDocument["minKey"]!! as BsonMinKey, + allBsonTypesDocument["null"]!! as BsonNull, + allBsonTypesDocument["objectId"]!!.asObjectId(), + allBsonTypesDocument["regex"]!!.asRegularExpression(), + allBsonTypesDocument["string"]!!.asString(), + allBsonTypesDocument["symbol"]!!.asSymbol(), + allBsonTypesDocument["timestamp"]!!.asTimestamp(), + allBsonTypesDocument["undefined"]!! as BsonUndefined) + + assertRoundTrips(allBsonTypesJson, dataClass) + + val emptyDataClass = + DataClassOptionalBsonValues( + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + null, + ) + + assertRoundTrips("{}", emptyDataClass) } @Test 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 c5ba953e7c2..9bcd6ff26e7 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 @@ -21,8 +21,27 @@ import kotlinx.serialization.ExperimentalSerializationApi import kotlinx.serialization.Required import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable +import org.bson.BsonArray +import org.bson.BsonBinary +import org.bson.BsonBoolean +import org.bson.BsonDateTime +import org.bson.BsonDecimal128 import org.bson.BsonDocument +import org.bson.BsonDouble +import org.bson.BsonInt32 +import org.bson.BsonInt64 +import org.bson.BsonJavaScript +import org.bson.BsonJavaScriptWithScope +import org.bson.BsonMaxKey +import org.bson.BsonMinKey +import org.bson.BsonNull +import org.bson.BsonObjectId +import org.bson.BsonRegularExpression +import org.bson.BsonString +import org.bson.BsonSymbol +import org.bson.BsonTimestamp import org.bson.BsonType +import org.bson.BsonUndefined import org.bson.codecs.pojo.annotations.BsonCreator import org.bson.codecs.pojo.annotations.BsonDiscriminator import org.bson.codecs.pojo.annotations.BsonExtraElements @@ -162,15 +181,63 @@ enum class Key { @Serializable data class DataClassWithDataClassMapKey(val map: Map) @Serializable -data class DataClassWithObjectIdAndBsonDocument( - @Contextual val objectId: ObjectId, - @Contextual val bsonDocument: BsonDocument +data class DataClassBsonValues( + @Contextual val id: ObjectId, + @Contextual val arrayEmpty: BsonArray, + @Contextual val arraySimple: BsonArray, + @Contextual val arrayComplex: BsonArray, + @Contextual val arrayMixedTypes: BsonArray, + @Contextual val arrayComplexMixedTypes: BsonArray, + @Contextual val binary: BsonBinary, + @Contextual val boolean: BsonBoolean, + @Contextual val code: BsonJavaScript, + @Contextual val codeWithScope: BsonJavaScriptWithScope, + @Contextual val dateTime: BsonDateTime, + @Contextual val decimal128: BsonDecimal128, + @Contextual val documentEmpty: BsonDocument, + @Contextual val document: BsonDocument, + @Contextual val double: BsonDouble, + @Contextual val int32: BsonInt32, + @Contextual val int64: BsonInt64, + @Contextual val maxKey: BsonMaxKey, + @Contextual val minKey: BsonMinKey, + @Contextual val `null`: BsonNull, + @Contextual val objectId: BsonObjectId, + @Contextual val regex: BsonRegularExpression, + @Contextual val string: BsonString, + @Contextual val symbol: BsonSymbol, + @Contextual val timestamp: BsonTimestamp, + @Contextual val undefined: BsonUndefined, ) @Serializable -data class DataClassWithOptionalObjectIdAndBsonDocument( - @Contextual val objectId: ObjectId?, - @Contextual val bsonDocument: BsonDocument? +data class DataClassOptionalBsonValues( + @Contextual val id: ObjectId?, + @Contextual val arrayEmpty: BsonArray?, + @Contextual val arraySimple: BsonArray?, + @Contextual val arrayComplex: BsonArray?, + @Contextual val arrayMixedTypes: BsonArray?, + @Contextual val arrayComplexMixedTypes: BsonArray?, + @Contextual val binary: BsonBinary?, + @Contextual val boolean: BsonBoolean?, + @Contextual val code: BsonJavaScript?, + @Contextual val codeWithScope: BsonJavaScriptWithScope?, + @Contextual val dateTime: BsonDateTime?, + @Contextual val decimal128: BsonDecimal128?, + @Contextual val documentEmpty: BsonDocument?, + @Contextual val document: BsonDocument?, + @Contextual val double: BsonDouble?, + @Contextual val int32: BsonInt32?, + @Contextual val int64: BsonInt64?, + @Contextual val maxKey: BsonMaxKey?, + @Contextual val minKey: BsonMinKey?, + @Contextual val `null`: BsonNull?, + @Contextual val objectId: BsonObjectId?, + @Contextual val regex: BsonRegularExpression?, + @Contextual val string: BsonString?, + @Contextual val symbol: BsonSymbol?, + @Contextual val timestamp: BsonTimestamp?, + @Contextual val undefined: BsonUndefined?, ) @Serializable sealed class DataClassSealed From 331c66609d1b66e205ec7ef254948b4574b6b317 Mon Sep 17 00:00:00 2001 From: Ross Lawley Date: Mon, 19 Jun 2023 12:00:06 +0100 Subject: [PATCH 3/3] Simplify element decoding logic and ensuring all element indexes are accounted for --- .../org/bson/codecs/kotlinx/BsonDecoder.kt | 23 ++++--------------- .../kotlinx/KotlinSerializerCodecTest.kt | 18 ++++++++------- .../codecs/kotlinx/samples/DataClasses.kt | 3 --- config/detekt/baseline.xml | 1 + 4 files changed, 16 insertions(+), 29 deletions(-) diff --git a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonDecoder.kt b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonDecoder.kt index 25c39a122dc..b4cbad3b9dd 100644 --- a/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonDecoder.kt +++ b/bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonDecoder.kt @@ -63,7 +63,6 @@ internal open class DefaultBsonDecoder( private data class ElementMetadata(val name: String, val nullable: Boolean, var processed: Boolean = false) private var elementsMetadata: Array? = null private var currentIndex: Int = UNKNOWN_INDEX - private var level = 0 companion object { val validKeyKinds = setOf(PrimitiveKind.STRING, PrimitiveKind.CHAR, SerialKind.ENUM) @@ -85,35 +84,24 @@ internal open class DefaultBsonDecoder( override fun decodeElementIndex(descriptor: SerialDescriptor): Int { initElementMetadata(descriptor) currentIndex = decodeElementIndexImpl(descriptor) + elementsMetadata?.getOrNull(currentIndex)?.processed = true return currentIndex } @Suppress("ReturnCount", "ComplexMethod") private fun decodeElementIndexImpl(descriptor: SerialDescriptor): Int { + val elementMetadata = elementsMetadata ?: error("elementsMetadata may not be null.") val name: String? = when (reader.state ?: error("State of reader may not be null.")) { AbstractBsonReader.State.NAME -> reader.readName() AbstractBsonReader.State.VALUE -> reader.currentName AbstractBsonReader.State.TYPE -> { - val type = reader.readBsonType() - if (type.isContainer) { - level++ - } + reader.readBsonType() return decodeElementIndexImpl(descriptor) } AbstractBsonReader.State.END_OF_DOCUMENT, - AbstractBsonReader.State.END_OF_ARRAY -> { - level-- - val elementMetadata = elementsMetadata ?: error("elementsMetadata may not be null.") - - return if (level == 0) { - DECODE_DONE - } else { - currentIndex = elementMetadata.indexOfFirst { it.nullable && !it.processed } - if (currentIndex >= 0) elementMetadata[currentIndex].processed = true - return currentIndex - } - } + AbstractBsonReader.State.END_OF_ARRAY -> + return elementMetadata.indexOfFirst { it.nullable && !it.processed } else -> null } @@ -194,7 +182,6 @@ internal open class DefaultBsonDecoder( private inline fun readOrThrow(action: () -> T, bsonType: BsonType): T { return try { - elementsMetadata?.get(currentIndex)?.processed = true action() } catch (e: BsonInvalidOperationException) { throw BsonInvalidOperationException( 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 ed62e3176e4..146e897c59b 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 @@ -29,7 +29,6 @@ import org.bson.BsonDocumentWriter import org.bson.BsonInvalidOperationException import org.bson.BsonMaxKey import org.bson.BsonMinKey -import org.bson.BsonNull import org.bson.BsonUndefined import org.bson.codecs.DecoderContext import org.bson.codecs.EncoderContext @@ -119,7 +118,6 @@ class KotlinSerializerCodecTest { | "int64": {"${'$'}numberLong": "52"}, | "maxKey": {"${'$'}maxKey": 1}, | "minKey": {"${'$'}minKey": 1}, - | "null": null, | "objectId": {"${'$'}oid": "211111111111111111111112"}, | "regex": {"${'$'}regularExpression": {"pattern": "^test.*regex.*xyz$", "options": "i"}}, | "string": "the fox ...", @@ -494,7 +492,6 @@ class KotlinSerializerCodecTest { allBsonTypesDocument["int64"]!!.asInt64(), allBsonTypesDocument["maxKey"]!! as BsonMaxKey, allBsonTypesDocument["minKey"]!! as BsonMinKey, - allBsonTypesDocument["null"]!! as BsonNull, allBsonTypesDocument["objectId"]!!.asObjectId(), allBsonTypesDocument["regex"]!!.asRegularExpression(), allBsonTypesDocument["string"]!!.asString(), @@ -507,7 +504,6 @@ class KotlinSerializerCodecTest { @Test fun testDataClassOptionalBsonValues() { - val dataClass = DataClassOptionalBsonValues( allBsonTypesDocument["id"]!!.asObjectId().value, @@ -529,7 +525,6 @@ class KotlinSerializerCodecTest { allBsonTypesDocument["int64"]!!.asInt64(), allBsonTypesDocument["maxKey"]!! as BsonMaxKey, allBsonTypesDocument["minKey"]!! as BsonMinKey, - allBsonTypesDocument["null"]!! as BsonNull, allBsonTypesDocument["objectId"]!!.asObjectId(), allBsonTypesDocument["regex"]!!.asRegularExpression(), allBsonTypesDocument["string"]!!.asString(), @@ -565,11 +560,18 @@ class KotlinSerializerCodecTest { null, null, null, - null, - null, - ) + null) assertRoundTrips("{}", emptyDataClass) + assertRoundTrips( + """{ "id": null, "arrayEmpty": null, "arraySimple": null, "arrayComplex": null, "arrayMixedTypes": null, + | "arrayComplexMixedTypes": null, "binary": null, "boolean": null, "code": null, "codeWithScope": null, + | "dateTime": null, "decimal128": null, "documentEmpty": null, "document": null, "double": null, + | "int32": null, "int64": null, "maxKey": null, "minKey": null, "objectId": null, "regex": null, + | "string": null, "symbol": null, "timestamp": null, "undefined": null }""" + .trimMargin(), + emptyDataClass, + BsonConfiguration(explicitNulls = true)) } @Test 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 9bcd6ff26e7..ea5e3fea3cd 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 @@ -34,7 +34,6 @@ import org.bson.BsonJavaScript import org.bson.BsonJavaScriptWithScope import org.bson.BsonMaxKey import org.bson.BsonMinKey -import org.bson.BsonNull import org.bson.BsonObjectId import org.bson.BsonRegularExpression import org.bson.BsonString @@ -201,7 +200,6 @@ data class DataClassBsonValues( @Contextual val int64: BsonInt64, @Contextual val maxKey: BsonMaxKey, @Contextual val minKey: BsonMinKey, - @Contextual val `null`: BsonNull, @Contextual val objectId: BsonObjectId, @Contextual val regex: BsonRegularExpression, @Contextual val string: BsonString, @@ -231,7 +229,6 @@ data class DataClassOptionalBsonValues( @Contextual val int64: BsonInt64?, @Contextual val maxKey: BsonMaxKey?, @Contextual val minKey: BsonMinKey?, - @Contextual val `null`: BsonNull?, @Contextual val objectId: BsonObjectId?, @Contextual val regex: BsonRegularExpression?, @Contextual val string: BsonString?, diff --git a/config/detekt/baseline.xml b/config/detekt/baseline.xml index 0f6b17d2460..c899728c7be 100644 --- a/config/detekt/baseline.xml +++ b/config/detekt/baseline.xml @@ -6,6 +6,7 @@ LargeClass:MongoCollectionTest.kt$MongoCollectionTest LongMethod:FindFlowTest.kt$FindFlowTest$@Suppress("DEPRECATION") @Test fun shouldCallTheUnderlyingMethods() LongMethod:FindIterableTest.kt$FindIterableTest$@Suppress("DEPRECATION") @Test fun shouldCallTheUnderlyingMethods() + LongMethod:KotlinSerializerCodecTest.kt$KotlinSerializerCodecTest$@Test fun testDataClassOptionalBsonValues() MaxLineLength:MapReduceFlow.kt$MapReduceFlow$* MaxLineLength:MapReduceIterable.kt$MapReduceIterable$* SwallowedException:MockitoHelper.kt$MockitoHelper.DeepReflectionEqMatcher$e: Throwable