From 6d180651b4eb40289ecea3df3575f207aa6c5d1f Mon Sep 17 00:00:00 2001 From: David Luecke Date: Mon, 4 Jan 2021 20:17:30 -0800 Subject: [PATCH] feat(transport-commons): New built-in high performance radix router (#2177) --- packages/socketio-client/test/index.test.ts | 4 +- packages/transport-commons/package.json | 3 +- packages/transport-commons/src/routing.ts | 42 --------- .../transport-commons/src/routing/index.ts | 45 ++++++++++ .../transport-commons/src/routing/router.ts | 87 +++++++++++++++++++ .../index.test.ts} | 15 ++-- .../test/routing/router.test.ts | 78 +++++++++++++++++ 7 files changed, 220 insertions(+), 54 deletions(-) delete mode 100644 packages/transport-commons/src/routing.ts create mode 100644 packages/transport-commons/src/routing/index.ts create mode 100644 packages/transport-commons/src/routing/router.ts rename packages/transport-commons/test/{routing.test.ts => routing/index.test.ts} (82%) create mode 100644 packages/transport-commons/test/routing/router.test.ts diff --git a/packages/socketio-client/test/index.test.ts b/packages/socketio-client/test/index.test.ts index 365143c066..da3f861c15 100644 --- a/packages/socketio-client/test/index.test.ts +++ b/packages/socketio-client/test/index.test.ts @@ -69,10 +69,10 @@ describe('@feathersjs/socketio-client', () => { it('return 404 for non-existent service', async () => { try { - await app.service('not-me').create({}); + await app.service('not/me').create({}); assert.fail('Should never get here'); } catch(e) { - assert.strictEqual(e.message, 'Service \'not-me\' not found') + assert.strictEqual(e.message, 'Service \'not/me\' not found') } }); diff --git a/packages/transport-commons/package.json b/packages/transport-commons/package.json index 19f9e05fbb..6c7dc21655 100644 --- a/packages/transport-commons/package.json +++ b/packages/transport-commons/package.json @@ -55,8 +55,7 @@ "@feathersjs/commons": "^5.0.0-pre.1", "@feathersjs/errors": "^5.0.0-pre.1", "debug": "^4.3.1", - "lodash": "^4.17.20", - "radix-router": "^3.0.1" + "lodash": "^4.17.20" }, "devDependencies": { "@feathersjs/feathers": "^5.0.0-pre.1", diff --git a/packages/transport-commons/src/routing.ts b/packages/transport-commons/src/routing.ts deleted file mode 100644 index 6c9696e36e..0000000000 --- a/packages/transport-commons/src/routing.ts +++ /dev/null @@ -1,42 +0,0 @@ -// @ts-ignore -import Router from 'radix-router'; -import { stripSlashes } from '@feathersjs/commons'; -import { Application } from '@feathersjs/feathers'; - -export const ROUTER = Symbol('@feathersjs/transport-commons/router'); - -declare module '@feathersjs/feathers/lib/declarations' { - interface Application { // eslint-disable-line - lookup (path: string): { [key: string]: string }; - } -} - -export const routing = () => (app: Application) => { - if (typeof app.lookup === 'function') { - return; - } - - const router = new Router(); - - Object.assign(app, { - [ROUTER]: router, - lookup (path: string): { [key: string]: string } { - if (!path) { - return null; - } - - return this[ROUTER].lookup(stripSlashes('' + path) || '/'); - } - }); - - // Add a mixin that registers a service on the router - app.mixins.push((service, path) => { - // @ts-ignore - app[ROUTER].insert({ path, service }); - // @ts-ignore - app[ROUTER].insert({ - path: `${path}/:__id`, - service - }); - }); -}; diff --git a/packages/transport-commons/src/routing/index.ts b/packages/transport-commons/src/routing/index.ts new file mode 100644 index 0000000000..3b9a35562a --- /dev/null +++ b/packages/transport-commons/src/routing/index.ts @@ -0,0 +1,45 @@ +import { Application, Service } from '@feathersjs/feathers'; +import { Router } from './router'; + +declare module '@feathersjs/feathers/lib/declarations' { + interface RouteLookup { + service: Service, + params: { [key: string]: string } + } + + interface Application { // eslint-disable-line + routes: Router; + lookup (path: string): RouteLookup; + } +} + +export * from './router'; + +export const routing = () => (app: Application) => { + if (typeof app.lookup === 'function') { + return; + } + + const routes = new Router(); + + Object.assign(app, { + routes, + lookup (this: Application, path: string) { + const result = this.routes.lookup(path); + + if (result !== null) { + const { params, data: service } = result; + + return { params, service }; + } + + return result; + } + }); + + // Add a mixin that registers a service on the router + app.mixins.push((service: Service, path: string) => { + app.routes.insert(path, service); + app.routes.insert(`${path}/:__id`, service); + }); +}; diff --git a/packages/transport-commons/src/routing/router.ts b/packages/transport-commons/src/routing/router.ts new file mode 100644 index 0000000000..074a11775b --- /dev/null +++ b/packages/transport-commons/src/routing/router.ts @@ -0,0 +1,87 @@ +import { stripSlashes } from '@feathersjs/commons'; +import { BadRequest } from '@feathersjs/errors'; + +export interface LookupData { + params: { [key: string]: string }; +} + +export interface LookupResult extends LookupData { + data?: T; +} + +export class RouteNode { + data?: T; + children: { [key: string]: RouteNode } = {}; + placeholder?: RouteNode; + + constructor (public name: string) {} + + insert (path: string[], data: T): RouteNode { + if (path.length === 0) { + this.data = data; + return this; + } + + const [ current, ...rest ] = path; + + if (current.startsWith(':')) { + const { placeholder } = this; + const name = current.substring(1); + + if (!placeholder) { + this.placeholder = new RouteNode(name); + } else if(placeholder.name !== name) { + throw new BadRequest(`Can not add route with placeholder ':${name}' because placeholder ':${placeholder.name}' already exists`); + } + + return this.placeholder.insert(rest, data); + } + + this.children[current] = this.children[current] || new RouteNode(current); + + return this.children[current].insert(rest, data); + } + + lookup (path: string[], info: LookupData): LookupResult|null { + if (path.length === 0) { + return { + ...info, + data: this.data + } + } + + const [ current, ...rest ] = path; + const child = this.children[current]; + + if (child) { + return child.lookup(rest, info); + } + + if (this.placeholder) { + info.params[this.placeholder.name] = current; + return this.placeholder.lookup(rest, info); + } + + return null; + } +} + +export class Router { + root: RouteNode = new RouteNode(''); + + getPath (path: string) { + return stripSlashes(path).split('/'); + } + + insert (path: string, data: T) { + return this.root.insert(this.getPath(path), data); + } + + lookup (path: string) { + if (typeof path !== 'string') { + return null; + } + + return this.root.lookup(this.getPath(path), { params: {} }); + } +} diff --git a/packages/transport-commons/test/routing.test.ts b/packages/transport-commons/test/routing/index.test.ts similarity index 82% rename from packages/transport-commons/test/routing.test.ts rename to packages/transport-commons/test/routing/index.test.ts index e43666de16..d4e4d6aceb 100644 --- a/packages/transport-commons/test/routing.test.ts +++ b/packages/transport-commons/test/routing/index.test.ts @@ -1,8 +1,8 @@ import assert from 'assert'; import feathers, { Application } from '@feathersjs/feathers'; -import { routing, ROUTER } from '../src/routing'; +import { routing } from '../../src/routing'; -describe('app.router', () => { +describe('app.routes', () => { let app: Application; beforeEach(() => { @@ -19,10 +19,9 @@ describe('app.router', () => { feathers().configure(routing()).configure(routing()); }); - it('has app.lookup and ROUTER symbol', () => { + it('has app.lookup and app.routes', () => { assert.strictEqual(typeof app.lookup, 'function'); - // @ts-ignore - assert.ok(app[ROUTER]); + assert.ok(app.routes); }); it('returns null when nothing is found', () => { @@ -40,7 +39,7 @@ describe('app.router', () => { it('can look up and strips slashes', () => { const result = app.lookup('my/service'); - assert.strictEqual(result.service, app.service('/my/service')); + assert.strictEqual(result.service, app.service('/my/service/')); }); it('can look up with id', () => { @@ -56,8 +55,8 @@ describe('app.router', () => { const path = '/test/:first/my/:second'; app.use(path, { - get (id: string|number) { - return Promise.resolve({ id }); + async get (id: string|number) { + return { id }; } }); diff --git a/packages/transport-commons/test/routing/router.test.ts b/packages/transport-commons/test/routing/router.test.ts new file mode 100644 index 0000000000..48bdb81ea5 --- /dev/null +++ b/packages/transport-commons/test/routing/router.test.ts @@ -0,0 +1,78 @@ +import assert from 'assert'; +import { Router } from '../../src/routing'; + +describe('router', () => { + it('can lookup and insert a simple path and returns null for invalid path', () => { + const r = new Router(); + + r.insert('/hello/there/you', 'test'); + + const result = r.lookup('hello/there/you/'); + + assert.deepStrictEqual(result, { + params: {}, + data: 'test' + }); + + assert.strictEqual(r.lookup('not/there'), null); + assert.strictEqual(r.lookup('not-me'), null); + }); + + it('can insert data at the root', () => { + const r = new Router(); + + r.insert('', 'hi'); + + const result = r.lookup('/'); + + assert.deepStrictEqual(result, { + params: {}, + data: 'hi' + }); + }); + + it('can insert with placeholder and has proper specificity', () => { + const r = new Router(); + + r.insert('/hello/:id', 'one'); + r.insert('/hello/:id/you', 'two'); + r.insert('/hello/:id/:other', 'three'); + + const first = r.lookup('hello/there/'); + + assert.deepStrictEqual(first, { + params: { id: 'there' }, + data: 'one' + }); + + const second = r.lookup('hello/yes/you'); + + assert.deepStrictEqual(second, { + params: { id: 'yes' }, + data: 'two' + }); + + const third = r.lookup('hello/yes/they'); + + assert.deepStrictEqual(third, { + params: { + id: 'yes', + other: 'they' + }, + data: 'three' + }); + + assert.strictEqual(r.lookup('hello/yes/they/here'), null); + }); + + it('errors when placeholder in a path is different', () => { + const r = new Router(); + + assert.throws(() => { + r.insert('/hello/:id', 'one'); + r.insert('/hello/:test/you', 'two'); + }, { + message: 'Can not add route with placeholder \':test\' because placeholder \':id\' already exists' + }); + }); +});