From 57daad0c264c7132b1dd5ca635df11bd08939ec3 Mon Sep 17 00:00:00 2001 From: Sabir Hassan Date: Fri, 22 Jul 2022 16:58:26 +0500 Subject: [PATCH] chore: error response codes for drive api --- api/src/controllers/drive.ts | 185 +++++++++++++++++--------- api/src/routes/api/drive.ts | 50 +++++-- api/src/routes/api/spec/drive.spec.ts | 106 ++++++++------- 3 files changed, 224 insertions(+), 117 deletions(-) diff --git a/api/src/controllers/drive.ts b/api/src/controllers/drive.ts index 36d2127..1b7a6ca 100644 --- a/api/src/controllers/drive.ts +++ b/api/src/controllers/drive.ts @@ -318,13 +318,19 @@ const getFile = async (req: express.Request, filePath: string) => { .join(getFilesFolder(), filePath) .replace(new RegExp('/', 'g'), path.sep) - if (!filePathFull.includes(driveFilesPath)) { - throw new Error('Cannot get file outside drive.') - } + if (!filePathFull.includes(driveFilesPath)) + throw { + code: 400, + status: 'Bad Request', + message: `Can't get file outside drive.` + } - if (!(await fileExists(filePathFull))) { - throw new Error("File doesn't exist.") - } + if (!(await fileExists(filePathFull))) + throw { + code: 404, + status: 'Not Found', + message: `File doesn't exist.` + } const extension = path.extname(filePathFull).toLowerCase() if (extension === '.sas') { @@ -342,17 +348,26 @@ const getFolder = async (folderPath?: string) => { .join(getFilesFolder(), folderPath) .replace(new RegExp('/', 'g'), path.sep) - if (!folderPathFull.includes(driveFilesPath)) { - throw new Error('Cannot get folder outside drive.') - } + if (!folderPathFull.includes(driveFilesPath)) + throw { + code: 400, + status: 'Bad Request', + message: `Can't get folder outside drive.` + } - if (!(await folderExists(folderPathFull))) { - throw new Error("Folder doesn't exist.") - } + if (!(await folderExists(folderPathFull))) + throw { + code: 404, + status: 'Not Found', + message: `Folder doesn't exist.` + } - if (!(await isFolder(folderPathFull))) { - throw new Error('Not a Folder.') - } + if (!(await isFolder(folderPathFull))) + throw { + code: 400, + status: 'Bad Request', + message: 'Not a Folder.' + } const files: string[] = await listFilesInFolder(folderPathFull) const folders: string[] = await listSubFoldersInFolder(folderPathFull) @@ -371,13 +386,19 @@ const deleteFile = async (filePath: string) => { .join(getFilesFolder(), filePath) .replace(new RegExp('/', 'g'), path.sep) - if (!filePathFull.includes(driveFilesPath)) { - throw new Error('Cannot delete file outside drive.') - } + if (!filePathFull.includes(driveFilesPath)) + throw { + code: 400, + status: 'Bad Request', + message: `Can't delete file outside drive.` + } - if (!(await fileExists(filePathFull))) { - throw new Error('File does not exist.') - } + if (!(await fileExists(filePathFull))) + throw { + code: 404, + status: 'Not Found', + message: `File doesn't exist.` + } await deleteFileOnSystem(filePathFull) @@ -391,13 +412,19 @@ const deleteFolder = async (folderPath: string) => { .join(getFilesFolder(), folderPath) .replace(new RegExp('/', 'g'), path.sep) - if (!folderPathFull.includes(driveFolderPath)) { - throw new Error('Cannot delete file outside drive.') - } + if (!folderPathFull.includes(driveFolderPath)) + throw { + code: 400, + status: 'Bad Request', + message: `Can't delete folder outside drive.` + } - if (!(await folderExists(folderPathFull))) { - throw new Error('Folder does not exist.') - } + if (!(await folderExists(folderPathFull))) + throw { + code: 404, + status: 'Not Found', + message: `Folder doesn't exist.` + } await deleteFolderOnSystem(folderPathFull) @@ -414,13 +441,19 @@ const saveFile = async ( .join(driveFilesPath, filePath) .replace(new RegExp('/', 'g'), path.sep) - if (!filePathFull.includes(driveFilesPath)) { - throw new Error('Cannot put file outside drive.') - } + if (!filePathFull.includes(driveFilesPath)) + throw { + code: 400, + status: 'Bad Request', + message: `Can't put file outside drive.` + } - if (await fileExists(filePathFull)) { - throw new Error('File already exists.') - } + if (await fileExists(filePathFull)) + throw { + code: 409, + status: 'Conflict', + message: 'File already exists.' + } const folderPath = path.dirname(filePathFull) await createFolder(folderPath) @@ -436,13 +469,19 @@ const addFolder = async (folderPath: string): Promise => { .join(drivePath, folderPath) .replace(new RegExp('/', 'g'), path.sep) - if (!folderPathFull.includes(drivePath)) { - throw new Error('Cannot put folder outside drive.') - } + if (!folderPathFull.includes(drivePath)) + throw { + code: 400, + status: 'Bad Request', + message: `Can't put folder outside drive.` + } - if (await folderExists(folderPathFull)) { - throw new Error('Folder already exists.') - } + if (await folderExists(folderPathFull)) + throw { + code: 409, + status: 'Conflict', + message: 'Folder already exists.' + } await createFolder(folderPathFull) @@ -463,30 +502,46 @@ const rename = async ( .join(drivePath, newPath) .replace(new RegExp('/', 'g'), path.sep) - if (!oldPathFull.includes(drivePath)) { - throw new Error('Old path is outside drive.') - } + if (!oldPathFull.includes(drivePath)) + throw { + code: 400, + status: 'Bad Request', + message: `Old path can't be outside of drive.` + } - if (!newPathFull.includes(drivePath)) { - throw new Error('New path is outside drive.') - } + if (!newPathFull.includes(drivePath)) + throw { + code: 400, + status: 'Bad Request', + message: `New path can't be outside of drive.` + } - if (await folderExists(oldPathFull)) { - if (await folderExists(newPathFull)) { - throw new Error('Folder already exists.') - } else moveFile(oldPathFull, newPathFull) + if (await isFolder(oldPathFull)) { + if (await folderExists(newPathFull)) + throw { + code: 409, + status: 'Conflict', + message: 'Folder with new name already exists.' + } + else moveFile(oldPathFull, newPathFull) + return { status: 'success' } + } else if (await fileExists(oldPathFull)) { + if (await fileExists(newPathFull)) + throw { + code: 409, + status: 'Conflict', + message: 'File with new name already exists.' + } + else moveFile(oldPathFull, newPathFull) return { status: 'success' } } - if (await fileExists(oldPathFull)) { - if (await fileExists(newPathFull)) { - throw new Error('File already exists.') - } else moveFile(oldPathFull, newPathFull) - return { status: 'success' } + throw { + code: 404, + status: 'Not Found', + message: 'No file/folder found for provided path.' } - - throw new Error('No file/folder found for provided path.') } const updateFile = async ( @@ -499,13 +554,19 @@ const updateFile = async ( .join(driveFilesPath, filePath) .replace(new RegExp('/', 'g'), path.sep) - if (!filePathFull.includes(driveFilesPath)) { - throw new Error('Cannot modify file outside drive.') - } + if (!filePathFull.includes(driveFilesPath)) + throw { + code: 400, + status: 'Bad Request', + message: `Can't modify file outside drive.` + } - if (!(await fileExists(filePathFull))) { - throw new Error(`File doesn't exist.`) - } + if (!(await fileExists(filePathFull))) + throw { + code: 404, + status: 'Not Found', + message: `File doesn't exist.` + } await moveFile(multerFile.path, filePathFull) diff --git a/api/src/routes/api/drive.ts b/api/src/routes/api/drive.ts index c1e94c0..e404821 100644 --- a/api/src/routes/api/drive.ts +++ b/api/src/routes/api/drive.ts @@ -121,7 +121,11 @@ driveRouter.get('/file', async (req, res) => { try { await controller.getFile(req, query._filePath) } catch (err: any) { - res.status(403).send(err.toString()) + const statusCode = err.code + + delete err.code + + res.status(statusCode).send(err.message) } }) @@ -134,7 +138,11 @@ driveRouter.get('/folder', async (req, res) => { const response = await controller.getFolder(query._folderPath) res.send(response) } catch (err: any) { - res.status(403).send(err.toString()) + const statusCode = err.code + + delete err.code + + res.status(statusCode).send(err.message) } }) @@ -147,7 +155,11 @@ driveRouter.delete('/file', async (req, res) => { const response = await controller.deleteFile(query._filePath) res.send(response) } catch (err: any) { - res.status(403).send(err.toString()) + const statusCode = err.code + + delete err.code + + res.status(statusCode).send(err.message) } }) @@ -160,7 +172,11 @@ driveRouter.delete('/folder', async (req, res) => { const response = await controller.deleteFolder(query._folderPath) res.send(response) } catch (err: any) { - res.status(403).send(err.toString()) + const statusCode = err.code + + delete err.code + + res.status(statusCode).send(err.message) } }) @@ -187,7 +203,12 @@ driveRouter.post( res.send(response) } catch (err: any) { await deleteFile(req.file.path) - res.status(403).send(err.toString()) + + const statusCode = err.code + + delete err.code + + res.status(statusCode).send(err.message) } } ) @@ -201,7 +222,11 @@ driveRouter.post('/folder', async (req, res) => { const response = await controller.addFolder(body) res.send(response) } catch (err: any) { - res.status(403).send(err.toString()) + const statusCode = err.code + + delete err.code + + res.status(statusCode).send(err.message) } }) @@ -228,7 +253,12 @@ driveRouter.patch( res.send(response) } catch (err: any) { await deleteFile(req.file.path) - res.status(403).send(err.toString()) + + const statusCode = err.code + + delete err.code + + res.status(statusCode).send(err.message) } } ) @@ -242,7 +272,11 @@ driveRouter.post('/rename', async (req, res) => { const response = await controller.rename(body) res.send(response) } catch (err: any) { - res.status(403).send(err.toString()) + const statusCode = err.code + + delete err.code + + res.status(statusCode).send(err.message) } }) diff --git a/api/src/routes/api/spec/drive.spec.ts b/api/src/routes/api/spec/drive.spec.ts index 5651ec3..4e8a875 100644 --- a/api/src/routes/api/spec/drive.spec.ts +++ b/api/src/routes/api/spec/drive.spec.ts @@ -549,29 +549,29 @@ describe('drive', () => { expect(res.body).toEqual({}) }) - it('should respond with Forbidden if folder is not present', async () => { + it('should respond with Not Found if folder is not present', async () => { const res = await request(app) .get(getFolderApi) .auth(accessToken, { type: 'bearer' }) .query({ _folderPath: `/my/path/code-${generateTimestamp()}` }) - .expect(403) + .expect(404) - expect(res.text).toEqual(`Error: Folder doesn't exist.`) + expect(res.text).toEqual(`Folder doesn't exist.`) expect(res.body).toEqual({}) }) - it('should respond with Forbidden if folderPath outside Drive', async () => { + it('should respond with Bad Request if folderPath outside Drive', async () => { const res = await request(app) .get(getFolderApi) .auth(accessToken, { type: 'bearer' }) .query({ _folderPath: '/../path/code.sas' }) - .expect(403) + .expect(400) - expect(res.text).toEqual('Error: Cannot get folder outside drive.') + expect(res.text).toEqual(`Can't get folder outside drive.`) expect(res.body).toEqual({}) }) - it('should respond with Forbidden if folderPath is of a file', async () => { + it('should respond with Bad Request if folderPath is of a file', async () => { const fileToCopyPath = path.join(__dirname, 'files', 'sample.sas') const filePath = '/my/path/code.sas' @@ -582,9 +582,9 @@ describe('drive', () => { .get(getFolderApi) .auth(accessToken, { type: 'bearer' }) .query({ _folderPath: filePath }) - .expect(403) + .expect(400) - expect(res.text).toEqual('Error: Not a Folder.') + expect(res.text).toEqual('Not a Folder.') expect(res.body).toEqual({}) }) }) @@ -609,24 +609,28 @@ describe('drive', () => { }) }) - it('should respond with Forbidden if the folder already exists', async () => { + it('should respond with Conflict if the folder already exists', async () => { await createFolder(path.join(pathToDrive, '/post/folder')) const res = await request(app) .post(folderApi) .auth(accessToken, { type: 'bearer' }) .send({ folderPath: '/post/folder' }) + .expect(409) - expect(res.statusCode).toEqual(403) + expect(res.text).toEqual(`Folder already exists.`) + + expect(res.statusCode).toEqual(409) }) - it('should respond with Forbidden if the folderPath is outside drive', async () => { + it('should respond with Bad Request if the folderPath is outside drive', async () => { const res = await request(app) .post(folderApi) .auth(accessToken, { type: 'bearer' }) .send({ folderPath: '../sample' }) + .expect(400) - expect(res.statusCode).toEqual(403) + expect(res.text).toEqual(`Can't put folder outside drive.`) }) }) @@ -648,22 +652,24 @@ describe('drive', () => { }) }) - it('should respond with Forbidden if the folder does not exists', async () => { + it('should respond with Not Found if the folder does not exists', async () => { const res = await request(app) .delete(folderApi) .auth(accessToken, { type: 'bearer' }) .query({ _folderPath: 'notExists' }) + .expect(404) - expect(res.statusCode).toEqual(403) + expect(res.text).toEqual(`Folder doesn't exist.`) }) - it('should respond with Forbidden if the folderPath is outside drive', async () => { + it('should respond with Bad Request if the folderPath is outside drive', async () => { const res = await request(app) .delete(folderApi) .auth(accessToken, { type: 'bearer' }) .query({ _folderPath: '../outsideDrive' }) + .expect(400) - expect(res.statusCode).toEqual(403) + expect(res.text).toEqual(`Can't delete folder outside drive.`) }) }) }) @@ -711,7 +717,7 @@ describe('drive', () => { expect(res.body).toEqual({}) }) - it('should respond with Forbidden if file is already present', async () => { + it('should respond with Conflict if file is already present', async () => { const fileToAttachPath = path.join(__dirname, 'files', 'sample.sas') const pathToUpload = `/my/path/code-${generateTimestamp()}.sas` @@ -726,13 +732,13 @@ describe('drive', () => { .auth(accessToken, { type: 'bearer' }) .field('filePath', pathToUpload) .attach('file', fileToAttachPath) - .expect(403) + .expect(409) - expect(res.text).toEqual('Error: File already exists.') + expect(res.text).toEqual('File already exists.') expect(res.body).toEqual({}) }) - it('should respond with Forbidden if filePath outside Drive', async () => { + it('should respond with Bad Request if filePath outside Drive', async () => { const fileToAttachPath = path.join(__dirname, 'files', 'sample.sas') const pathToUpload = '/../path/code.sas' @@ -741,9 +747,9 @@ describe('drive', () => { .auth(accessToken, { type: 'bearer' }) .field('filePath', pathToUpload) .attach('file', fileToAttachPath) - .expect(403) + .expect(400) - expect(res.text).toEqual('Error: Cannot put file outside drive.') + expect(res.text).toEqual(`Can't put file outside drive.`) expect(res.body).toEqual({}) }) @@ -878,19 +884,19 @@ describe('drive', () => { expect(res.body).toEqual({}) }) - it('should respond with Forbidden if file is not present', async () => { + it('should respond with Not Found if file is not present', async () => { const res = await request(app) .patch('/SASjsApi/drive/file') .auth(accessToken, { type: 'bearer' }) .field('filePath', `/my/path/code-3.sas`) .attach('file', path.join(__dirname, 'files', 'sample.sas')) - .expect(403) + .expect(404) - expect(res.text).toEqual(`Error: File doesn't exist.`) + expect(res.text).toEqual(`File doesn't exist.`) expect(res.body).toEqual({}) }) - it('should respond with Forbidden if filePath outside Drive', async () => { + it('should respond with Bad Request if filePath outside Drive', async () => { const fileToAttachPath = path.join(__dirname, 'files', 'sample.sas') const pathToUpload = '/../path/code.sas' @@ -899,9 +905,9 @@ describe('drive', () => { .auth(accessToken, { type: 'bearer' }) .field('filePath', pathToUpload) .attach('file', fileToAttachPath) - .expect(403) + .expect(400) - expect(res.text).toEqual('Error: Cannot modify file outside drive.') + expect(res.text).toEqual(`Can't modify file outside drive.`) expect(res.body).toEqual({}) }) @@ -1006,25 +1012,25 @@ describe('drive', () => { expect(res.body).toEqual({}) }) - it('should respond with Forbidden if file is not present', async () => { + it('should respond with Not Found if file is not present', async () => { const res = await request(app) .get('/SASjsApi/drive/file') .auth(accessToken, { type: 'bearer' }) .query({ _filePath: `/my/path/code-4.sas` }) - .expect(403) + .expect(404) - expect(res.text).toEqual(`Error: File doesn't exist.`) + expect(res.text).toEqual(`File doesn't exist.`) expect(res.body).toEqual({}) }) - it('should respond with Forbidden if filePath outside Drive', async () => { + it('should respond with Bad Request if filePath outside Drive', async () => { const res = await request(app) .get('/SASjsApi/drive/file') .auth(accessToken, { type: 'bearer' }) .query({ _filePath: '/../path/code.sas' }) - .expect(403) + .expect(400) - expect(res.text).toEqual('Error: Cannot get file outside drive.') + expect(res.text).toEqual(`Can't get file outside drive.`) expect(res.body).toEqual({}) }) @@ -1093,54 +1099,59 @@ describe('drive', () => { }) }) - it('should respond with forbidden if the oldPath is outside drive', async () => { + it('should respond with Bad Request if the oldPath is outside drive', async () => { const res = await request(app) .post(renameApi) .auth(accessToken, { type: 'bearer' }) .send({ oldPath: '../outside', newPath: 'renamed' }) + .expect(400) - expect(res.statusCode).toEqual(403) + expect(res.text).toEqual(`Old path can't be outside of drive.`) }) - it('should respond with forbidden if the newPath is outside drive', async () => { + it('should respond with Bad Request if the newPath is outside drive', async () => { const res = await request(app) .post(renameApi) .auth(accessToken, { type: 'bearer' }) .send({ oldPath: 'older', newPath: '../outside' }) + .expect(400) - expect(res.statusCode).toEqual(403) + expect(res.text).toEqual(`New path can't be outside of drive.`) }) - it('should respond with forbidden if the folder does not exist', async () => { + it('should respond with Not Found if the folder does not exist', async () => { const res = await request(app) .post(renameApi) .auth(accessToken, { type: 'bearer' }) .send({ oldPath: '/rename/not exists', newPath: '/rename/renamed' }) + .expect(404) - expect(res.statusCode).toEqual(403) + expect(res.text).toEqual('No file/folder found for provided path.') }) - it('should respond with forbidden if the folder already exists', async () => { + it('should respond with Conflict if the folder already exists', async () => { await createFolder(path.join(pathToDrive, 'rename', 'folder')) await createFolder(path.join(pathToDrive, 'rename', 'exists')) const res = await request(app) .post(renameApi) .auth(accessToken, { type: 'bearer' }) .send({ oldPath: '/rename/folder', newPath: '/rename/exists' }) + .expect(409) - expect(res.statusCode).toEqual(403) + expect(res.text).toEqual('Folder with new name already exists.') }) - it('should respond with forbidden if the file does not exist', async () => { + it('should respond with Not Found if the file does not exist', async () => { const res = await request(app) .post(renameApi) .auth(accessToken, { type: 'bearer' }) .send({ oldPath: '/rename/file.txt', newPath: '/rename/renamed.txt' }) + .expect(404) - expect(res.statusCode).toEqual(403) + expect(res.text).toEqual('No file/folder found for provided path.') }) - it('should respond with forbidden if the file already exists', async () => { + it('should respond with Conflict if the file already exists', async () => { await createFile( path.join(pathToDrive, 'rename', 'file.txt'), 'some file content' @@ -1153,8 +1164,9 @@ describe('drive', () => { .post(renameApi) .auth(accessToken, { type: 'bearer' }) .send({ oldPath: '/rename/file.txt', newPath: '/rename/exists.txt' }) + .expect(409) - expect(res.statusCode).toEqual(403) + expect(res.text).toEqual('File with new name already exists.') }) }) })