Enhance KotlinSerializer with value codecs for widening primitive conversion. #1301
Conversation
…version. JAVA-5303
vbabanin
requested review from
a team and
rozza
and removed request for
a team
|
Kotlinx Json has a configuration to allow for lenient parsing: https://github.com/Kotlin/kotlinx.serialization/blob/master/docs/json.md#lenient-parsing Should this be configurable? |
|
@rozza I think in this case it's better to be consistent with other codecs and also with the way numeric values are treated in MongoDB is basically interchangeable and compatible with each other, so long as there is no loss of precision or magnitude. |
|
@vbabanin can this come out of Draft now? |
JAVA-5303
| <!-- 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"/> |
There was a problem hiding this comment.
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.
| @@ -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 | |||
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
| } | ||
|
|
||
| public static char decodeChar(final BsonReader reader) { | ||
| String string = reader.readString(); |
There was a problem hiding this comment.
Lets add a check if the bsonType isnt a string type.
throw new BsonInvalidOperationException(format("Invalid string type, found: %s", bsonType));
There was a problem hiding this comment.
The method readString internally calls verifyBSONType, which already includes a type check and throws a BsonInvalidOperationException if the types don't match: link to code. The current exception message is: 'methodName can only be called when CurrentBSONType is X, not when CurrentBSONType is Y.'
Are you suggesting we add additional check to override this exception message?
There was a problem hiding this comment.
Adding the check would make it inline with the NumberCodecHelper.
There was a problem hiding this comment.
Thank you for clarification! I have added a check for BsonType.STRING.
| override fun decodeDouble(): Double = NumberCodecHelper.decodeDouble(reader) | ||
| override fun decodeInt(): Int = NumberCodecHelper.decodeInt(reader) | ||
| override fun decodeLong(): Long = NumberCodecHelper.decodeLong(reader) | ||
| override fun decodeString(): String = reader.readString() |
There was a problem hiding this comment.
Lets keep the readOrThrow({ reader.readString() }, BsonType.STRING) so that users know if an invalid type is present for a string field.
| @@ -176,7 +181,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 = reader.readObjectId() | |||
There was a problem hiding this comment.
Lets keep the readOrThrow({ reader.readObjectId() }, BsonType.OBJECT_ID) so that users know if an invalid type is present for the objectId field.
JAVA-5303
bson-kotlinx/src/main/kotlin/org/bson/codecs/kotlinx/BsonDecoder.kt
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| public static char decodeChar(final BsonReader reader) { | ||
| String string = reader.readString(); |
There was a problem hiding this comment.
Adding the check would make it inline with the NumberCodecHelper.
…er.kt Co-authored-by: Ross Lawley <[email protected]>
JAVA-5303
…version. (mongodb#1301) JAVA-5303
…version. (mongodb#1301) JAVA-5303
JAVA-5303