From 954d495d2834f76983a70034624a3d4f5749e97e Mon Sep 17 00:00:00 2001 From: Aura Date: Tue, 5 Nov 2024 19:01:53 +0100 Subject: [PATCH] fix(route): handle `index` case correctly (#657) --- packages/api/src/lib/structures/Route.ts | 24 +-- .../src/lib/structures/router/RouterRoot.ts | 14 +- packages/api/src/register.ts | 4 +- .../api/tests/lib/structures/Route.test.ts | 139 ++++++++++++++++++ 4 files changed, 164 insertions(+), 17 deletions(-) create mode 100644 packages/api/tests/lib/structures/Route.test.ts diff --git a/packages/api/src/lib/structures/Route.ts b/packages/api/src/lib/structures/Route.ts index 75485788..8bed4573 100644 --- a/packages/api/src/lib/structures/Route.ts +++ b/packages/api/src/lib/structures/Route.ts @@ -62,17 +62,23 @@ export abstract class Route exten super(context, options); const api = this.container.server.options; - const path = [ - ...RouterRoot.normalize(api.prefix), - ...RouterRoot.normalize(options.route ?? RouterRoot.makeRoutePathForPiece(this.location.directories, this.name)) - ]; + const path = RouterRoot.normalize(api.prefix); const methods = new Set(options.methods); - const implied = RouterRoot.extractMethod(path); - if (!isNullish(implied)) { - const lastIndex = path.length - 1; - path[lastIndex] = path[lastIndex].slice(0, path[lastIndex].length - implied.length - 1); - methods.add(implied); + if (options.route) { + // If a route is specified, no extra processing is made: + path.push(...RouterRoot.normalize(options.route)); + } else { + // If a route is not specified, extra processing is made to calculate + // one from the file system if it's possible: + let lastPart = context.name; + const implied = RouterRoot.extractMethod(lastPart); + if (!isNullish(implied)) { + lastPart = lastPart.slice(0, lastPart.length - implied.length - 1); + methods.add(implied); + } + + path.push(...RouterRoot.normalize(RouterRoot.makeRoutePathForPiece(this.location.directories, lastPart))); } this.path = path; diff --git a/packages/api/src/lib/structures/router/RouterRoot.ts b/packages/api/src/lib/structures/router/RouterRoot.ts index 2df5196a..d4d86ac4 100644 --- a/packages/api/src/lib/structures/router/RouterRoot.ts +++ b/packages/api/src/lib/structures/router/RouterRoot.ts @@ -81,14 +81,16 @@ export class RouterRoot extends RouterBranch { return parts; } - public static extractMethod(path: readonly string[]): MethodName | null { + public static extractMethod(path: string | readonly string[]): MethodName | null { if (path.length === 0) return null; + if (typeof path === 'string') { + const methodSeparatorPositionIndex = path.lastIndexOf('.'); + if (methodSeparatorPositionIndex === -1 || methodSeparatorPositionIndex === path.length - 1) return null; - const lastIndex = path.length - 1; - const last = path[lastIndex]; - const methodIndex = last.lastIndexOf('.'); - if (methodIndex === -1 || methodIndex === last.length - 1) return null; + return path.slice(methodSeparatorPositionIndex + 1).toUpperCase() as MethodName; + } - return last.slice(methodIndex + 1).toUpperCase() as MethodName; + const lastIndex = path.length - 1; + return RouterRoot.extractMethod(path[lastIndex]); } } diff --git a/packages/api/src/register.ts b/packages/api/src/register.ts index e62e4fa1..9c6865e0 100644 --- a/packages/api/src/register.ts +++ b/packages/api/src/register.ts @@ -9,7 +9,7 @@ export class Api extends Plugin { /** * @since 1.0.0 */ - public static [postInitialization](this: SapphireClient, options: ClientOptions): void { + public static override [postInitialization](this: SapphireClient, options: ClientOptions): void { this.server = new Server(options.api); this.stores .register(this.server.routes) // @@ -23,7 +23,7 @@ export class Api extends Plugin { /** * @since 1.0.0 */ - public static async [preLogin](this: SapphireClient): Promise { + public static override async [preLogin](this: SapphireClient): Promise { if (!(this.server.options.automaticallyConnect ?? true)) { return; } diff --git a/packages/api/tests/lib/structures/Route.test.ts b/packages/api/tests/lib/structures/Route.test.ts new file mode 100644 index 00000000..49c4fe9f --- /dev/null +++ b/packages/api/tests/lib/structures/Route.test.ts @@ -0,0 +1,139 @@ +import { container, VirtualPath } from '@sapphire/pieces'; +import { Route as BaseRoute, type Server, type ServerOptions } from '../../../src'; + +class Route extends BaseRoute { + public run(_request: BaseRoute.Request, _response: BaseRoute.Response) {} +} + +function useFakeServer(options: ServerOptions = {}): void { + const server = { options } as Partial; + container.server = server as Server; +} + +describe('Route', () => { + let previousValue: Server; + beforeEach(() => (previousValue = container.server)); + afterEach(() => (container.server = previousValue)); + + describe('virtual', () => { + test('GIVEN no options THEN generates an empty path with just the name', () => { + useFakeServer(); + const route = new Route({ name: 'route', root: VirtualPath, path: VirtualPath, store: null! }); + + expect(route.path).toEqual(['route']); + expect(route.methods).toEqual(new Set()); + expect(route.maximumBodyLength).toBe(1024 * 1024 * 50); + }); + + test('GIVEN route THEN generates the specified path', () => { + useFakeServer(); + const route = new Route({ name: 'route', root: VirtualPath, path: VirtualPath, store: null! }, { route: 'routes/test' }); + + expect(route.path).toEqual(['routes', 'test']); + expect(route.methods).toEqual(new Set()); + expect(route.maximumBodyLength).toBe(1024 * 1024 * 50); + }); + + test('GIVEN no options with prefix THEN generates an empty path with just the name', () => { + useFakeServer({ prefix: 'v1/' }); + const route = new Route({ name: 'route', root: VirtualPath, path: VirtualPath, store: null! }); + + expect(route.path).toEqual(['v1', 'route']); + expect(route.methods).toEqual(new Set()); + expect(route.maximumBodyLength).toBe(1024 * 1024 * 50); + }); + + test('GIVEN route with prefix THEN generates the specified path', () => { + useFakeServer({ prefix: 'v1/' }); + const route = new Route({ name: 'route', root: VirtualPath, path: VirtualPath, store: null! }, { route: 'routes/test' }); + + expect(route.path).toEqual(['v1', 'routes', 'test']); + expect(route.methods).toEqual(new Set()); + expect(route.maximumBodyLength).toBe(1024 * 1024 * 50); + }); + }); + + describe('non-virtual', () => { + test('GIVEN no options THEN generates a path', () => { + useFakeServer(); + const route = new Route({ name: 'route', root: '/usr/src/app/dist/routes', path: '/usr/src/app/dist/routes/index.ts', store: null! }); + + expect(route.path).toEqual(['route']); + expect(route.methods).toEqual(new Set()); + expect(route.maximumBodyLength).toBe(1024 * 1024 * 50); + }); + + test('GIVEN no options and an index file path with method THEN generates a path with method', () => { + useFakeServer(); + const route = new Route({ + name: 'index.get', + root: '/usr/src/app/dist/routes', + path: '/usr/src/app/dist/routes/index.get.ts', + store: null! + }); + + expect(route.path).toEqual([]); + expect(route.methods).toEqual(new Set(['GET'])); + expect(route.maximumBodyLength).toBe(1024 * 1024 * 50); + }); + + test('GIVEN no options and an index file path with a group and method THEN generates a path with method', () => { + useFakeServer(); + const route = new Route({ + name: 'index.get', + root: '/usr/src/app/dist/routes', + path: '/usr/src/app/dist/routes/(administrator)/index.get.ts', + store: null! + }); + + expect(route.path).toEqual([]); + expect(route.methods).toEqual(new Set(['GET'])); + expect(route.maximumBodyLength).toBe(1024 * 1024 * 50); + }); + + test('GIVEN no options and an index file path with a folder and method THEN generates a path with method', () => { + useFakeServer(); + const route = new Route({ + name: 'index.get', + root: '/usr/src/app/dist/routes', + path: '/usr/src/app/dist/routes/administrator/index.get.ts', + store: null! + }); + + expect(route.path).toEqual(['administrator']); + expect(route.methods).toEqual(new Set(['GET'])); + expect(route.maximumBodyLength).toBe(1024 * 1024 * 50); + }); + + test('GIVEN no options and a non-index file path with a folder and method THEN generates a path with method', () => { + useFakeServer(); + const route = new Route({ + name: 'create.post', + root: '/usr/src/app/dist/routes', + path: '/usr/src/app/dist/routes/administrator/create.post.ts', + store: null! + }); + + expect(route.path).toEqual(['administrator', 'create']); + expect(route.methods).toEqual(new Set(['POST'])); + expect(route.maximumBodyLength).toBe(1024 * 1024 * 50); + }); + + test('GIVEN name and a file path with method THEN generates a path with method', () => { + useFakeServer(); + const route = new Route( + { + name: 'index.get', + root: '/usr/src/app/dist/routes', + path: '/usr/src/app/dist/routes/index.get.ts', + store: null! + }, + { name: 'MyLittleMainRoute' } + ); + + expect(route.path).toEqual([]); + expect(route.methods).toEqual(new Set(['GET'])); + expect(route.maximumBodyLength).toBe(1024 * 1024 * 50); + }); + }); +});