Skip to content

Enhance KotlinSerializer with value codecs for widening primitive conversion. #1301

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 11 commits into from
Jul 16, 2024
Merged

Enhance KotlinSerializer with value codecs for widening primitive conversion. #1301

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
80a36cb
Enhance KotlinSerializer with value codecs for widening primitive con…
vbabanin Jan 31, 2024
4c6c2ca
Rename variable.
vbabanin Jul 1, 2024
de50db6
Merge branch 'master' into JAVA-5303
vbabanin Jul 1, 2024
dc12357
Fix checkstyle issues.
vbabanin Jul 2, 2024
8818c6e
Add SpotBugs exclusion for BC_BAD_CAST_TO_ABSTRACT_COLLECTION.
vbabanin Jul 2, 2024
1f9933c
Replace Codecs with CodeHelper to enhance separation of concerns.
vbabanin Jul 9, 2024
c80b17c
Rename StringCodeHelper.
vbabanin Jul 9, 2024
f984a8a
Throw exceptions with clear message.
vbabanin Jul 11, 2024
8e4bee5
Update bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonDecod…
vbabanin Jul 11, 2024
bfe713a
Add a check for BsonType.
vbabanin Jul 12, 2024
636daa1
Merge branch 'master' into JAVA-5303
vbabanin Jul 12, 2024
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
40 changes: 30 additions & 10 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 @@ -34,8 +34,18 @@ import org.bson.BsonReader
import org.bson.BsonReaderMark
import org.bson.BsonType
import org.bson.BsonValue
import org.bson.codecs.BooleanCodec
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we discussed not using codecs (so to not confuse the two concepts of codecs and kserializers. Instead use the NumberCodecHelper directly (which is what the numeric codecs are doing in decode anyway).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the reminder. We did discuss prioritizing clarity by not mixing codecs with serializers. I've now implemented the changes to use the codec helpers directly as we discussed.

import org.bson.codecs.BsonValueCodec
import org.bson.codecs.ByteCodec
import org.bson.codecs.CharacterCodec
import org.bson.codecs.DecoderContext
import org.bson.codecs.DoubleCodec
import org.bson.codecs.FloatCodec
import org.bson.codecs.IntegerCodec
import org.bson.codecs.LongCodec
import org.bson.codecs.ObjectIdCodec
import org.bson.codecs.ShortCodec
import org.bson.codecs.StringCodec
import org.bson.types.ObjectId

/**
Expand Down Expand Up @@ -68,6 +78,16 @@ internal open class DefaultBsonDecoder(
companion object {
val validKeyKinds = setOf(PrimitiveKind.STRING, PrimitiveKind.CHAR, SerialKind.ENUM)
val bsonValueCodec = BsonValueCodec()
val longCodec = LongCodec()
val shortCodec = ShortCodec()
val intCodec = IntegerCodec()
val doubleCodec = DoubleCodec()
val floatCodec = FloatCodec()
val byteCodec = ByteCodec()
val booleanCodec = BooleanCodec()
val charCodec = CharacterCodec()
val stingCodec = StringCodec()
val objectIdCodec = ObjectIdCodec()
const val UNKNOWN_INDEX = -10
fun validateCurrentBsonType(
reader: AbstractBsonReader,
Expand Down Expand Up @@ -154,15 +174,15 @@ internal open class DefaultBsonDecoder(
}
}

override fun decodeByte(): Byte = decodeInt().toByte()
override fun decodeChar(): Char = decodeString().single()
override fun decodeFloat(): Float = decodeDouble().toFloat()
override fun decodeShort(): Short = decodeInt().toShort()
override fun decodeBoolean(): Boolean = readOrThrow({ reader.readBoolean() }, BsonType.BOOLEAN)
override fun decodeDouble(): Double = readOrThrow({ reader.readDouble() }, BsonType.DOUBLE)
override fun decodeInt(): Int = readOrThrow({ reader.readInt32() }, BsonType.INT32)
override fun decodeLong(): Long = readOrThrow({ reader.readInt64() }, BsonType.INT64)
override fun decodeString(): String = readOrThrow({ reader.readString() }, BsonType.STRING)
override fun decodeByte(): Byte = byteCodec.decode(reader, DecoderContext.builder().build())
override fun decodeChar(): Char = charCodec.decode(reader, DecoderContext.builder().build())
override fun decodeFloat(): Float = floatCodec.decode(reader, DecoderContext.builder().build())
override fun decodeShort(): Short = shortCodec.decode(reader, DecoderContext.builder().build())
override fun decodeBoolean(): Boolean = booleanCodec.decode(reader, DecoderContext.builder().build())
override fun decodeDouble(): Double = doubleCodec.decode(reader, DecoderContext.builder().build())
override fun decodeInt(): Int = intCodec.decode(reader, DecoderContext.builder().build())
override fun decodeLong(): Long = longCodec.decode(reader, DecoderContext.builder().build())
override fun decodeString(): String = stingCodec.decode(reader, DecoderContext.builder().build())

override fun decodeNull(): Nothing? {
if (reader.state == AbstractBsonReader.State.VALUE) {
Expand All @@ -176,7 +196,7 @@ internal open class DefaultBsonDecoder(
return reader.state != AbstractBsonReader.State.END_OF_DOCUMENT && reader.currentBsonType != BsonType.NULL
}

override fun decodeObjectId(): ObjectId = readOrThrow({ reader.readObjectId() }, BsonType.OBJECT_ID)
override fun decodeObjectId(): ObjectId = objectIdCodec.decode(reader, DecoderContext.builder().build())
override fun decodeBsonValue(): BsonValue = bsonValueCodec.decode(reader, DecoderContext.builder().build())
override fun reader(): BsonReader = reader

Expand Down
69 changes: 60 additions & 9 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 @@ -15,6 +15,7 @@
*/
package org.bson.codecs.kotlinx

import java.util.stream.Stream
import kotlin.test.assertEquals
import kotlinx.serialization.ExperimentalSerializationApi
import kotlinx.serialization.MissingFieldException
Expand All @@ -23,12 +24,17 @@ import kotlinx.serialization.modules.SerializersModule
import kotlinx.serialization.modules.plus
import kotlinx.serialization.modules.polymorphic
import kotlinx.serialization.modules.subclass
import org.bson.BsonBoolean
import org.bson.BsonDocument
import org.bson.BsonDocumentReader
import org.bson.BsonDocumentWriter
import org.bson.BsonDouble
import org.bson.BsonInt32
import org.bson.BsonInt64
import org.bson.BsonInvalidOperationException
import org.bson.BsonMaxKey
import org.bson.BsonMinKey
import org.bson.BsonString
import org.bson.BsonUndefined
import org.bson.codecs.DecoderContext
import org.bson.codecs.EncoderContext
Expand Down Expand Up @@ -90,11 +96,12 @@ import org.bson.codecs.kotlinx.samples.SealedInterface
import org.bson.codecs.kotlinx.samples.ValueClass
import org.junit.jupiter.api.Test
import org.junit.jupiter.api.assertThrows
import org.junit.jupiter.params.ParameterizedTest
import org.junit.jupiter.params.provider.MethodSource

@OptIn(ExperimentalSerializationApi::class)
@Suppress("LargeClass")
class KotlinSerializerCodecTest {
private val numberLong = "\$numberLong"
private val oid = "\$oid"
private val emptyDocument = "{}"
private val altConfiguration =
Expand Down Expand Up @@ -134,15 +141,59 @@ class KotlinSerializerCodecTest {

private val allBsonTypesDocument = BsonDocument.parse(allBsonTypesJson)

@Test
fun testDataClassWithSimpleValues() {
val expected =
"""{"char": "c", "byte": 0, "short": 1, "int": 22, "long": {"$numberLong": "42"}, "float": 4.0,
| "double": 4.2, "boolean": true, "string": "String"}"""
.trimMargin()
val dataClass = DataClassWithSimpleValues('c', 0, 1, 22, 42L, 4.0f, 4.2, true, "String")
companion object {
@JvmStatic
fun testTypesCastingDataClassWithSimpleValues(): Stream<BsonDocument> {
return Stream.of(
BsonDocument()
.append("char", BsonString("c"))
.append("byte", BsonInt32(1))
.append("short", BsonInt32(2))
.append("int", BsonInt32(10))
.append("long", BsonInt32(10))
.append("float", BsonInt32(2))
.append("double", BsonInt32(3))
.append("boolean", BsonBoolean.TRUE)
.append("string", BsonString("String")),
BsonDocument()
.append("char", BsonString("c"))
.append("byte", BsonDouble(1.0))
.append("short", BsonDouble(2.0))
.append("int", BsonDouble(9.9999999999999992))
.append("long", BsonDouble(9.9999999999999992))
.append("float", BsonDouble(2.0))
.append("double", BsonDouble(3.0))
.append("boolean", BsonBoolean.TRUE)
.append("string", BsonString("String")),
BsonDocument()
.append("char", BsonString("c"))
.append("byte", BsonDouble(1.0))
.append("short", BsonDouble(2.0))
.append("int", BsonDouble(10.0))
.append("long", BsonDouble(10.0))
.append("float", BsonDouble(2.0))
.append("double", BsonDouble(3.0))
.append("boolean", BsonBoolean.TRUE)
.append("string", BsonString("String")),
BsonDocument()
.append("char", BsonString("c"))
.append("byte", BsonInt64(1))
.append("short", BsonInt64(2))
.append("int", BsonInt64(10))
.append("long", BsonInt64(10))
.append("float", BsonInt64(2))
.append("double", BsonInt64(3))
.append("boolean", BsonBoolean.TRUE)
.append("string", BsonString("String")))
}
}

assertRoundTrips(expected, dataClass)
@ParameterizedTest
@MethodSource("testTypesCastingDataClassWithSimpleValues")
fun testTypesCastingDataClassWithSimpleValues(data: BsonDocument) {
val expectedDataClass = DataClassWithSimpleValues('c', 1, 2, 10, 10L, 2.0f, 3.0, true, "String")

assertDecodesTo(data, expectedDataClass)
}

@Test
Expand Down
6 changes: 6 additions & 0 deletions config/spotbugs/exclude.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,12 @@
<Method name="~.*validateAnnotations.*"/>
<Bug pattern="UC_USELESS_OBJECT"/>
</Match>
<Match>
<!-- MongoDB status: "False Positive", SpotBugs rank: 17 -->
<Class name="org.bson.codecs.kotlinx.KotlinSerializerCodec$Companion"/>
<Method name="~.*validateAnnotations.*"/>
<Bug pattern="BC_BAD_CAST_TO_ABSTRACT_COLLECTION"/>
Copy link
Member Author

@vbabanin vbabanin Jul 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warning is triggered by an implicit cast generated during Kotlin to Java bytecode compilation, which is not directly controllable in the source Kotlin code. As this casting is safely handled by the Kotlin compiler and does not reflect an actual error in our Kotlin source, we can suppress this warning.

Copy link
Member

@rozza rozza Jul 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still needed?

</Match>

<!-- Spotbugs reports false positives for suspendable operations with default params
see: https://github.com/Kotlin/kotlinx.coroutines/issues/3099
Expand Down