Browse Source

skip any broken ops when viewing history diffs

Brian Gough 11 years ago
parent
commit
23c43b8042

+ 10 - 3
services/track-changes/app/coffee/DiffGenerator.coffee

@@ -11,8 +11,15 @@ module.exports = DiffGenerator =
 	ConsistencyError: ConsistencyError
 	ConsistencyError: ConsistencyError
 
 
 	rewindUpdate: (content, update) ->
 	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
 		return content
 
 
 	rewindOp: (content, op) ->
 	rewindOp: (content, op) ->
@@ -90,7 +97,7 @@ module.exports = DiffGenerator =
 		return newDiff
 		return newDiff
 
 
 	applyUpdateToDiff: (diff, update) ->
 	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
 			diff = DiffGenerator.applyOpToDiff diff, op, update.meta
 		return diff
 		return diff
 
 

+ 6 - 3
services/track-changes/app/coffee/DiffManager.coffee

@@ -45,9 +45,12 @@ module.exports = DiffManager =
 			if lastUpdate? and lastUpdate.v != version - 1
 			if lastUpdate? and lastUpdate.v != version - 1
 				return callback new Error("latest update version, #{lastUpdate.v}, does not match doc version, #{version}")
 				return callback new Error("latest update version, #{lastUpdate.v}, does not match doc version, #{version}")
 
 
+			tryUpdates = updates.slice().reverse()
+
 			try
 			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
 			catch e
 				return callback(e)
 				return callback(e)
-			
-			callback(null, startingContent, updates)
+
+			callback(null, startingContent, tryUpdates)

+ 6 - 4
services/track-changes/test/unit/coffee/DiffManager/DiffManagerTests.coffee

@@ -108,7 +108,11 @@ describe "DiffManager", ->
 		describe "with matching versions", ->
 		describe "with matching versions", ->
 			beforeEach ->
 			beforeEach ->
 				@DiffManager.getLatestDocAndUpdates = sinon.stub().callsArgWith(4, null, @content, @version, @updates)
 				@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
 				@DiffManager.getDocumentBeforeVersion @project_id, @doc_id, @fromVersion, @callback
 
 
 			it "should get the latest doc and version with all recent updates", ->
 			it "should get the latest doc and version with all recent updates", ->
@@ -117,9 +121,7 @@ describe "DiffManager", ->
 					.should.equal true
 					.should.equal true
 
 
 			it "should rewind the diff", ->
 			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", ->
 			it "should call the callback with the rewound document and updates", ->
 				@callback.calledWith(null, @rewound_content, @updates).should.equal true
 				@callback.calledWith(null, @rewound_content, @updates).should.equal true