DDP mishandles non-JSON objects: Mongo ObjectIds, Dates, etc #61

Closed
antoviaque opened this Issue Apr 18, 2012 · 28 comments

Projects

None yet

Calls to update() on pre-existing mongodb records which use ObjectId() for their _id are ignored. _Mongo.prototype.update passes the string representation of the ObjectId() to collection.update().

As a result, update() doesn't match any record. This doesn't raise any error - the update is silently ignored by the server, and the change reverted on the client.

Not sure what should be changed, but at the minimum it would be nice to be able to debug this more easily.

In my case, I've converted the _ids in my database to strings:

db.applications_hackers.find({}).forEach(function(app){
    db.applications_hackers.remove({_id:app._id}); 
    app._id = app._id.toString(); 
    db.applications_hackers.insert(app); 
});
Owner
n1mmy commented Apr 19, 2012

Yeah, this is a known problem. Meteor does not handle ObjectID correctly.

callesg commented Jun 28, 2012

This BUG causes me problems as well.

I use a database that is connected to other clients. So i cant use the "HACK" that antoviaque used.
also remove() dosen't work. (just a side note)

Contributor
Tarang commented Aug 16, 2012

This is quite a serious bug is there no fix yet? Its odd this works with find() and findOne() but not with update(), or remove()

Contributor
Tarang commented Aug 16, 2012

Tried to fix this at #279

badslug commented Aug 29, 2012

I guess a lot of the early Meteor projects only dealt with documents created using Meteor - so the object IDs always were Meteor generated. I just ran into the bug now because we have foreign systems starting to import data into our MongoDB and they use ObjectIDs for all the new documents. It took a while to figure out that some documents were magically failing to not update - and this turns out to be the reason why. If there's anything I can do to help get the PR merged I'm happy to help - this has become a blocker to our current Meteor project. Thanks

@glasser glasser was assigned Sep 7, 2012
Owner
glasser commented Sep 16, 2012

Let me be very specific about the precise issue here, as I understand it:

  • When MongoDB autogenerates an _id, it uses a special ObjectId class.
  • It may look like a string, but matching against this with a string in a selector will fail.
  • When such an object is put into a DDP collection for transmission to the client, the _id is converted to a string (this happens in Meteor._LivedataSubscription.set.
  • So on the client, _id is always a string.
  • That means that on the client, doing _id queries (find, findOne, etc) with strings that only touch minimongo will succeed, because it's a string both in the minimongo structures and the query code
  • But doing write operations using _id as a selector from the client will always fail, because the client can't actually create ObjectIds.

There are a couple possible solutions here. We could add an id-type field to the DDP set messages, though that seems ugly. We could just make the id field of the set messages not a string in this case, but some other object that the client knows how to identify as a Mongo ObjectId.

It would be nice to be able to standardize on strings, but it's important to allow users to write to Mongo from outside of Meteor, and in addition, it's entirely possible for a collection to have two objects with _id where one is ObjectId("123456789012345678901234") and the other is "123456789012345678901234". So we probably do have to come up with a client-side representation of ObjectIds. Probably with a scary name like MongoGeneratedObjectId.

Owner
glasser commented Sep 16, 2012

Note that while id is particularly bad (because of its use as a dictionary key in various internals) there are other non-JSON types that might be found in Mongo: http://www.mongodb.org/display/DOCS/Mongo+Extended+JSON

badslug commented Sep 17, 2012

Hi David,

I agreed that it's impossible to standardize on strings and have 100% of the use-cases handled. If the object IDs (as objects) are preserved as objects on the client, at least there will be ways to code around the various cases of the _id being a string or an ObjectId. I think the most common scenario will be a collection published to the client, bound to template, updating properties as a field in an html form, and then round-trip back through the template event handler to the client collection, then through the auth Collection.allow() and then to MongoDB. Any opaque object used to represent the ObjectId (with the developer not caring if it's a string or ObjectId) will work in this scenario without any mental overhead from the developer as long as the _id's object is preserved throughout the loop.

I'm guessing that if we want the full power of MongoDB - having the complete set of extended objects will be necessary at some point. From an API perspective, will this tie the implementation even further to MongoDB? On the client (and on the server) we will need to be able to generate some of these special objects to form queries. Perhaps a set of factory methods on either Meteor or Meteor.Collection and no explicit definition of the object's implementation would be the best way to make this both easier to be portable and easier to use... e.g.

var fruits = new Meteor.Collection('fruits');

// Generates an opaque object used as an object ID for this particular collection.
// If the collection is a MongoDB collection, this uses either MongoDB's ObjectID directly, 
// or MongoGeneratedObjectId.
//
// Other collection implementations may use a different object 
// (say CouchDB on the backend and a Couch specific ID object)
//
// It's still up to the developer not the framework to know whether the IDs are strings 
// (where it should use strings in the query)
// or database object IDs (where it should use the Meteor.Collection.id() factory method).
var objectId = fruits.id('abcdefg'); 
var orange = fruits.findOne(objectId);

// Generates an opaque object used as a "timestamp" object for the collection.
// Again, if the collection is MongoDB, this uses MongoDB's Timestamp object.
var since = fruits.timestamp(new Date()); 
var recentFruits = fruits.find({modified:{$gt:since}});

This is actually critical. Even documents created in the shell with >meteor mongo use standard ObjectId's which then cannot be searched from within a meteor app.

Makes testing on sample data created in the shell impossible. Which then requires writing input screens in the test app just so the ID fields are created (incorrectly) by meteor.

Owner
glasser commented Sep 27, 2012

Yeah, I agree this is critical.

We don't want to add Mongo-specific ObjectID support to the DDP protocol right now. After more thought, something along the lines of #279 probably is a good idea, with clear documentation that we support Meteor-generated UUID-style string _ids and Mongo-generated ObjectId _ids and nothing else.

@lrobbins, I think you can create test data in the mongo db shell just fine, you just would have to explicitly set the _id (to a string value). You could also import the Minimongo UUID generator into the MongoDB Shell (./packages/minimongo/uuid.js IIANM) if you want random Meteor style IDs.

Still -- I also believe being able to handle Mongo-Style ObjectIDs in Meteor is crucial for interoperability.

Quite right. I did just that by explicitly setting {_id: ObjectId().str}.
Maybe meteor will one day conform to mongodb standards.

Owner
glasser commented Sep 28, 2012

Well, the problem is that it would be easy to make Meteor "conform to mongodb standards", if we were happy just saying "Meteor is based on MongoDB now and forever", and baked MongoDB-specific concepts into our APIs and wire protocol. But we'd like to eventually make the database pluggable, supporting any data store that can have a livedata adapter. Since we're trying to design a wire protocol and API to last a while, that makes backend-specific hacks expensive in the long term.

On the other hand, we don't want to be so focused on purity that we make Meteor difficult to use, and that's the case right now.

Contributor

Actually, I'd like to encourage you to take the mongodb route all the way (or at least one step further). One reason I chose meteor over other frameworks was that is uses the mongodb api directly. Having used a lot of frameworks that try to be database-agnostic made me really hate ORM wrappers and other abstractions and indirections. All they do is introduce subtle gotchas everywhere, and most of the time there really is just one "blessed" backend and the others are somewhat undermaintained. I'd really worry more about interoperability with standard mongodb than (at this stage still non-existing) other database backends for Meteor.

That said, if you can pull this (pluggable backends) off, great! But I've been bitten by the messy state with Meteors _id more than once and would love to see you resolving this issue by embracing the mongodb way of doing it.

Definitely disagree with Ed-von-Schleck. We're on the verge of trying meteor and one of the only hesitations is that it only supports MongoDB. The other hesitation is that it lives outside of a thriving and happy node.js ecosystem. By that I mean, why couldn't meteor be installed with npm install meteor? As you can see, customization and friendliness to the current ecosystem is of prime importance.

mcbain commented Nov 9, 2012

I started playing with meteor just a few days ago as an evaluation for my company and already has to stop due this blocker.
If the meteor aims to be used as part of bigger projects, with already existing data, then it should conform to the formats/standards the data is currently in - and that's ObjectId for the mongodb.
Beside, meterors current 'collection' abstraction is just a copy of the mongodb-collection features, not a fully general database access api which prevents usage of a specific db-implementation element like ObjectId.

Can we vote this issue to be solved ?

Member
gschmidt commented Dec 4, 2012

We will fix this soon. Thanks for your patience.

Owner
glasser commented Dec 14, 2012

This is in our roadmap as a part of "Official DDP specification": https://trello.com/card/official-ddp-specification/508721606e02bb9d570016ae/53

Owner
glasser commented Jan 31, 2013

@sixolet fixed this on the ddp-pre1 branch. The release that incorporates this branch will work seamlessly with many more types in Mongo, DDP, Session, etc. ObjectID, Date, Binary, and more are supported.

Contributor
sixolet commented Feb 6, 2013

Closing since it's fixed on a branch scheduled for devel soon.

@sixolet sixolet closed this Feb 6, 2013
@ghost
ghost commented Apr 3, 2013

in console:
var categoryBooks = store.findOne({Category:"Livros"}); store.remove({_id:categoryBooks._id}); delete categoryBooks;

I think this issue might be back?

I can't Articles.findOne("572bdb811ab1829622aeee78"), but I can Articles.findOne(new Mongo.ObjectID("572bdb811ab1829622aeee78")) after creating the Article in meteor mongo.

Here's a more detailed post I wrote about it on StackOverflow.

I think this issue might be back?

I can't Articles.findOne("572bdb811ab1829622aeee78"), but I can Articles.findOne(new >Mongo.ObjectID("572bdb811ab1829622aeee78")) after creating the Article in meteor mongo.

I have also seen this again today.

Collaborator
abernix commented Jun 1, 2016 edited

The _id that is created when using Meteor (through a Method, or .insert() within Meteor code) is by default, not the same as the _id that would be created using Mongo (with either mongo or meteor mongo). If not specified, Mongo generates a Mongo.ObjectID (you can specify whatever you want for the _id though).

In Meteor, the idGeneration option when defining a Mongo.Collection is what determines how the _id is generated (see these docs).

If you want to .insert rows manually using the mongo shell, you should use the same technique as Meteor uses to generate the ID. Though as long as you are sure the ID is unique, you can use whatever random string you want – Random.id() works well for this.

But really, the easiest way is to just use the meteor shell instead of meteor mongo. This will handle the ID generation for you and works exactly the same as .insert inside your usual Mongo shell.

The record has to be retrieved with however the _id was created...

...If it's a Mongo.ObjectId(), then .find({_id: Mongo.ObjectId('...')}). If it was a string, then .find({_id: '...'}) with a string.

p.s. If you want to know the particular reasons that things are this way in Meteor (someone actually this great explanation earlier today!).

Aha! That's very useful! Yes, of course, I created my collection using mongoimport rather than from inside meteor :)

just use the meteor shell instead of meteor mongo

Oh whoa, I didn't even know this existed. The tutorials should be updated to use this instead - thanks @abernix 😄

er1
command is retrived findOne!!
But Meteor Programming
let data1 = Tasks.findOne({mobile:mobile_number});
output console is undefined ?

Collaborator

@aboobakkar - see my comments in #8072 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment