Add a bulkUpdate function #1399
Conversation
There was a problem hiding this comment.
@ssh24 TBH, I am not sure if we need an atomic version of bulkUpdate, considering that the current implementation inside loopback goes a long way to ensure consistency of data and prevent unintentional overwriting. Perhaps all we need is to modify DAO.update() API to support the needs of applyUpdate() when run against CouchDB?
Having said that, if we can find a good way how to implement atomic bulkUpdate in connectors while preserving the important features of the current implementation, then I will happily applause.
The first major issue I see in this proposal is how to correctly invoke different operation hooks that are triggered by create/update/delete operations, preserving the context options that are expected by existing hook implementations. The current design of operation hooks is a result of long design and refinement process, we need to carefully asses impact of changes.
As I commented in strongloop/loopback#3433 (comment), I would like to see a robust test suite covering bulkUpdate in juggler's shared test suite run by all connectors. This test suite should not only verify that bulkUpdate performs the right data changes (including when a conflict occurs), but also verify that operation hooks are invoked with correct context values.
|
|
||
| var context = { | ||
| Model: Model, | ||
| options: options, |
There was a problem hiding this comment.
This context is not honouring the fields provided by other DAO methods for access hook. See https://loopback.io/doc/en/lb3/Operation-hooks.html#access
| datas: datas, | ||
| options: options, | ||
| }; | ||
| Model.notifyObserversOf('before save', context, |
There was a problem hiding this comment.
This context is not honouring the fields provided by other DAO methods for access hook, see https://loopback.io/doc/en/lb3/Operation-hooks.html#before-save
Also "before save" is not a correct hook name considering that bulkUpdate performs delete operations too.
| var context = { | ||
| Model: Model, | ||
| datas: datas, | ||
| options: options, |
There was a problem hiding this comment.
See https://loopback.io/doc/en/lb3/Operation-hooks.html#persist
Again, we shouldn't be invoking "persist" for instances that are going to be deleted.
| options: options, | ||
| info: info, | ||
| }; | ||
| Model.notifyObserversOf('after save', context, function(err, ctx) { |
There was a problem hiding this comment.
See https://loopback.io/doc/en/lb3/Operation-hooks.html#after-save
Also "after save" shouldn't be invoked for records that are going to be deleted.
|
Closing this PR. Found another way to call just the cloudant implementation of the |
Description
Add a static method for
bulkUpdate. Connectors now can callbulkUpdateto perform a bulk update on data.connect to loopbackio/loopback-connector-cloudant#35