search

scottwrobinson / camo

A class-based ES6 ODM for Mongo-like databases.
557 stars 81 forks source link

Id's not being recognized by mongodb 3.x unless in native formate #10

Closed paullryan closed 9 years ago

paullryan commented 9 years ago

With mongodb 3.x the requirement that _id be in ObjectId format is hard. This pull request checks a query for an _id that is not in Native format and makes it native.

scottwrobinson commented 9 years ago

Thanks for the pull request!

So your changes broke some of the tests for MongoClient (to test MongoClient you have to change the uri in each test manually, which is probably why you didn't catch this - I need to fix this...), so I'll have to look in to this more when I have time this weekend.

scottwrobinson commented 9 years ago

Finally got around to this. So I tested out Camo v0.8.0 on MongoDB v3.0.7 and haven't had any problems. I ran all of the tests and a web-app on it and I'm not seeing any failed tests or any problems with the data stored.

Can you tell me more about the problems you're seeing? Thanks!

paullryan commented 9 years ago

@scottwrobinson My issue isn't with failing tests it's with the need for the user of Camo to make all id's into an ObjectId prior to passing it to Camo. This makes working with rest requests more tricky as the cast has to be done by each request or a custom middleware needs to be added. By pushing the cast into camo it eliminates a decent bit of boilerplate code for your users.

scottwrobinson commented 9 years ago

Ah I see. I misunderstood your original post. Yes, this is an issue.

The problem is a bit more involved than just looking at query.id and converting it. Some of the queries use other operators like this: { '_id': { $in: ['abc123'] } }. So I'll need to figure out all the ways an id can be specified in the query and handle converting them all. Still shouldn't be too bad, but unfortunately your code won't work.

I should be able to get to this in the next day or two. Thanks again.

Scott

paullryan commented 9 years ago

No worries, your donating your time, thank you.

scottwrobinson commented 9 years ago

Finally got around to fixing this. Thanks for your patience.

I had to handle all the different ways an ID could be used in a query, like { _id: { '$in': [ 'K1cbMk7T8A0OU83IAT4dFa91', 'Y1cbak7T8A1OU83IBT6aPq11' ] } }, so hopefully I covered everything. If you find a query that contains an _id that doesn't get cast, please let me know.

Thanks again for bringing up the issue!

Scott