From fc58db5d1a09b6bd04884a77c7fee6e533011b51 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Wed, 13 Jan 2021 21:50:43 +0000 Subject: [PATCH] Server: Removed controller dependency from route --- .../src/controllers/api/FileController.ts | 10 --- packages/server/src/routes/api/files.ts | 84 +++++++++++++++---- packages/server/src/utils/routeUtils.ts | 12 +++ .../server/src/utils/testing/testUtils.ts | 7 +- 4 files changed, 84 insertions(+), 29 deletions(-) diff --git a/packages/server/src/controllers/api/FileController.ts b/packages/server/src/controllers/api/FileController.ts index 227973e75..41f8e450e 100644 --- a/packages/server/src/controllers/api/FileController.ts +++ b/packages/server/src/controllers/api/FileController.ts @@ -7,16 +7,6 @@ import { ChangePagination, PaginatedChanges } from '../../models/ChangeModel'; export default class FileController extends BaseController { - // Note: this is only used in tests. To create files with no content - // or directories, use postChild() - public async postFile_(sessionId: string, file: File): Promise { - const user = await this.initSession(sessionId); - const fileModel = this.models.file({ userId: user.id }); - let newFile = fileModel.fromApiInput(file); - newFile = await fileModel.save(file); - return fileModel.toApiOutput(newFile); - } - public async getFile(sessionId: string, fileId: string): Promise { const user = await this.initSession(sessionId); const fileModel = this.models.file({ userId: user.id }); diff --git a/packages/server/src/routes/api/files.ts b/packages/server/src/routes/api/files.ts index d5d4028a1..542cfaa3d 100644 --- a/packages/server/src/routes/api/files.ts +++ b/packages/server/src/routes/api/files.ts @@ -1,6 +1,6 @@ import { ErrorNotFound, ErrorMethodNotAllowed, ErrorBadRequest } from '../../utils/errors'; import { File } from '../../db'; -import { bodyFields, formParse, headerSessionId } from '../../utils/requestUtils'; +import { bodyFields, formParse } from '../../utils/requestUtils'; import { SubPath, Route, respondWithFileContent } from '../../utils/routeUtils'; import { AppContext } from '../../utils/types'; import * as fs from 'fs-extra'; @@ -9,29 +9,61 @@ import { requestChangePagination, requestPagination } from '../../models/utils/p const route: Route = { exec: async function(path: SubPath, ctx: AppContext) { - const fileController = ctx.controllers.apiFile(); - // console.info(`${ctx.method} ${path.id}${path.link ? `/${path.link}` : ''}`); + // ------------------------------------------- + // ROUTE api/files/:id + // ------------------------------------------- + if (!path.link) { + const fileModel = ctx.models.file({ userId: ctx.owner.id }); + const fileId = path.id; + if (ctx.method === 'GET') { - return fileController.getFile(headerSessionId(ctx.headers), path.id); + const file: File = await fileModel.entityFromItemId(fileId); + const loadedFile = await fileModel.load(file.id); + if (!loadedFile) throw new ErrorNotFound(); + return fileModel.toApiOutput(loadedFile); } if (ctx.method === 'PATCH') { - return fileController.patchFile(headerSessionId(ctx.headers), path.id, await bodyFields(ctx.req)); + const inputFile: File = await bodyFields(ctx.req); + const existingFile: File = await fileModel.entityFromItemId(fileId); + const newFile = fileModel.fromApiInput(inputFile); + newFile.id = existingFile.id; + return fileModel.toApiOutput(await fileModel.save(newFile)); } if (ctx.method === 'DELETE') { - return fileController.deleteFile(headerSessionId(ctx.headers), path.id); + try { + const file: File = await fileModel.entityFromItemId(fileId, { mustExist: false }); + if (!file.id) return; + await fileModel.delete(file.id); + } catch (error) { + if (error instanceof ErrorNotFound) { + // That's ok - a no-op + } else { + throw error; + } + } + return; } throw new ErrorMethodNotAllowed(); } + // ------------------------------------------- + // ROUTE api/files/:id/content + // ------------------------------------------- + if (path.link === 'content') { + const fileModel = ctx.models.file({ userId: ctx.owner.id }); + const fileId = path.id; + if (ctx.method === 'GET') { - const file: File = await fileController.getFileContent(headerSessionId(ctx.headers), path.id); + let file: File = await fileModel.entityFromItemId(fileId); + file = await fileModel.loadWithContent(file.id); + if (!file) throw new ErrorNotFound(); return respondWithFileContent(ctx.response, file); } @@ -39,35 +71,55 @@ const route: Route = { const result = await formParse(ctx.req); if (!result?.files?.file) throw new ErrorBadRequest('File data is missing'); const buffer = await fs.readFile(result.files.file.path); - return fileController.putFileContent(headerSessionId(ctx.headers), path.id, buffer); + + const file: File = await fileModel.entityFromItemId(fileId, { mustExist: false }); + file.content = buffer; + return fileModel.toApiOutput(await fileModel.save(file, { validationRules: { mustBeFile: true } })); } if (ctx.method === 'DELETE') { - return fileController.deleteFileContent(headerSessionId(ctx.headers), path.id); + const file: File = await fileModel.entityFromItemId(fileId, { mustExist: false }); + if (!file) return; + file.content = Buffer.alloc(0); + await fileModel.save(file, { validationRules: { mustBeFile: true } }); + return; } throw new ErrorMethodNotAllowed(); } + // ------------------------------------------- + // ROUTE api/files/:id/delta + // ------------------------------------------- + if (path.link === 'delta') { if (ctx.method === 'GET') { - return fileController.getDelta( - headerSessionId(ctx.headers), - path.id, - requestChangePagination(ctx.query) - ); + const fileModel = ctx.models.file({ userId: ctx.owner.id }); + const dir: File = await fileModel.entityFromItemId(path.id, { mustExist: true }); + const changeModel = ctx.models.change({ userId: ctx.owner.id }); + return changeModel.byDirectoryId(dir.id, requestChangePagination(ctx.query)); } throw new ErrorMethodNotAllowed(); } + // ------------------------------------------- + // ROUTE api/files/:id/children + // ------------------------------------------- + if (path.link === 'children') { + const fileModel = ctx.models.file({ userId: ctx.owner.id }); + if (ctx.method === 'GET') { - return fileController.getChildren(headerSessionId(ctx.headers), path.id, requestPagination(ctx.query)); + const parent: File = await fileModel.entityFromItemId(path.id); + return fileModel.toApiOutput(await fileModel.childrens(parent.id, requestPagination(ctx.query))); } if (ctx.method === 'POST') { - return fileController.postChild(headerSessionId(ctx.headers), path.id, await bodyFields(ctx.req)); + const child: File = fileModel.fromApiInput(await bodyFields(ctx.req)); + const parent: File = await fileModel.entityFromItemId(path.id); + child.parent_id = parent.id; + return fileModel.toApiOutput(await fileModel.save(child)); } throw new ErrorMethodNotAllowed(); diff --git a/packages/server/src/utils/routeUtils.ts b/packages/server/src/utils/routeUtils.ts index 3ddb5c5a3..c16cd7443 100644 --- a/packages/server/src/utils/routeUtils.ts +++ b/packages/server/src/utils/routeUtils.ts @@ -153,13 +153,25 @@ export function routeResponseFormat(match: MatchedRoute, rawPath: string): Route return path.indexOf('api') === 0 || path.indexOf('/api') === 0 ? RouteResponseFormat.Json : RouteResponseFormat.Html; } +// In a path such as "/api/files/SOME_ID/content" we want to find: +// - The base path: "api/files" +// - The ID: "SOME_ID" +// - The link: "content" export function findMatchingRoute(path: string, routes: Routes): MatchedRoute { const splittedPath = path.split('/'); + + // Because the path starts with "/", we remove the first element, which is + // an empty string. So for example we now have ['api', 'files', 'SOME_ID', 'content']. splittedPath.splice(0, 1); if (splittedPath.length >= 2) { + // Create the base path, eg. "api/files", to match it to one of the + // routes.s const basePath = `${splittedPath[0]}/${splittedPath[1]}`; if (routes[basePath]) { + // Remove the base path from the array so that parseSubPath() can + // extract the ID and link from the URL. So the array will contain + // at this point: ['SOME_ID', 'content']. splittedPath.splice(0, 2); return { route: routes[basePath], diff --git a/packages/server/src/utils/testing/testUtils.ts b/packages/server/src/utils/testing/testUtils.ts index b375c577e..2cc1ed2b0 100644 --- a/packages/server/src/utils/testing/testUtils.ts +++ b/packages/server/src/utils/testing/testUtils.ts @@ -65,7 +65,7 @@ export async function beforeEachDb() { } interface AppContextTestOptions { - owner?: User; + // owner?: User; sessionId?: string; request?: any; } @@ -100,6 +100,8 @@ export async function koaAppContext(options: AppContextTestOptions = null): Prom const req = httpMocks.createRequest(reqOptions); req.__isMocked = true; + const owner = options.sessionId ? await models().session().sessionUser(options.sessionId) : null; + const appLogger = Logger.create('AppTest'); // Set type to "any" because the Koa context has many properties and we @@ -111,9 +113,8 @@ export async function koaAppContext(options: AppContextTestOptions = null): Prom appContext.models = models(); appContext.controllers = controllers(); appContext.appLogger = () => appLogger; - appContext.path = req.url; - appContext.owner = options.owner; + appContext.owner = owner; appContext.cookies = new FakeCookies(); appContext.request = new FakeRequest(req); appContext.response = new FakeResponse();