erikolson186 / zangodb

MongoDB-like interface for HTML5 IndexedDB
https://erikolson186.github.io/zangodb/
MIT License
1.07k stars 72 forks source link

Add Collection#findOne #4

Open keithnorm opened 7 years ago

keithnorm commented 7 years ago

Noticed this method was missing and it was easy to add, so here you go if this is helpful.

Not sure how closely you intend to mirror mongodb, but I'm using zango on the front end and mongodb on the backend in an isomorphic React app, so getting as close as possible to full mongodb compatibility is best for me.

Thanks for a great implementation.

keithnorm commented 7 years ago

FYI most of the changes are because I compiled with a newer version of babel. Aside from that and regenerating the docs, the real changes are in this commit https://github.com/erikolson186/zangodb/pull/4/commits/d74a5feb837eff100ac29c0aaa0c51f3bb03618d

erikolson186 commented 7 years ago

I am glad that you are using ZangoDB and I appreciate the contribution.

I looked at the API documentation for the MongoDB node driver and saw that the findOne method supports a callback: http://mongodb.github.io/node-mongodb-native/2.0/api/Collection.html#findOne

I went ahead and implemented the method for ZangoDB a little differently than you did so as to support a callback: https://github.com/erikolson186/zangodb/commit/a8bc729cf5e0a561fd8e8c84e831e32feaf4a5e6

If you could comment on the change to create_next_fn.js I would appreciate it. Is there a benefit to retrieving each next function in the same order as they were created?

Thanks!

keithnorm commented 7 years ago

Yeah, if you run the find_one.js test suite I added without that change to create_next_fn it fails the should perform disjunction equality test and I traced that failure down to the order of executing next fn's. I don't recall the actual details of what was going wrong now, but I can go through it again later on and get back to you with more details on that if you want.

erikolson186 commented 7 years ago

Don't worry about going through it, I know what the problem is. What I would like to know, though, is if MongoDB ensures document order with disjunction queries. If so, your change is useful for more reasons than just tests.

keithnorm commented 7 years ago

I ran through the test case in mongo shell and it does concur with the test expectation:

db.docs.insert([{x:2,g:3}, {x:2,g:8} ,{x:2,g:8,z:10}, {x:3,z:3}, {x:4,k:8}, {x:6,g:9}, {x:10,k:4}, {x:undefined}, {x:null}, {x:[{k:2}, {k:8}]}])
db.docs.findOne({ x: { $in: [2, 4, 8] } })
// => { "_id" : ObjectId("591606893f674ab10d9e9c0a"), "x" : 2, "g" : 3 }

So without that change the test fails with AssertionError: expected { x: 2, g: 3 } to deeply equal { x: 4, k: 8 }

You might want to cherry pick in this commit https://github.com/erikolson186/zangodb/pull/4/commits/176829233f61f27236f80108c2a5667c9ac30167 to get the test coverage for your implementation of findOne too.

erikolson186 commented 7 years ago

Great work. I committed a fix, although iterating in reverse rather than using shift: https://github.com/erikolson186/zangodb/commit/cd3e37762d8a63eb2be44a9cff8063eadfabc55a

I am going to go through some files to cherry pick. The conf.json, package.json, .gitignore, and find_one.js files all look promising.