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

[ENHANCEMENT] working with updateOne() #91

Open
zsimoes opened this issue Dec 12, 2019 · 5 comments
Open

[ENHANCEMENT] working with updateOne() #91

zsimoes opened this issue Dec 12, 2019 · 5 comments

Comments

@zsimoes
Copy link

zsimoes commented Dec 12, 2019

Is there a viable way to adopt a similar strategy with Model.updateOne()?

Pondering on how the pre-save hook works, I understand any document that is retrieved from the database (that can subsequently be save()d) is guaranteed to have a version key which can be used in the $where clause of the current query. An updateOne() operation might not have a versionKey suitable for this purpose.

I believe a pre('updateOne') hook could have the same functionality as pre('save') if the query passed has a key that matches the schema versionKey. Is this too naïve, or am I missing some obvious difficulty?

Otherwise, is there an interest in adding this functionality to this package, or does it fall out of scope?

@zsimoes zsimoes added the bug label Dec 12, 2019
@zsimoes zsimoes changed the title [IMPROVEMENT] working with updateOne() [ENHANCEMENT] working with updateOne() Dec 12, 2019
@eoin-obrien
Copy link
Owner

Good question! I've thought it over, and I think I can definitely add this once a few design decisions have been made. There would be two elements to getting this working:

  1. Passing the current version of the document to the query.
  2. Incrementing the document's version key if the update succeeds.

Grabbing the version key from the filter parameter in the pre-update hook is straightforward, as is adding a $set or $inc for the version key. An issue could arise where the version key is absent from the filter or the version key is a complex query, e.g. version: { $gte: 4, $lte: 10 }, rather than a number.

To avoid any unexpected behaviour and stick to the principle of least astonishment, the best way to implement this might be to pass a flag to the updateOne call to explicitly switch on OCC functionality. A call might look something like this:

await Person.updateOne({ name: 'Jean-Luc Picard' }, { ship: 'USS Enterprise' }, { occ: <version> });

Having the version key for OCC explicitly passed in the options parameter would help the library keep to the principle of least astonishment and allow the user to update an existing codebase relatively simply.

Let me know what you think of the above!

@zsimoes
Copy link
Author

zsimoes commented Dec 16, 2019

Thanks for the feedback :)

I agree, an explicit flag in the updateOne options sounds like the best strategy to prevent unwanted/unexpected behaviour, and allow incremental codebase updates. It also takes care of the complex version key query problem.

Another point: Is this also applicable to timestamps? E.g. depending on the plugin used on a given schema (version key or timestamp) the value of the occ flag could be assumed to be a versionKey or a timestamp.

@eoin-obrien
Copy link
Owner

Thanks! Yes, I'd think that the timestamp plugin would work in much the same way as the version key plugin; the only difference would be that Mongoose automatically updates the updatedAt timestamp field, so we wouldn't need to include a $set for that at all.

@zsimoes
Copy link
Author

zsimoes commented Dec 17, 2019

Great! I don't have time to submit a PR right now, but if you want help with this feature around mid-January I'll gladly help.

@GitProdEnv
Copy link

Any update on this?

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

No branches or pull requests

3 participants