Skip to content

Fix duplicate field name serialization with @BsonDiscriminator and getter #1610

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

Jump to bottom
Merged
rozza merged 9 commits into from
Sep 3, 2025
Merged

Fix duplicate field name serialization with @BsonDiscriminator and getter #1610

rozza merged 9 commits into from
Sep 3, 2025

Conversation

@raffaeleflorio
Copy link
Contributor

@raffaeleflorio raffaeleflorio commented Jan 16, 2025 •
edited by jyemin
Loading

This PR is about the JAVA-5764 issue. It changes the encode method by skipping the encode of a property if it's already encoded as the discriminator.

@rozza
Copy link
Member

rozza commented Jan 22, 2025

@raffaeleflorio could you include a test case for the issue?

@raffaeleflorio
Copy link
Contributor Author

raffaeleflorio commented Jan 22, 2025 •
edited
Loading

@rozza I've just added the test case. The testDiscriminatorEncodedOnceWhenItIsAlsoAGetter fails without the change because the discriminator will be encoded two times instead of one when encoding a DiscriminatorWithModelGetter object.

@raffaeleflorio
Copy link
Contributor Author

raffaeleflorio commented Jan 23, 2025 •
edited
Loading

@rozza I also added the testDiscriminatorEncodedOnceWhenItIsAlsoAProperty in order to test a getter with the readName as the discriminationKey. It fails without the fix because the same reasons of the aforesaid test.

@jyemin jyemin added the external label Feb 4, 2025
@raffaeleflorio raffaeleflorio force-pushed the main branch 2 times, most recently from 89f904e to 0e7dfd4 Compare February 12, 2025 08:05
@raffaeleflorio raffaeleflorio force-pushed the main branch 2 times, most recently from 67731e5 to 0b34a9b Compare February 19, 2025 16:12
@raffaeleflorio raffaeleflorio force-pushed the main branch 3 times, most recently from 2c7d0d1 to 01c4820 Compare February 28, 2025 08:00
@raffaeleflorio raffaeleflorio force-pushed the main branch 2 times, most recently from ba54a86 to 443d36c Compare April 2, 2025 07:23
@raffaeleflorio
Copy link
Contributor Author

raffaeleflorio commented Apr 10, 2025

Hello @rozza @jyemin, what else could I improve about this PR? Thanks

@jyemin jyemin requested a review from rozza April 24, 2025 11:46
@jyemin jyemin changed the title fix JAVA-5764 Fix duplicate field name serialization with @BsonDiscriminator and getter May 1, 2025
@raffaeleflorio raffaeleflorio force-pushed the main branch 2 times, most recently from b7b2ec1 to e8446d6 Compare May 18, 2025 07:03
@raffaeleflorio raffaeleflorio force-pushed the main branch 2 times, most recently from a6de0b5 to 3ae1b70 Compare May 30, 2025 14:07
@raffaeleflorio raffaeleflorio requested a review from a team as a code owner July 9, 2025 08:25
@codeowners-service-app
Copy link

codeowners-service-app bot commented Aug 4, 2025

Assigned vbabanin for team dbx-java because rozza is out of office.

rozza
rozza requested changes Sep 3, 2025
View reviewed changes
Copy link
Member

@rozza rozza left a comment

Choose a reason for hiding this comment

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

A couple of checkstyle nits

@rozza
Copy link
Member

rozza commented Sep 3, 2025

Looks good - just going to run the CI again then should be able to merge.

Thanks @raffaeleflorio

rozza
rozza approved these changes Sep 3, 2025
View reviewed changes
Copy link
Member

@rozza rozza left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you @raffaeleflorio for the excellent PR.

I'm pleased to say this improvement will be part of the 5.6 release.

@rozza rozza merged commit bcf15c0 into mongodb:main Sep 3, 2025
53 of 55 checks passed
@owen-obrien owen-obrien mentioned this pull request Sep 22, 2025
Merged
rozza added a commit that referenced this pull request Sep 22, 2025
@owen-obrien @rozza
JAVA-5971 Reverse equals check to avoid NPEs (#1806)
a5d636b 
Fixes NPE due to BSONIgnore fields introduced in #1610

JAVA-5971

---------

Co-authored-by: Ross Lawley <[email protected]>
rozza added a commit to rozza/mongo-java-driver that referenced this pull request Sep 22, 2025
@owen-obrien @rozza
JAVA-5971 Reverse equals check to avoid NPEs (mongodb#1806)
002adfe 
Fixes NPE due to BSONIgnore fields introduced in mongodb#1610

JAVA-5971

---------

Co-authored-by: Ross Lawley <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rozza rozza rozza approved these changes

@vbabanin vbabanin Awaiting requested review from vbabanin

Assignees

No one assigned

Labels

external

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@raffaeleflorio @rozza @jyemin