From 3fccf79ca802828f112835c8a1c7f647e1ac1bcc Mon Sep 17 00:00:00 2001 From: Henry Oswald Date: Tue, 5 Feb 2019 11:32:02 +0000 Subject: [PATCH] cleanup --- services/filestore/app.coffee | 2 +- .../app/coffee/FSPersistorManager.coffee | 6 +-- .../app/coffee/HealthCheckController.coffee | 1 - .../app/coffee/S3PersistorManager.coffee | 3 +- services/filestore/docker-compose.ci.yml | 2 +- .../coffee/FSPersistorManagerTests.coffee | 27 +++++++----- .../coffee/S3PersistorManagerTests.coffee | 44 +++++++++++-------- 7 files changed, 48 insertions(+), 37 deletions(-) diff --git a/services/filestore/app.coffee b/services/filestore/app.coffee index c1484142b1..b4fff5b698 100644 --- a/services/filestore/app.coffee +++ b/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) diff --git a/services/filestore/app/coffee/FSPersistorManager.coffee b/services/filestore/app/coffee/FSPersistorManager.coffee index 733202e4cd..353e2ef099 100644 --- a/services/filestore/app/coffee/FSPersistorManager.coffee +++ b/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)->)-> diff --git a/services/filestore/app/coffee/HealthCheckController.coffee b/services/filestore/app/coffee/HealthCheckController.coffee index 7b7a80bfc8..db3f111c5e 100644 --- a/services/filestore/app/coffee/HealthCheckController.coffee +++ b/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) -> diff --git a/services/filestore/app/coffee/S3PersistorManager.coffee b/services/filestore/app/coffee/S3PersistorManager.coffee index 0c55f8b0e6..2b183730d6 100644 --- a/services/filestore/app/coffee/S3PersistorManager.coffee +++ b/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 diff --git a/services/filestore/docker-compose.ci.yml b/services/filestore/docker-compose.ci.yml index 5a7200e4f1..e7ac6e84a7 100644 --- a/services/filestore/docker-compose.ci.yml +++ b/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 diff --git a/services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee b/services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee index c4a7d83d06..7df7adfd8b 100644 --- a/services/filestore/test/unit/coffee/FSPersistorManagerTests.coffee +++ b/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)=> diff --git a/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee b/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee index e552886995..1eee09dc29 100644 --- a/services/filestore/test/unit/coffee/S3PersistorManagerTests.coffee +++ b/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() \ No newline at end of file