Delete doc from redis after a full set if it wasn't in redis before

services/document-updater/app/coffee/DocumentManager.coffee

@@ -12,7 +12,7 @@ module.exports = DocumentManager =
 			timer.done()
 			_callback(args...)
 
-		RedisManager.getDoc doc_id, (error, lines, version) ->
+		RedisManager.getDoc doc_id, (error, lines, version, alreadyLoaded) ->
 			return callback(error) if error?
 			if !lines? or !version?
 				logger.log project_id: project_id, doc_id: doc_id, "doc not in redis so getting from persistence API"
@@ -21,9 +21,9 @@ module.exports = DocumentManager =
 					logger.log project_id: project_id, doc_id: doc_id, lines: lines, version: version, "got doc from persistence API"
 					RedisManager.putDocInMemory project_id, doc_id, lines, version, (error) ->
 						return callback(error) if error?
-						callback null, lines, version
+						callback null, lines, version, false
 			else
-				callback null, lines, version
+				callback null, lines, version, true
 
 	getDocAndRecentOps: (project_id, doc_id, fromVersion, _callback = (error, lines, version, recentOps) ->) ->
 		timer = new Metrics.Timer("docManager.getDocAndRecentOps")
@@ -50,7 +50,7 @@ module.exports = DocumentManager =
 			return callback(new Error("No lines were provided to setDoc"))
 
 		UpdateManager = require "./UpdateManager"
-		DocumentManager.getDoc project_id, doc_id, (error, oldLines, version) ->
+		DocumentManager.getDoc project_id, doc_id, (error, oldLines, version, alreadyLoaded) ->
 			return callback(error) if error?
 			
 			if oldLines? and oldLines.length > 0 and oldLines[0].text?
@@ -70,9 +70,18 @@ module.exports = DocumentManager =
 						user_id: user_id
 				UpdateManager.applyUpdates project_id, doc_id, [update], (error) ->
 					return callback(error) if error?
-					DocumentManager.flushDocIfLoaded project_id, doc_id, (error) ->
-						return callback(error) if error?
-						callback null
+					# If the document was loaded already, then someone has it open
+					# in a project, and the usual flushing mechanism will happen.
+					# Otherwise we should remove it immediately since nothing else
+					# is using it.
+					if alreadyLoaded
+						DocumentManager.flushDocIfLoaded project_id, doc_id, (error) ->
+							return callback(error) if error?
+							callback null
+					else
+						DocumentManager.flushAndDeleteDoc project_id, doc_id, (error) ->
+							return callback(error) if error?
+							callback null
 		
 
 	flushDocIfLoaded: (project_id, doc_id, _callback = (error) ->) ->

services/document-updater/test/acceptance/coffee/ApplyingUpdatesToADocTests.coffee

@@ -360,7 +360,7 @@ describe "Applying updates to a large doc (uses compression)", ->
 				throw error if error?
 				DocUpdaterClient.sendUpdates @project_id, @doc_id, updates, (error) =>
 					throw error if error?
-					setTimeout done, 200
+					setTimeout done, 500
 
 		after ->
 			MockTrackChangesApi.flushDoc.restore()

services/document-updater/test/acceptance/coffee/SettingADocumentTests.coffee

@@ -1,14 +1,16 @@
 sinon = require "sinon"
 chai = require("chai")
 chai.should()
+expect = require("chai").expect
 {db, ObjectId} = require "../../../app/js/mongojs"
+rclient = require("redis").createClient()
 
+MockTrackChangesApi = require "./helpers/MockTrackChangesApi"
 MockWebApi = require "./helpers/MockWebApi"
 DocUpdaterClient = require "./helpers/DocUpdaterClient"
 
 describe "Setting a document", ->
-	before (done) ->
-		[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
+	before ->
 		@lines = ["one", "two", "three"]
 		@version = 42
 		@update =
@@ -23,30 +25,31 @@ describe "Setting a document", ->
 		@source = "dropbox"
 		@user_id = "user-id-123"
 		
-		MockWebApi.insertDoc @project_id, @doc_id, lines: @lines
-		db.docOps.insert {
-			doc_id: ObjectId(@doc_id)
-			version: @version
-		}, (error) =>
-			throw error if error?
-			done()
+		sinon.spy MockTrackChangesApi, "flushDoc"
+		sinon.spy MockWebApi, "setDocumentLines"
+	
+	after ->
+		MockWebApi.setDocumentLines.restore()
+		MockTrackChangesApi.flushDoc.restore()
 				
-
 	describe "when the updated doc exists in the doc updater", ->
 		before (done) ->
-			sinon.spy MockWebApi, "setDocumentLines"
-			DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) =>
+			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
+			MockWebApi.insertDoc @project_id, @doc_id, lines: @lines
+			db.docOps.insert {
+				doc_id: ObjectId(@doc_id)
+				version: @version
+			}, (error) =>
 				throw error if error?
-				DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) =>
+				DocUpdaterClient.preloadDoc @project_id, @doc_id, (error) =>
 					throw error if error?
-					setTimeout () =>
-						DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, (error, res, body) =>
-							@statusCode = res.statusCode
-							done()
-					, 200
-
-		after ->
-			MockWebApi.setDocumentLines.restore()
+					DocUpdaterClient.sendUpdate @project_id, @doc_id, @update, (error) =>
+						throw error if error?
+						setTimeout () =>
+							DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, (error, res, body) =>
+								@statusCode = res.statusCode
+								done()
+						, 200
 
 		it "should return a 204 status code", ->
 			@statusCode.should.equal 204
@@ -65,4 +68,39 @@ describe "Setting a document", ->
 			DocUpdaterClient.getDoc @project_id, @doc_id, (error, res, doc) =>
 				doc.version.should.equal @version + 2
 				done()
+		
+		it "should leave the document in redis", (done) ->
+			rclient.get "doclines:#{@doc_id}", (error, lines) =>
+				throw error if error?
+				expect(JSON.parse(lines)).to.deep.equal @newLines
+				done()
+	
+	describe "when the updated doc does not exist in the doc updater", ->
+		before (done) ->
+			[@project_id, @doc_id] = [DocUpdaterClient.randomId(), DocUpdaterClient.randomId()]
+			MockWebApi.insertDoc @project_id, @doc_id, lines: @lines
+			db.docOps.insert {
+				doc_id: ObjectId(@doc_id)
+				version: @version
+			}, (error) =>
+				throw error if error?
+				DocUpdaterClient.setDocLines @project_id, @doc_id, @newLines, @source, @user_id, (error, res, body) =>
+					@statusCode = res.statusCode
+					done()
+		
+		it "should return a 204 status code", ->
+			@statusCode.should.equal 204
 
+		it "should send the updated doc lines to the web api", ->
+			MockWebApi.setDocumentLines
+				.calledWith(@project_id, @doc_id, @newLines)
+				.should.equal true
+		
+		it "should flush track changes"#, ->
+			# MockTrackChangesApi.flushDoc.calledWith(@doc_id).should.equal true
+		
+		it "should remove the document from redis", (done) ->
+			rclient.get "doclines:#{@doc_id}", (error, lines) =>
+				throw error if error?
+				expect(lines).to.not.exist
+				done()

services/document-updater/test/unit/coffee/DocumentManager/getDocTests.coffee

@@ -32,7 +32,7 @@ describe "DocumentUpdater - getDoc", ->
 				.should.equal true
 		
 		it "should call the callback with the doc info", ->
-			@callback.calledWith(null, @lines, @version).should.equal true
+			@callback.calledWith(null, @lines, @version, true).should.equal true
 
 		it "should time the execution", ->
 			@Metrics.Timer::done.called.should.equal true
@@ -60,7 +60,7 @@ describe "DocumentUpdater - getDoc", ->
 				.should.equal true
 
 		it "should call the callback with the doc info", ->
-			@callback.calledWith(null, @lines, @version).should.equal true
+			@callback.calledWith(null, @lines, @version, false).should.equal true
 
 		it "should time the execution", ->
 			@Metrics.Timer::done.called.should.equal true

services/document-updater/test/unit/coffee/DocumentManager/setDocTests.coffee

@@ -29,13 +29,14 @@ describe "DocumentManager - setDoc", ->
 		beforeEach ->
 			@beforeLines = ["before", "lines"]
 			@afterLines = ["after", "lines"]
+			@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, true)
+			@DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops)
+			@UpdateManager.applyUpdates = sinon.stub().callsArgWith(3, null)
+			@DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2)
+			@DocumentManager.flushAndDeleteDoc = sinon.stub().callsArg(2)
 
-		describe "successfully", ->
+		describe "when already loaded", ->
 			beforeEach ->
-				@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version)
-				@DiffCodec.diffAsShareJsOp = sinon.stub().callsArgWith(2, null, @ops)
-				@UpdateManager.applyUpdates = sinon.stub().callsArgWith(3, null)
-				@DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2)
 				@DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, @callback
 
 			it "should get the current doc lines", ->
@@ -76,17 +77,26 @@ describe "DocumentManager - setDoc", ->
 
 			it "should time the execution", ->
 				@Metrics.Timer::done.called.should.equal true
+		
+		describe "when not already loaded", ->
+			beforeEach ->
+				@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version, false)
+				@DocumentManager.setDoc @project_id, @doc_id, @afterLines, @source, @user_id, @callback
 
-	describe "without new lines", ->
-		beforeEach ->
-			@DocumentManager.getDoc = sinon.stub().callsArgWith(2, null, @beforeLines, @version)
-			@DocumentManager.setDoc @project_id, @doc_id, null, @callback
+			it "should flush and delete the doc from the doc updater", ->
+				@DocumentManager.flushAndDeleteDoc
+					.calledWith(@project_id, @doc_id)
+					.should.equal true
 
-		it "should return teh callback with an error", ->
-			@callback.calledWith(new Error("No lines were passed to setDoc"))
-			
-		it "should not try to get the doc lines", ->
-			@DocumentManager.getDoc.called.should.equal false
+		describe "without new lines", ->
+			beforeEach ->
+				@DocumentManager.setDoc @project_id, @doc_id, null, @source, @user_id, @callback
+
+			it "should return the callback with an error", ->
+				@callback.calledWith(new Error("No lines were passed to setDoc"))
+				
+			it "should not try to get the doc lines", ->
+				@DocumentManager.getDoc.called.should.equal false