Merge pull request #21320 from overleaf/jpa-filestore-controller

[filestore] only call handler from controller

GitOrigin-RevId: 1d1bbe4e961b300a919ae79e3c760322304783bc

services/filestore/app/js/FileController.js

@@ -1,4 +1,3 @@
-const PersistorManager = require('./PersistorManager')
 const FileHandler = require('./FileHandler')
 const metrics = require('@overleaf/metrics')
 const parseRange = require('range-parser')
@@ -139,17 +138,17 @@ function copyFile(req, res, next) {
   })
   req.requestLogger.setMessage('copying file')
 
-  PersistorManager.copyObject(bucket, `${oldProjectId}/${oldFileId}`, key)
+  FileHandler.copyObject(bucket, `${oldProjectId}/${oldFileId}`, key, err => {
-    .then(() => res.sendStatus(200))
+    if (err) {
-    .catch(err => {
+      if (err instanceof Errors.NotFoundError) {
-      if (err) {
+        res.sendStatus(404)
-        if (err instanceof Errors.NotFoundError) {
+      } else {
-          res.sendStatus(404)
+        next(err)
-        } else {
-          next(err)
-        }
       }
-    })
+    } else {
+      res.sendStatus(200)
+    }
+  })
 }
 
 function deleteFile(req, res, next) {

services/filestore/app/js/FileHandler.js

@@ -10,6 +10,7 @@ const { ConversionError, InvalidParametersError } = require('./Errors')
 const metrics = require('@overleaf/metrics')
 
 module.exports = {
+  copyObject: callbackify(copyObject),
   insertFile: callbackify(insertFile),
   deleteFile: callbackify(deleteFile),
   deleteProject: callbackify(deleteProject),
@@ -18,6 +19,7 @@ module.exports = {
   getFileSize: callbackify(getFileSize),
   getDirectorySize: callbackify(getDirectorySize),
   promises: {
+    copyObject,
     getFile,
     getRedirectUrl,
     insertFile,
@@ -28,6 +30,10 @@ module.exports = {
   },
 }
 
+async function copyObject(bucket, sourceKey, destinationKey) {
+  await PersistorManager.copyObject(bucket, sourceKey, destinationKey)
+}
+
 async function insertFile(bucket, key, stream) {
   const convertedKey = KeyBuilder.getConvertedFolderKey(key)
   if (!convertedKey.match(/^[0-9a-f]{24}\/([0-9a-f]{24}|v\/[0-9]+\/[a-z]+)/i)) {

services/filestore/test/unit/js/FileControllerTests.js

@@ -6,14 +6,7 @@ const Errors = require('../../../app/js/Errors')
 const modulePath = '../../../app/js/FileController.js'
 
 describe('FileController', function () {
-  let PersistorManager,
+  let FileHandler, LocalFileWriter, FileController, req, res, next, stream
-    FileHandler,
-    LocalFileWriter,
-    FileController,
-    req,
-    res,
-    next,
-    stream
   const settings = {
     s3: {
       buckets: {
@@ -32,13 +25,8 @@ describe('FileController', function () {
   const error = new Error('incorrect utensil')
 
   beforeEach(function () {
-    PersistorManager = {
-      sendStream: sinon.stub().yields(),
-      copyObject: sinon.stub().resolves(),
-      deleteObject: sinon.stub().yields(),
-    }
-
     FileHandler = {
+      copyObject: sinon.stub().yields(),
       getFile: sinon.stub().yields(null, fileStream),
       getFileSize: sinon.stub().yields(null, fileSize),
       deleteFile: sinon.stub().yields(),
@@ -57,7 +45,6 @@ describe('FileController', function () {
       requires: {
         './LocalFileWriter': LocalFileWriter,
         './FileHandler': FileHandler,
-        './PersistorManager': PersistorManager,
         './Errors': Errors,
         stream,
         '@overleaf/settings': settings,
@@ -239,7 +226,7 @@ describe('FileController', function () {
   })
 
   describe('insertFile', function () {
-    it('should send bucket name key and res to PersistorManager', function (done) {
+    it('should send bucket name key and res to FileHandler', function (done) {
       res.sendStatus = code => {
         expect(FileHandler.insertFile).to.have.been.calledWith(bucket, key, req)
         expect(code).to.equal(200)
@@ -263,10 +250,10 @@ describe('FileController', function () {
       }
     })
 
-    it('should send bucket name and both keys to PersistorManager', function (done) {
+    it('should send bucket name and both keys to FileHandler', function (done) {
       res.sendStatus = code => {
         code.should.equal(200)
-        expect(PersistorManager.copyObject).to.have.been.calledWith(
+        expect(FileHandler.copyObject).to.have.been.calledWith(
           bucket,
           oldKey,
           key
@@ -277,7 +264,7 @@ describe('FileController', function () {
     })
 
     it('should send a 404 if the original file was not found', function (done) {
-      PersistorManager.copyObject.rejects(
+      FileHandler.copyObject.yields(
         new Errors.NotFoundError({ message: 'not found', info: {} })
       )
       res.sendStatus = code => {
@@ -288,7 +275,7 @@ describe('FileController', function () {
     })
 
     it('should send an error if there was an error', function (done) {
-      PersistorManager.copyObject.rejects(error)
+      FileHandler.copyObject.yields(error)
       FileController.copyFile(req, res, err => {
         expect(err).to.equal(error)
         done()