Skip to content

Commit fec5d3f

Browse files
rozzanhachicha
rozza
authored and
nhachicha
committed
Fix Kotlin bson decoding of optionals (#1941)
* Fix Kotlin bson decoding of optionals Previously, DataClassCodec pre-populated all constructor parameters with null before reading the document, which prevented callBy from using Kotlin default parameter values. Now optional parameters missing from the document are left absent from the args map so callBy invokes their defaults, and a clear CodecConfigurationException is thrown when a required non-nullable field is missing. Ported the bson-kotlin test cases to bson-kotlinx to ensure coverage and prevent future regressions. JAVA-6162
1 parent 78ad78f commit fec5d3f

5 files changed

Lines changed: 147 additions & 18 deletions

File tree

‎bson-kotlin/src/main/kotlin/org/bson/codecs/kotlin/DataClassCodec.kt‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ internal data class DataClassCodec<T : Any>(
7676
@Suppress("TooGenericExceptionCaught")
7777
override fun decode(reader: BsonReader, decoderContext: DecoderContext): T {
7878
val args: MutableMap<KParameter, Any?> = mutableMapOf()
79-
fieldNamePropertyModelMap.values.forEach { args[it.param] = null }
8079

8180
reader.readStartDocument()
8281
while (reader.readBsonType() != BsonType.END_OF_DOCUMENT) {
@@ -89,6 +88,7 @@ internal data class DataClassCodec<T : Any>(
8988
}
9089
} else if (propertyModel.param.type.isMarkedNullable && reader.currentBsonType == BsonType.NULL) {
9190
reader.readNull()
91+
args[propertyModel.param] = null
9292
} else {
9393
try {
9494
args[propertyModel.param] = decoderContext.decodeWithChildContext(propertyModel.codec, reader)
@@ -100,6 +100,23 @@ internal data class DataClassCodec<T : Any>(
100100
}
101101
reader.readEndDocument()
102102

103+
// For non-optional parameters missing from the document, fail with a clear message
104+
// if non-nullable, or pass null explicitly if nullable.
105+
// Optional parameters (with defaults) are left absent so callBy uses the default value.
106+
fieldNamePropertyModelMap.values.forEach {
107+
if (it.param !in args && !it.param.isOptional) {
108+
// Only error for concrete types (KClass). Generic type parameters (KTypeParameter)
109+
// may be nullable at runtime even though isMarkedNullable is false at the
110+
// declaration site (e.g. Box<T>(val boxed: T) instantiated as Box<String?>).
111+
if (!it.param.type.isMarkedNullable && it.param.type.classifier is KClass<*>) {
112+
throw CodecConfigurationException(
113+
"Required field '${it.fieldName}' is missing from the document for " +
114+
"${kClass.simpleName} data class")
115+
}
116+
args[it.param] = null
117+
}
118+
}
119+
103120
try {
104121
return primaryConstructor.callBy(args)
105122
} catch (e: Exception) {

‎bson-kotlin/src/test/kotlin/org/bson/codecs/kotlin/DataClassCodecTest.kt‎

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import org.bson.codecs.kotlin.samples.DataClassWithBsonProperty
4848
import org.bson.codecs.kotlin.samples.DataClassWithCollections
4949
import org.bson.codecs.kotlin.samples.DataClassWithDataClassMapKey
5050
import org.bson.codecs.kotlin.samples.DataClassWithDefaults
51+
import org.bson.codecs.kotlin.samples.DataClassWithDefaultsAndNulls
5152
import org.bson.codecs.kotlin.samples.DataClassWithEmbedded
5253
import org.bson.codecs.kotlin.samples.DataClassWithEnum
5354
import org.bson.codecs.kotlin.samples.DataClassWithEnumMapKey
@@ -177,17 +178,71 @@ class DataClassCodecTest {
177178
|}"""
178179
.trimMargin()
179180

180-
val defaultDataClass = DataClassWithDefaults()
181-
assertRoundTrips(expectedDefault, defaultDataClass)
181+
assertRoundTrips(expectedDefault, DataClassWithDefaults())
182+
183+
// Assert no data decodes as expected
184+
assertDecodesTo(BsonDocument.parse(emptyDocument), DataClassWithDefaults())
185+
186+
// Assert some data
187+
assertDecodesTo(BsonDocument.parse("""{"string": "Custom"}"""), DataClassWithDefaults(string = "Custom"))
188+
189+
// Assert all data
190+
val expected =
191+
"""{
192+
| "boolean": true,
193+
| "string": "Custom",
194+
| "listSimple": ["x"]
195+
|}"""
196+
.trimMargin()
197+
198+
assertRoundTrips(expected, DataClassWithDefaults(boolean = true, string = "Custom", listSimple = listOf("x")))
182199
}
183200

184201
@Test
185202
fun testDataClassWithNulls() {
186203
val dataClass = DataClassWithNulls(null, null, null)
187204
assertRoundTrips(emptyDocument, dataClass)
188205

189-
val withStoredNulls = BsonDocument.parse("""{"boolean": null, "string": null, "listSimple": null}""")
190-
assertDecodesTo(withStoredNulls, dataClass)
206+
// Assert all null data decodes as expected
207+
assertDecodesTo(BsonDocument.parse("""{"boolean": null, "string": null, "listSimple": null}"""), dataClass)
208+
209+
// Assert some data
210+
assertDecodesTo(BsonDocument.parse("""{"string": "Custom"}"""), DataClassWithNulls(null, "Custom", null))
211+
212+
// Assert all data
213+
val expected =
214+
"""{
215+
| "boolean": true,
216+
| "string": "Custom",
217+
| "listSimple": ["x"]
218+
|}"""
219+
.trimMargin()
220+
assertRoundTrips(expected, DataClassWithNulls(true, "Custom", listOf("x")))
221+
}
222+
223+
@Test
224+
fun testDataClassWithDefaultsAndNulls() {
225+
// All fields provided
226+
val expected = """{"required": "req", "optional": "opt", "nullable": "nul"}"""
227+
assertRoundTrips(expected, DataClassWithDefaultsAndNulls("req", "opt", "nul"))
228+
229+
// Only required field — optional gets default, nullable gets default (null)
230+
assertDecodesTo(BsonDocument.parse("""{"required": "req"}"""), DataClassWithDefaultsAndNulls("req"))
231+
232+
// Required + nullable explicit null in document
233+
assertDecodesTo(
234+
BsonDocument.parse("""{"required": "req", "nullable": null}"""), DataClassWithDefaultsAndNulls("req"))
235+
236+
// Required + optional overridden, nullable absent
237+
assertDecodesTo(
238+
BsonDocument.parse("""{"required": "req", "optional": "custom"}"""),
239+
DataClassWithDefaultsAndNulls("req", "custom"))
240+
241+
// Missing required field throws
242+
assertThrows<CodecConfigurationException> {
243+
val codec = DataClassCodec.create(DataClassWithDefaultsAndNulls::class, registry())
244+
codec?.decode(BsonDocumentReader(BsonDocument()), DecoderContext.builder().build())
245+
}
191246
}
192247

193248
@Test

‎bson-kotlin/src/test/kotlin/org/bson/codecs/kotlin/samples/DataClasses.kt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,12 @@ data class DataClassWithDefaults(
142142

143143
data class DataClassWithNulls(val boolean: Boolean?, val string: String?, val listSimple: List<String?>?)
144144

145+
data class DataClassWithDefaultsAndNulls(
146+
val required: String,
147+
val optional: String = "default",
148+
val nullable: String? = null
149+
)
150+
145151
data class DataClassWithListThatLastItemDefaultsToNull(val elements: List<DataClassLastItemDefaultsToNull>)
146152

147153
data class DataClassLastItemDefaultsToNull(val required: String, val optional: String? = null)

‎bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/KotlinSerializerCodecTest.kt‎

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,7 @@ import org.bson.codecs.kotlinx.samples.DataClassWithContextualDateValues
8888
import org.bson.codecs.kotlinx.samples.DataClassWithDataClassMapKey
8989
import org.bson.codecs.kotlinx.samples.DataClassWithDateValues
9090
import org.bson.codecs.kotlinx.samples.DataClassWithDefaults
91+
import org.bson.codecs.kotlinx.samples.DataClassWithDefaultsAndNulls
9192
import org.bson.codecs.kotlinx.samples.DataClassWithEmbedded
9293
import org.bson.codecs.kotlinx.samples.DataClassWithEncodeDefault
9394
import org.bson.codecs.kotlinx.samples.DataClassWithEnum
@@ -333,28 +334,71 @@ class KotlinSerializerCodecTest {
333334
|}"""
334335
.trimMargin()
335336

336-
val defaultDataClass = DataClassWithDefaults()
337-
assertRoundTrips(expectedDefault, defaultDataClass)
338-
assertRoundTrips(emptyDocument, defaultDataClass, altConfiguration)
337+
assertRoundTrips(expectedDefault, DataClassWithDefaults())
339338

340-
val expectedSomeOverrides = """{"boolean": true, "listSimple": ["a"]}"""
341-
val someOverridesDataClass = DataClassWithDefaults(boolean = true, listSimple = listOf("a"))
342-
assertRoundTrips(expectedSomeOverrides, someOverridesDataClass, altConfiguration)
339+
// Assert no data decodes as expected
340+
assertDecodesTo(BsonDocument.parse(emptyDocument), DataClassWithDefaults())
341+
342+
// Assert some data
343+
assertDecodesTo(BsonDocument.parse("""{"string": "Custom"}"""), DataClassWithDefaults(string = "Custom"))
344+
345+
// Assert all data
346+
val expected =
347+
"""{
348+
| "boolean": true,
349+
| "string": "Custom",
350+
| "listSimple": ["x"]
351+
|}"""
352+
.trimMargin()
353+
354+
assertRoundTrips(expected, DataClassWithDefaults(boolean = true, string = "Custom", listSimple = listOf("x")))
343355
}
344356

345357
@Test
346358
fun testDataClassWithNulls() {
347-
val expectedNulls =
359+
val dataClass = DataClassWithNulls(null, null, null)
360+
assertRoundTrips(emptyDocument, dataClass)
361+
362+
// Assert all null data decodes as expected
363+
assertDecodesTo(BsonDocument.parse("""{"boolean": null, "string": null, "listSimple": null}"""), dataClass)
364+
365+
// Assert some data
366+
assertDecodesTo(BsonDocument.parse("""{"string": "Custom"}"""), DataClassWithNulls(null, "Custom", null))
367+
368+
// Assert all data
369+
val expected =
348370
"""{
349-
| "boolean": null,
350-
| "string": null,
351-
| "listSimple": null
371+
| "boolean": true,
372+
| "string": "Custom",
373+
| "listSimple": ["x"]
352374
|}"""
353375
.trimMargin()
376+
assertRoundTrips(expected, DataClassWithNulls(true, "Custom", listOf("x")))
377+
}
354378

355-
val dataClass = DataClassWithNulls(null, null, null)
356-
assertRoundTrips(emptyDocument, dataClass)
357-
assertRoundTrips(expectedNulls, dataClass, altConfiguration)
379+
@Test
380+
fun testDataClassWithDefaultsAndNulls() {
381+
// All fields provided
382+
val expected = """{"required": "req", "optional": "opt", "nullable": "nul"}"""
383+
assertRoundTrips(expected, DataClassWithDefaultsAndNulls("req", "opt", "nul"))
384+
385+
// Only required field — optional gets default, nullable gets default (null)
386+
assertDecodesTo(BsonDocument.parse("""{"required": "req"}"""), DataClassWithDefaultsAndNulls("req"))
387+
388+
// Required + nullable explicit null in document
389+
assertDecodesTo(
390+
BsonDocument.parse("""{"required": "req", "nullable": null}"""), DataClassWithDefaultsAndNulls("req"))
391+
392+
// Required + optional overridden, nullable absent
393+
assertDecodesTo(
394+
BsonDocument.parse("""{"required": "req", "optional": "custom"}"""),
395+
DataClassWithDefaultsAndNulls("req", "custom"))
396+
397+
// Missing required field throws
398+
assertThrows<MissingFieldException> {
399+
val codec = KotlinSerializerCodec.create(DataClassWithDefaultsAndNulls::class)
400+
codec?.decode(BsonDocumentReader(BsonDocument()), DecoderContext.builder().build())
401+
}
358402
}
359403

360404
@Test

‎bson-kotlinx/src/test/kotlin/org/bson/codecs/kotlinx/samples/DataClasses.kt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,13 @@ data class DataClassWithKotlinAllowedName(
129129

130130
@Serializable data class DataClassWithNulls(val boolean: Boolean?, val string: String?, val listSimple: List<String?>?)
131131

132+
@Serializable
133+
data class DataClassWithDefaultsAndNulls(
134+
val required: String,
135+
val optional: String = "default",
136+
val nullable: String? = null
137+
)
138+
132139
@Serializable
133140
data class DataClassWithListThatLastItemDefaultsToNull(val elements: List<DataClassLastItemDefaultsToNull>)
134141

0 commit comments

Comments
 (0)