Skip to content

Commit

Permalink
Fix bson-kotlinx encodeNullableSerializableValue null handling (#1453)
Browse files Browse the repository at this point in the history
Ensures that the deferredElement name is reset correctly.
Test case ported to bson-kotlin

JAVA-5524
  • Loading branch information
rozza committed Jul 29, 2024
1 parent f339684 commit d988c37
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ data class DataClassWithDefaults(

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

data class DataClassWithListThatLastItemDefaultsToNull(val elements: List<DataClassLastItemDefaultsToNull>)

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

data class DataClassSelfReferential(
val name: String,
val left: DataClassSelfReferential? = null,
Expand Down
69 changes: 42 additions & 27 deletions bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonEncoder.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -117,7 +117,7 @@ internal class DefaultBsonEncoder(
is StructureKind.CLASS -> {
val elementName = descriptor.getElementName(index)
if (descriptor.getElementDescriptor(index).isNullable) {
deferredElementName = elementName
deferredElementHandler.set(elementName)
} else {
encodeName(elementName)
}
Expand All @@ -140,25 +140,27 @@ internal class DefaultBsonEncoder(
}

override fun <T> encodeSerializableValue(serializer: SerializationStrategy<T>, value: T) {
deferredElementName?.let {
if (value != null || configuration.explicitNulls) {
encodeName(it)
super.encodeSerializableValue(serializer, value)
} else {
deferredElementName = null
}
}
?: super.encodeSerializableValue(serializer, value)
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)
}
},
{ super.encodeSerializableValue(serializer, value) })
}

override fun <T : Any> encodeNullableSerializableValue(serializer: SerializationStrategy<T>, value: T?) {
deferredElementName?.let {
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())
Expand All @@ -170,14 +172,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) {
Expand Down Expand Up @@ -206,7 +201,6 @@ internal class DefaultBsonEncoder(

private fun encodeName(value: Any) {
writer.writeName(value.toString())
deferredElementName = null
state = STATE.VALUE
}

Expand All @@ -229,4 +223,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
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,11 @@ data class DataClassWithDefaults(

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

@Serializable
data class DataClassWithListThatLastItemDefaultsToNull(val elements: List<DataClassLastItemDefaultsToNull>)

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

@Serializable
data class DataClassSelfReferential(
val name: String,
Expand Down

0 comments on commit d988c37

Please sign in to comment.