Merge pull request #52 from overleaf/em-get-file-size

Return file size on HEAD request

services/filestore/app.coffee

@@ -63,22 +63,23 @@ app.use (req, res, next) ->
 
 Metrics.injectMetricsRoute(app)
 
+app.head "/project/:project_id/file/:file_id", keyBuilder.userFileKey, fileController.getFileHead
 app.get  "/project/:project_id/file/:file_id", keyBuilder.userFileKey, fileController.getFile
 app.post "/project/:project_id/file/:file_id", keyBuilder.userFileKey, fileController.insertFile
+app.put  "/project/:project_id/file/:file_id", keyBuilder.userFileKey, bodyParser.json(), fileController.copyFile
+app.del  "/project/:project_id/file/:file_id", keyBuilder.userFileKey, fileController.deleteFile
 
-app.put "/project/:project_id/file/:file_id", keyBuilder.userFileKey, bodyParser.json(), fileController.copyFile
-app.del "/project/:project_id/file/:file_id", keyBuilder.userFileKey, fileController.deleteFile
-
+app.head "/template/:template_id/v/:version/:format", keyBuilder.templateFileKey, fileController.getFileHead
 app.get  "/template/:template_id/v/:version/:format", keyBuilder.templateFileKey, fileController.getFile
 app.get  "/template/:template_id/v/:version/:format/:sub_type", keyBuilder.templateFileKey, fileController.getFile
 app.post "/template/:template_id/v/:version/:format", keyBuilder.templateFileKey, fileController.insertFile
 
 
+app.head "/project/:project_id/public/:public_file_id", keyBuilder.publicFileKey, fileController.getFileHead
 app.get  "/project/:project_id/public/:public_file_id", keyBuilder.publicFileKey, fileController.getFile
 app.post "/project/:project_id/public/:public_file_id", keyBuilder.publicFileKey, fileController.insertFile
-
-app.put "/project/:project_id/public/:public_file_id", keyBuilder.publicFileKey, bodyParser.json(), fileController.copyFile
-app.del "/project/:project_id/public/:public_file_id", keyBuilder.publicFileKey, fileController.deleteFile
+app.put  "/project/:project_id/public/:public_file_id", keyBuilder.publicFileKey, bodyParser.json(), fileController.copyFile
+app.del  "/project/:project_id/public/:public_file_id", keyBuilder.publicFileKey, fileController.deleteFile
 
 app.get "/project/:project_id/size", keyBuilder.publicProjectKey, fileController.directorySize
 

services/filestore/app/coffee/FSPersistorManager.coffee

@@ -1,5 +1,6 @@
 logger = require("logger-sharelatex")
 fs = require("fs")
+path = require("path")
 LocalFileWriter = require("./LocalFileWriter")
 Errors = require('./Errors')
 rimraf = require("rimraf")
@@ -35,7 +36,7 @@ module.exports =
       if err?
         logger.err  location:location, target:target, fsPath:fsPath, err:err, "something went wrong writing stream to disk"
         return callback err
-      @sendFile location, target, fsPath, (err) -> 
+      @sendFile location, target, fsPath, (err) ->
         # delete the temporary file created above and return the original error
         LocalFileWriter.deleteFile fsPath, () ->
           callback(err)
@@ -57,6 +58,18 @@ module.exports =
       # ensures the callback is only called once
       return callback null, sourceStream
 
+  getFileSize: (location, filename, callback) ->
+    fullPath = path.join(location, filterName(filename))
+    fs.stat fullPath, (err, stats) ->
+      if err?
+        if err.code == 'ENOENT'
+          logger.log({location:location, filename:filename}, "file not found")
+          callback(new Errors.NotFoundError(err.message))
+        else
+          logger.err({err:err, location:location, filename:filename}, "failed to stat file")
+          callback(err)
+        return
+      callback(null, stats.size)
 
   copyFile: (location, fromName, toName, callback = (err)->)->
     filteredFromName=filterName fromName

services/filestore/app/coffee/FileController.coffee

@@ -21,7 +21,7 @@ module.exports = FileController =
 			style: style,
 		}
 		metrics.inc "getFile"
-		logger.log key:key, bucket:bucket, format:format, style: style, "reciving request to get file"
+		logger.log key:key, bucket:bucket, format:format, style: style, "receiving request to get file"
 		if req.headers.range?
 			range = FileController._get_range(req.headers.range)
 			options.start = range.start
@@ -41,10 +41,24 @@ module.exports = FileController =
 				logger.log key:key, bucket:bucket, format:format, style:style, "sending file to response"
 				fileStream.pipe res
 
+	getFileHead: (req, res) ->
+		{key, bucket} = req
+		metrics.inc("getFileSize")
+		logger.log({ key: key, bucket: bucket }, "receiving request to get file metadata")
+		FileHandler.getFileSize bucket, key, (err, fileSize) ->
+			if err?
+				if err instanceof Errors.NotFoundError
+					res.status(404).end()
+				else
+					res.status(500).end()
+				return
+			res.set("Content-Length", fileSize)
+			res.status(200).end()
+
 	insertFile: (req, res)->
 		metrics.inc "insertFile"
 		{key, bucket} = req
-		logger.log key:key, bucket:bucket, "reciving request to insert file"
+		logger.log key:key, bucket:bucket, "receiving request to insert file"
 		FileHandler.insertFile bucket, key, req, (err)->
 			if err?
 				logger.log err: err, key: key, bucket: bucket, "error inserting file"
@@ -57,7 +71,7 @@ module.exports = FileController =
 		{key, bucket} = req
 		oldProject_id = req.body.source.project_id
 		oldFile_id = req.body.source.file_id
-		logger.log key:key, bucket:bucket, oldProject_id:oldProject_id, oldFile_id:oldFile_id, "reciving request to copy file"
+		logger.log key:key, bucket:bucket, oldProject_id:oldProject_id, oldFile_id:oldFile_id, "receiving request to copy file"
 		PersistorManager.copyFile bucket, "#{oldProject_id}/#{oldFile_id}", key, (err)->
 			if err?
 				if err instanceof Errors.NotFoundError
@@ -71,7 +85,7 @@ module.exports = FileController =
 	deleteFile: (req, res)->
 		metrics.inc "deleteFile"
 		{key, bucket} = req
-		logger.log key:key, bucket:bucket,  "reciving request to delete file"
+		logger.log key:key, bucket:bucket,  "receiving request to delete file"
 		FileHandler.deleteFile bucket, key, (err)->
 			if err?
 				logger.log err:err, key:key, bucket:bucket, "something went wrong deleting file"
@@ -90,7 +104,7 @@ module.exports = FileController =
 	directorySize: (req, res)->
 		metrics.inc "projectSize"
 		{project_id, bucket} = req
-		logger.log project_id:project_id, bucket:bucket, "reciving request to project size"
+		logger.log project_id:project_id, bucket:bucket, "receiving request to project size"
 		FileHandler.getDirectorySize bucket, project_id, (err, size)->
 			if err?
 				logger.log err: err, project_id: project_id, bucket: bucket, "error inserting file"

services/filestore/app/coffee/FileHandler.coffee

@@ -31,6 +31,9 @@ module.exports = FileHandler =
 		else
 			@_getConvertedFile bucket, key, opts, callback
 
+	getFileSize: (bucket, key, callback) ->
+		PersistorManager.getFileSize(bucket, key, callback)
+
 	_getStandardFile: (bucket, key, opts, callback)->
 		PersistorManager.getFileStream bucket, key, opts, (err, fileStream)->
 			if err? and !(err instanceof Errors.NotFoundError)

services/filestore/app/coffee/S3PersistorManager.coffee

@@ -91,12 +91,36 @@ 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
-			else 
+			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
 
+	getFileSize: (bucketName, key, callback) ->
+		logger.log({ bucketName: bucketName, key: key }, "getting file size from S3")
+		s3.headObject { Bucket: bucketName, Key: key }, (err, data) ->
+			if err?
+				if err.statusCode in [403, 404]
+					# S3 returns a 403 instead of a 404 when the user doesn't have
+					# permission to list the bucket contents.
+					logger.log({
+						bucketName: bucketName,
+						key: key
+					}, "file not found in s3")
+					callback(
+						new Errors.NotFoundError("File not found in S3: #{bucketName}:#{key}")
+					)
+				else
+					logger.err({
+						bucketName: bucketName,
+						key: key,
+						err: err
+					}, "error performing S3 HeadObject")
+					callback(err)
+				return
+			callback(null, data.ContentLength)
+
 	copyFile: (bucketName, sourceKey, destKey, callback)->
 		logger.log bucketName:bucketName, sourceKey:sourceKey, destKey: destKey, "copying file in s3"
 		source = bucketName + '/' + sourceKey

services/filestore/test/acceptance/coffee/SendingFileTest.coffee

@@ -32,7 +32,7 @@ describe "Filestore", ->
 
 
 
-	it "should send a 200 for status endpoing", (done)->
+	it "should send a 200 for status endpoint", (done)->
 		request "#{@filestoreUrl}/status", (err, response, body)->
 			response.statusCode.should.equal 200
 			body.indexOf("filestore").should.not.equal -1
@@ -59,6 +59,13 @@ describe "Filestore", ->
 				response.statusCode.should.equal 404
 				done()
 
+		it "should return the file size on a HEAD request", (done) ->
+			expectedLength = Buffer.byteLength(@constantFileContent)
+			request.head @fileUrl, (err, res) =>
+				expect(res.statusCode).to.equal(200)
+				expect(res.headers['content-length']).to.equal(expectedLength.toString())
+				done()
+
 		it "should be able get the file back", (done)->
 			@timeout(1000 * 10)
 			request.get @fileUrl, (err, response, body)=>

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

@@ -22,6 +22,7 @@ describe "FSPersistorManagerTests", ->
       openSync:sinon.stub()
       fstatSync:sinon.stub()
       closeSync:sinon.stub()
+      stat:sinon.stub()
     @Rimraf = sinon.stub()
     @LocalFileWriter =
       writeStream: sinon.stub()
@@ -156,7 +157,35 @@ describe "FSPersistorManagerTests", ->
             expect(err instanceof Error).to.equal true
             done()
 
+  describe "getFileSize", ->
+    it "should return the file size", (done) ->
+      expectedFileSize = 75382
+      @Fs.stat.yields(new Error("fs.stat got unexpected arguments"))
+      @Fs.stat.withArgs("#{@location}/#{@name1Filtered}")
+        .yields(null, { size: expectedFileSize })
+
+      @FSPersistorManager.getFileSize @location, @name1, (err, fileSize) =>
+        if err?
+          return done(err)
+        expect(fileSize).to.equal(expectedFileSize)
+        done()
+
+    it "should throw a NotFoundError if the file does not exist", (done) ->
+      error = new Error()
+      error.code = "ENOENT"
+      @Fs.stat.yields(error)
+
+      @FSPersistorManager.getFileSize @location, @name1, (err, fileSize) =>
+        expect(err).to.be.instanceof(@Errors.NotFoundError)
+        done()
+
+    it "should rethrow any other error", (done) ->
+      error = new Error()
+      @Fs.stat.yields(error)
 
+      @FSPersistorManager.getFileSize @location, @name1, (err, fileSize) =>
+        expect(err).to.equal(error)
+        done()
 
   describe "copyFile", ->
     beforeEach ->

services/filestore/test/unit/coffee/FileControllerTests.coffee

@@ -20,6 +20,7 @@ describe "FileController", ->
 					user_files:"user_files"
 		@FileHandler =
 			getFile: sinon.stub()
+			getFileSize: sinon.stub()
 			deleteFile: sinon.stub()
 			insertFile: sinon.stub()
 			getDirectorySize: sinon.stub()
@@ -49,7 +50,8 @@ describe "FileController", ->
 				file_id:@file_id
 			headers: {}
 		@res =
-			setHeader: ->
+			set: sinon.stub().returnsThis()
+			status: sinon.stub().returnsThis()
 		@fileStream = {}
 
 	describe "getFile", ->
@@ -89,6 +91,39 @@ describe "FileController", ->
 					done()
 				@controller.getFile @req, @res
 
+	describe "getFileHead", ->
+		it "should return the file size in a Content-Length header", (done) ->
+			expectedFileSize = 84921
+			@FileHandler.getFileSize.yields(
+				new Error("FileHandler.getFileSize: unexpected arguments")
+			)
+			@FileHandler.getFileSize.withArgs(@bucket, @key).yields(null, expectedFileSize)
+
+			@res.end = () =>
+				expect(@res.status.lastCall.args[0]).to.equal(200)
+				expect(@res.set.calledWith("Content-Length", expectedFileSize)).to.equal(true)
+				done()
+
+			@controller.getFileHead(@req, @res)
+
+		it "should return a 404 is the file is not found", (done) ->
+			@FileHandler.getFileSize.yields(new @Errors.NotFoundError())
+
+			@res.end = () =>
+				expect(@res.status.lastCall.args[0]).to.equal(404)
+				done()
+
+			@controller.getFileHead(@req, @res)
+
+		it "should return a 500 on internal errors", (done) ->
+			@FileHandler.getFileSize.yields(new Error())
+
+			@res.end = () =>
+				expect(@res.status.lastCall.args[0]).to.equal(500)
+				done()
+
+			@controller.getFileHead(@req, @res)
+
 	describe "insertFile", ->
 
 		it "should send bucket name key and res to PersistorManager", (done)->

services/filestore/test/unit/coffee/FileHandlerTests.coffee

@@ -108,7 +108,6 @@ describe "FileHandler", ->
 				@handler._getConvertedFile.called.should.equal true
 				done()
 
-
 	describe "_getStandardFile", ->
 
 		beforeEach ->

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

@@ -26,11 +26,13 @@ describe "S3PersistorManagerTests", ->
 		@knox =
 			createClient: sinon.stub().returns(@stubbedKnoxClient)
 		@stubbedS3Client =
-			copyObject:sinon.stub()
+			copyObject: sinon.stub()
+			headObject: sinon.stub()
 		@awsS3 = sinon.stub().returns @stubbedS3Client
 		@LocalFileWriter =
 			writeStream: sinon.stub()
 			deleteFile: sinon.stub()
+		@request = sinon.stub()
 		@requires =
 			"knox": @knox
 			"aws-sdk/clients/s3": @awsS3
@@ -39,15 +41,16 @@ describe "S3PersistorManagerTests", ->
 			"logger-sharelatex":
 				log:->
 				err:->
+			"request": @request
 			"./Errors": @Errors =
 				NotFoundError: sinon.stub()
 		@key = "my/key"
 		@bucketName = "my-bucket"
 		@error = "my errror"
+		@S3PersistorManager = SandboxedModule.require modulePath, requires: @requires
 
 	describe "getFileStream", ->
 		beforeEach ->
-			@S3PersistorManager = SandboxedModule.require modulePath, requires: @requires
 			@opts = {}
 
 		it "should use correct key", (done)->
@@ -74,7 +77,6 @@ describe "S3PersistorManagerTests", ->
 
 		describe "with supplied auth", ->
 			beforeEach ->
-				@S3PersistorManager = SandboxedModule.require modulePath, requires: @requires
 				@credentials =
 					auth_key: "that_key"
 					auth_secret: "that_secret"
@@ -156,10 +158,45 @@ describe "S3PersistorManagerTests", ->
 						@Errors.NotFoundError.called.should.equal false
 						done()
 
+	describe "getFileSize", ->
+		it "should obtain the file size from S3", (done) ->
+			expectedFileSize = 123
+			@stubbedS3Client.headObject.yields(new Error(
+				"s3Client.headObject got unexpected arguments"
+			))
+			@stubbedS3Client.headObject.withArgs({
+				Bucket: @bucketName
+				Key: @key
+			}).yields(null, { ContentLength: expectedFileSize })
+
+			@S3PersistorManager.getFileSize @bucketName, @key, (err, fileSize) =>
+				if err?
+					return done(err)
+				expect(fileSize).to.equal(expectedFileSize)
+				done()
+
+		[403, 404].forEach (statusCode) ->
+			it "should throw NotFoundError when S3 responds with #{statusCode}", (done) ->
+				error = new Error()
+				error.statusCode = statusCode
+				@stubbedS3Client.headObject.yields(error)
+
+				@S3PersistorManager.getFileSize @bucketName, @key, (err, fileSize) =>
+					expect(err).to.be.an.instanceof(@Errors.NotFoundError)
+					done()
+
+		it "should rethrow any other error", (done) ->
+			error = new Error()
+			@stubbedS3Client.headObject.yields(error)
+			@stubbedS3Client.headObject.yields(error)
+
+			@S3PersistorManager.getFileSize @bucketName, @key, (err, fileSize) =>
+				expect(err).to.equal(error)
+				done()
+
 	describe "sendFile", ->
 
 		beforeEach ->
-			@S3PersistorManager = SandboxedModule.require modulePath, requires: @requires
 			@stubbedKnoxClient.putFile.returns on:->
 
 		it "should put file with knox", (done)->
@@ -183,7 +220,6 @@ describe "S3PersistorManagerTests", ->
 			@fsPath = "to/some/where"
 			@origin =
 				on:->
-			@S3PersistorManager = SandboxedModule.require modulePath, requires: @requires
 			@S3PersistorManager.sendFile = sinon.stub().callsArgWith(3)
 
 		it "should send stream to LocalFileWriter", (done)->
@@ -211,7 +247,6 @@ describe "S3PersistorManagerTests", ->
 		beforeEach ->
 			@sourceKey = "my/key"
 			@destKey = "my/dest/key"
-			@S3PersistorManager = SandboxedModule.require modulePath, requires: @requires
 
 		it "should use AWS SDK to copy file", (done)->
 			@stubbedS3Client.copyObject.callsArgWith(1, @error)
@@ -229,9 +264,6 @@ describe "S3PersistorManagerTests", ->
 
 	describe "deleteDirectory", ->
 
-		beforeEach ->
-			@S3PersistorManager = SandboxedModule.require modulePath, requires: @requires
-
 		it "should list the contents passing them onto multi delete", (done)->
 			data =
 				Contents: [{Key:"1234"}, {Key: "456"}]
@@ -244,9 +276,7 @@ describe "S3PersistorManagerTests", ->
 	describe "deleteFile", ->
 
 		it "should use correct options", (done)->
-			@request = sinon.stub().callsArgWith(1)
-			@requires["request"] = @request
-			@S3PersistorManager = SandboxedModule.require modulePath, requires: @requires
+			@request.callsArgWith(1)
 
 			@S3PersistorManager.deleteFile @bucketName, @key, (err)=>
 				opts = @request.args[0][0]
@@ -257,9 +287,7 @@ describe "S3PersistorManagerTests", ->
 				done()
 
 		it "should return the error", (done)->
-			@request = sinon.stub().callsArgWith(1, @error)
-			@requires["request"] = @request
-			@S3PersistorManager = SandboxedModule.require modulePath, requires: @requires
+			@request.callsArgWith(1, @error)
 
 			@S3PersistorManager.deleteFile @bucketName, @key, (err)=>
 				err.should.equal @error
@@ -268,9 +296,7 @@ describe "S3PersistorManagerTests", ->
 	describe "checkIfFileExists", ->
 
 		it "should use correct options", (done)->
-			@request = sinon.stub().callsArgWith(1,  null, statusCode:200)
-			@requires["request"] = @request
-			@S3PersistorManager = SandboxedModule.require modulePath, requires: @requires
+			@request.callsArgWith(1,  null, statusCode:200)
 
 			@S3PersistorManager.checkIfFileExists @bucketName, @key, (err)=>
 				opts = @request.args[0][0]
@@ -281,25 +307,21 @@ describe "S3PersistorManagerTests", ->
 				done()
 
 		it "should return true for a 200", (done)->
-			@request = sinon.stub().callsArgWith(1, null, statusCode:200)
-			@requires["request"] = @request
-			@S3PersistorManager = SandboxedModule.require modulePath, requires: @requires
+			@request.callsArgWith(1, null, statusCode:200)
+
 			@S3PersistorManager.checkIfFileExists @bucketName, @key, (err, exists)=>
 				exists.should.equal true
 				done()
 
 		it "should return false for a non 200", (done)->
-			@request = sinon.stub().callsArgWith(1, null, statusCode:404)
-			@requires["request"] = @request
-			@S3PersistorManager = SandboxedModule.require modulePath, requires: @requires
+			@request.callsArgWith(1, null, statusCode:404)
+
 			@S3PersistorManager.checkIfFileExists @bucketName, @key, (err, exists)=>
 				exists.should.equal false
 				done()
 
 		it "should return the error", (done)->
-			@request = sinon.stub().callsArgWith(1, @error, {})
-			@requires["request"] = @request
-			@S3PersistorManager = SandboxedModule.require modulePath, requires: @requires
+			@request.callsArgWith(1, @error, {})
 
 			@S3PersistorManager.checkIfFileExists @bucketName, @key, (err)=>
 				err.should.equal @error
@@ -307,13 +329,10 @@ describe "S3PersistorManagerTests", ->
 
 	describe "directorySize", ->
 
-		beforeEach ->
-			@S3PersistorManager = SandboxedModule.require modulePath, requires: @requires
-
 		it "should sum directory files size", (done) ->
 			data =
 				Contents: [ {Size: 1024}, {Size: 2048} ]
 			@stubbedKnoxClient.list.callsArgWith(1, null, data)
 			@S3PersistorManager.directorySize @bucketName, @key, (err, totalSize)=>
 				totalSize.should.equal 3072
-				done()
+				done()