BSON::ObjectId#as_json reimplementation

[p-mongo]

In https://jira.mongodb.org/browse/MONGOID-5158 @johnnyshields requested changing BSON::ObjectId#as_json to return the hex-serialized ObjectId value as a string instead of the current {"oid" => "..."} syntax. I provided some feedback on that initial proposal in #5054, which outlines the current situation. In discussion with @comandeo we considered how to move forward with the change to ObjectId in particular (not yet discussing the other classes) and identified the following options:

Have bson-ruby 5.0 return a simple string from ObjectId#as_json. This is a breaking change.

Remove ObjectId#as_json from bson-ruby altogether and provided it in a separate library. Technically the required functionality in bson-ruby is serialization to extended JSON, which is now done via ObjectId#as_extended_json, and ObjectId#as_json is possibly not needed for bson-ruby itself at all.

Because bson-ruby does provide functionality for integrating with ActiveSupport (such as handling of TimeWithZone objects), another option is to leave ObjectId#as_json in bson-ruby but make it a separate component that is requred from bson/active_support such that applications can opt into it if needed.

With either of the last two options, two implementations of ObjectId#as_json may be provided in the same library version (be that bson-ruby or a new library): the existing one that returns extended JSON oid syntax, and the proposed new one that returns a simple string. This way applications can be configured at runtime with the desired implementation. This would hopefully make upgrades simpler as applications would be able to move to the new version of bson-ruby without needing changes to account for the ObjectId#as_json behavior.

@johnnyshields would love to hear your thoughts on the above.

[p-mongo]

Note also the open bson-ruby PR: mongodb/bson-ruby#238

Unfortunately it seems that referinging PRs from discussions doesn't add a reference back to discussion from the PR.

[johnnyshields]

@p-mongo my vote is #1. Let's keep it simple and have this work "out-of-the-box" for first-time Mongoid / Mongo Ruby users using Rails and other serialization libraries which also rely on #as_json.

[comandeo]

I also vote for #1.

1. I think that having json serialization in bson-ruby gem is actually a good thing. We provide driver and Mongoid users with the same serialization tools. So, if one uses our driver with, say, sinatra, they can just call to_json on what driver methods return. This is very nice.
2. This is a breaking change indeed; however, as @johnnyshields mentioned, 99% of users who serialize Mondoid documents with _id field, applied some monkeypatchng anyway. I honestly cannot come up with a real life scenario when one need {"_id": {"$oid": "xxx" }, "attr1" : "value" } in their API.
3. Assuming that most of our users somehow fixed current behavior, we should be sure that we do not break their apps. I think we are safe here. I looked over what people suggest to to to fix the current behavior, and all the fixes I should work with the new behavior.
4. We can add some kind of toggle to enable/disable the new behavior. However, I do not think it worth the effort of maintaining this code later. We can add something like this if there is a complaint.

[johnnyshields]

Agreed with all @comandeo's points.

Re: #4, agreed a toggle to enable/disable is not needed at this time.

[johnnyshields]

@p-mongo are you in agreement?

I think the only remaining decision is how we handle BSON types which do not cleanly map to JSON. Specifically these types:

lib/bson/binary.rb
lib/bson/code.rb
lib/bson/code_with_scope.rb
lib/bson/db_pointer.rb
lib/bson/max_key.rb
lib/bson/min_key.rb

Here are our choices:

(a) alias #as_json to #as_extended_json
(b) return a custom representation (i.e. usually a hash) which attempts to represent the full object.
(c) compacted (lossy) single value representation

Taking binary as an example, here are the 3 options:

# a) alias `#as_json` to `#as_extended_json`
{ "$binary" => { "base64" => "yO2rw/c4TKO2jauSqRR4ow==", "subType": "04" } }

# b) custom representation, in this case the inner part of extended JSON
{ "base64" => "yO2rw/c4TKO2jauSqRR4ow==", "subType": "04" }

# c) compacted (lossy) single value representation, in this case the binary string
"yO2rw/c4TKO2jauSqRR4ow=="

My vote here is (a) to alias these to #as_extended_json

[p-mongo]

Hi @johnnyshields, we discussed the situation with @comandeo and our current thinking is as follows:

• Given the ObjectId#as_json override in Mongoid, any change to its implementation in bson-ruby won't affect Mongoid prior to 7.4.
• We don't know for sure whether changing the behavior of ObjectId#as_json would cause problems for applications, but we believe the likelihood of that to be low. We expect that, as you said, most applications would patch it to return the string only.
• Therefore we would like to change ObjectId#as_json in the next bson-ruby 4.x release. This would effectively be a breaking change for users of bson-ruby and driver directly, and for Mongoid 7.4+, but not for any Mongoid 7.3 and earlier users.
• The benefit of doing this change in 4.x is we avoid a major version bump.
• We considered adding an alternative implementation that would provide the corrent oid behavior. We are inclined to not add this until there are user reports of compatibility breaks, so that we don't need to do this work if there are no users affected by the change.
• If users report compatibility breaks due to the ObjectId#as_json change, we well either revert to the oid serialization by default with the option provided by bson-ruby to return the string only (such as by requiring a file like bson/new_object_id_serialization), or by providing the oid serialization as such an option.
• As part of ObjectId#as_json change we would like to also consider the other as_json methods and develop a rationalization for what as_json is supposed to do (this rationalization being the same for all classes implementing this method)
• However, the actual work of adding/changing as_json implementations on other classes (not ObjectId) does not need to be done as part of the ObjectId#as_json change, to keep PR sizes down and to get the ObjectId#as_json change in faster as the more desired one.

Does this sound agreeable to you?

[johnnyshields]

Yep this sounds perfect. Let's do it!

[johnnyshields]

@p-mongo @comandeo after further reflection, I think this should be a major version bump.

From the bson-ruby README.md:

As of 2.0.0, this project adheres to the Semantic Versioning Specification.

Semver says that any API public behavior changes to existing methods should be a major version bump. Although this is trivial, we should still follow Semver, just in case. Given how stable BSON is, I think there's no major concern here for a major version bump; it's unlikely we'll need to backport any changes to the 4.x-stable branch, and if we do, they will be minimal.

Following Semver strictly is very helpful for application owners (like me).

[johnnyshields]

@p-mongo @comandeo I've implemented the correct #as_json and #as_extended_json logic for all classes in 4 separate PRs. Each PR can be merged individually.

Ready for your review.