Skip to content

Kotlinx serialization decoding optional ObjectId / BsonValues fails to hydrate properly #1143

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jun 20, 2023
Merged

Kotlinx serialization decoding optional ObjectId / BsonValues fails to hydrate properly #1143

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6a200ed
Kotlinx serialization decoding optional ObjectId / BsonValues fails t…
rozza Jun 16, 2023
0db6607
Updated BsonValues tests
rozza Jun 19, 2023
331c666
Simplify element decoding logic and ensuring all element indexes are …
rozza Jun 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 26 additions & 26 deletions bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonDecoder.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -61,55 +60,56 @@ 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<ElementMetadata>? = null
private var currentIndex: Int = UNKNOWN_INDEX

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)
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 -> {
reader.readBsonType()
return decodeElementIndex(descriptor)
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 }

return if (indexOfNullableElement == -1) {
DECODE_DONE
} else {
isNullableIndexes[indexOfNullableElement] = false
indexOfNullableElement
}
}
AbstractBsonReader.State.END_OF_ARRAY ->
return elementMetadata.indexOfFirst { it.nullable && !it.processed }
else -> null
}

return name?.let {
val index = descriptor.getElementIndex(it)
return if (index == UNKNOWN_NAME) {
reader.skipValue()
decodeElementIndex(descriptor)
decodeElementIndexImpl(descriptor)
} else {
index
}
Expand Down
178 changes: 140 additions & 38 deletions bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodecTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,13 @@ 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.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
Expand All @@ -43,6 +47,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
Expand Down Expand Up @@ -72,7 +77,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.DataClassWithPair
import org.bson.codecs.kotlinx.samples.DataClassWithParameterizedDataClass
import org.bson.codecs.kotlinx.samples.DataClassWithRequired
Expand All @@ -81,7 +85,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

Expand All @@ -92,6 +95,40 @@ 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},
| "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 =
Expand Down Expand Up @@ -432,44 +469,109 @@ 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)
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["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 =
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["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)

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
Expand Down
Loading