Add aws-sdk unit test.

Fix Aws-Sdk persistor to return a correct error on file not found.
Fix FileHandler after some change were lost on a previous merge

services/filestore/app/coffee/AWSSDKPersistorManager.coffee

@@ -2,6 +2,7 @@ logger = require "logger-sharelatex"
 aws = require "aws-sdk"
 _ = require "underscore"
 fs = require "fs"
+Errors = require("./Errors")
 
 s3 = new aws.S3()
 
@@ -35,6 +36,8 @@ module.exports =
       callback null, stream
     stream.on 'error', (err) ->
       logger.err err:err, bucketName:bucketName, key:key, "error getting file stream from s3"
+      if err.code == 'NoSuchKey'
+        return callback new Errors.NotFoundError "File not found in S3: #{bucketName}:#{key}"
       callback err
 
   copyFile: (bucketName, sourceKey, destKey, callback)->

services/filestore/app/coffee/FileHandler.coffee

@@ -41,16 +41,15 @@ module.exports =
 			if err?
 				return callback err
 			if exists
-				PersistorManager.getFileStream bucket, convertedKey, callback
+				PersistorManager.getFileStream bucket, convertedKey, null, callback
 			else
 				@_getConvertedFileAndCache bucket, key, convertedKey, opts, callback
 
 	_getConvertedFileAndCache: (bucket, key, convertedKey, opts, callback)->
-		self = @
 		convertedFsPath = ""
 		async.series [
-			(cb)->
-				self._convertFile bucket, key, opts, (err, fileSystemPath)->
+			(cb) =>
+				@_convertFile bucket, key, opts, (err, fileSystemPath) ->
 					convertedFsPath = fileSystemPath
 					cb err
 			(cb)->
@@ -60,7 +59,7 @@ module.exports =
 		], (err)->
 			if err?
 				return callback(err)
-			PersistorManager.getFileStream bucket, convertedKey, callback
+			PersistorManager.getFileStream bucket, convertedKey, opts, callback
 
 	_convertFile: (bucket, originalKey, opts, callback)->
 		@_writeS3FileToDisk bucket, originalKey, opts, (err, originalFsPath)->

services/filestore/test/unit/coffee/AWSSDKPersistorManagerTests.coffee

@@ -0,0 +1,249 @@
+sinon = require 'sinon'
+chai = require 'chai'
+
+should = chai.should()
+expect = chai.expect
+
+modulePath = "../../../app/js/AWSSDKPersistorManager.js"
+SandboxedModule = require 'sandboxed-module'
+
+describe "AWSSDKPersistorManager", ->
+  beforeEach ->
+    @settings =
+      filestore:
+        backend: "aws-sdk"
+    @s3 =
+      upload: sinon.stub()
+      getObject: sinon.stub()
+      copyObject: sinon.stub()
+      deleteObject: sinon.stub()
+      listObjects: sinon.stub()
+      deleteObjects: sinon.stub()
+      headObject: sinon.stub()
+    @awssdk =
+      S3: sinon.stub().returns @s3
+
+    @requires =
+      "aws-sdk": @awssdk
+      "settings-sharelatex": @settings
+      "logger-sharelatex":
+        log:->
+        err:->
+      "fs": @fs =
+        createReadStream: sinon.stub()
+      "./Errors": @Errors =
+        NotFoundError: sinon.stub()
+    @key = "my/key"
+    @bucketName = "my-bucket"
+    @error = "my error"
+    @AWSSDKPersistorManager = SandboxedModule.require modulePath, requires: @requires
+
+  describe "sendFile", ->
+    beforeEach ->
+      @stream = {}
+      @fsPath = "/usr/local/some/file"
+      @fs.createReadStream.returns @stream
+
+    it "should put the file with s3.upload", (done) ->
+      @s3.upload.callsArgWith 1
+      @AWSSDKPersistorManager.sendFile @bucketName, @key, @fsPath, (err) =>
+        expect(err).to.not.be.ok
+        expect(@s3.upload.calledOnce, "called only once").to.be.true
+        expect((@s3.upload.calledWith Bucket: @bucketName, Key: @key, Body: @stream)
+               , "called with correct arguments").to.be.true
+        done()
+
+    it "should dispatch the error from s3.upload", (done) ->
+      @s3.upload.callsArgWith 1, @error
+      @AWSSDKPersistorManager.sendFile @bucketName, @key, @fsPath, (err) =>
+        expect(err).to.equal @error
+        done()
+
+
+  describe "sendStream", ->
+    beforeEach ->
+      @stream = {}
+
+    it "should put the file with s3.upload", (done) ->
+      @s3.upload.callsArgWith 1
+      @AWSSDKPersistorManager.sendStream @bucketName, @key, @stream, (err) =>
+        expect(err).to.not.be.ok
+        expect(@s3.upload.calledOnce, "called only once").to.be.true
+        expect((@s3.upload.calledWith Bucket: @bucketName, Key: @key, Body: @stream),
+               "called with correct arguments").to.be.true
+        done()
+
+    it "should dispatch the error from s3.upload", (done) ->
+      @s3.upload.callsArgWith 1, @error
+      @AWSSDKPersistorManager.sendStream @bucketName, @key, @stream, (err) =>
+        expect(err).to.equal @error
+        done()
+
+  describe "getFileStream", ->
+    beforeEach ->
+      @opts = {}
+      @stream = {}
+      @read_stream =
+        on: @read_stream_on = sinon.stub()
+      @object =
+        createReadStream: sinon.stub().returns @read_stream
+      @s3.getObject.returns @object
+
+    it "should return a stream from s3.getObject", (done) ->
+      @read_stream_on.withArgs('readable').callsArgWith 1
+
+      @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) =>
+        expect(@read_stream_on.calledTwice)
+        expect(err).to.not.be.ok
+        expect(stream, "returned the stream").to.equal @read_stream
+        expect((@s3.getObject.calledWith Bucket: @bucketName, Key: @key),
+               "called with correct arguments").to.be.true
+        done()
+
+    describe "with start and end options", ->
+      beforeEach ->
+        @opts =
+          start: 0
+          end: 8
+      it "should pass headers to the s3.GetObject", (done) ->
+        @read_stream_on.withArgs('readable').callsArgWith 1
+        @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) =>
+          expect((@s3.getObject.calledWith Bucket: @bucketName, Key: @key, Range: 'bytes=0-8'),
+            "called with correct arguments").to.be.true
+        done()
+
+    describe "error conditions", ->
+      describe "when the file doesn't exist", ->
+        beforeEach ->
+          @error = new Error()
+          @error.code = 'NoSuchKey'
+        it "should produce a NotFoundError", (done) ->
+          @read_stream_on.withArgs('error').callsArgWith 1, @error
+          @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) =>
+            expect(stream).to.not.be.ok
+            expect(err).to.be.ok
+            expect(err instanceof @Errors.NotFoundError, "error is a correct instance").to.equal true
+            done()
+
+      describe "when there is some other error", ->
+        beforeEach ->
+          @error = new Error()
+        it "should dispatch the error from s3 object stream", (done) ->
+          @read_stream_on.withArgs('error').callsArgWith 1, @error
+          @AWSSDKPersistorManager.getFileStream @bucketName, @key, @opts, (err, stream) =>
+            expect(stream).to.not.be.ok
+            expect(err).to.be.ok
+            expect(err).to.equal @error
+            done()
+
+  describe "copyFile", ->
+    beforeEach ->
+      @destKey = "some/key"
+      @stream = {}
+
+    it "should copy the file with s3.copyObject", (done) ->
+      @s3.copyObject.callsArgWith 1
+      @AWSSDKPersistorManager.copyFile @bucketName, @key, @destKey, (err) =>
+        expect(err).to.not.be.ok
+        expect(@s3.copyObject.calledOnce, "called only once").to.be.true
+        expect((@s3.copyObject.calledWith Bucket: @bucketName, Key: @destKey, CopySource: @bucketName + '/' + @key),
+          "called with correct arguments").to.be.true
+        done()
+
+    it "should dispatch the error from s3.copyObject", (done) ->
+      @s3.copyObject.callsArgWith 1, @error
+      @AWSSDKPersistorManager.copyFile @bucketName, @key, @destKey, (err) =>
+        expect(err).to.equal @error
+        done()
+
+  describe "deleteFile", ->
+    it "should delete the file with s3.deleteObject", (done) ->
+      @s3.deleteObject.callsArgWith 1
+      @AWSSDKPersistorManager.deleteFile @bucketName, @key, (err) =>
+        expect(err).to.not.be.ok
+        expect(@s3.deleteObject.calledOnce, "called only once").to.be.true
+        expect((@s3.deleteObject.calledWith Bucket: @bucketName, Key: @key),
+          "called with correct arguments").to.be.true
+        done()
+
+    it "should dispatch the error from s3.deleteObject", (done) ->
+      @s3.deleteObject.callsArgWith 1, @error
+      @AWSSDKPersistorManager.deleteFile @bucketName, @key, (err) =>
+        expect(err).to.equal @error
+        done()
+
+  describe "deleteDirectory", ->
+
+    it "should list the directory content using s3.listObjects", (done) ->
+      @s3.listObjects.callsArgWith 1, null, Contents: []
+      @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) =>
+        expect(err).to.not.be.ok
+        expect(@s3.listObjects.calledOnce, "called only once").to.be.true
+        expect((@s3.listObjects.calledWith Bucket: @bucketName, Prefix: @key),
+          "called with correct arguments").to.be.true
+        done()
+
+    it "should dispatch the error from s3.listObjects", (done) ->
+      @s3.listObjects.callsArgWith 1, @error
+      @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) =>
+        expect(err).to.equal @error
+        done()
+
+    describe "with directory content", ->
+      beforeEach ->
+        @fileList = [
+          Key: 'foo'
+        , Key: 'bar'
+        , Key: 'baz'
+        ]
+
+      it "should forward the file keys to s3.deleteObjects", (done) ->
+        @s3.listObjects.callsArgWith 1, null, Contents: @fileList
+        @s3.deleteObjects.callsArgWith 1
+        @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) =>
+          expect(err).to.not.be.ok
+          expect(@s3.deleteObjects.calledOnce, "called only once").to.be.true
+          expect((@s3.deleteObjects.calledWith
+              Bucket: @bucketName
+              Delete:
+                Quiet: true
+                Objects: @fileList),
+            "called with correct arguments").to.be.true
+          done()
+
+      it "should dispatch the error from s3.deleteObjects", (done) ->
+        @s3.listObjects.callsArgWith 1, null, Contents: @fileList
+        @s3.deleteObjects.callsArgWith 1, @error
+        @AWSSDKPersistorManager.deleteDirectory @bucketName, @key, (err) =>
+          expect(err).to.equal @error
+          done()
+
+
+  describe "checkIfFileExists", ->
+
+    it "should check for the file with s3.headObject", (done) ->
+      @s3.headObject.callsArgWith 1, null, {}
+      @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) =>
+        expect(err).to.not.be.ok
+        expect(@s3.headObject.calledOnce, "called only once").to.be.true
+        expect((@s3.headObject.calledWith Bucket: @bucketName, Key: @key),
+          "called with correct arguments").to.be.true
+        done()
+
+    it "should return false on an inexistant file", (done) ->
+      @s3.headObject.callsArgWith 1, null, {}
+      @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) =>
+        expect(exists).to.be.false
+        done()
+
+    it "should return true on an existing file", (done) ->
+      @s3.headObject.callsArgWith 1, null, ETag: "etag"
+      @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) =>
+        expect(exists).to.be.true
+        done()
+
+    it "should dispatch the error from s3.headObject", (done) ->
+      @s3.headObject.callsArgWith 1, @error
+      @AWSSDKPersistorManager.checkIfFileExists @bucketName, @key, (err, exists) =>
+        expect(err).to.equal @error
+        done()