Merge pull request #15 from sharelatex/ja-copy-paste-diff

Run a diff against big delete - insert changes which are likely copy-…

services/track-changes/app/coffee/UpdateCompressor.coffee

@@ -1,6 +1,9 @@
 strInject = (s1, pos, s2) -> s1[...pos] + s2 + s1[pos..]
 strRemove = (s1, pos, length) -> s1[...pos] + s1[(pos + length)..]
 
+diff_match_patch = require("../lib/diff_match_patch").diff_match_patch
+dmp = new diff_match_patch()
+
 module.exports = UpdateCompressor =
 	NOOP: "noop"
 
@@ -155,6 +158,59 @@ module.exports = UpdateCompressor =
 				# This will only happen if the delete extends outside the insert
 				return [firstUpdate, secondUpdate]
 
+		# A delete then an insert at the same place, likely a copy-paste of a chunk of content
+		else if firstOp.d? and secondOp.i? and firstOp.p == secondOp.p
+			offset = firstOp.p
+			diff_ops = @diffAsShareJsOps(firstOp.d, secondOp.i)
+			if diff_ops.length == 0
+				return [{ # Noop
+					meta:
+						start_ts: firstUpdate.meta.start_ts
+						end_ts:   secondUpdate.meta.end_ts
+						user_id:  firstUpdate.meta.user_id
+					op:
+						p: firstOp.p
+						i: ""
+					v: secondUpdate.v
+				}]
+			else
+				return diff_ops.map (op) ->
+					op.p += offset
+					return {
+						meta:
+							start_ts: firstUpdate.meta.start_ts
+							end_ts:   secondUpdate.meta.end_ts
+							user_id:  firstUpdate.meta.user_id
+						op: op
+						v: secondUpdate.v
+					}
+
 		else
 			return [firstUpdate, secondUpdate]
 
+	ADDED: 1
+	REMOVED: -1
+	UNCHANGED: 0
+	diffAsShareJsOps: (before, after, callback = (error, ops) ->) ->
+		diffs = dmp.diff_main(before, after)
+		dmp.diff_cleanupSemantic(diffs)
+
+		ops = []
+		position = 0
+		for diff in diffs
+			type = diff[0]
+			content = diff[1]
+			if type == @ADDED
+				ops.push
+					i: content
+					p: position
+				position += content.length
+			else if type == @REMOVED
+				ops.push
+					d: content
+					p: position
+			else if type == @UNCHANGED
+				position += content.length
+			else
+				throw "Unknown type"
+		return ops

services/track-changes/test/unit/coffee/UpdateCompressor/UpdateCompressorTests.coffee

@@ -5,13 +5,15 @@ expect = chai.expect
 modulePath = "../../../../app/js/UpdateCompressor.js"
 SandboxedModule = require('sandboxed-module')
 
+bigstring = ("a" for [0 .. 2*1024*1024]).join("")
+mediumstring = ("a" for [0 .. 1024*1024]).join("")
+
 describe "UpdateCompressor", ->
 	beforeEach ->
-		@UpdateCompressor = SandboxedModule.require modulePath
+		@UpdateCompressor = SandboxedModule.require modulePath, requires:
+			"../lib/diff_match_patch": require("../../../../app/lib/diff_match_patch")
 		@user_id = "user-id-1"
 		@other_user_id = "user-id-2"
-		@bigstring = ("a" for [0 .. 2*1024*1024]).join("")
-		@mediumstring = ("a" for [0 .. 1024*1024]).join("")
 		@ts1 = Date.now()
 		@ts2 = Date.now() + 1000
 
@@ -149,7 +151,7 @@ describe "UpdateCompressor", ->
 					meta: ts: @ts1, user_id: @user_id
 					v: 42
 				}, {
-					op: { p: 6, i: @bigstring }
+					op: { p: 6, i: bigstring }
 					meta: ts: @ts2, user_id: @user_id
 					v: 43
 				}])
@@ -158,47 +160,47 @@ describe "UpdateCompressor", ->
 					meta: start_ts: @ts1, end_ts: @ts1, user_id: @user_id
 					v: 42
 				}, {
-					op: { p: 6, i: @bigstring }
+					op: { p: 6, i: bigstring }
 					meta: start_ts: @ts2, end_ts: @ts2, user_id: @user_id
 					v: 43
 				}]
 
 			it "should not append inserts that are too big (first op)", ->
 				expect(@UpdateCompressor.compressUpdates [{
-					op: { p: 3, i: @bigstring }
+					op: { p: 3, i: bigstring }
 					meta: ts: @ts1, user_id: @user_id
 					v: 42
 				}, {
-					op: { p: 3 + @bigstring.length, i: "bar" }
+					op: { p: 3 + bigstring.length, i: "bar" }
 					meta: ts: @ts2, user_id: @user_id
 					v: 43
 				}])
 				.to.deep.equal [{
-					op: { p: 3, i: @bigstring }
+					op: { p: 3, i: bigstring }
 					meta: start_ts: @ts1, end_ts: @ts1, user_id: @user_id
 					v: 42
 				}, {
-					op: { p: 3 + @bigstring.length, i: "bar" }
+					op: { p: 3 + bigstring.length, i: "bar" }
 					meta: start_ts: @ts2, end_ts: @ts2, user_id: @user_id
 					v: 43
 				}]
 
 			it "should not append inserts that are too big (first and second op)", ->
 				expect(@UpdateCompressor.compressUpdates [{
-					op: { p: 3, i: @mediumstring }
+					op: { p: 3, i: mediumstring }
 					meta: ts: @ts1, user_id: @user_id
 					v: 42
 				}, {
-					op: { p: 3 + @mediumstring.length, i: @mediumstring }
+					op: { p: 3 + mediumstring.length, i: mediumstring }
 					meta: ts: @ts2, user_id: @user_id
 					v: 43
 				}])
 				.to.deep.equal [{
-					op: { p: 3, i: @mediumstring }
+					op: { p: 3, i: mediumstring }
 					meta: start_ts: @ts1, end_ts: @ts1, user_id: @user_id
 					v: 42
 				}, {
-					op: { p: 3 + @mediumstring.length, i: @mediumstring }
+					op: { p: 3 + mediumstring.length, i: mediumstring }
 					meta: start_ts: @ts2, end_ts: @ts2, user_id: @user_id
 					v: 43
 				}]
@@ -345,6 +347,43 @@ describe "UpdateCompressor", ->
 					meta: start_ts: @ts2, end_ts: @ts2, user_id: @user_id
 					v: 43
 				}]
+		
+		describe "delete - insert", ->
+			it "should do a diff of the content", ->
+				expect(@UpdateCompressor.compressUpdates [{
+					op: { p: 3, d: "one two three four five six seven eight" }
+					meta: ts: @ts1, user_id: @user_id
+					v: 42
+				}, {
+					op: { p: 3, i: "one 2 three four five six seven eight" }
+					meta: ts: @ts2, user_id: @user_id
+					v: 43
+				}])
+				.to.deep.equal [{
+					op: { p: 7, d: "two" }
+					meta: start_ts: @ts1, end_ts: @ts2, user_id: @user_id
+					v: 43
+				}, {
+					op: { p: 7, i: "2" }
+					meta: start_ts: @ts1, end_ts: @ts2, user_id: @user_id
+					v: 43
+				}]
+			
+			it "should return a no-op if the delete and insert are the same", ->
+				expect(@UpdateCompressor.compressUpdates [{
+					op: { p: 3, d: "one two three four five six seven eight" }
+					meta: ts: @ts1, user_id: @user_id
+					v: 42
+				}, {
+					op: { p: 3, i: "one two three four five six seven eight" }
+					meta: ts: @ts2, user_id: @user_id
+					v: 43
+				}])
+				.to.deep.equal [{
+					op: { p: 3, i: "" }
+					meta: start_ts: @ts1, end_ts: @ts2, user_id: @user_id
+					v: 43
+				}]
 
 		describe "noop - insert", ->
 			it "should leave them untouched", ->