cleanup

services/filestore/app.coffee

@@ -94,7 +94,7 @@ app.post "/shutdown", (req, res)->
 
 app.get '/status', (req, res)->
 	if appIsOk
-		res.send('filestore sharelatex up - hello james')
+		res.send('filestore sharelatex up')
 	else
 		logger.log "app is not ok - shutting down"
 		res.send("server is being shut down", 500)

services/filestore/app/coffee/FSPersistorManager.coffee

@@ -49,13 +49,13 @@ module.exports =
     sourceStream.on 'error', (err) ->
       logger.err err:err, location:location, name:name, "Error reading from file"
       if err.code == 'ENOENT'
-        callback new Errors.NotFoundError(err.message), null
+        return callback new Errors.NotFoundError(err.message), null
       else
-        callback err, null
+        return callback err, null
     sourceStream.on 'readable', () ->
       # This can be called multiple times, but the callback wrapper
       # ensures the callback is only called once
-      callback null, sourceStream
+      return callback null, sourceStream
 
 
   copyFile: (location, fromName, toName, callback = (err)->)->

services/filestore/app/coffee/HealthCheckController.coffee

@@ -15,7 +15,6 @@ checkCanStoreFiles = (callback)->
 	req = {params:{}, query:{}, headers:{}}
 	req.params.project_id = settings.health_check.project_id
 	req.params.file_id = settings.health_check.file_id
-	console.log settings
 	myWritableStreamBuffer = new streamBuffers.WritableStreamBuffer(initialSize: 100)
 	res = {
 		send: (code) ->

services/filestore/app/coffee/S3PersistorManager.coffee

@@ -91,7 +91,8 @@ module.exports =
 			else if res.statusCode not in [200, 206]
 				logger.log bucketName:bucketName, key:key, "error getting file from s3: #{res.statusCode}"
 				return callback new Error("Got non-200 response from S3: #{res.statusCode}"), null
-			callback null, res
+			else 
+				return callback null, res
 		s3Stream.on 'error', (err) ->
 			logger.err err:err, bucketName:bucketName, key:key, "error getting file stream from s3"
 			callback err

services/filestore/docker-compose.ci.yml

@@ -21,7 +21,7 @@ services:
       MONGO_HOST: mongo
       POSTGRES_HOST: postgres
       MOCHA_GREP: ${MOCHA_GREP}
-      ENABLE_CONVERSIONS: true
+      ENABLE_CONVERSIONS: "true"
     depends_on:
       - mongo
       - redis

services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee

@@ -34,6 +34,8 @@ describe "FSPersistorManagerTests", ->
         err:->
       "response":response
       "rimraf":@Rimraf
+      "./Errors": @Errors =
+        NotFoundError: sinon.stub()
     @location = "/tmp"
     @name1 = "530f2407e7ef165704000007/530f838b46d9a9e859000008"
     @name1Filtered ="530f2407e7ef165704000007_530f838b46d9a9e859000008"
@@ -119,32 +121,33 @@ describe "FSPersistorManagerTests", ->
 
     describe "error conditions", ->
 
-      beforeEach ->
-        @fakeCode = 'ENOENT'
-        @Fs.createReadStream.returns(
-          on: (key, callback) =>
-            err = new Error()
-            err.message = "this is from a test"
-            err.code = @fakeCode
-            callback(err, null)
-        )
-
       describe "when the file does not exist", ->
 
         beforeEach ->
           @fakeCode = 'ENOENT'
-
+          @Fs.createReadStream.returns(
+            on: (key, callback) =>
+              err = new Error()
+              err.code = @fakeCode
+              callback(err, null)
+          )
         it "should give a NotFoundError", (done) ->
           @FSPersistorManager.getFileStream @location, @name1, @opts, (err,res)=>
             expect(res).to.equal null
             expect(err).to.not.equal null
-            expect(err.name == "NotFoundError").to.equal true
+            expect(err instanceof @Errors.NotFoundError).to.equal true
             done()
 
       describe "when some other error happens", ->
 
         beforeEach ->
           @fakeCode = 'SOMETHINGHORRIBLE'
+          @Fs.createReadStream.returns(
+            on: (key, callback) =>
+              err = new Error()
+              err.code = @fakeCode
+              callback(err, null)
+          )
 
         it "should give an Error", (done) ->
           @FSPersistorManager.getFileStream @location, @name1, @opts, (err,res)=>

services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee

@@ -39,6 +39,8 @@ describe "S3PersistorManagerTests", ->
 			"logger-sharelatex":
 				log:->
 				err:->
+			"./Errors": @Errors =
+				NotFoundError: sinon.stub()
 		@key = "my/key"
 		@bucketName = "my-bucket"
 		@error = "my errror"
@@ -108,44 +110,50 @@ describe "S3PersistorManagerTests", ->
 
 		describe "error conditions", ->
 
-			beforeEach ->
-				@fakeResponse =
-					statusCode: 500
-				@stubbedKnoxClient.get.returns(
-					on: (key, callback) =>
-						if key == 'response'
-							callback(@fakeResponse)
-					end: ->
-				)
-
 			describe "when the file doesn't exist", ->
 
 				beforeEach ->
-					@bucketName = "mybucket"
-					@key = "somekey"
-					@fakeResponse.statusCode = 404
+					@fakeResponse =
+						statusCode: 404
+					@stubbedKnoxClient.get.returns(
+						on: (key, callback) =>
+							if key == 'response'
+								callback(@fakeResponse)
+						end: ->
+					)
 
 				it "should produce a NotFoundError", (done) ->
 					@S3PersistorManager.getFileStream @bucketName, @key, @opts, (err, stream)=> # empty callback
 						expect(stream).to.equal null
 						expect(err).to.not.equal null
-						expect(err.name == "NotFoundError").to.equal true
+						expect(err instanceof @Errors.NotFoundError).to.equal true
 						done()
 
 				it "should have bucket and key in the Error message", (done) ->
 					@S3PersistorManager.getFileStream @bucketName, @key, @opts, (err, stream)=> # empty callback
-						expect(err.message).to.not.equal null
-						err.message.should.match(new RegExp(".*#{@bucketName}.*"))
-						err.message.should.match(new RegExp(".*#{@key}.*"))
+						error_message = @Errors.NotFoundError.lastCall.args[0]
+						expect(error_message).to.not.equal null
+						error_message.should.match(new RegExp(".*#{@bucketName}.*"))
+						error_message.should.match(new RegExp(".*#{@key}.*"))
 						done()
 
 			describe "when the S3 service produces an error", ->
+				beforeEach ->
+					@fakeResponse =
+						statusCode: 500
+					@stubbedKnoxClient.get.returns(
+						on: (key, callback) =>
+							if key == 'response'
+								callback(@fakeResponse)
+						end: ->
+					)
 
 				it "should produce an error", (done) ->
 					@S3PersistorManager.getFileStream @bucketName, @key, @opts, (err, stream)=> # empty callback
 						expect(stream).to.equal null
 						expect(err).to.not.equal null
 						expect(err instanceof Error).to.equal true
+						@Errors.NotFoundError.called.should.equal false
 						done()
 
 	describe "sendFile", ->
@@ -308,4 +316,4 @@ describe "S3PersistorManagerTests", ->
 			@stubbedKnoxClient.list.callsArgWith(1, null, data)
 			@S3PersistorManager.directorySize @bucketName, @key, (err, totalSize)=>
 				totalSize.should.equal 3072
-				done()
+				done()