chore: add test for atomic upsertWithWhere (#1864) #1871
Conversation
jannyHou
force-pushed
the
backport-atomic-upsert
branch
from
792c416 to
89989f9
Compare
test/manipulation.test.js
Outdated
| id: '4', name: 'Brian', | ||
| }, function(err) { | ||
| err.message.should.equal('There are multiple instances found.' + | ||
| bdd.itIf(connectorCapabilities.atomicUpsertWithWhere !== true, |
There was a problem hiding this comment.
When adding a new connector capability flag, we should pick a reasonable default so that we don't have to update all connectors with the new flag. Considering that DAO does not require connectors to implement atomic upsertWithWhere and provides a non-atomic implementation in such case, I think we should assume connectors don't support atomic version by default.
We need to ensure one of the bdd.itIf cases is executed when connectorCapabilities.atomicUpsertWithWhere is not set to any value (it's undefined). I think it will be this particular case, right?
I think the condition here could be simplified as follows:
| bdd.itIf(connectorCapabilities.atomicUpsertWithWhere !== true, | |
| bdd.itIf(!connectorCapabilities.atomicUpsertWithWhere, |
| 'upsertWithWhere update the first matching instance when multiple instances are ' + | ||
| 'retrieved based on the filter criteria', async () => { | ||
| // The first matching instance is determinate from specific connector implementation | ||
| // For example for mongodb connector the sort parameter is used (default to _id asc) |
There was a problem hiding this comment.
I find it problematic that connectors implement different behavior than our default non-atomic implementation. Are we sure that SQL connectors like MySQL and PostgreSQL will update the first record only, the same way as MongoDB connector works? It would be great if we could align juggler-provided non-atomic version with the behavior of real connectors (both SQL and noSQL). If different connectors behave differently, then IMO we need to clearly document these differences and probably need to add a new connectorCapability flag to allow the test suite to execute the right set of expectations?
There was a problem hiding this comment.
Are we sure that SQL connectors like MySQL and PostgreSQL will update the first record only, the same way as MongoDB connector works?
@bajtos Currently only mongodb supports that, no other connectors have atomic implementation.
Hmm isn't atomicUpsertWithWhere already a connector flag? I will document the behaviour after PRs merge :)
jannyHou
force-pushed
the
backport-atomic-upsert
branch
from
f8adbd1 to
8869c5d
Compare
Introduce the new property atomicUpsertWithWhere for connector that implement specific method. See loopbackio/loopback-connector-mongodb#563 for mongodb implementation. Signed-off-by: Matteo Padovano <mrbatista@users.noreply.github.com>
jannyHou
force-pushed
the
backport-atomic-upsert
branch
from
8869c5d to
fdd2bda
Compare
Introduce the new property atomicUpsertWithWhere for
connector that implement specific method.
See loopbackio/loopback-connector-mongodb#563
for mongodb implementation.
Backport #1864
Signed-off-by: Matteo Padovano mrbatista@users.noreply.github.com
Checklist
npm testpasses on your machine