Skip to content
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.

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

Support discriminators not being the first field when decoding #1324

Merged
merged 5 commits into from
Mar 15, 2024

Conversation

rozza
Copy link
Member

@rozza rozza commented Mar 4, 2024

@rozza rozza requested review from stIncMale, a team, vbabanin and jyemin and removed request for stIncMale and a team March 4, 2024 15:14
@@ -75,6 +79,41 @@ class KotlinSerializerCodecProviderTest {
assertEquals(DataClassWithSimpleValues::class.java, codec.encoderClass)
}

@Test
fun testDataClassWithSimpleValuesFieldOrdering() {
Copy link
Member Author

Choose a reason for hiding this comment

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

This functionality was already supported but added a regression test.

val actual =
codec.decode(
JsonReader(
"""{"name": "string", "_id": {$oid: "${objectId.toHexString()}"},
Copy link
Member Author

Choose a reason for hiding this comment

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

Here all the fields are out of order and this confirms it does support the scenario where the discriminator isn't the first field.

@@ -213,13 +213,37 @@ private class PolymorphicDecoder(
configuration: BsonConfiguration
) : DefaultBsonDecoder(reader, serializersModule, configuration) {
private var index = 0
private var mark: BsonReaderMark? = reader.mark
Copy link
Member Author

Choose a reason for hiding this comment

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

Here a mark is set allowing the PolymorphicDecoder to iterate to the discriminator field instead of expecting it to be the first field.

override fun <T> decodeSerializableValue(deserializer: DeserializationStrategy<T>): T =
deserializer.deserialize(DefaultBsonDecoder(reader, serializersModule, configuration))
override fun <T> decodeSerializableValue(deserializer: DeserializationStrategy<T>): T {
mark?.let {
Copy link
Member Author

Choose a reason for hiding this comment

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

clears the mark on the first decoding of values.

@robbamberg
Copy link

@rozza this seems a solution for the polymorphic serializer on root level (with _id field), but this is only part of the fix. Currently the polymorphic serializer is not called while inserting a document to Mongo, so without that also having a fix, this won't do much.

In driver-core in the Operations class we see that during insert of a document that the function documentToBsonDocument is called. It fetches the Codec for the class that is inserted (data class), but it should fetch the Codec of collection (basically the getCodec function). This is causing the wrong serializer to be used.

@rozza
Copy link
Member Author

rozza commented Mar 5, 2024

Hi @robbamberg - thanks for the follow up, that is a slightly different issue to the one being fixed here. I've added #1327 for that fix.

@rozza rozza requested a review from jyemin March 14, 2024 10:47
Copy link
Contributor

@jyemin jyemin left a comment

Choose a reason for hiding this comment

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

Looking good, just one DRY-related suggestion.

@rozza rozza requested a review from jyemin March 15, 2024 11:42
@rozza rozza merged commit dc6c38b into mongodb:master Mar 15, 2024
2 of 3 checks passed
@rozza rozza deleted the JAVA-5304 branch March 15, 2024 14:36
rozza added a commit to rozza/mongo-java-driver that referenced this pull request Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants