-
Notifications
You must be signed in to change notification settings - Fork 1.4k
MONGOID-5158 - BSON::ObjectId#as_json and #to_json should serialize as a String without "$oid" #5054
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
Conversation
Hi Johnny, This is a non-trivial change to make and it probably belongs in bson-ruby rather than Mongoid. I've done some investigation to establish the history behind the current behavior: The In 2013, Subsequently in 3c3496a (2 months after the bson-ruby commit), Mongoid's Since this, and as of today, I believe Mongoid's definition of Similarly, changing this method would need to be done in bson-ruby, not in Mongoid. Otherwise applications that use the driver would end up having their behavior changed when Mongoid is loaded, which I would say is undesirable. The behavior of So... Given that bson-ruby now uses I support in principle making the change proposed in this PR in bson-ruby, but at this point I believe it requires more research to determine impact and any related changes that should also be made. |
Since In the release notes I've included a patch to revert to the |
I created https://jira.mongodb.org/browse/MONGOID-5162 for removing the method from Mongoid. |
Can you elaborate how you arrived at this conclusion? My expectation is this method is used in the vast majority of Ruby and Mongoid applications, and it is not at all clear to me that altering its behavior will produce "basically no impact". |
Yes, the I will ask you the opposite: please name one use-case for the You can see clearly in Stack Overflow and in Mongoid repo you're getting 1000s of complaints for the existing behavior. If you change it (with a proper change notice) I cannot imagine you will receive a single complaint. |
@p-mongo If there is serious concern that this may break applications, may I suggest the following:
Would this be acceptable to you? If so, I'm glad to raise the PRs for both the deprecation in 7.4 and the removal in 8.0. |
As I mentioned earlier, I believe that Mongoid shouldn't have a |
OK sure... but then what? What's the plan to switch |
As the next step I suggest defining what
Notably absent from that list are Time, Date and DateTime (which define The Decimal128 implementation is perhaps the one which could be improved, presently it produces extended json which people may not want. (https://github.com/mongodb/bson-ruby/blob/master/lib/bson/decimal128.rb#L67) A related question is whether all of the other Once this is decided the next step would be to implement the change and come up with the transition plan. Perhaps this could be the change that gets bson-ruby 5.0 out into the world. |
Replaced by #5057. I'll also be raising a bson-ruby PR soon. Thanks |
@p-mongo BSON library discussion here: mongodb/bson-ruby#238 |
The Change
Currently,
BSON::ObjectId#as_json
will return{ "$oid" => "03d623ca58edf9046e207565" }
This PR does what 99% of users prefer: return a plain String
"03d623ca58edf9046e207565"
I've included a "Breaking change" notice in the docs for this.
Why
While
$oid
may be the correct way to represent an ObjectID in strict "BSON", it is not a practical way to represent an ObjectID in "JSON" as used in real world apps. The standard use case for JSON (.as_json
method) is to serialize objects for HTTP APIs. The primary goal of Mongoid is for us as developers not to have to worry about BSON internal representations; instead we think in terms of the Ruby app domain. (Mongoid / Mongo Ruby Driver handles the BSON nuts-and-bolts for us.)Other Ideas
We could also add
BSON::ObjectId#as_bson
andObject#as_bson
(which delegates to.as_json
in all cases exceptObjectId
.) I don't think it's particularly useful, but I'd be happy to implement it if the Mongo team would like. (We'd probably want to callmongoize
orevolve
as well...)More Background
The current behavior drives Rails users absolutely crazy as you can see in the following issues, and nearly everyone ends up monkey-patching this method after hours of debugging frustration. Here are some a few examples out of hundreds:
Incidentally, Mongoid 3.1 and earlier used to serialize this as a String; when it changed to
$oid
it broke my and many other users apps--this unexpected behavior causes lots of bugs. When I looked up the commit git blame I found this: