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

Add JsonEncoder and JsonDecoder implmentations to their Bson counterparts #1253

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

markvogelnomad
Copy link

Kotlinx requires the encoder to implement JsonEncoder and JsonDecoder in order to encode generic JsonElement types.

The lack of this support for this came up many times when using KBson and KMongo previously and I'm hoping we can quickly mitigate this now that MongoDB has official support.

I've added each implementation to support most types on decoding and determine the best possible type on encoding to insert. It is somewhat assumed that if someone is encoding a JsonElement, then they will probably be decoding one as well and, therefore, type information is primarily for storage optimization.

Unfortunately, Kotlinx writes all numbers as 64 bit integers first and never attempts to check for 32 bits or allow overriding the JsonElementSerializer behavior; this means that all integers will probably be written as int64. Regardless, this should be a valuable add.

Signed-off-by: Mark <mark.vogel@nomadgcs.com>
Signed-off-by: Mark <mark.vogel@nomadgcs.com>
Signed-off-by: Mark <mark.vogel@nomadgcs.com>
Signed-off-by: Mark <mark.vogel@nomadgcs.com>
Signed-off-by: Mark <mark.vogel@nomadgcs.com>
@rozza
Copy link
Member

rozza commented Nov 8, 2023

Hi @markvogelnomad, many thanks for the excellent PR!

I've added JAVA-5239 to track the feature request.

Cheers,

Ross

Signed-off-by: Mark <mark.vogel@nomadgcs.com>
@markvogelnomad
Copy link
Author

Appreciate it!

I've pushed one more non-functional change to attempt to comply with the checks; I will leave it alone at this point to allow feedback or modification, if required.

@tom-selander
Copy link

Hi @markvogelnomad thanks for the PR. We are backlogging this ticket for now, as we would like to dedicate a little more time reviewing this PR and ensuring we do our proper due diligence before merging. Please follow JAVA-5239 for updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants