From 33de2b8cab3dad5470bb8140d0d2e31bd766f922 Mon Sep 17 00:00:00 2001 From: scott-wyatt Date: Thu, 19 Jul 2018 16:09:52 -0400 Subject: [PATCH] [chore] update router --- lib/config/index.ts | 5 +- lib/config/router.ts | 3 + lib/server.ts | 4 +- package-lock.json | 20 +-- package.json | 12 +- .../api/controllers/DefaultController.js | 8 ++ test/fixtures/app.js | 10 ++ test/integration/controller.test.js | 35 +++++ test/unit/utils/routeSortOrder.test.js | 135 +++++++++++++++++- 9 files changed, 205 insertions(+), 27 deletions(-) create mode 100644 lib/config/router.ts diff --git a/lib/config/index.ts b/lib/config/index.ts index acebad6..0b2b2df 100755 --- a/lib/config/index.ts +++ b/lib/config/index.ts @@ -1,9 +1,10 @@ import { spool } from './spool' import { web } from './web' import { session } from './session' - +import { router } from './router' export { spool, web, - session + session, + router } diff --git a/lib/config/router.ts b/lib/config/router.ts new file mode 100644 index 0000000..451f643 --- /dev/null +++ b/lib/config/router.ts @@ -0,0 +1,3 @@ +export const router = { + sortOrder: 'asc' +} diff --git a/lib/server.ts b/lib/server.ts index fb7e208..fb95286 100755 --- a/lib/server.ts +++ b/lib/server.ts @@ -259,8 +259,8 @@ export const Server = { }) } - Object.keys(routes).forEach(r => { - const route = routes[r] + routes.forEach((route, r) => { + RouterUtils.methods.forEach(m => { if (route[m]) { this.serverRoutes[m.toLowerCase() + ' ' + r] = { diff --git a/package-lock.json b/package-lock.json index bac1811..b00f324 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,6 +1,6 @@ { "name": "@fabrix/spool-express", - "version": "1.1.2", + "version": "1.1.3", "lockfileVersion": 1, "requires": true, "dependencies": { @@ -125,9 +125,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", @@ -151,9 +151,9 @@ } }, "@fabrix/spool-router": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/@fabrix/spool-router/-/spool-router-1.1.1.tgz", - "integrity": "sha512-hahiq1BoSQBdsPtReoJONF/Qo0pJ6Znzi6foKz+mXofe9/Fp/EhuzguO+Gx+1rvWNssIEJZC9+8JhitSH1bTxA==", + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/@fabrix/spool-router/-/spool-router-1.1.2.tgz", + "integrity": "sha512-a+EF5G8NCq9T5nP1Hn9nNzZWC306IEBA7Sh3Nt1DoziVCwOcd6lRMrvY7FdfLIi+I/TPqn5UC22GSQgd99N1Hw==", "dev": true, "requires": { "call": "5.0.1", @@ -174,9 +174,9 @@ } }, "@fabrix/spool-tapestries": { - "version": "1.1.0", - "resolved": "https://registry.npmjs.org/@fabrix/spool-tapestries/-/spool-tapestries-1.1.0.tgz", - "integrity": "sha512-Y8dxJpm+52yHz64tHo3Fh+VUQIEPvL4V6cHtMr7cMzlcUs7yUR6L0v5xfXMdROPDZGGZFosCog34mH5F/W2K5g==", + "version": "1.1.2", + "resolved": "https://registry.npmjs.org/@fabrix/spool-tapestries/-/spool-tapestries-1.1.2.tgz", + "integrity": "sha512-PiMPrCoq4kQOWEPWQcZbzYmhgnP4GRdC4fy8ypL5DKeDA2KZggP7I7CiP5Qw7dkaCUhbJp5e/c+0JLJCChrrGA==", "dev": true, "requires": { "lodash": "4.17.10" diff --git a/package.json b/package.json index fd75b88..bee2caa 100755 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@fabrix/spool-express", - "version": "1.1.2", + "version": "1.1.3", "description": "Spool Express - Binds the routes compiled in spool-router to a Express 4 Server.", "homepage": "https://fabrix.app", "author": "Fabrix-app Team ", @@ -56,12 +56,12 @@ "methods": "^1.1.2" }, "devDependencies": { - "@fabrix/fabrix": "^1.1.0", + "@fabrix/fabrix": "^1.1.1", "@fabrix/lint": "^1.0.0-alpha.3", "@fabrix/spool-i18n": "^1.1.0", - "@fabrix/spool-router": "^1.1.1", + "@fabrix/spool-router": "^1.1.2", "@fabrix/spool-sequelize": "^1.1.0", - "@fabrix/spool-tapestries": "^1.1.0", + "@fabrix/spool-tapestries": "^1.1.2", "@types/body-parser": "^1.17.0", "@types/compression": "0.0.36", "@types/cookie-parser": "^1.4.1", @@ -91,8 +91,8 @@ "typescript": "~2.8.1" }, "peerDependencies": { - "@fabrix/fabrix": "^1.1.0", - "@fabrix/spool-router": "^1.1.1", + "@fabrix/fabrix": "^1.1.1", + "@fabrix/spool-router": "^1.1.2", "@fabrix/spool-i18n": "^1.1.0" }, "engines": { diff --git a/test/fixtures/api/controllers/DefaultController.js b/test/fixtures/api/controllers/DefaultController.js index ac0ed19..37b3f77 100755 --- a/test/fixtures/api/controllers/DefaultController.js +++ b/test/fixtures/api/controllers/DefaultController.js @@ -39,4 +39,12 @@ module.exports = class DefaultController extends Controller { const where = req.jsonCriteria(req.query.where) res.json(where) } + worldController(req, res) { + const where = req.jsonCriteria(req.query.where) + res.json({world: where}) + } + planetController(req, res) { + const where = req.jsonCriteria(req.query.where) + res.json({[req.params.planet]: where}) + } } diff --git a/test/fixtures/app.js b/test/fixtures/app.js index d97ea2e..ff179c7 100755 --- a/test/fixtures/app.js +++ b/test/fixtures/app.js @@ -240,6 +240,16 @@ const App = { }, '/jsonCriteria': { 'GET': 'DefaultController.jsonCriteria' + }, + // Routes that potentially could get out of order from the router. + '/test/earth': { + 'GET': 'DefaultController.worldController' + }, + '/test/:planet': { + 'GET': 'DefaultController.planetController' + }, + '/test/world': { + 'GET': 'DefaultController.worldController' } }, web: { diff --git a/test/integration/controller.test.js b/test/integration/controller.test.js index 0f664bb..2c09bc2 100755 --- a/test/integration/controller.test.js +++ b/test/integration/controller.test.js @@ -81,6 +81,41 @@ describe('express controllers', () => { done(err) }) }) + + it('should test order', (done) => { + request + .get('/test/world') + .query({ where: {hello: 'world'}}) + .expect(200) + .end((err, res) => { + assert.ok(res.body) + assert.deepEqual(res.body, { world: { hello: 'world' } }) + done(err) + }) + }) + // TODO fix spool-router and come back to this + it.skip('should test order', (done) => { + request + .get('/test/earth') + .query({ where: {hello: 'world'}}) + .expect(200) + .end((err, res) => { + assert.ok(res.body) + assert.deepEqual(res.body, { world: { hello: 'world' } }) + done(err) + }) + }) + it('should test order', (done) => { + request + .get('/test/mars') + .query({ where: {hello: 'mars'}}) + .expect(200) + .end((err, res) => { + assert.ok(res.body) + assert.deepEqual(res.body, {mars: { hello: 'mars' } }) + done(err) + }) + }) }) describe('ViewController', () => { describe('helloWorld', () => { diff --git a/test/unit/utils/routeSortOrder.test.js b/test/unit/utils/routeSortOrder.test.js index e9c4075..7be58e2 100644 --- a/test/unit/utils/routeSortOrder.test.js +++ b/test/unit/utils/routeSortOrder.test.js @@ -1,13 +1,15 @@ 'use strict' /* global describe, it */ const assert = require('assert') +const routeOrder = require('@fabrix/spool-router').Utils.createSpecificityComparator const sort = require('@fabrix/spool-router').Utils.sortRoutes describe('Utils Route Sort Order', () => { it('should exist', () => { + assert(routeOrder) assert(sort) }) - it('should sort the routes for express', () => { + it('should sort the routes for free variables (express style)', () => { let routes = { '/a': {}, '/a/:id': {}, @@ -23,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/*': {}, @@ -37,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}': {}, @@ -58,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/*': {}, @@ -72,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' + ]) }) })