Skip to content
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

findOneAndUpdate using nedb client returns not updated document with updated nested property #55

Closed
royaltm opened this issue Feb 27, 2016 · 4 comments
Labels

Comments

@royaltm
Copy link

royaltm commented Feb 27, 2016

complete example in gist

updating with nested property:

SomeDoc.findOneAndUpdate({_id: someid}, {"foo.bar": "some new value"})

resolves to a document before the update, however the document in nedb storage gets updated.

in case of mongo the mongo's internal findOneAndUpdate implementation is responsible for correct behavior
in this instance however there are no equivalents for atomic find and update in nedb, so you are doing a hackish thing here, while instead you probably should just re-read the document from nedb:

                        db.findOne({_id: data._id}, function(error, doc) {
                            if (error) return reject(error);
                            resolve(doc);
                        });

nedb is already keeping the document in memory so this another call to db.findOne would most probably return consistent result with the last update.

@scottwrobinson
Copy link
Owner

NeDB does occasionally do file reads/writes (when using a file as the backend), so I wanted to avoid as many find/save/update calls as possible, but in hindsight this was a pretty bad idea that wasn't adequately backed up with tests...

Thanks for bringing this up, and for providing full example code 😃

@royaltm
Copy link
Author

royaltm commented Mar 2, 2016

IMHO just after the update, nedb has 99.999% chance that the referred document will still be in memory (AFAIK it keeps everything in mem once it's there), so the question is how does nedb schedule client calls. There's a race possibility between update and re-read but only if the promise on the nedb side isn't fullfilled immediately. We won't know without RTFS of nedb.

@royaltm
Copy link
Author

royaltm commented Mar 2, 2016

Maybe the proper way would be to ask for an atomic update function in nedb. I wonder why it's not already there.

@scottwrobinson
Copy link
Owner

Okay, just a few notes...

Looks like in NeDB v1.8+ db.update() now returns the updated doc data in the callback. This solves our issue, but the problem with v1.4+ is that there was a breaking change in the storage system. Camo currently uses v1.1.2.

So for the next patch version I'm about to release, I'll have to use a temporary fix (likely the one you posted above). In the next minor release I'll update the required NeDB version in package.json and provide a real fix to this issue.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants