fix(drive): update file API is same as create file

api/src/controllers/drive.ts

@@ -108,7 +108,7 @@ export class DriveController {
   /**
    * It's optional to either provide `_filePath` in url as query parameter
    * Or provide `filePath` in body as form field.
-   * But it's required to provided else API will respond with Bad Request.
+   * But it's required to provide else API will respond with Bad Request.
    *
    * @summary Create a file in SASjs Drive
    * @param _filePath Location of SAS program
@@ -118,7 +118,7 @@ export class DriveController {
   @Example<UpdateFileResponse>({
     status: 'success'
   })
-  @Response<UpdateFileResponse>(400, 'File already exists', {
+  @Response<UpdateFileResponse>(403, 'File already exists', {
     status: 'failure',
     message: 'File request failed.'
   })
@@ -132,21 +132,29 @@ export class DriveController {
   }
 
   /**
+   * It's optional to either provide `_filePath` in url as query parameter
+   * Or provide `filePath` in body as form field.
+   * But it's required to provide else API will respond with Bad Request.
+   *
    * @summary Modify a file in SASjs Drive
+   * @param _filePath Location of SAS program
+   * @example _filePath "/Public/somefolder/some.file.sas"
    *
    */
   @Example<UpdateFileResponse>({
     status: 'success'
   })
-  @Response<UpdateFileResponse>(400, 'Unable to get File', {
+  @Response<UpdateFileResponse>(403, `File doesn't exist`, {
     status: 'failure',
     message: 'File request failed.'
   })
   @Patch('/file')
   public async updateFile(
-    @Body() body: FilePayload
+    @UploadedFile() file: Express.Multer.File,
+    @Query() _filePath?: string,
+    @FormField() filePath?: string
   ): Promise<UpdateFileResponse> {
-    return updateFile(body)
+    return updateFile((_filePath ?? filePath)!, file)
   }
 
   /**
@@ -227,25 +235,27 @@ const saveFile = async (
   return { status: 'success' }
 }
 
-const updateFile = async (body: FilePayload): Promise<GetFileResponse> => {
-  const { filePath, fileContent } = body
-  try {
-    const filePathFull = path
-      .join(getTmpFilesFolderPath(), filePath)
-      .replace(new RegExp('/', 'g'), path.sep)
+const updateFile = async (
+  filePath: string,
+  multerFile: Express.Multer.File
+): Promise<GetFileResponse> => {
+  const driveFilesPath = getTmpFilesFolderPath()
 
-    await validateFilePath(filePathFull)
-    await createFile(filePathFull, fileContent)
+  const filePathFull = path
+    .join(driveFilesPath, filePath)
+    .replace(new RegExp('/', 'g'), path.sep)
 
-    return { status: 'success' }
-  } catch (err: any) {
-    throw {
-      code: 400,
-      status: 'failure',
-      message: 'File request failed.',
-      error: typeof err === 'object' ? err.toString() : err
-    }
+  if (!filePathFull.includes(driveFilesPath)) {
+    throw new Error('Cannot modify file outside drive.')
   }
+
+  if (!(await fileExists(filePathFull))) {
+    throw new Error(`File doesn't exist.`)
+  }
+
+  await moveFile(multerFile.path, filePathFull)
+
+  return { status: 'success' }
 }
 
 const validateFilePath = async (filePath: string) => {

api/src/routes/api/drive.ts

@@ -66,21 +66,33 @@ driveRouter.post(
   }
 )
 
-driveRouter.patch('/file', async (req, res) => {
-  const { error, value: body } = updateFileDriveValidation(req.body)
-  if (error) return res.status(400).send(error.details[0].message)
+driveRouter.patch(
+  '/file',
+  (...arg) => multerSingle('file', arg),
+  async (req, res) => {
+    const { error: errQ, value: query } = uploadFileParamValidation(req.query)
+    const { error: errB, value: body } = uploadFileBodyValidation(req.body)
 
-  try {
-    const response = await controller.updateFile(body)
-    res.send(response)
-  } catch (err: any) {
-    const statusCode = err.code
+    if (errQ && errB) {
+      if (req.file) await deleteFile(req.file.path)
+      return res.status(400).send(errB.details[0].message)
+    }
 
-    delete err.code
+    if (!req.file) return res.status(400).send('"file" is not present.')
 
-    res.status(statusCode).send(err)
+    try {
+      const response = await controller.updateFile(
+        req.file,
+        query._filePath,
+        body.filePath
+      )
+      res.send(response)
+    } catch (err: any) {
+      await deleteFile(req.file.path)
+      res.status(403).send(err.toString())
+    }
   }
-})
+)
 
 driveRouter.get('/fileTree', async (req, res) => {
   try {

api/src/routes/api/spec/drive.spec.ts

@@ -321,6 +321,166 @@ describe('files', () => {
         expect(res.body).toEqual({})
       })
     })
+
+    describe('update', () => {
+      it('should update a SAS file on drive having filePath as form field', async () => {
+        const fileToAttachPath = path.join(__dirname, 'files', 'sample.sas')
+        const pathToUpload = '/my/path/code.sas'
+
+        const pathToCopy = path.join(
+          fileUtilModules.getTmpFilesFolderPath(),
+          pathToUpload
+        )
+        await copy(fileToAttachPath, pathToCopy)
+
+        const res = await request(app)
+          .patch('/SASjsApi/drive/file')
+          .auth(accessToken, { type: 'bearer' })
+          .field('filePath', pathToUpload)
+          .attach('file', fileToAttachPath)
+
+        expect(res.statusCode).toEqual(200)
+        expect(res.body).toEqual({
+          status: 'success'
+        })
+      })
+
+      it('should update a SAS file on drive having _filePath as query param', async () => {
+        const fileToAttachPath = path.join(__dirname, 'files', 'sample.sas')
+        const pathToUpload = '/my/path/code.sas'
+
+        const pathToCopy = path.join(
+          fileUtilModules.getTmpFilesFolderPath(),
+          pathToUpload
+        )
+        await copy(fileToAttachPath, pathToCopy)
+
+        const res = await request(app)
+          .patch('/SASjsApi/drive/file')
+          .auth(accessToken, { type: 'bearer' })
+          .field('filePath', pathToUpload)
+          .attach('file', fileToAttachPath)
+
+        expect(res.statusCode).toEqual(200)
+        expect(res.body).toEqual({
+          status: 'success'
+        })
+      })
+
+      it('should respond with Unauthorized if access token is not present', async () => {
+        const res = await request(app)
+          .patch('/SASjsApi/drive/file')
+          .field('filePath', '/my/path/code.sas')
+          .attach('file', path.join(__dirname, 'files', 'sample.sas'))
+          .expect(401)
+
+        expect(res.text).toEqual('Unauthorized')
+        expect(res.body).toEqual({})
+      })
+
+      it('should respond with Forbidden if file is not present', async () => {
+        const res = await request(app)
+          .patch('/SASjsApi/drive/file')
+          .auth(accessToken, { type: 'bearer' })
+          .field('filePath', `/my/path/code-${generateTimestamp()}.sas`)
+          .attach('file', path.join(__dirname, 'files', 'sample.sas'))
+          .expect(403)
+
+        expect(res.text).toEqual(`Error: File doesn't exist.`)
+        expect(res.body).toEqual({})
+      })
+
+      it('should respond with Forbidden if filePath outside Drive', async () => {
+        const fileToAttachPath = path.join(__dirname, 'files', 'sample.sas')
+        const pathToUpload = '/../path/code.sas'
+
+        const res = await request(app)
+          .patch('/SASjsApi/drive/file')
+          .auth(accessToken, { type: 'bearer' })
+          .field('filePath', pathToUpload)
+          .attach('file', fileToAttachPath)
+          .expect(403)
+
+        expect(res.text).toEqual('Error: Cannot modify file outside drive.')
+        expect(res.body).toEqual({})
+      })
+
+      it('should respond with Bad Request if filePath is missing', async () => {
+        const fileToAttachPath = path.join(__dirname, 'files', 'sample.sas')
+
+        const res = await request(app)
+          .patch('/SASjsApi/drive/file')
+          .auth(accessToken, { type: 'bearer' })
+          .attach('file', fileToAttachPath)
+          .expect(400)
+
+        expect(res.text).toEqual(`"filePath" is required`)
+        expect(res.body).toEqual({})
+      })
+
+      it("should respond with Bad Request if filePath doesn't has correct extension", async () => {
+        const fileToAttachPath = path.join(__dirname, 'files', 'sample.sas')
+        const pathToUpload = '/my/path/code.oth'
+
+        const res = await request(app)
+          .patch('/SASjsApi/drive/file')
+          .auth(accessToken, { type: 'bearer' })
+          .field('filePath', pathToUpload)
+          .attach('file', fileToAttachPath)
+          .expect(400)
+
+        expect(res.text).toEqual('Valid extensions for filePath: .sas')
+        expect(res.body).toEqual({})
+      })
+
+      it('should respond with Bad Request if file is missing', async () => {
+        const pathToUpload = '/my/path/code.sas'
+
+        const res = await request(app)
+          .patch('/SASjsApi/drive/file')
+          .auth(accessToken, { type: 'bearer' })
+          .field('filePath', pathToUpload)
+          .expect(400)
+
+        expect(res.text).toEqual('"file" is not present.')
+        expect(res.body).toEqual({})
+      })
+
+      it("should respond with Bad Request if attached file doesn't has correct extension", async () => {
+        const fileToAttachPath = path.join(__dirname, 'files', 'sample.oth')
+        const pathToUpload = '/my/path/code.sas'
+
+        const res = await request(app)
+          .patch('/SASjsApi/drive/file')
+          .auth(accessToken, { type: 'bearer' })
+          .field('filePath', pathToUpload)
+          .attach('file', fileToAttachPath)
+          .expect(400)
+
+        expect(res.text).toEqual(
+          `File extension '.oth' not acceptable. Valid extension(s): .sas`
+        )
+        expect(res.body).toEqual({})
+      })
+
+      it('should respond with Bad Request if attached file exceeds file limit', async () => {
+        const pathToUpload = '/my/path/code.sas'
+
+        const attachedFile = Buffer.from('.'.repeat(20 * 1024 * 1024))
+
+        const res = await request(app)
+          .patch('/SASjsApi/drive/file')
+          .auth(accessToken, { type: 'bearer' })
+          .field('filePath', pathToUpload)
+          .attach('file', attachedFile, 'another.sas')
+          .expect(400)
+
+        expect(res.text).toEqual(
+          'File size is over limit. File limit is: 10 MB'
+        )
+        expect(res.body).toEqual({})
+      })
+    })
   })
 })