Detect 3rd-party changes made during replication #1214
Conversation
bajtos
force-pushed
the
fix/bulkUpdate-race-condition
branch
from
dd4e039
to
552be04
Compare
| }); | ||
| break; | ||
| case Change.DELETE: | ||
| // NOTE(bajtos) Delete does not care whether the model has | ||
| // the same revision as expected, we are going to delete it anyway |
There was a problem hiding this comment.
Delete does not care whether the model has the same revision as expected
The delete should be a conflict if the current target revision doesn't match the previous revision of the source. If someone has changed the target, we shouldn't blindly delete it (although I'm not certain that is what you are doing).
There was a problem hiding this comment.
OK, I'll add a check for that too.
There was a problem hiding this comment.
The fix for this requires loopbackio/loopback-datasource-juggler#511 and possibly implementation in all connectors.
|
@ritch I have addressed all your comments, PTAL again. |
|
LGTM |
bajtos
force-pushed
the
fix/bulkUpdate-race-condition
branch
from
ed32072
to
126f9c4
Compare
|
@slnode test please |
Modify `Change.diff()` to include current data revision in each delta reported back. The current data revision is stored in `delta.prev`. Modify `PersistedModel.bulkUpdate()` to check that the current data revision matches `delta.prev` and report a conflict if a third party has modified the database under our hands. Fix `Change` implementation and tests so that they are no longer attempting to create instances with duplicate ids. (This used to work because the memory connector was silently converting such requests to updateOrCreate/findOrCreate.)
bajtos
force-pushed
the
fix/bulkUpdate-race-condition
branch
from
b7a552a
to
87940a4
Compare
|
The Jenkins build is passing on v0.10 and v0.12 and failing on io.js 1.6.0 due to nodejs/node#1215 that has been fixed in 1.6.1. @rmg could you please upgrade our build slaves? |
Detect 3rd-party changes made during replication
|
Yep, I'll upgrade in the morning. |
@chandadharap @dashby3000 @sumitha Can you check with our customers if they are using |
Change.diff()to include current data revision in each delta reported back. The current data revision is stored indelta.prev.PersistedModel.bulkUpdate()to check that the current data revision matchesdelta.prevand report a conflict if a third party has modified the database under our hands.Changeimplementation and tests so that they are no longer attempting to create instances with duplicate ids. (This used to work because the memory connector was silently converting such requests to updateOrCreate/findOrCreate.)Closes #986
Requires loopbackio/loopback-datasource-juggler#502
Requires loopbackio/loopback-datasource-juggler#511
Requires loopbackio/loopback-datasource-juggler#510
/to @ritch please review
/cc @BerkeleyTrue
Note: This change may be viewed as a breaking change, because a
bulkUpdatecalled with a hand-crafted list of updates will probably start reporting conflict errors. If you think this is a valid concern and we should preserve BC for such users, then I am proposing to add a flag tobulkUpdatethat will control this behaviour.Thoughts?