diff --git a/services/track-changes/app/coffee/DiffGenerator.coffee b/services/track-changes/app/coffee/DiffGenerator.coffee index e0fcf744cd..e57033ab40 100644 --- a/services/track-changes/app/coffee/DiffGenerator.coffee +++ b/services/track-changes/app/coffee/DiffGenerator.coffee @@ -11,8 +11,15 @@ module.exports = DiffGenerator = ConsistencyError: ConsistencyError rewindUpdate: (content, update) -> - for op in update.op.slice().reverse() - content = DiffGenerator.rewindOp content, op + for op in update.op by -1 + try + content = DiffGenerator.rewindOp content, op + catch e + if e instanceof ConsistencyError + logger.error {update, op: JSON.stringify(op)}, "marking op as broken" + op.broken = true + else + throw e # rethrow the execption return content rewindOp: (content, op) -> @@ -90,7 +97,7 @@ module.exports = DiffGenerator = return newDiff applyUpdateToDiff: (diff, update) -> - for op in update.op + for op in update.op when op.broken isnt true diff = DiffGenerator.applyOpToDiff diff, op, update.meta return diff diff --git a/services/track-changes/app/coffee/DiffManager.coffee b/services/track-changes/app/coffee/DiffManager.coffee index 29016fc42c..91d41a909f 100644 --- a/services/track-changes/app/coffee/DiffManager.coffee +++ b/services/track-changes/app/coffee/DiffManager.coffee @@ -45,9 +45,12 @@ module.exports = DiffManager = if lastUpdate? and lastUpdate.v != version - 1 return callback new Error("latest update version, #{lastUpdate.v}, does not match doc version, #{version}") + tryUpdates = updates.slice().reverse() + try - startingContent = DiffGenerator.rewindUpdates content, updates.slice().reverse() + startingContent = DiffGenerator.rewindUpdates content, tryUpdates + # tryUpdates is reversed, and any unapplied ops are marked as broken catch e return callback(e) - - callback(null, startingContent, updates) \ No newline at end of file + + callback(null, startingContent, tryUpdates) diff --git a/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee b/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee index e3214a2f36..388520293e 100644 --- a/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee +++ b/services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee @@ -108,7 +108,11 @@ describe "DiffManager", -> describe "with matching versions", -> beforeEach -> @DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(4, null, @content, @version, @updates) - @DiffGenerator.rewindUpdates = sinon.stub().returns(@rewound_content) + @DiffGenerator.rewindUpdates = sinon.spy (content, updates) => + # the rewindUpdates method reverses the 'updates' array + updates.reverse() + return @rewound_content + @rewindUpdatesWithArgs = @DiffGenerator.rewindUpdates.withArgs(@content, @updates.slice().reverse()) @DiffManager.getDocumentBeforeVersion @project_id, @doc_id, @fromVersion, @callback it "should get the latest doc and version with all recent updates", -> @@ -117,9 +121,7 @@ describe "DiffManager", -> .should.equal true it "should rewind the diff", -> - @DiffGenerator.rewindUpdates - .calledWith(@content, @updates.slice().reverse()) - .should.equal true + sinon.assert.calledOnce(@rewindUpdatesWithArgs) it "should call the callback with the rewound document and updates", -> @callback.calledWith(null, @rewound_content, @updates).should.equal true