diff --git a/lib/RouterSpool.ts b/lib/RouterSpool.ts index cef815e..9e95175 100644 --- a/lib/RouterSpool.ts +++ b/lib/RouterSpool.ts @@ -18,7 +18,7 @@ import * as pkg from '../package.json' * @see https://github.com/fabrix-app/spool-hapi */ export class RouterSpool extends SystemSpool { - private _routes = {} + private _routes: Map // = new Map constructor (app) { super(app, { @@ -63,7 +63,7 @@ export class RouterSpool extends SystemSpool { /** * Get's the routes from spool-router */ - get routes(): {[key: string]: any} { + get routes(): Map { return this._routes } diff --git a/lib/config/router.ts b/lib/config/router.ts index a8d4a04..d756620 100755 --- a/lib/config/router.ts +++ b/lib/config/router.ts @@ -1,4 +1,5 @@ export const router = { - prefix: '', - sortOrder: 'asc' + prefix: null, + sortOrder: 'asc', + debug: false } diff --git a/lib/interfaces/IRoute.ts b/lib/interfaces/IRoute.ts index 29c8de7..9b7999a 100644 --- a/lib/interfaces/IRoute.ts +++ b/lib/interfaces/IRoute.ts @@ -1,5 +1,6 @@ export interface IRoute { - [key: string]: { [key: string]: any }, + [key: string]: any, + _orgPath: string, config?: { pre?: any, [key: string]: { [key: string]: any } diff --git a/lib/schemas/router.ts b/lib/schemas/router.ts index 1f10115..2c8a14f 100644 --- a/lib/schemas/router.ts +++ b/lib/schemas/router.ts @@ -2,5 +2,6 @@ import * as joi from 'joi' export const routerSchema = joi.object().keys({ sortOrder: joi.string().allow('asc', 'desc').required(), - prefix: joi.string().allow('').required() + prefix: joi.string().allow('', null).required(), + debug: joi.any() }) diff --git a/lib/utils.ts b/lib/utils.ts index 5e3eed7..072930b 100755 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -1,5 +1,5 @@ import { FabrixApp } from '@fabrix/fabrix' -import { get } from 'lodash' +import { get, omit } from 'lodash' import { Router } from 'call' import { IRoute } from './interfaces/IRoute' @@ -20,35 +20,46 @@ export const Utils = { * Build a complete route, with bound handler and attached preconditions */ buildRoute (app: FabrixApp, path: string, rawRoute: IRoute) { - const route = Object.assign({ }, rawRoute) - route.config = route.config || (route.config = { }) - route.config.pre = route.config.pre || (route.config.pre = [ ]) + const orgRoute = Object.assign({ }, rawRoute) + orgRoute.config = orgRoute.config || (orgRoute.config = { }) + orgRoute.config.pre = orgRoute.config.pre || (orgRoute.config.pre = [ ]) - path = Utils.getPathFromRoute(app, path, route) - Utils.getHandlerFromString(app, route) + if (app.config.get('router.debug')) { + orgRoute._orgPath = path + } + + path = Utils.getPathFromRoute(app, path, orgRoute) + + if (app.config.get('router.debug')) { + orgRoute._newPath = path + } - route.config.pre = route.config.pre + Utils.getHandlerFromString(app, orgRoute) + + orgRoute.config.pre = orgRoute.config.pre .map(pre => Utils.getHandlerFromPrerequisite(app, pre)) .filter(handler => !!handler) - const routeHandlers = Object.keys(route).filter(value => -1 !== Utils.methods.indexOf(value)) + const orgRouteHandlers = Object.keys(orgRoute).filter(value => -1 !== Utils.methods.indexOf(value)) - if (!routeHandlers.some(v => Utils.methods.indexOf(v) >= 0 || !!route[v])) { - app.log.error('spool-router: route ', path, ' handler [', routeHandlers.join(', '), ']', + if (!orgRouteHandlers.some(v => Utils.methods.indexOf(v) >= 0 || !!orgRoute[v])) { + app.log.error('spool-orgRouter: orgRoute ', path, ' handler [', orgRouteHandlers.join(', '), ']', 'does not correspond to any defined Controller handler') return {} } - routeHandlers.forEach(method => { - if (route[method]) { - route[method].config = route[method].config || route.config - route[method].config.pre = route[method].config.pre || route.config.pre - route[method].config.pre = route[method].config.pre + orgRouteHandlers.forEach(method => { + if (orgRoute[method]) { + orgRoute[method].config = orgRoute[method].config || orgRoute.config + orgRoute[method].config.pre = orgRoute[method].config.pre || orgRoute.config.pre + orgRoute[method].config.pre = orgRoute[method].config.pre .map(pre => Utils.getHandlerFromPrerequisite(app, pre)) .filter(handler => !!handler) } }) + const route = omit(orgRoute, 'config') + return { path, route } }, @@ -180,10 +191,10 @@ export const Utils = { * Sort a route collection by object key */ sortRoutes(routes, order) { - const toReturn = {} + const toReturn = new Map const sorted = Object.keys(routes).sort(Utils.createSpecificityComparator({ order: order })) sorted.forEach((r, i) => { - toReturn[r] = routes[r] + toReturn.set(r, routes[r]) }) return toReturn }, @@ -212,19 +223,19 @@ export const Utils = { || routeA === catchAllRoute ) { return asc ? 1 : -1 - // Also push index route down to end, but not past the default } + // Also push index route down to end, but not past the default else if ( routeB === defaultRoute || routeB === catchAllRoute ) { return asc ? -1 : 1 - // Also push index route down to end, but not past the default } + // Also push index route down to end, but not past the default else if (/^\/$/.test(routeA) && (routeB !== defaultRoute && routeB !== catchAllRoute)) { return asc ? 1 : -1 - // Otherwise, sort based on either depth or free variable priority } + // Otherwise, sort based on either depth or free variable priority else { const slicedA = routeA.split('/') // .normalize('/' + routeA + '/').split('/').join('/') const slicedB = routeB.split('/') // .normalize('/' + routeB + '/').split('/').join('/') diff --git a/package-lock.json b/package-lock.json index ec88711..f2c6380 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@fabrix/spool-router", - "version": "1.1.1", + "version": "1.1.2", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -114,9 +114,9 @@ } }, "@fabrix/fabrix": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/@fabrix/fabrix/-/fabrix-1.1.0.tgz", - "integrity": "sha512-1r7TmtG24rX0Xemr6cWu0L7vUpUhINxyRaPwwc2bhts+NoIFR+5jJBSP1b1+dGsKV2GLYDQwIqmMgavNImVI8g==", + "version": "1.1.1", + "resolved": "https://registry.npmjs.org/@fabrix/fabrix/-/fabrix-1.1.1.tgz", + "integrity": "sha512-CL06baNKFPUB5dFKVCtwgZzKNeQeJyXVwaZhs0JPe7hL/7+LhAZ8zmh0ugcva1YuXecGASp3zXuTdGODGhgCBA==", "dev": true, "requires": { "lodash": "4.17.10", diff --git a/package.json b/package.json index e87abd8..44f8166 100755 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@fabrix/spool-router", - "version": "1.1.1", + "version": "1.1.2", "description": "Spool - Router for Fabrix", "scripts": { "build": "tsc -p ./lib/tsconfig.release.json", @@ -49,7 +49,7 @@ "lodash": "^4.17.10" }, "devDependencies": { - "@fabrix/fabrix": "^1.1.0", + "@fabrix/fabrix": "^1.1.1", "@fabrix/lint": "^1.0.0-alpha.3", "@types/lodash": "^4.14.109", "@types/node": "~10.3.4", @@ -64,7 +64,7 @@ "typescript": "~2.8.1" }, "peerDependencies": { - "@fabrix/fabrix": "^1.1.0" + "@fabrix/fabrix": "^1.1.1" }, "license": "MIT", "bugs": { diff --git a/test/fixtures/app.js b/test/fixtures/app.js index 6f44e72..9a1f7f7 100755 --- a/test/fixtures/app.js +++ b/test/fixtures/app.js @@ -34,6 +34,9 @@ module.exports = { customPrefixer: { prefix: '/prefix' }, + router: { + debug: true + }, routes: { '/test/foo': { 'GET': 'TestController.foo' diff --git a/test/integration/lib/util.test.js b/test/integration/lib/util.test.js index bc919b2..3d0bbfa 100755 --- a/test/integration/lib/util.test.js +++ b/test/integration/lib/util.test.js @@ -36,7 +36,15 @@ describe('lib.Util', () => { } }) - assert.equal(route.config.pre[0], global.app.policies.FooPolicy.bar) + assert.equal(route.GET.config.pre[0], global.app.policies.FooPolicy.bar) + assert.equal(route.HEAD.config.pre[0], global.app.policies.FooPolicy.bar) + assert.equal(route.POST.config.pre[0], global.app.policies.FooPolicy.bar) + assert.equal(route.PUT.config.pre[0], global.app.policies.FooPolicy.bar) + assert.equal(route.DELETE.config.pre[0], global.app.policies.FooPolicy.bar) + assert.equal(route.CONNECT.config.pre[0], global.app.policies.FooPolicy.bar) + assert.equal(route.OPTIONS.config.pre[0], global.app.policies.FooPolicy.bar) + assert.equal(route.TRACE.config.pre[0], global.app.policies.FooPolicy.bar) + assert.equal(route.PATCH.config.pre[0], global.app.policies.FooPolicy.bar) }) it('should resolve the prerequisite handler (object) to the correct policy method', () => { const { path, route} = lib.Utils.buildRoute(global.app, '/foo/bar', { @@ -49,8 +57,6 @@ describe('lib.Util', () => { ] } }) - - assert.equal(route.config.pre[0], global.app.policies.FooPolicy.bar) assert.equal(route.GET.config.pre[0], global.app.policies.FooPolicy.bar) }) it('should resolve the prerequisite handler (string) to the correct policy method', () => { diff --git a/test/integration/spool.test.js b/test/integration/spool.test.js index 864c6db..7f43836 100755 --- a/test/integration/spool.test.js +++ b/test/integration/spool.test.js @@ -15,11 +15,11 @@ describe('Router Spool', () => { // assert(_.isFunction(routes[2].handler)) // assert(_.isPlainObject(routes[3].handler)) - assert(global.app.routes['/test/foo1']) - assert(global.app.routes['/test/foo2']) - assert(global.app.routes['/prefix/test/custom/prefix']) + assert(global.app.routes.get('/test/foo1')) + assert(global.app.routes.get('/test/foo2')) + assert(global.app.routes.get('/prefix/test/custom/prefix')) - assert.equal(Object.keys(global.app.routes).length, 9) + assert.equal(global.app.routes.size, 9) }) }) @@ -27,10 +27,10 @@ describe('Router Spool', () => { it('tags could be an array', () => { const routes = global.app.routes - const route = routes['/test/foo/tags'] - assert(_.isObject(route.config)) - assert(_.isArray(route.config.tags)) - assert(_.includes(route.config.tags, 'test', 'other')) + const route = routes.get('/test/foo/tags') + assert(_.isObject(route.GET.config)) + assert(_.isArray(route.GET.config.tags)) + assert(_.includes(route.GET.config.tags, 'test', 'other')) }) }) }) diff --git a/test/unit/lib/routeSortOrder.test.js b/test/unit/lib/routeSortOrder.test.js index ef82680..2ddd93b 100644 --- a/test/unit/lib/routeSortOrder.test.js +++ b/test/unit/lib/routeSortOrder.test.js @@ -7,8 +7,9 @@ const sort = require('../../../dist/utils').Utils.sortRoutes describe('Utils Route Sort Order', () => { it('should exist', () => { assert(routeOrder) + assert(sort) }) - it('should sort the routes for free variables', () => { + it('should sort the routes for free variables (express style)', () => { let routes = { '/a': {}, '/a/:id': {}, @@ -24,9 +25,11 @@ describe('Utils Route Sort Order', () => { '/b/*': {} } - routes = sort(routes, {order: 'asc'}) - - assert.deepEqual(routes, { + routes = sort(routes, 'asc') + assert(routes) + console.log(routes) + const ordered = new Map() + const order = { '/a': {}, '/a/:id': {}, '/a/*': {}, @@ -38,11 +41,15 @@ describe('Utils Route Sort Order', () => { '/b/:id/:world': {}, '/b/:id/*': {}, '/': {}, - '*': {} + '*': {}, + } + Object.keys(order).forEach(k => { + ordered.set(k, order[k]) }) + assert.deepEqual(routes, ordered) }) - it('should sort the routes for free variables', () => { + it('should sort the routes for free variables (hapi style)', () => { let routes = { '/a': {}, '/a/{id}': {}, @@ -59,8 +66,49 @@ describe('Utils Route Sort Order', () => { } routes = sort(routes, 'asc') + assert(routes) + + const ordered = new Map() + const order = { + '/a': {}, + '/a/{id}': {}, + '/a/*': {}, + '/a/{id}/{world}': {}, + '/a/{id}/*': {}, + '/b': {}, + '/b/{id}': {}, + '/b/*': {}, + '/b/{id}/{world}': {}, + '/b/{id}/*': {}, + '/': {}, + '*': {}, + } + Object.keys(order).forEach(k => { + ordered.set(k, order[k]) + }) + assert.deepEqual(routes, ordered) + }) + + it('should sort the routes for free variables desc', () => { + let routes = { + '/a': {}, + '/a/{id}': {}, + '/a/*': {}, + '/b': {}, + '/a/{id}/{world}': {}, + '/a/{id}/*': {}, + '*': {}, + '/b/{id}/{world}': {}, + '/': {}, + '/b/{id}/*': {}, + '/b/{id}': {}, + '/b/*': {}, + } - assert.deepEqual(routes, { + routes = sort(routes, 'desc') + assert(routes) + const ordered = new Map() + const order = { '/a': {}, '/a/{id}': {}, '/a/*': {}, @@ -73,6 +121,78 @@ describe('Utils Route Sort Order', () => { '/b/{id}/*': {}, '/': {}, '*': {}, + } + Object.keys(order).forEach(k => { + ordered.set(k, order[k]) }) + assert.deepEqual(routes, ordered) + }) + + it('should sort the routes for free variables asc', () => { + let routes = { + '/a': {}, + '/a/{id}': {}, + '/a/*': {}, + '/b': {}, + '/a/{id}/{world}': {}, + '/a/{id}/*': {}, + '*': {}, + '/b/{id}/{world}': {}, + '/': {}, + '/b/{id}/*': {}, + '/b/{id}': {}, + '/b/*': {}, + } + + const order = Object.keys(routes).sort(routeOrder({order: 'asc'})) + assert.deepEqual(order, [ + '/a', + '/a/{id}', + '/a/*', + '/a/{id}/{world}', + '/a/{id}/*', + '/b', + '/b/{id}', + '/b/*', + '/b/{id}/{world}', + '/b/{id}/*', + '/', + '*', + ]) + }) + + // TODO + it.skip('should sort the routes for free variables desc', () => { + let routes = { + '/a': {}, + '/a/{id}': {}, + '/a/*': {}, + '/b': {}, + '/a/{id}/{world}': {}, + '/a/{id}/*': {}, + '*': {}, + '/b/{id}/{world}': {}, + '/': {}, + '/b/{id}/*': {}, + '/b/{id}': {}, + '/b/*': {}, + } + + const order = Object.keys(routes).sort(routeOrder({order: 'desc'})) + console.log('BROKE', order) + assert.deepEqual(order, [ + '*', + '/', + '/b/{id}/*', + '/b/{id}/{world}', + '/b/*', + '/b/{id}', + '/b', + '/a/{id}/*', + '/a/{id}/{world}', + '/a/*', + '/a/{id}', + '/a' + ]) }) }) diff --git a/test/unit/lib/util.test.js b/test/unit/lib/util.test.js index 9fbdf97..acb654c 100755 --- a/test/unit/lib/util.test.js +++ b/test/unit/lib/util.test.js @@ -1,6 +1,7 @@ const assert = require('assert') const lib = require('../../../dist') + describe('lib.utils', () => { describe('#buildRoutes errors', () => { it('should log an error if there is no handler and return undefined', () => { @@ -82,6 +83,5 @@ describe('lib.utils', () => { assert.equal(path, '/prefix/a') }) }) - })