From dfd3ec993b64fe49c1dcaa5d33b186ae419fd94f Mon Sep 17 00:00:00 2001
From: James Allen <james@sharelatex.com>
Date: Wed, 26 Feb 2014 14:49:52 +0000
Subject: [PATCH] Ensure version is still returned from redis

---
 services/document-updater/app/coffee/DocOpsManager.coffee  | 6 ++++--
 services/document-updater/app/coffee/ShareJsDB.coffee      | 7 ++++---
 .../unit/coffee/DocOpsManager/DocOpsManagerTests.coffee    | 6 +++---
 .../test/unit/coffee/ShareJsDB/WriteOpsTests.coffee        | 7 ++-----
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/services/document-updater/app/coffee/DocOpsManager.coffee b/services/document-updater/app/coffee/DocOpsManager.coffee
index 43fd7a4aac..971db83358 100644
--- a/services/document-updater/app/coffee/DocOpsManager.coffee
+++ b/services/document-updater/app/coffee/DocOpsManager.coffee
@@ -44,9 +44,11 @@ module.exports = DocOpsManager =
 				callback null, ops
 
 	pushDocOp: (project_id, doc_id, op, callback = (error) ->) ->
-		RedisManager.pushDocOp doc_id, op, (error) ->
+		RedisManager.pushDocOp doc_id, op, (error, version) ->
 			return callback(error) if error?
-			RedisManager.pushUncompressedHistoryOp doc_id, op, callback
+			RedisManager.pushUncompressedHistoryOp doc_id, op, (error) ->
+				return callback(error) if error?
+				callback null, version
 
 	_ensureOpsAreLoaded: (project_id, doc_id, backToVersion, callback = (error) ->) ->
 		RedisManager.getDocVersion doc_id, (error, redisVersion) ->
diff --git a/services/document-updater/app/coffee/ShareJsDB.coffee b/services/document-updater/app/coffee/ShareJsDB.coffee
index 6eaf21846c..da6640685b 100644
--- a/services/document-updater/app/coffee/ShareJsDB.coffee
+++ b/services/document-updater/app/coffee/ShareJsDB.coffee
@@ -4,6 +4,7 @@ DocumentManager = require "./DocumentManager"
 RedisManager = require "./RedisManager"
 DocOpsManager = require "./DocOpsManager"
 Errors = require "./Errors"
+logger = require "logger-sharelatex"
 
 module.exports = ShareJsDB =
 	getOps: (doc_key, start, end, callback) ->
@@ -29,9 +30,9 @@ module.exports = ShareJsDB =
 			if version == opData.v + 1
 				callback()
 			else
-				# The document has been corrupted by the change. For now, throw an exception.
-				# Later, rebuild the snapshot.
-				callback "Version mismatch in db.append. '#{doc_id}' is corrupted."
+				error = new Error("Version mismatch. '#{doc_id}' is corrupted.")
+				logger.error err: error, doc_id: doc_id, project_id: project_id, opVersion: opData.v, expectedVersion: version, "doc is corrupt"
+				callback error
 
 	getSnapshot: (doc_key, callback) ->
 		[project_id, doc_id] = Keys.splitProjectIdAndDocId(doc_key)
diff --git a/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee b/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee
index c26171d98f..df2a546480 100644
--- a/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee
+++ b/services/document-updater/test/unit/coffee/DocOpsManager/DocOpsManagerTests.coffee
@@ -309,7 +309,7 @@ describe "DocOpsManager", ->
 	describe "pushDocOp", ->
 		beforeEach ->
 			@op = "mock-op"
-			@RedisManager.pushDocOp = sinon.stub().callsArg(2)
+			@RedisManager.pushDocOp = sinon.stub().callsArgWith(2, null, @version = 42)
 			@RedisManager.pushUncompressedHistoryOp = sinon.stub().callsArg(2)
 			@DocOpsManager.pushDocOp @project_id, @doc_id, @op, @callback
 
@@ -323,7 +323,7 @@ describe "DocOpsManager", ->
 				.calledWith(@doc_id, @op)
 				.should.equal true
 
-		it "should call the callback", ->
-			@callback.called.should.equal true
+		it "should call the callback with the version", ->
+			@callback.calledWith(null, @version).should.equal true
 
 	
diff --git a/services/document-updater/test/unit/coffee/ShareJsDB/WriteOpsTests.coffee b/services/document-updater/test/unit/coffee/ShareJsDB/WriteOpsTests.coffee
index b28f23d2f4..6088de77f4 100644
--- a/services/document-updater/test/unit/coffee/ShareJsDB/WriteOpsTests.coffee
+++ b/services/document-updater/test/unit/coffee/ShareJsDB/WriteOpsTests.coffee
@@ -26,11 +26,8 @@ describe "ShareJsDB.writeOps", ->
 			@ShareJsDB.writeOp @doc_key, @opData, @callback
 
 		it "should write the op to redis", ->
-			op =
-				op: @opData.op
-				meta: @opData.meta
 			@DocOpsManager.pushDocOp
-				.calledWith(@project_id, @doc_id, op)
+				.calledWith(@project_id, @doc_id, @opData)
 				.should.equal true
 
 		it "should call the callback without an error", ->
@@ -46,7 +43,7 @@ describe "ShareJsDB.writeOps", ->
 			@ShareJsDB.writeOp @doc_key, @opData, @callback
 
 		it "should call the callback with an error", ->
-			@callback.calledWith(sinon.match.string).should.equal true
+			@callback.calledWith(new Error()).should.equal true