Merge pull request #13 from sharelatex/ja-delete-refactor

Include deleted docs when archiving

services/docstore/app/coffee/DocArchiveManager.coffee

@@ -11,7 +11,7 @@ thirtySeconds = 30 * 1000
 module.exports = DocArchive =
 
 	archiveAllDocs: (project_id, callback = (err, docs) ->) ->
-		MongoManager.getProjectsDocs project_id, (err, docs) ->
+		MongoManager.getProjectsDocs project_id, {include_deleted: true}, (err, docs) ->
 			if err?
 				return callback(err)
 			else if !docs?

services/docstore/app/coffee/DocManager.coffee

@@ -30,9 +30,9 @@ module.exports = DocManager =
 				else
 					callback err, doc
 
-	getAllDocs: (project_id, callback = (error, docs) ->) ->
+	getAllNonDeletedDocs: (project_id, callback = (error, docs) ->) ->
 		DocArchive.unArchiveAllDocs project_id, (error) ->
-			MongoManager.getProjectsDocs project_id, (error, docs) ->
+			MongoManager.getProjectsDocs project_id, {include_deleted: false}, (error, docs) ->
 				if err?
 					return callback(error)
 				else if !docs?

services/docstore/app/coffee/HttpController.coffee

@@ -35,7 +35,7 @@ module.exports = HttpController =
 	getAllDocs: (req, res, next = (error) ->) ->
 		project_id = req.params.project_id
 		logger.log project_id: project_id, "getting all docs"
-		DocManager.getAllDocs project_id, (error, docs = []) ->
+		DocManager.getAllNonDeletedDocs project_id, (error, docs = []) ->
 			return next(error) if error?
 			docViews = []
 			for doc in docs

services/docstore/app/coffee/MongoManager.coffee

@@ -6,8 +6,11 @@ module.exports = MongoManager =
 		db.docs.find {_id: ObjectId(doc_id.toString()), project_id: ObjectId(project_id.toString())}, {}, (error, docs = []) ->
 			callback error, docs[0]
 
-	getProjectsDocs: (project_id, callback)->
-		db.docs.find {project_id: ObjectId(project_id.toString()), deleted: { $ne: true }}, {}, callback
+	getProjectsDocs: (project_id, options = {include_deleted: true}, callback)->
+		query = {project_id: ObjectId(project_id.toString())}
+		if !options.include_deleted
+			query.deleted = { $ne: true }
+		db.docs.find query, {}, callback
 
 	getArchivedProjectDocs: (project_id, callback)->
 		query =

services/docstore/test/acceptance/coffee/ArchiveDocsTests.coffee

@@ -31,6 +31,9 @@ describe "Archiving all docs", ->
 					DocstoreClient.createDoc @project_id, doc._id, doc.lines, (err)=>
 						doc.lines[0] = doc.lines[0]+" added"
 						DocstoreClient.updateDoc @project_id, doc._id, doc.lines, null, callback
+		# Make sure archiving works on deleted docs too
+		jobs.push (cb) =>
+			DocstoreClient.deleteDoc @project_id, @docs[2]._id, cb
 		async.series jobs, done 
 
 	afterEach (done) ->
@@ -141,10 +144,11 @@ describe "Archiving all docs", ->
 	describe "Unarchiving all docs", ->
 
 		it "should unarchive all the docs", (done) ->
+			non_deleted_docs = @docs.slice(0,2)
 			DocstoreClient.archiveAllDoc @project_id, (error, res) =>
 				DocstoreClient.getAllDocs @project_id, (error, res, docs) =>
 					throw error if error?
-					docs.length.should.equal @docs.length
-					for doc, i in docs
+					docs.length.should.equal non_deleted_docs.length
+					for doc, i in non_deleted_docs
 						doc.lines.should.deep.equal @docs[i].lines
 					done()

services/docstore/test/unit/coffee/DocArchiveManager.coffee

@@ -64,7 +64,7 @@ describe "DocArchiveManager", ->
 		@MongoManager =
 			markDocAsArchived: sinon.stub().callsArgWith(2, null)
 			upsertIntoDocCollection: sinon.stub().callsArgWith(3, null)
-			getProjectsDocs: sinon.stub().callsArgWith(1, null, @mongoDocs)
+			getProjectsDocs: sinon.stub().callsArgWith(2, null, @mongoDocs)
 			getArchivedProjectDocs: sinon.stub().callsArgWith(1, null, @mongoDocs)
 
 		@requires = 
@@ -127,7 +127,7 @@ describe "DocArchiveManager", ->
 	describe "archiveAllDocs", ->
 
 		it "should archive all project docs which are not in s3", (done)->
-			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(1, null, @mongoDocs)
+			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, null, @mongoDocs)
 			@DocArchiveManager.archiveDoc = sinon.stub().callsArgWith(2, null)
 
 			@DocArchiveManager.archiveAllDocs @project_id, (err)=>
@@ -142,14 +142,14 @@ describe "DocArchiveManager", ->
 				done()
 
 		it "should return error if have no docs", (done)->
-			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(1, null, null)
+			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, null, null)
 
 			@DocArchiveManager.archiveAllDocs @project_id, (err)=>
 				should.exist err
 				done()
 
 		it "should return the error", (done)->
-			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(1, @error, null)
+			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, @error, null)
 
 			@DocArchiveManager.archiveAllDocs @project_id, (err)=>
 				err.should.equal @error
@@ -163,7 +163,7 @@ describe "DocArchiveManager", ->
 				while --numberOfDocs != 0
 					@mongoDocs.push({inS3:true, _id: ObjectId()})
 
-				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(1, null, @mongoDocs)
+				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, null, @mongoDocs)
 				@DocArchiveManager.archiveDoc = sinon.stub().callsArgWith(2, null)	
 
 			it "should not throw and error", (done)->

services/docstore/test/unit/coffee/DocManagerTests.coffee

@@ -96,17 +96,17 @@ describe "DocManager", ->
 					.calledWith(new Errors.NotFoundError("No such doc: #{@doc_id} in project #{@project_id}"))
 					.should.equal true
 
-	describe "getAllDocs", ->
+	describe "getAllNonDeletedDocs", ->
 		describe "when the project exists", ->
 			beforeEach -> 
 				@docs = [{ _id: @doc_id, project_id: @project_id, lines: ["mock-lines"] }]
-				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(1, null, @docs)
+				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, null, @docs)
 				@DocArchiveManager.unArchiveAllDocs = sinon.stub().callsArgWith(1, null, @docs)
-				@DocManager.getAllDocs @project_id, @callback
+				@DocManager.getAllNonDeletedDocs @project_id, @callback
 
 			it "should get the project from the database", ->
 				@MongoManager.getProjectsDocs
-					.calledWith(@project_id)
+					.calledWith(@project_id, {include_deleted: false})
 					.should.equal true
 
 			it "should return the docs", ->
@@ -114,9 +114,9 @@ describe "DocManager", ->
 
 		describe "when there are no docs for the project", ->
 			beforeEach -> 
-				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(1, null, null)
+				@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(2, null, null)
 				@DocArchiveManager.unArchiveAllDocs = sinon.stub().callsArgWith(1, null, null)
-				@DocManager.getAllDocs @project_id, @callback
+				@DocManager.getAllNonDeletedDocs @project_id, @callback
 
 			it "should return a NotFoundError", ->
 				@callback

services/docstore/test/unit/coffee/HttpControllerTests.coffee

@@ -120,11 +120,11 @@ describe "HttpController", ->
 					lines: ["mock", "lines", "two"]
 					rev: 4
 				}]
-				@DocManager.getAllDocs = sinon.stub().callsArgWith(1, null, @docs)
+				@DocManager.getAllNonDeletedDocs = sinon.stub().callsArgWith(1, null, @docs)
 				@HttpController.getAllDocs @req, @res, @next
 
-			it "should get all the docs", ->
-				@DocManager.getAllDocs
+			it "should get all the (non-deleted) docs", ->
+				@DocManager.getAllNonDeletedDocs
 					.calledWith(@project_id)
 					.should.equal true
 
@@ -158,7 +158,7 @@ describe "HttpController", ->
 					lines: ["mock", "lines", "two"]
 					rev: 4
 				}]
-				@DocManager.getAllDocs = sinon.stub().callsArgWith(1, null, @docs)
+				@DocManager.getAllNonDeletedDocs = sinon.stub().callsArgWith(1, null, @docs)
 				@HttpController.getAllDocs @req, @res, @next
 
 			it "should return the non null docs as JSON", ->

services/docstore/test/unit/coffee/MongoManagerTests.coffee

@@ -40,18 +40,35 @@ describe "MongoManager", ->
 			@doc3 = { name: "mock-doc3" }
 			@doc4 = { name: "mock-doc4" }
 			@db.docs.find = sinon.stub().callsArgWith(2, null, [@doc, @doc3, @doc4])
-			@MongoManager.getProjectsDocs @project_id, @callback
+		
+		describe "with included_deleted = false", ->
+			beforeEach -> 
+				@MongoManager.getProjectsDocs @project_id, include_deleted: false, @callback
+
+			it "should find the non-deleted docs via the project_id", ->
+				@db.docs.find
+					.calledWith({
+						project_id: ObjectId(@project_id)
+						deleted: { $ne: true }
+					}, {})
+					.should.equal true
 
-		it "should find the non-deleted docs via the project_id", ->
-			@db.docs.find
-				.calledWith({
-					project_id: ObjectId(@project_id)
-					deleted: { $ne: true }
-				}, {})
-				.should.equal true
+			it "should call the callback with the docs", ->
+				@callback.calledWith(null, [@doc, @doc3, @doc4]).should.equal true
+				
+		describe "with included_deleted = true", ->
+			beforeEach -> 
+				@MongoManager.getProjectsDocs @project_id, include_deleted: true, @callback
+
+			it "should find all via the project_id", ->
+				@db.docs.find
+					.calledWith({
+						project_id: ObjectId(@project_id)
+					}, {})
+					.should.equal true
 
-		it "should call the callback with the docs", ->
-			@callback.calledWith(null, [@doc, @doc3, @doc4]).should.equal true
+			it "should call the callback with the docs", ->
+				@callback.calledWith(null, [@doc, @doc3, @doc4]).should.equal true
 
 	describe "upsertIntoDocCollection", ->
 		beforeEach ->