Atomic upsertWithWhere

[mrbatista]

Implement atomic upsertWithWhere method.

The out-of-the-box implementation catch error if multiple instances are founds.
With atomic implementation one test present in the juggler datasource will be disabled.
I added a new option to specify if connector has an atomic implementation. See loopbackio/loopback-datasource-juggler#1864.

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

• npm test passes on your machine
• New tests added or existing tests modified to cover all changes
• Code conforms with the style guide
• Commit messages are following our guidelines

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[agnes512]

@mrbatista Thank you for the PR. Could you add some tests for the changes?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[dhmlau]

We just switch the contribution method from CLA to DCO, making your contribution easier in the future. Please sign the commits with DCO by amending your commit messages with -s flag and push the changes again. If you're using the command line, you can:

git commit --amend -s
git push --force-with-lease

Please refer to this docs page for details. Thanks!

For more questions, please join our Slack workspace - #loopback-contributors.

[jamesjjk]

@mrbatista @fabien I don't understand why you would want to use a transactional operation? Maybe I'm missing something. Atomic upsert and document return is achieved with findAndModify operation with the "new" and "update" flags. transactional seems overkill.

[mrbatista]

@jamesjjk If the connector does not implement the upsertWithWhere method, the DAO use a non atomic implementation. See the relevant code.
I need the atomic implementation for concurrent requests.

[jamesjjk]

@jamesjjk If the connector does not implement the upsertWithWhere method, the DAO use a non atomic implementation. See the relevant code.
I need the atomic implementation for concurrent requests.

@mrbatista Atomic I fully understand but why transactional? I too need an atomic version which is implemented by MongoDB native findAndModify with the appropriate parameters. Your ticket mentions transactional, which is not the same as an atomic opetation. Would it not be sufficient to implement findAndModify (with new and upsert parameters) in the connector to parse any of the DOA's upsertWithWhere operations?

[mrbatista]

@jamesjjk I changed the title to respect the implementation of this PR.
For the implementation the DAO define the methods that each connector (oracle, mysql, ecc..) can implement.
If use findAndModfy (with specific options set) or updateMany (like in this PR) is not relevant.

[jamesjjk]

@mrbatista Understood, are you updating the respective connector? Which connector are you using if I may ask?

[mrbatista]

@mrbatista Understood, are you updating the respective connector? Which connector are you using if I may ask?

I'm currently using my fork which implements atomic upsertWithWhere.
Today add more test to permit review by mainteners of this repo. 😃

[mrbatista]

@agnes512 I have update the PR and first comment. Please review.
No test needed because the functionality of upsertWithWhere method is tested in loopback-datasource-juggler.
See relevant test:

1. normalize-undefined test
2. crud-with-options test
3. manipulation test
4. persistence-hooks suite

To have green tests need to merge this PR loopbackio/loopback-datasource-juggler#1864

[jannyHou]

Thanks @mrbatista , I merged and published the juggler test, but it fails due to

1) juggler-v3

       manipulation

         upsertWithWhere

           fails the upsertWithWhere operation when multiple instances are retrieved based on the filter criteria:

      Uncaught AssertionError: expected 'Performing an update on the path \'_id\' would modify the immutable field \'_id\'' to be 'There are multiple instances found.Upsert Operation will not be performed!'

      + expected - actual

      -Performing an update on the path '_id' would modify the immutable field '_id'

      +There are multiple instances found.Upsert Operation will not be performed!

juggler-v3 complains the same problem. Could you take a look?

[jannyHou]

cc @mrbatista any update about the test?

[mrbatista]

@jannyHou You have release only juggler 4.x. Please backport the change of this commit in 3.x juggler branch.
Ping me when release a new 3.x version.

[mrbatista]

@jannyHou On my local environment only the juggler 3.x fail.

[jannyHou]

@mrbatista Thanks, I see created PR loopbackio/loopback-datasource-juggler#1871 to back port it.

[mrbatista]

@jannyHou Any update on this PR?

[jannyHou]

Hi @mrbatista sorry for late update, I am making time to address the feedback in loopbackio/loopback-datasource-juggler#1871, after that, this PR should be good to land.

[jannyHou]

@slnode test please

[bajtos]

I wanted to re-run Travis CI build but it looks like the old Travis at https://travis-ci.org does not allow that (or I don't have permissions to do so). I think we will need to force-push to the feature branch to trigger a new build, can you @jannyHou PTAL?

We need a new run to pick the recently released versions of juggler v3 and v4 that should fix the failure of the test it fails the upsertWithWhere operation when multiple instances are retrieved based on the filter criteria

[jannyHou]

@bajtos thank you for looking into the error, triggered a test.

[jannyHou]

It failed because I didn't release the change in juggler v3. Just released 3.36.1, let me rerun it.

Great it passed.

[mrbatista]

@jannyHou green CI 🎉🥳

[jannyHou]

👍 Cheers!

[bajtos]

👏🏻