- deal with calling archive twice on same project

- clean up unit tests
- change s3 keys
- use err not error

services/docstore/app/coffee/DocArchiveManager.coffee

@@ -10,14 +10,18 @@ thirtySeconds = 30 * 1000
 
 module.exports = DocArchive =
 
-	archiveAllDocs: (project_id, callback = (error, docs) ->) ->
-		MongoManager.getProjectsDocs project_id, (error, docs) ->
-			if error?
-				return callback(error)
+	archiveAllDocs: (project_id, callback = (err, docs) ->) ->
+		MongoManager.getProjectsDocs project_id, (err, docs) ->
+			if err?
+				return callback(err)
 			else if !docs?
 				return callback new Errors.NotFoundError("No docs for project #{project_id}")
 			jobs = _.map docs, (doc) ->
-				(cb)-> DocArchive.archiveDoc project_id, doc, cb
+				(cb)-> 
+					if doc.inS3
+						return cb()
+					else
+						DocArchive.archiveDoc project_id, doc, cb
 			async.series jobs, callback
 
 
@@ -25,22 +29,23 @@ module.exports = DocArchive =
 		logger.log project_id: project_id, doc_id: doc._id, "sending doc to s3"
 		options = DocArchive.buildS3Options(doc.lines, project_id+"/"+doc._id)
 		request.put options, (err, res)->
+			if err? || res.statusCode != 200
+				logger.err err:err, res:res, project_id:project_id, doc_id: doc._id, statusCode: res?.statusCode, "something went wrong archiving doc in aws"
+				return callback new Error("Error in S3 request")
 			md5lines = crypto.createHash("md5").update(JSON.stringify(doc.lines)).digest("hex")
 			md5response = res.headers.etag.toString().replace(/\"/g, '')
-			if err? || res.statusCode != 200
-				logger.err err:err, res:res, "something went wrong archiving doc in aws"
-				return callback new Errors.NotFoundError("Error in S3 request")
 			if md5lines != md5response
-				logger.err responseMD5:md5response, linesMD5:md5lines, "error in response md5 from s3"
-				return callback new Errors.NotFoundError("Error in S3 md5 response")
-			MongoManager.markDocAsArchived doc._id, doc.rev, (error) ->
-				return callback(error) if error?
+				logger.err responseMD5:md5response, linesMD5:md5lines,  project_id:project_id, doc_id: doc._id, "err in response md5 from s3"
+				return callback new Error("Error in S3 md5 response")
+			MongoManager.markDocAsArchived doc._id, doc.rev, (err) ->
+				return callback(err) if err?
 				callback()
 
-	unArchiveAllDocs: (project_id, callback = (error) ->) ->
-		MongoManager.getArchivedProjectDocs project_id, (error, docs) ->
-			if error?
-				return callback(error)
+	unArchiveAllDocs: (project_id, callback = (err) ->) ->
+		MongoManager.getArchivedProjectDocs project_id, (err, docs) ->
+			if err?
+				logger.err err:err, project_id:project_id, "error unarchiving all docs"
+				return callback(err)
 			else if !docs?
 				return callback new Errors.NotFoundError("No docs for project #{project_id}")
 			jobs = _.map docs, (doc) ->
@@ -58,8 +63,8 @@ module.exports = DocArchive =
 			if err? || res.statusCode != 200
 				logger.err err:err, res:res, "something went wrong unarchiving doc from aws"
 				return callback new Errors.NotFoundError("Error in S3 request")
-			MongoManager.upsertIntoDocCollection project_id, doc_id.toString(), lines, (error) ->
-				return callback(error) if error?
+			MongoManager.upsertIntoDocCollection project_id, doc_id.toString(), lines, (err) ->
+				return callback(err) if err?
 				logger.log project_id: project_id, doc_id: doc_id, "deleting doc from s3"
 				request.del options, (err, res, body)->
 					if err? || res.statusCode != 204
@@ -70,12 +75,12 @@ module.exports = DocArchive =
 	buildS3Options: (content, key)->
 		return {
 				aws:
-					key: settings.filestore.s3.key
-					secret: settings.filestore.s3.secret
-					bucket: settings.filestore.stores.user_files
+					key: settings.docstore.s3.key
+					secret: settings.docstore.s3.secret
+					bucket: settings.docstore.s3.bucket
 				timeout: thirtySeconds
 				json: content
 				#headers:
 				#	'content-md5': crypto.createHash("md5").update(JSON.stringify(content)).digest("hex")
-				uri:"https://#{settings.filestore.stores.user_files}.s3.amazonaws.com/#{key}"
+				uri:"https://#{settings.docstore.s3.bucket}.s3.amazonaws.com/#{key}"
 		}

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

@@ -28,7 +28,7 @@ if Settings.filestore?.backend == "s3"
 			}]
 			jobs = for doc in @docs
 				do (doc) =>
-					(callback) => 
+					(callback) =>
 						DocstoreClient.createDoc @project_id, doc._id, doc.lines, (err)=>
 							doc.lines[0] = doc.lines[0]+" added"
 							DocstoreClient.updateDoc @project_id, doc._id, doc.lines, callback
@@ -37,58 +37,78 @@ if Settings.filestore?.backend == "s3"
 		afterEach (done) ->
 			db.docs.remove({project_id: @project_id}, done)
 
-		if !Settings.filestore?.fail
 
-			describe "Archiving all docs", ->
-				beforeEach (done) ->
+		describe "Archiving all docs", ->
+			beforeEach (done) ->
 
-					DocstoreClient.archiveAllDoc @project_id, (error, @res) =>
-						done()
-
-				it "should archive all the docs", (done) ->
-					@res.statusCode.should.equal 204
+				DocstoreClient.archiveAllDoc @project_id, (error, @res) =>
 					done()
 
-				it "should set inS3 and unset lines in each doc", (done) ->
+			it "should archive all the docs", (done) ->
+				@res.statusCode.should.equal 204
+				done()
 
-					jobs = for archiveDoc in @docs
-						do (archiveDoc) =>
-							(callback) => 
-								db.docs.findOne _id: archiveDoc._id, (error, doc) =>
-									should.not.exist doc.lines
-									doc.inS3.should.equal true
-									callback()
-					async.series jobs, done
+			it "should set inS3 and unset lines in each doc", (done) ->
 
-				it "should be able get the same docs back", (done) ->
+				jobs = for archiveDoc in @docs
+					do (archiveDoc) =>
+						(callback) => 
+							db.docs.findOne _id: archiveDoc._id, (error, doc) =>
+								should.not.exist doc.lines
+								doc.inS3.should.equal true
+								callback()
+				async.series jobs, done
 
-					jobs = for archiveDoc in @docs
-						do (archiveDoc) =>
-							(callback) => 
-								DocstoreClient.getS3Doc @project_id, archiveDoc._id, (error, res, doc) =>
-									doc.toString().should.equal archiveDoc.lines.toString()
-									callback()
-					async.series jobs, done
+			it "should be able get the same docs back", (done) ->
 
+				jobs = for archiveDoc in @docs
+					do (archiveDoc) =>
+						(callback) => 
+							DocstoreClient.getS3Doc @project_id, archiveDoc._id, (error, res, doc) =>
+								doc.toString().should.equal archiveDoc.lines.toString()
+								callback()
+				async.series jobs, done
 
-			describe "Unarchiving all docs", ->
-
-				it "should unarchive all the docs", (done) ->
-					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
-								doc.lines.should.deep.equal @docs[i].lines
-							done()
-
-		# set fail to true and also run the docstore service with a denied config
-		if Settings.filestore?.fail
-
-			describe "Test S3 fail request", ->
-
-				it "should return a 404", (done) ->
-
-					DocstoreClient.archiveAllDoc @project_id, (error, res) =>
-						res.statusCode.should.equal 404
+		describe "Arching all docs twice", ->
+			beforeEach (done) ->
+				DocstoreClient.archiveAllDoc @project_id, (error, @res) =>
+					@res.statusCode.should.equal 204
+					DocstoreClient.archiveAllDoc @project_id, (error, @res) =>
+						@res.statusCode.should.equal 204
+						done()
+
+			it "should archive all the docs", (done) ->
+				@res.statusCode.should.equal 204
+				done()
+
+			it "should set inS3 and unset lines in each doc", (done) ->
+
+				jobs = for archiveDoc in @docs
+					do (archiveDoc) =>
+						(callback) => 
+							db.docs.findOne _id: archiveDoc._id, (error, doc) =>
+								should.not.exist doc.lines
+								doc.inS3.should.equal true
+								callback()
+				async.series jobs, done
+
+			it "should be able get the same docs back", (done) ->
+
+				jobs = for archiveDoc in @docs
+					do (archiveDoc) =>
+						(callback) => 
+							DocstoreClient.getS3Doc @project_id, archiveDoc._id, (error, res, doc) =>
+								doc.toString().should.equal archiveDoc.lines.toString()
+								callback()
+				async.series jobs, done
+
+		describe "Unarchiving all docs", ->
+
+			it "should unarchive all the docs", (done) ->
+				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
+							doc.lines.should.deep.equal @docs[i].lines
 						done()

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

@@ -14,41 +14,58 @@ describe "DocArchiveManager", ->
 	beforeEach ->
 
 		@settings =
-			filestore:
-				backend: "s3"
+			docstore:
 				s3:
 					secret: "secret"
 					key: "this_key"
-				stores:
-					user_files:"sl_user_files"
+					bucket:"doc-archive-unit-test"
 
 		@request = 
 			put: {}
 			get: {}
 			del: {}
 
-		@docs = [{
+		@archivedDocs = [{
+			_id: ObjectId()
+			inS3:true
+			rev: 2
+		}, {
+			_id: ObjectId()
+			inS3:true
+			rev: 4
+		}, {
+			_id: ObjectId()
+			inS3:true
+			rev: 6
+		}]
+
+		@mongoDocs = [{
 			_id: ObjectId()
 			lines: ["one", "two", "three"]
 			rev: 2
-			inS3: true
 		}, {
 			_id: ObjectId()
 			lines: ["aaa", "bbb", "ccc"]
 			rev: 4
+		}, {
+			_id: ObjectId()
 			inS3: true
+			rev: 6
+		}, {
+			_id: ObjectId()
+			inS3: true
+			rev: 6
 		}, {
 			_id: ObjectId()
 			lines: ["111", "222", "333"]
 			rev: 6
-			inS3: true
 		}]
 
 		@MongoManager =
 			markDocAsArchived: sinon.stub().callsArgWith(2, null)
 			upsertIntoDocCollection: sinon.stub().callsArgWith(3, null)
-			getProjectsDocs: sinon.stub().callsArgWith(1, null, @docs)
-			getArchivedProjectDocs: sinon.stub().callsArgWith(1, null, @docs)
+			getProjectsDocs: sinon.stub().callsArgWith(1, null, @mongoDocs)
+			getArchivedProjectDocs: sinon.stub().callsArgWith(1, null, @mongoDocs)
 
 		@requires = 
 			"settings-sharelatex": @settings
@@ -61,84 +78,71 @@ describe "DocArchiveManager", ->
 		@error = "my errror"
 		@project_id = ObjectId().toString()
 		@stubbedError = new Errors.NotFoundError("Error in S3 request")
+		@DocArchiveManager = SandboxedModule.require modulePath, requires: @requires
 
 	describe "archiveDoc", ->
 
 		it "should use correct options", (done)->
 			@request.put = sinon.stub().callsArgWith(1,  null, {statusCode:200,headers:{etag:""}})
-			@requires["request"] = @request
-			@DocArchiveManager = SandboxedModule.require modulePath, requires: @requires
-
-			@DocArchiveManager.archiveDoc @project_id, @docs[0], (err)=>
+			@DocArchiveManager.archiveDoc @project_id, @mongoDocs[0], (err)=>
 				opts = @request.put.args[0][0]
-				assert.deepEqual(opts.aws, {key:@settings.filestore.s3.key, secret:@settings.filestore.s3.secret, bucket:@settings.filestore.stores.user_files})
-				opts.json.should.equal @docs[0].lines
+				assert.deepEqual(opts.aws, {key:@settings.docstore.s3.key, secret:@settings.docstore.s3.secret, bucket:@settings.docstore.s3.bucket})
+				opts.json.should.equal @mongoDocs[0].lines
 				opts.timeout.should.equal (30*1000)
-				opts.uri.should.equal "https://#{@settings.filestore.stores.user_files}.s3.amazonaws.com/#{@project_id}/#{@docs[0]._id}"
+				opts.uri.should.equal "https://#{@settings.docstore.s3.bucket}.s3.amazonaws.com/#{@project_id}/#{@mongoDocs[0]._id}"
 				done()
 
 		it "should return no md5 error", (done)->
-			@md5 = crypto.createHash("md5").update(JSON.stringify(@docs[0].lines)).digest("hex")
+			@md5 = crypto.createHash("md5").update(JSON.stringify(@mongoDocs[0].lines)).digest("hex")
 			@request.put = sinon.stub().callsArgWith(1,  null, {statusCode:200,headers:{etag:@md5}})
-			@requires["request"] = @request
-			@DocArchiveManager = SandboxedModule.require modulePath, requires: @requires
-
-			@DocArchiveManager.archiveDoc @project_id, @docs[0], (err)=>
+			@DocArchiveManager.archiveDoc @project_id, @mongoDocs[0], (err)=>
 				should.not.exist err
 				done()
 
 		it "should return the error", (done)->
 			@request.put = sinon.stub().callsArgWith(1, @stubbedError, {statusCode:400,headers:{etag:""}})
-			@requires["request"] = @request
-			@DocArchiveManager = SandboxedModule.require modulePath, requires: @requires
-
-			@DocArchiveManager.archiveDoc @project_id, @docs[0], (err)=>
+			@DocArchiveManager.archiveDoc @project_id, @mongoDocs[0], (err)=>
 				should.exist err
 				done()
 
 	describe "unarchiveDoc", ->
 
 		it "should use correct options", (done)->
-			@request.get = sinon.stub().callsArgWith(1, null, statusCode:200, @docs[0].lines)
+			@request.get = sinon.stub().callsArgWith(1, null, statusCode:200, @mongoDocs[0].lines)
 			@request.del = sinon.stub().callsArgWith(1, null, statusCode:204, {})
-			@requires["request"] = @request
-			@DocArchiveManager = SandboxedModule.require modulePath, requires: @requires
-
-			@DocArchiveManager.unarchiveDoc @project_id, @docs[0]._id, (err)=>
+			@DocArchiveManager.unarchiveDoc @project_id, @mongoDocs[0]._id, (err)=>
 				opts = @request.get.args[0][0]
-				assert.deepEqual(opts.aws, {key:@settings.filestore.s3.key, secret:@settings.filestore.s3.secret, bucket:@settings.filestore.stores.user_files})
+				assert.deepEqual(opts.aws, {key:@settings.docstore.s3.key, secret:@settings.docstore.s3.secret, bucket:@settings.docstore.s3.bucket})
 				opts.json.should.equal true
 				opts.timeout.should.equal (30*1000)
-				opts.uri.should.equal "https://#{@settings.filestore.stores.user_files}.s3.amazonaws.com/#{@project_id}/#{@docs[0]._id}"
+				opts.uri.should.equal "https://#{@settings.docstore.s3.bucket}.s3.amazonaws.com/#{@project_id}/#{@mongoDocs[0]._id}"
 				done()
 
 		it "should return the error", (done)->
 			@request.get = sinon.stub().callsArgWith(1, @stubbedError, {}, {})
-			@requires["request"] = @request
-			@DocArchiveManager = SandboxedModule.require modulePath, requires: @requires
-
-			@DocArchiveManager.unarchiveDoc @project_id, @docs[0], (err)=>
+			@DocArchiveManager.unarchiveDoc @project_id, @mongoDocs[0], (err)=>
 				should.exist err
 				done()
 
 	describe "archiveAllDocs", ->
 
-		it "should archive all project docs", (done)->
-			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(1, null, @docs)
-			@requires["./MongoManager"] = @MongoManager
-			@DocArchiveManager = SandboxedModule.require modulePath, requires: @requires
+		it "should archive all project docs which are not in s3", (done)->
+			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(1, null, @mongoDocs)
 			@DocArchiveManager.archiveDoc = sinon.stub().callsArgWith(2, null)
 
 			@DocArchiveManager.archiveAllDocs @project_id, (err)=>
-				for doc in @docs
-					@DocArchiveManager.archiveDoc.calledWith(@project_id, doc).should.equal true
+				@DocArchiveManager.archiveDoc.calledWith(@project_id, @mongoDocs[0]).should.equal true
+				@DocArchiveManager.archiveDoc.calledWith(@project_id, @mongoDocs[1]).should.equal true
+				@DocArchiveManager.archiveDoc.calledWith(@project_id, @mongoDocs[4]).should.equal true
+
+				@DocArchiveManager.archiveDoc.calledWith(@project_id, @mongoDocs[2]).should.equal false
+				@DocArchiveManager.archiveDoc.calledWith(@project_id, @mongoDocs[3]).should.equal false
+				
 				should.not.exist err
 				done()
 
 		it "should return error if have no docs", (done)->
 			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(1, null, null)
-			@requires["./MongoManager"] = @MongoManager
-			@DocArchiveManager = SandboxedModule.require modulePath, requires: @requires
 
 			@DocArchiveManager.archiveAllDocs @project_id, (err)=>
 				should.exist err
@@ -146,8 +150,6 @@ describe "DocArchiveManager", ->
 
 		it "should return the error", (done)->
 			@MongoManager.getProjectsDocs = sinon.stub().callsArgWith(1, @error, null)
-			@requires["./MongoManager"] = @MongoManager
-			@DocArchiveManager = SandboxedModule.require modulePath, requires: @requires
 
 			@DocArchiveManager.archiveAllDocs @project_id, (err)=>
 				err.should.equal @error
@@ -156,31 +158,22 @@ describe "DocArchiveManager", ->
 	describe "unArchiveAllDocs", ->
 
 		it "should unarchive all inS3 docs", (done)->
-			@MongoManager.getArchivedProjectDocs = sinon.stub().callsArgWith(1, null, @docs)
-			@requires["./MongoManager"] = @MongoManager
-			@DocArchiveManager = SandboxedModule.require modulePath, requires: @requires
+			@MongoManager.getArchivedProjectDocs = sinon.stub().callsArgWith(1, null, @archivedDocs)
 			@DocArchiveManager.unarchiveDoc = sinon.stub().callsArgWith(2, null)
-
 			@DocArchiveManager.unArchiveAllDocs @project_id, (err)=>
-				for doc in @docs
+				for doc in @archivedDocs
 					@DocArchiveManager.unarchiveDoc.calledWith(@project_id, doc._id).should.equal true
 				should.not.exist err
 				done()
 
 		it "should return error if have no docs", (done)->
 			@MongoManager.getArchivedProjectDocs = sinon.stub().callsArgWith(1, null, null)
-			@requires["./MongoManager"] = @MongoManager
-			@DocArchiveManager = SandboxedModule.require modulePath, requires: @requires
-
 			@DocArchiveManager.unArchiveAllDocs @project_id, (err)=>
 				should.exist err
 				done()
 
 		it "should return the error", (done)->
 			@MongoManager.getArchivedProjectDocs = sinon.stub().callsArgWith(1, @error, null)
-			@requires["./MongoManager"] = @MongoManager
-			@DocArchiveManager = SandboxedModule.require modulePath, requires: @requires
-
 			@DocArchiveManager.unArchiveAllDocs @project_id, (err)=>
 				err.should.equal @error
 				done()