From 65c14c177922a969bdde2c96e770f06f0f0bed8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Fri, 20 Mar 2015 17:47:07 +0100 Subject: [PATCH] Add conflict resolution API New methods: conflict.resolveUsingSource(cb) conflict.resolveUsingTarget(cb) conflict.resolveManually(data, cb) --- common/models/change.js | 75 ++++++++++++++++++++++++++ test/replication.test.js | 114 +++++++++++++++++++++++++++++---------- 2 files changed, 161 insertions(+), 28 deletions(-) diff --git a/common/models/change.js b/common/models/change.js index afd6625d..556a9450 100644 --- a/common/models/change.js +++ b/common/models/change.js @@ -616,6 +616,13 @@ module.exports = function(Change) { /** * Resolve the conflict. * + * Set the source change's previous revision to the current revision of the + * (conflicting) target change. Since the changes are no longer conflicting + * and appear as if the source change was based on the target, they will be + * replicated normally as part of the next replicate() call. + * + * This is effectively resolving the conflict using the source version. + * * @callback {Function} callback * @param {Error} err */ @@ -629,6 +636,74 @@ module.exports = function(Change) { }); }; + /** + * Resolve the conflict using the instance data in the source model. + * + * @callback {Function} callback + * @param {Error} err + */ + Conflict.prototype.resolveUsingSource = function(cb) { + this.resolve(function(err) { + // don't forward any cb arguments from resolve() + cb(err); + }); + }; + + /** + * Resolve the conflict using the instance data in the target model. + * + * @callback {Function} callback + * @param {Error} err + */ + Conflict.prototype.resolveUsingTarget = function(cb) { + var conflict = this; + + conflict.models(function(err, source, target) { + if (err) return done(err); + if (target === null) { + return conflict.SourceModel.deleteById(conflict.modelId, done); + } + var inst = new conflict.SourceModel(target); + inst.save(done); + }); + + function done(err) { + // don't forward any cb arguments from internal calls + cb(err); + } + }; + + /** + * Resolve the conflict using the supplied instance data. + * + * @param {Object} data The set of changes to apply on the model + * instance. Use `null` value to delete the source instance instead. + * @callback {Function} callback + * @param {Error} err + */ + + Conflict.prototype.resolveManually = function(data, cb) { + var conflict = this; + if (!data) { + return conflict.SourceModel.deleteById(conflict.modelId, done); + } + + conflict.models(function(err, source, target) { + if (err) return done(err); + var inst = source || new conflict.SourceModel(target); + inst.setAttributes(data); + inst.save(function(err) { + if (err) return done(err); + conflict.resolve(done); + }); + }); + + function done(err) { + // don't forward any cb arguments from internal calls + cb(err); + } + }; + /** * Determine the conflict type. * diff --git a/test/replication.test.js b/test/replication.test.js index 6c875d25..cc091d03 100644 --- a/test/replication.test.js +++ b/test/replication.test.js @@ -963,50 +963,70 @@ describe('Replication / Change APIs', function() { ], done); }); - it('handles conflict resolved using "ours"', function(done) { - testResolvedConflictIsHandledWithNoMoreConflicts( + it('handles UPDATE conflict resolved using "ours"', function(done) { + testUpdateConflictIsResolved( function resolveUsingOurs(conflict, cb) { - conflict.resolve(cb); + conflict.resolveUsingSource(cb); }, done); }); - it('handles conflict resolved using "theirs"', function(done) { - testResolvedConflictIsHandledWithNoMoreConflicts( + it('handles UPDATE conflict resolved using "theirs"', function(done) { + testUpdateConflictIsResolved( function resolveUsingTheirs(conflict, cb) { - conflict.models(function(err, source, target) { - if (err) return cb(err); - // We sync ClientA->Server first - expect(conflict.SourceModel.modelName) - .to.equal(ClientB.modelName); - var m = new conflict.SourceModel(target); - m.save(cb); - }); + // We sync ClientA->Server first + expect(conflict.SourceModel.modelName) + .to.equal(ClientB.modelName); + conflict.resolveUsingTarget(cb); }, done); }); - it('handles conflict resolved manually', function(done) { - testResolvedConflictIsHandledWithNoMoreConflicts( + it('handles UPDATE conflict resolved manually', function(done) { + testUpdateConflictIsResolved( function resolveManually(conflict, cb) { - conflict.models(function(err, source, target) { - if (err) return cb(err); - var m = source || new conflict.SourceModel(target); - m.name = 'manual'; - m.save(function(err) { - if (err) return cb(err); - conflict.resolve(function(err) { - if (err) return cb(err); - cb(); - }); - }); - }); + conflict.resolveManually({ name: 'manual' }, cb); + }, + done); + }); + + it('handles DELETE conflict resolved using "ours"', function(done) { + testDeleteConflictIsResolved( + function resolveUsingOurs(conflict, cb) { + conflict.resolveUsingSource(cb); + }, + done); + }); + + it('handles DELETE conflict resolved using "theirs"', function(done) { + testDeleteConflictIsResolved( + function resolveUsingTheirs(conflict, cb) { + // We sync ClientA->Server first + expect(conflict.SourceModel.modelName) + .to.equal(ClientB.modelName); + conflict.resolveUsingTarget(cb); + }, + done); + }); + + it('handles DELETE conflict resolved as manual delete', function(done) { + testDeleteConflictIsResolved( + function resolveManually(conflict, cb) { + conflict.resolveManually(null, cb); + }, + done); + }); + + it('handles DELETE conflict resolved manually', function(done) { + testDeleteConflictIsResolved( + function resolveManually(conflict, cb) { + conflict.resolveManually({ name: 'manual' }, cb); }, done); }); }); - function testResolvedConflictIsHandledWithNoMoreConflicts(resolver, cb) { + function testUpdateConflictIsResolved(resolver, cb) { async.series([ // sync the new model to ClientB sync(ClientB, Server), @@ -1041,6 +1061,44 @@ describe('Replication / Change APIs', function() { ], cb); } + function testDeleteConflictIsResolved(resolver, cb) { + async.series([ + // sync the new model to ClientB + sync(ClientB, Server), + verifyInstanceWasReplicated(ClientA, ClientB, sourceInstanceId), + + // ClientA makes a change + function deleteInstanceOnClientA(next) { + ClientA.deleteById(sourceInstanceId, next); + }, + + sync(ClientA, Server), + + // ClientB changes the same instance + updateClientB('b'), + + function syncAndResolveConflict(next) { + replicate(ClientB, Server, function(err, conflicts, cps) { + if (err) return next(err); + + expect(conflicts).to.have.length(1); + expect(conflicts[0].SourceModel.modelName) + .to.equal(ClientB.modelName); + + debug('Resolving the conflict %j', conflicts[0]); + resolver(conflicts[0], next); + }); + }, + + // repeat the last sync, it should pass now + sync(ClientB, Server), + // and sync back to ClientA too + sync(ClientA, Server), + + verifyInstanceWasReplicated(ClientB, ClientA, sourceInstanceId) + ], cb); + } + function updateClientB(name) { return function updateInstanceB(next) { ClientB.findById(sourceInstanceId, function(err, instance) {