From 200b613a8da305ee54b75fdea35ff95010239e4d Mon Sep 17 00:00:00 2001 From: Gunjan Pandya Date: Thu, 4 Feb 2016 02:05:38 -0500 Subject: [PATCH 01/13] Implement rest router from POC. - Implement trie data structure - Implement rest router and integrate in rest-adapter.js, in harmony with express router - Include test cases --- lib/rest-adapter.js | 24 ++++++++++++-- lib/rest-router.js | 37 +++++++++++++++++++++ lib/trie.js | 66 ++++++++++++++++++++++++++++++++++++++ test/rest-router.test.js | 60 ++++++++++++++++++++++++++++++++++ test/trie.test.js | 69 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 254 insertions(+), 2 deletions(-) create mode 100644 lib/rest-router.js create mode 100644 lib/trie.js create mode 100644 test/rest-router.test.js create mode 100644 test/trie.test.js diff --git a/lib/rest-adapter.js b/lib/rest-adapter.js index 188af27..337dc3e 100644 --- a/lib/rest-adapter.js +++ b/lib/rest-adapter.js @@ -28,6 +28,7 @@ var async = require('async'); var HttpInvocation = require('./http-invocation'); var ContextBase = require('./context-base'); var HttpContext = require('./http-context'); +var RestRouter = require('./rest-router'); var json = bodyParser.json; var urlencoded = bodyParser.urlencoded; @@ -263,6 +264,10 @@ RestAdapter.prototype.createHandler = function() { var handleUnknownPaths = this._shouldHandleUnknownPaths(); + // Mount rest-router + var customRouter = new RestRouter(); + root.use(customRouter.handle()); + classes.forEach(function(restClass) { var router = express.Router(); var className = restClass.sharedClass.name; @@ -277,7 +282,13 @@ RestAdapter.prototype.createHandler = function() { var sharedMethod = restMethod.sharedMethod; debug(' method %s', sharedMethod.stringName); restMethod.routes.forEach(function(route) { +<<<<<<< HEAD methods.push({ route: route, method: sharedMethod }); +======= + //add fullPath for Trie insertion + route.fullPath = joinPaths(restClass.getPath(), route.path); + methods.push({route: route, method: sharedMethod}); +>>>>>>> cd24be5... Implement rest router from POC. }); }); @@ -285,7 +296,7 @@ RestAdapter.prototype.createHandler = function() { methods.sort(sortRoutes); methods.forEach(function(m) { - adapter._registerMethodRouteHandlers(router, m.method, m.route); + adapter._registerMethodRouteHandlers(router, m.method, m.route, customRouter); }); if (handleUnknownPaths) { @@ -414,7 +425,8 @@ RestAdapter.errorHandler = function(options) { RestAdapter.prototype._registerMethodRouteHandlers = function(router, sharedMethod, - route) { + route, + customRouter) { var handler = sharedMethod.isStatic ? this._createStaticMethodHandler(sharedMethod) : this._createPrototypeMethodHandler(sharedMethod); @@ -425,6 +437,14 @@ RestAdapter.prototype._registerMethodRouteHandlers = function(router, // Express 4.x only supports delete verb = 'delete'; } + + //For now: exclude paths with parameters like :id + //and handler functions with err obj i.e. (err, req,res,next) + if (!route.path.includes(':') && handler.length < 4) { + handler.methodName = sharedMethod.name; + handler.path = route.fullPath; + customRouter.registerPathAndHandlers({ verb: verb, fullPath: route.fullPath }, handler); + } router[verb](route.path, handler); }; diff --git a/lib/rest-router.js b/lib/rest-router.js new file mode 100644 index 0000000..2848a6e --- /dev/null +++ b/lib/rest-router.js @@ -0,0 +1,37 @@ +module.exports = RestRouter; + +var url = require('url'); +var Trie = require('./trie'); + +function RestRouter(options) { + this.trie = new Trie(); + this.options = options; +} + +//register path and related handler to Trie ds +RestRouter.prototype.registerPathAndHandlers = function(route, handler) { + var path = normalizePath(route.fullPath); + var trie = this.trie; + trie = trie.add(path, route.verb, handler); +}; + +//handle request, match path and if found, invoke handler method +RestRouter.prototype.handle = function() { + var trie = this.trie; + return function(req, res, next) { + var verb = req.method.toLowerCase() || 'all'; + var path = normalizePath(req.url, this.options); + var methods = trie.find(path); + if (methods && verb in methods) { + methods[verb](req, res, next); + } else next(); + }; +}; + +function normalizePath(path, options) { + path = url.parse(path).pathname; + if (!options || !(options.caseSensitive)) { + path = path.toLowerCase(); + } + return path; +} diff --git a/lib/trie.js b/lib/trie.js new file mode 100644 index 0000000..fbf4248 --- /dev/null +++ b/lib/trie.js @@ -0,0 +1,66 @@ +module.exports = Trie; + +function Trie() { + this.methods = {}; +} + +//insert new nodes to Trie +Trie.prototype.add = function(path, verb, handler) { + var node = this; + var parts = getPathParts(path); + if (parts.length) { + for (var i = 0; i < parts.length; i++) { + if (!(parts[i] in node)) { + node[parts[i]] = new Trie(); + } + if (i + 1 === parts.length) { + addVerbHandler(node[parts[i]], verb, handler); + } + node = node[parts[i]]; + } + } else { // if we would ever encounter this situation? + addVerbHandler(node, verb, handler); + } + return this; +}; + +//Look for matching path and return handler method for a match +Trie.prototype.find = function(path) { + var node = this; + var parts = getPathParts(path); + for (var i = 0; i < parts.length; i++) { + if (!(parts[i] in node)) { + return false; + } else { + if (i + 1 === parts.length) + return node[parts[i]].methods; + } + node = node[parts[i]]; + } +}; + +function getPathParts(path) { + path = path.trim(); + var chunks = path.split('/'); + var parts = []; + chunks.forEach(function(chunk) { + if (chunk) { + parts.push(chunk); + } + }); + return parts; +} + +function addVerbHandler(node, verb, handler) { + node.methods = node.methods || {}; + if (node.methods[verb]) { + console.warn( + 'WARN: A handler was already registered for %s /api%s : Ignoring the new handler-%s', + verb.toUpperCase(), + handler.path || '/[unknown]', + handler.methodName || '' + ); + return; + } + node.methods[verb] = handler; +} diff --git a/test/rest-router.test.js b/test/rest-router.test.js new file mode 100644 index 0000000..8972c37 --- /dev/null +++ b/test/rest-router.test.js @@ -0,0 +1,60 @@ +var assert = require('assert'); +var expect = require('chai').expect; +var RestRouter = require('../lib/rest-router'); + +describe('Custom Router', function() { + var restRouter; + var route = { verb: 'get', fullPath: '/Planets' }; + var handler = function(req, res, next) { res.handled = true; }; + var req = { + method: 'get', + url: '/Planets', + }; + var res = {}; + var next = function() { res.calledWhenPathNotFound = true; }; + + beforeEach(function() { + restRouter = new RestRouter(); + }); + + describe('RestRouter()', function() { + + it('initiates a restRouter obj with Trie and options', function() { + expect(restRouter).to.have.keys(['trie', 'options']); + }); + }); + + describe('restRouter.registerPathAndHandlers()', function() { + it('register handler for GET: /Planets', function() { + restRouter.registerPathAndHandlers(route, handler); + expect(restRouter.trie).to.have.deep.property('planets.methods.get'); + }); + }); + + describe('restRouter.handle()', function() { + it('returns a function --which invokes matching handler-- with three arguments', + function() { + restRouter.registerPathAndHandlers(route, handler); + var returned = restRouter.handle(); + expect(returned).to.have.length(3); + }); + + it('invokes the handler function for matching verb+path', + function() { + restRouter.registerPathAndHandlers(route, handler); + restRouter.handle()(req, res, next); + expect(res.handled).to.be.true; + }); + + it('invokes supplied next() if no matching handler found for verb+path', + function() { + restRouter.registerPathAndHandlers(route, handler); + req.method = 'post'; + res = {}; + restRouter.handle()(req, res, next); + expect(res.calledWhenPathNotFound).to.be.true; + expect(res.handled).to.be.undefined; + }); + }); + +}); diff --git a/test/trie.test.js b/test/trie.test.js new file mode 100644 index 0000000..bfed45e --- /dev/null +++ b/test/trie.test.js @@ -0,0 +1,69 @@ +var assert = require('assert'); +var expect = require('chai').expect; +var Trie = require('../lib/trie'); + +describe('Trie', function() { + var trie; + + describe('trie.add()', function() { + + beforeEach(function() { + trie = new Trie(); + }); + + it('has empty root node with only methods key', function() { + expect(trie).to.have.keys('methods'); + }); + + it('create a node for a path with single part, no verb-handler', function() { + trie.add('/Planets'); + expect(trie).to.have.property('Planets'); + expect(trie.Planets).to.have.property('methods'); + }); + + it('registers handler for path: /Planets', function() { + trie.add('/Planets', 'get', 'handler()'); + expect(trie).to.have.deep.property('Planets.methods.get', 'handler()'); + }); + + it('registers handler for path: /Planets/count', function() { + trie.add('/Planets/count', 'get', 'count()'); + expect(trie).to.have.deep.property('Planets.count.methods.get', 'count()'); + }); + + it('registers different HTTP verb handlers for the same path ', function() { + trie.add('/Planets', 'get', 'getHandler()'); + trie.add('/Planets', 'post', 'postHandler()'); + trie.add('/Planets', 'delete', 'deleteHandler()'); + + expect(trie).to.have.deep.property('Planets.methods.get', 'getHandler()'); + expect(trie).to.have.deep.property('Planets.methods.post', 'postHandler()'); + expect(trie).to.have.deep.property('Planets.methods.delete', 'deleteHandler()'); + }); + + it('While registering more than one handlers for one verb+path,' + + ' preserves the handler which was registered first', function() { + + trie.add('/Planets', 'get', 'getHandler()'); + trie.add('/Planets', 'get', 'anotherGetHandler()'); + expect(trie).to.have.deep.property('Planets.methods.get', 'getHandler()'); + expect(trie).to.have.deep.property('Planets.methods.get') + .to.not.equal('anotherGetHandler()'); + }); + }); + + describe('trie.find()', function() { + + it('should return handler for matching path', function() { + expect(trie).to.have.property('Planets'); + trie.add('/Planets/count', 'get', 'getCount()'); + trie.add('/Planets/count', 'post', 'postCount()'); + var handlers = trie.find('/Planets/count'); + expect(handlers.get).to.be.equal('getCount()'); + expect(handlers.post).to.be.equal('postCount()'); + + //handlers = trie.find('/Planets'); + //expect(handlers.get).to.be.equal('handler2()'); + }); + }); +}); From 8197ad2e61fdad592cae6c9f654ee96a68777a8f Mon Sep 17 00:00:00 2001 From: gunjpan Date: Mon, 8 Feb 2016 00:35:33 -0500 Subject: [PATCH 02/13] Apply review feedback. - change rest-router api to match express router style - improve trie implementation for consistency - alter trie testcases to reflect change --- lib/rest-adapter.js | 21 +++++--------- lib/rest-router.js | 69 ++++++++++++++++++++++++++++++++------------- lib/trie.js | 65 ++++++++++++++++++++++++------------------ test/trie.test.js | 40 +++++++++++++++++--------- 4 files changed, 120 insertions(+), 75 deletions(-) diff --git a/lib/rest-adapter.js b/lib/rest-adapter.js index 337dc3e..f2e483e 100644 --- a/lib/rest-adapter.js +++ b/lib/rest-adapter.js @@ -265,8 +265,8 @@ RestAdapter.prototype.createHandler = function() { var handleUnknownPaths = this._shouldHandleUnknownPaths(); // Mount rest-router - var customRouter = new RestRouter(); - root.use(customRouter.handle()); + var fastRouter = new RestRouter(); + root.use(fastRouter); classes.forEach(function(restClass) { var router = express.Router(); @@ -282,13 +282,9 @@ RestAdapter.prototype.createHandler = function() { var sharedMethod = restMethod.sharedMethod; debug(' method %s', sharedMethod.stringName); restMethod.routes.forEach(function(route) { -<<<<<<< HEAD - methods.push({ route: route, method: sharedMethod }); -======= //add fullPath for Trie insertion route.fullPath = joinPaths(restClass.getPath(), route.path); - methods.push({route: route, method: sharedMethod}); ->>>>>>> cd24be5... Implement rest router from POC. + methods.push({ route: route, method: sharedMethod }); }); }); @@ -296,7 +292,7 @@ RestAdapter.prototype.createHandler = function() { methods.sort(sortRoutes); methods.forEach(function(m) { - adapter._registerMethodRouteHandlers(router, m.method, m.route, customRouter); + adapter._registerMethodRouteHandlers(router, m.method, m.route, fastRouter); }); if (handleUnknownPaths) { @@ -426,7 +422,7 @@ RestAdapter.errorHandler = function(options) { RestAdapter.prototype._registerMethodRouteHandlers = function(router, sharedMethod, route, - customRouter) { + fastRouter) { var handler = sharedMethod.isStatic ? this._createStaticMethodHandler(sharedMethod) : this._createPrototypeMethodHandler(sharedMethod); @@ -438,12 +434,9 @@ RestAdapter.prototype._registerMethodRouteHandlers = function(router, verb = 'delete'; } - //For now: exclude paths with parameters like :id - //and handler functions with err obj i.e. (err, req,res,next) - if (!route.path.includes(':') && handler.length < 4) { + if (fastRouter.canRegister(route, handler)) { handler.methodName = sharedMethod.name; - handler.path = route.fullPath; - customRouter.registerPathAndHandlers({ verb: verb, fullPath: route.fullPath }, handler); + fastRouter[verb](route, handler); } router[verb](route.path, handler); }; diff --git a/lib/rest-router.js b/lib/rest-router.js index 2848a6e..6dca567 100644 --- a/lib/rest-router.js +++ b/lib/rest-router.js @@ -1,31 +1,53 @@ -module.exports = RestRouter; var url = require('url'); +var http = require('http'); var Trie = require('./trie'); -function RestRouter(options) { - this.trie = new Trie(); - this.options = options; -} +var methods = getCurrentNodeMethods(); +var RestRouter = module.exports = function(options) { + + var opts = options || {}; + var router = function(req, res, next) { + router.handle(req, res, next); + }; -//register path and related handler to Trie ds -RestRouter.prototype.registerPathAndHandlers = function(route, handler) { - var path = normalizePath(route.fullPath); - var trie = this.trie; - trie = trie.add(path, route.verb, handler); + router.__proto__ = RestRouter; + router._trie = new Trie(); + router.options = opts; + + return router; }; -//handle request, match path and if found, invoke handler method -RestRouter.prototype.handle = function() { - var trie = this.trie; - return function(req, res, next) { - var verb = req.method.toLowerCase() || 'all'; - var path = normalizePath(req.url, this.options); - var methods = trie.find(path); - if (methods && verb in methods) { - methods[verb](req, res, next); - } else next(); +// express style API for registering route handlers: ex. route[method](path, handler) +// add verb:hanlder for path to trie DS +methods.concat('all').forEach(function(method) { + RestRouter[method] = function(route, handler) { + route.fullPath = normalizePath(route.fullPath, this.opts); + var trie = this._trie; + trie = trie.add(route, handler); + return this; }; +}); + +// handle request, match path and if found, invoke handler method +RestRouter.handle = function(req, res, next) { + var trie = this._trie; + var verb = req.method.toLowerCase(); + var path = normalizePath(req.url, this.options); + var methods = trie.find(path); + + if (methods && verb in methods) { + methods[verb](req, res, next); + } else { + next(); + } +}; + +// For now: exclude paths with parameters like :id +// and handler functions with err obj i.e. (err, req, res, next) + +RestRouter.canRegister = function (route, handler) { + return !(route.path.includes(':') && handler.length < 4) }; function normalizePath(path, options) { @@ -35,3 +57,10 @@ function normalizePath(path, options) { } return path; } + +// get list of all HTTP methods +function getCurrentNodeMethods() { + return http.METHODS && http.METHODS.map(function lowerCaseMethod(method) { + return method.toLowerCase(); + }); +} diff --git a/lib/trie.js b/lib/trie.js index fbf4248..031113b 100644 --- a/lib/trie.js +++ b/lib/trie.js @@ -1,41 +1,50 @@ + module.exports = Trie; function Trie() { this.methods = {}; } -//insert new nodes to Trie -Trie.prototype.add = function(path, verb, handler) { +// insert new nodes to Trie +Trie.prototype.add = function(route, handler) { var node = this; - var parts = getPathParts(path); - if (parts.length) { - for (var i = 0; i < parts.length; i++) { - if (!(parts[i] in node)) { - node[parts[i]] = new Trie(); - } - if (i + 1 === parts.length) { - addVerbHandler(node[parts[i]], verb, handler); - } - node = node[parts[i]]; + var parts = getPathParts(route.fullPath); + var segment; + + if (!parts.length) { + addVerbHandler(node, route, handler); + return this; + } + for (var i = 0; i < parts.length; i++) { + segment = parts[i]; + if (!(segment in node)) { + node[segment] = new Trie(); } - } else { // if we would ever encounter this situation? - addVerbHandler(node, verb, handler); + if (i + 1 === parts.length) { + addVerbHandler(node[segment], route, handler); + } + node = node[segment]; } + return this; }; -//Look for matching path and return handler method for a match +// Look for matching path and return methods for a match Trie.prototype.find = function(path) { var node = this; var parts = getPathParts(path); + var segment; + for (var i = 0; i < parts.length; i++) { - if (!(parts[i] in node)) { + segment = parts[i]; + if (!(segment in node)) { return false; } else { - if (i + 1 === parts.length) - return node[parts[i]].methods; + if (i + 1 === parts.length) { + return node[segment].methods; + } } - node = node[parts[i]]; + node = node[segment]; } }; @@ -43,21 +52,23 @@ function getPathParts(path) { path = path.trim(); var chunks = path.split('/'); var parts = []; - chunks.forEach(function(chunk) { - if (chunk) { - parts.push(chunk); + + for (var i in chunks) { + if (chunks[i]) { + parts.push(chunks[i]); } - }); + } return parts; } -function addVerbHandler(node, verb, handler) { +function addVerbHandler(node, route, handler) { node.methods = node.methods || {}; + var verb = route.verb; if (node.methods[verb]) { console.warn( - 'WARN: A handler was already registered for %s /api%s : Ignoring the new handler-%s', - verb.toUpperCase(), - handler.path || '/[unknown]', + 'WARN: A handler was already registered for %s /api%s : Ignoring the new handler %s', + route.verb.toUpperCase(), + route.fullPath || '/[unknown]', handler.methodName || '' ); return; diff --git a/test/trie.test.js b/test/trie.test.js index bfed45e..dc2f93c 100644 --- a/test/trie.test.js +++ b/test/trie.test.js @@ -1,52 +1,61 @@ + var assert = require('assert'); var expect = require('chai').expect; var Trie = require('../lib/trie'); describe('Trie', function() { var trie; + var route; describe('trie.add()', function() { beforeEach(function() { trie = new Trie(); + route = { fullPath: '', verb: '' }; }); it('has empty root node with only methods key', function() { expect(trie).to.have.keys('methods'); }); + it('registers hanlder for path: / to root node', function() { + trie.add(setRoute(route, '/', 'get'), 'rootHanlder()'); + expect(trie.methods).to.have.keys('get');//{ 'get': 'rootHandler()' }); + }); + it('create a node for a path with single part, no verb-handler', function() { - trie.add('/Planets'); + trie.add(setRoute(route, '/Planets')); expect(trie).to.have.property('Planets'); expect(trie.Planets).to.have.property('methods'); }); it('registers handler for path: /Planets', function() { - trie.add('/Planets', 'get', 'handler()'); + + trie.add(setRoute(route, '/Planets', 'get'), 'handler()'); expect(trie).to.have.deep.property('Planets.methods.get', 'handler()'); }); it('registers handler for path: /Planets/count', function() { - trie.add('/Planets/count', 'get', 'count()'); + trie.add(setRoute(route, '/Planets/count', 'get'), 'count()'); expect(trie).to.have.deep.property('Planets.count.methods.get', 'count()'); }); it('registers different HTTP verb handlers for the same path ', function() { - trie.add('/Planets', 'get', 'getHandler()'); - trie.add('/Planets', 'post', 'postHandler()'); - trie.add('/Planets', 'delete', 'deleteHandler()'); - + trie.add(setRoute(route, '/Planets', 'get'), 'getHandler()'); expect(trie).to.have.deep.property('Planets.methods.get', 'getHandler()'); + trie.add(setRoute(route, '/Planets', 'post'), 'postHandler()'); expect(trie).to.have.deep.property('Planets.methods.post', 'postHandler()'); + trie.add(setRoute(route, '/Planets', 'delete'), 'deleteHandler()'); expect(trie).to.have.deep.property('Planets.methods.delete', 'deleteHandler()'); }); it('While registering more than one handlers for one verb+path,' + ' preserves the handler which was registered first', function() { - trie.add('/Planets', 'get', 'getHandler()'); - trie.add('/Planets', 'get', 'anotherGetHandler()'); + trie.add(setRoute(route, '/Planets', 'get'), 'getHandler()'); expect(trie).to.have.deep.property('Planets.methods.get', 'getHandler()'); + trie.add(setRoute(route,'/Planets', 'get'), 'anotherGetHandler()'); + expect(trie).to.have.deep.property('Planets.methods.get') .to.not.equal('anotherGetHandler()'); }); @@ -56,14 +65,17 @@ describe('Trie', function() { it('should return handler for matching path', function() { expect(trie).to.have.property('Planets'); - trie.add('/Planets/count', 'get', 'getCount()'); - trie.add('/Planets/count', 'post', 'postCount()'); + trie.add(setRoute(route, '/Planets/count', 'get'), 'getCount()'); + trie.add(setRoute(route,'/Planets/count', 'post'), 'postCount()'); var handlers = trie.find('/Planets/count'); expect(handlers.get).to.be.equal('getCount()'); expect(handlers.post).to.be.equal('postCount()'); - - //handlers = trie.find('/Planets'); - //expect(handlers.get).to.be.equal('handler2()'); }); }); }); + +function setRoute(route, fullPath, verb) { + route.fullPath = fullPath, + route.verb = verb; + return route; +} \ No newline at end of file From c05b300205eab89b8ec1e78b803c935b01ef5fff Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Fri, 6 May 2016 13:57:14 -0400 Subject: [PATCH 03/13] fixup! Apply review feedback. --- lib/rest-router.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rest-router.js b/lib/rest-router.js index 6dca567..a80d84c 100644 --- a/lib/rest-router.js +++ b/lib/rest-router.js @@ -46,8 +46,8 @@ RestRouter.handle = function(req, res, next) { // For now: exclude paths with parameters like :id // and handler functions with err obj i.e. (err, req, res, next) -RestRouter.canRegister = function (route, handler) { - return !(route.path.includes(':') && handler.length < 4) +RestRouter.canRegister = function(route, handler) { + return !(route.path.includes(':') && handler.length < 4); }; function normalizePath(path, options) { From 44a138e94ee8ad9c38ecab1ae5bdca37d25d97e4 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Fri, 6 May 2016 14:05:24 -0400 Subject: [PATCH 04/13] fixup! Apply review feedback. --- lib/rest-router.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/rest-router.js b/lib/rest-router.js index a80d84c..409d3bb 100644 --- a/lib/rest-router.js +++ b/lib/rest-router.js @@ -5,7 +5,6 @@ var Trie = require('./trie'); var methods = getCurrentNodeMethods(); var RestRouter = module.exports = function(options) { - var opts = options || {}; var router = function(req, res, next) { router.handle(req, res, next); From 65c553b88b7923783cf795715f45d24bfa0b486d Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Mon, 9 May 2016 14:17:25 -0400 Subject: [PATCH 05/13] fixup! fixup! Apply review feedback. --- lib/rest-adapter.js | 2 +- test/trie.test.js | 21 ++++++++------------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/lib/rest-adapter.js b/lib/rest-adapter.js index f2e483e..6ae5c1c 100644 --- a/lib/rest-adapter.js +++ b/lib/rest-adapter.js @@ -282,7 +282,7 @@ RestAdapter.prototype.createHandler = function() { var sharedMethod = restMethod.sharedMethod; debug(' method %s', sharedMethod.stringName); restMethod.routes.forEach(function(route) { - //add fullPath for Trie insertion + // add fullPath for Trie insertion route.fullPath = joinPaths(restClass.getPath(), route.path); methods.push({ route: route, method: sharedMethod }); }); diff --git a/test/trie.test.js b/test/trie.test.js index dc2f93c..db1e126 100644 --- a/test/trie.test.js +++ b/test/trie.test.js @@ -4,11 +4,9 @@ var expect = require('chai').expect; var Trie = require('../lib/trie'); describe('Trie', function() { - var trie; - var route; + var trie, route; describe('trie.add()', function() { - beforeEach(function() { trie = new Trie(); route = { fullPath: '', verb: '' }; @@ -30,7 +28,6 @@ describe('Trie', function() { }); it('registers handler for path: /Planets', function() { - trie.add(setRoute(route, '/Planets', 'get'), 'handler()'); expect(trie).to.have.deep.property('Planets.methods.get', 'handler()'); }); @@ -51,22 +48,20 @@ describe('Trie', function() { it('While registering more than one handlers for one verb+path,' + ' preserves the handler which was registered first', function() { + trie.add(setRoute(route, '/Planets', 'get'), 'getHandler()'); + expect(trie).to.have.deep.property('Planets.methods.get', 'getHandler()'); + trie.add(setRoute(route, '/Planets', 'get'), 'anotherGetHandler()'); - trie.add(setRoute(route, '/Planets', 'get'), 'getHandler()'); - expect(trie).to.have.deep.property('Planets.methods.get', 'getHandler()'); - trie.add(setRoute(route,'/Planets', 'get'), 'anotherGetHandler()'); - - expect(trie).to.have.deep.property('Planets.methods.get') + expect(trie).to.have.deep.property('Planets.methods.get') .to.not.equal('anotherGetHandler()'); - }); + }); }); describe('trie.find()', function() { - it('should return handler for matching path', function() { expect(trie).to.have.property('Planets'); trie.add(setRoute(route, '/Planets/count', 'get'), 'getCount()'); - trie.add(setRoute(route,'/Planets/count', 'post'), 'postCount()'); + trie.add(setRoute(route, '/Planets/count', 'post'), 'postCount()'); var handlers = trie.find('/Planets/count'); expect(handlers.get).to.be.equal('getCount()'); expect(handlers.post).to.be.equal('postCount()'); @@ -78,4 +73,4 @@ function setRoute(route, fullPath, verb) { route.fullPath = fullPath, route.verb = verb; return route; -} \ No newline at end of file +} From f0a667db6e39a693fa23b226f5e889af190fb407 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 25 May 2016 15:17:34 -0400 Subject: [PATCH 06/13] Add ability to handle routes with parameters. --- lib/rest-adapter.js | 10 +-- lib/rest-router.js | 7 ++- lib/trie.js | 25 +++----- test/rest-router.test.js | 127 ++++++++++++++++++++++++--------------- test/trie.test.js | 2 +- 5 files changed, 99 insertions(+), 72 deletions(-) diff --git a/lib/rest-adapter.js b/lib/rest-adapter.js index 6ae5c1c..956b660 100644 --- a/lib/rest-adapter.js +++ b/lib/rest-adapter.js @@ -434,11 +434,11 @@ RestAdapter.prototype._registerMethodRouteHandlers = function(router, verb = 'delete'; } - if (fastRouter.canRegister(route, handler)) { - handler.methodName = sharedMethod.name; - fastRouter[verb](route, handler); - } - router[verb](route.path, handler); + // if (fastRouter.canRegister(route, handler)) { + handler.methodName = sharedMethod.name; + fastRouter[verb](route, handler); + // } + // router[verb](route.path, handler); }; RestAdapter.prototype._createStaticMethodHandler = function(sharedMethod) { diff --git a/lib/rest-router.js b/lib/rest-router.js index 409d3bb..1e29e27 100644 --- a/lib/rest-router.js +++ b/lib/rest-router.js @@ -4,6 +4,7 @@ var http = require('http'); var Trie = require('./trie'); var methods = getCurrentNodeMethods(); + var RestRouter = module.exports = function(options) { var opts = options || {}; var router = function(req, res, next) { @@ -45,9 +46,9 @@ RestRouter.handle = function(req, res, next) { // For now: exclude paths with parameters like :id // and handler functions with err obj i.e. (err, req, res, next) -RestRouter.canRegister = function(route, handler) { - return !(route.path.includes(':') && handler.length < 4); -}; +// RestRouter.canRegister = function(route, handler) { +// return !(route.path.includes(':') && handler.length < 4); +// }; function normalizePath(path, options) { path = url.parse(path).pathname; diff --git a/lib/trie.js b/lib/trie.js index 031113b..43fcad1 100644 --- a/lib/trie.js +++ b/lib/trie.js @@ -19,6 +19,7 @@ Trie.prototype.add = function(route, handler) { segment = parts[i]; if (!(segment in node)) { node[segment] = new Trie(); + node.param = (segment[0] === ':'); } if (i + 1 === parts.length) { addVerbHandler(node[segment], route, handler); @@ -37,28 +38,20 @@ Trie.prototype.find = function(path) { for (var i = 0; i < parts.length; i++) { segment = parts[i]; - if (!(segment in node)) { - return false; - } else { - if (i + 1 === parts.length) { - return node[segment].methods; - } + if (!(segment in node) && !node.param) return false; + if (node.param) { + segment = Object.keys(node).find(function(key) { + if (key[0] === ':') return key; + }); + return node[segment].methods; } + if (i + 1 === parts.length) return node[segment].methods; node = node[segment]; } }; function getPathParts(path) { - path = path.trim(); - var chunks = path.split('/'); - var parts = []; - - for (var i in chunks) { - if (chunks[i]) { - parts.push(chunks[i]); - } - } - return parts; + return path.trim().split('/').filter(Boolean); } function addVerbHandler(node, route, handler) { diff --git a/test/rest-router.test.js b/test/rest-router.test.js index 8972c37..3cccb93 100644 --- a/test/rest-router.test.js +++ b/test/rest-router.test.js @@ -1,60 +1,93 @@ var assert = require('assert'); var expect = require('chai').expect; -var RestRouter = require('../lib/rest-router'); +var RestRouter = require('../lib/rest-router.js'); +var Trie = require('../lib/trie.js'); +var express = require('express'); +var supertest = require('supertest'); +var http = require('http'); -describe('Custom Router', function() { - var restRouter; - var route = { verb: 'get', fullPath: '/Planets' }; - var handler = function(req, res, next) { res.handled = true; }; - var req = { - method: 'get', - url: '/Planets', - }; - var res = {}; - var next = function() { res.calledWhenPathNotFound = true; }; - - beforeEach(function() { - restRouter = new RestRouter(); +describe('RestRouter', function() { + // Test init + it('initializes with Trie datastructure', function(done) { + var router = new RestRouter(); + expect(router).to.have.property('_trie'); + expect(router._trie).to.be.instanceOf(Trie); + done(); }); - describe('RestRouter()', function() { - - it('initiates a restRouter obj with Trie and options', function() { - expect(restRouter).to.have.keys(['trie', 'options']); + // test handle + it('handles a request', function(done) { + var app = express(); + var router = new RestRouter(); + var route = { fullPath: '/example', verb: 'get' }; + var resBody = 'Hello Wold!'; + // this doesn't make sense to me that I have to call router.get(...) + // as well as have route.verb = get + // it doesn't work without it + router.get(route, function(req, res) { + res.end(resBody); }); + app.use(router); + console.log(router._trie); + supertest(app) + .get(route.fullPath) + .expect(200) + .expect(resBody) + .end(function(err, res) { + if (err) return done(err); + done(); + }); }); - describe('restRouter.registerPathAndHandlers()', function() { - it('register handler for GET: /Planets', function() { - restRouter.registerPathAndHandlers(route, handler); - expect(restRouter.trie).to.have.deep.property('planets.methods.get'); + // test handle param + it('handles a request with a parameter', function(done) { + var app = express(); + var router = new RestRouter(); + var route = { fullPath: '/example/:id', verb: 'get' }; + var id = '1'; + // this doesn't make sense to me that I have to call router.get(...) + // as well as have route.verb = get + // it doesn't work without it + router.get(route, function(req, res, next) { + console.log('hit'); + res.end(id); }); - }); - - describe('restRouter.handle()', function() { - it('returns a function --which invokes matching handler-- with three arguments', - function() { - restRouter.registerPathAndHandlers(route, handler); - var returned = restRouter.handle(); - expect(returned).to.have.length(3); - }); - - it('invokes the handler function for matching verb+path', - function() { - restRouter.registerPathAndHandlers(route, handler); - restRouter.handle()(req, res, next); - expect(res.handled).to.be.true; - }); - - it('invokes supplied next() if no matching handler found for verb+path', - function() { - restRouter.registerPathAndHandlers(route, handler); - req.method = 'post'; - res = {}; - restRouter.handle()(req, res, next); - expect(res.calledWhenPathNotFound).to.be.true; - expect(res.handled).to.be.undefined; + app.use(router); + console.log(router._trie); + supertest(app) + .get(route.fullPath.replace(':id', id)) + .expect(200) + .expect(id) + .end(function(err, res) { + console.log(res.body); + if (err) return done(err); + done(); }); }); + // test handle param with method after + // it('handles a request for a instance method', function() { + // var app = express(); + // var router = new RestRouter(); + // var route = { fullPath: '/example/:id/properties', verb: 'get' }; + // var id = '1'; + // // this doesn't make sense to me that I have to call router.get(...) + // // as well as have route.verb = get + // // it doesn't work without it + // router.get(route, function(req, res, next) { + // // List the properties of a mock object + // res.end(req.fullPath); + // }); + // app.use(router); + // console.log(router._trie); + // supertest(app) + // .get(route.fullPath.replace(':id', id)) + // .expect(200) + // .expect(id) + // .end(function(err, res) { + // console.log(res.body); + // if (err) return done(err); + // done(); + // }); + // }); }); diff --git a/test/trie.test.js b/test/trie.test.js index db1e126..c0a45e5 100644 --- a/test/trie.test.js +++ b/test/trie.test.js @@ -46,7 +46,7 @@ describe('Trie', function() { expect(trie).to.have.deep.property('Planets.methods.delete', 'deleteHandler()'); }); - it('While registering more than one handlers for one verb+path,' + + it('While registering more than one handler for one verb+path,' + ' preserves the handler which was registered first', function() { trie.add(setRoute(route, '/Planets', 'get'), 'getHandler()'); expect(trie).to.have.deep.property('Planets.methods.get', 'getHandler()'); From 9a1bcef39547030ccb5b8b540fe69f11dab91a3a Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 25 May 2016 15:23:55 -0400 Subject: [PATCH 07/13] Change name of methods constant. --- lib/rest-router.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/rest-router.js b/lib/rest-router.js index 1e29e27..f3b8a6b 100644 --- a/lib/rest-router.js +++ b/lib/rest-router.js @@ -3,7 +3,7 @@ var url = require('url'); var http = require('http'); var Trie = require('./trie'); -var methods = getCurrentNodeMethods(); +var HTTP_METHODS = getCurrentNodeMethods(); var RestRouter = module.exports = function(options) { var opts = options || {}; @@ -20,7 +20,7 @@ var RestRouter = module.exports = function(options) { // express style API for registering route handlers: ex. route[method](path, handler) // add verb:hanlder for path to trie DS -methods.concat('all').forEach(function(method) { +HTTP_METHODS.concat('all').forEach(function(method) { RestRouter[method] = function(route, handler) { route.fullPath = normalizePath(route.fullPath, this.opts); var trie = this._trie; From 5638553dc7e5049336375ca460ecdfe7a136c2e4 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 25 May 2016 15:25:15 -0400 Subject: [PATCH 08/13] Fix typo. --- lib/rest-router.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rest-router.js b/lib/rest-router.js index f3b8a6b..b3df870 100644 --- a/lib/rest-router.js +++ b/lib/rest-router.js @@ -19,7 +19,7 @@ var RestRouter = module.exports = function(options) { }; // express style API for registering route handlers: ex. route[method](path, handler) -// add verb:hanlder for path to trie DS +// add verb:handler for path to trie DS HTTP_METHODS.concat('all').forEach(function(method) { RestRouter[method] = function(route, handler) { route.fullPath = normalizePath(route.fullPath, this.opts); From ecc5b677e142a560ccbe36645b81e6fa45932642 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 25 May 2016 15:35:01 -0400 Subject: [PATCH 09/13] Re-add express handler as backup for streams. --- lib/rest-adapter.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rest-adapter.js b/lib/rest-adapter.js index 956b660..6714233 100644 --- a/lib/rest-adapter.js +++ b/lib/rest-adapter.js @@ -438,7 +438,7 @@ RestAdapter.prototype._registerMethodRouteHandlers = function(router, handler.methodName = sharedMethod.name; fastRouter[verb](route, handler); // } - // router[verb](route.path, handler); + router[verb](route.path, handler); }; RestAdapter.prototype._createStaticMethodHandler = function(sharedMethod) { From 650a707636b82eb48187a824f5f667bd68319a71 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Wed, 25 May 2016 15:37:07 -0400 Subject: [PATCH 10/13] Add methods package. --- lib/rest-router.js | 9 +-------- package.json | 1 + 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/rest-router.js b/lib/rest-router.js index b3df870..b0db8a7 100644 --- a/lib/rest-router.js +++ b/lib/rest-router.js @@ -3,7 +3,7 @@ var url = require('url'); var http = require('http'); var Trie = require('./trie'); -var HTTP_METHODS = getCurrentNodeMethods(); +var HTTP_METHODS = require('methods'); var RestRouter = module.exports = function(options) { var opts = options || {}; @@ -57,10 +57,3 @@ function normalizePath(path, options) { } return path; } - -// get list of all HTTP methods -function getCurrentNodeMethods() { - return http.METHODS && http.METHODS.map(function lowerCaseMethod(method) { - return method.toLowerCase(); - }); -} diff --git a/package.json b/package.json index d08ebbc..d06e888 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,7 @@ "jayson": "^1.2.0", "js2xmlparser": "^0.1.9", "loopback-phase": "^1.3.0", + "methods": "^1.1.2", "mux-demux": "^3.7.9", "qs": "^2.4.2", "request": "^2.55.0", From eff2b9499c5ed87eb1f7fcf0aa75ce3de66dcab5 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Thu, 26 May 2016 11:00:43 -0400 Subject: [PATCH 11/13] Replace find() with filter() for ES5 support. --- lib/trie.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/trie.js b/lib/trie.js index 43fcad1..2398658 100644 --- a/lib/trie.js +++ b/lib/trie.js @@ -40,9 +40,10 @@ Trie.prototype.find = function(path) { segment = parts[i]; if (!(segment in node) && !node.param) return false; if (node.param) { - segment = Object.keys(node).find(function(key) { + segment = Object.keys(node).filter(function(key) { + console.log(key); if (key[0] === ':') return key; - }); + }).pop(); return node[segment].methods; } if (i + 1 === parts.length) return node[segment].methods; From 94166a0c3dc223227a6cd3b7dd224df24cedb3f4 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Thu, 26 May 2016 11:57:19 -0400 Subject: [PATCH 12/13] Add parameter to request object. --- lib/rest-router.js | 3 ++- lib/trie.js | 8 +++++--- test/rest-router.test.js | 10 +++++----- 3 files changed, 12 insertions(+), 9 deletions(-) diff --git a/lib/rest-router.js b/lib/rest-router.js index b0db8a7..b210e04 100644 --- a/lib/rest-router.js +++ b/lib/rest-router.js @@ -34,7 +34,8 @@ RestRouter.handle = function(req, res, next) { var trie = this._trie; var verb = req.method.toLowerCase(); var path = normalizePath(req.url, this.options); - var methods = trie.find(path); + var methods = trie.find(path, req); + console.log('req.params: ', req.params); if (methods && verb in methods) { methods[verb](req, res, next); diff --git a/lib/trie.js b/lib/trie.js index 2398658..d79d0a4 100644 --- a/lib/trie.js +++ b/lib/trie.js @@ -31,7 +31,7 @@ Trie.prototype.add = function(route, handler) { }; // Look for matching path and return methods for a match -Trie.prototype.find = function(path) { +Trie.prototype.find = function(path, req) { var node = this; var parts = getPathParts(path); var segment; @@ -41,8 +41,10 @@ Trie.prototype.find = function(path) { if (!(segment in node) && !node.param) return false; if (node.param) { segment = Object.keys(node).filter(function(key) { - console.log(key); - if (key[0] === ':') return key; + if (key[0] === ':') { + req.params[key.split(':')[1]] = parts[i]; + return key; + } }).pop(); return node[segment].methods; } diff --git a/test/rest-router.test.js b/test/rest-router.test.js index 3cccb93..f179660 100644 --- a/test/rest-router.test.js +++ b/test/rest-router.test.js @@ -28,7 +28,7 @@ describe('RestRouter', function() { res.end(resBody); }); app.use(router); - console.log(router._trie); + // console.log(router._trie); supertest(app) .get(route.fullPath) .expect(200) @@ -49,17 +49,17 @@ describe('RestRouter', function() { // as well as have route.verb = get // it doesn't work without it router.get(route, function(req, res, next) { - console.log('hit'); - res.end(id); + console.log('req.params: ', req.params); + res.end(req.params.id); }); app.use(router); - console.log(router._trie); + // console.log(router._trie); supertest(app) .get(route.fullPath.replace(':id', id)) .expect(200) .expect(id) .end(function(err, res) { - console.log(res.body); + // console.log(res.body); if (err) return done(err); done(); }); From c79b9e6c35a14248df75bf722817af3172e91660 Mon Sep 17 00:00:00 2001 From: Richard Pringle Date: Mon, 6 Jun 2016 21:42:40 -0400 Subject: [PATCH 13/13] Touch up console warn message. --- lib/trie.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/trie.js b/lib/trie.js index d79d0a4..4e425e0 100644 --- a/lib/trie.js +++ b/lib/trie.js @@ -64,8 +64,8 @@ function addVerbHandler(node, route, handler) { console.warn( 'WARN: A handler was already registered for %s /api%s : Ignoring the new handler %s', route.verb.toUpperCase(), - route.fullPath || '/[unknown]', - handler.methodName || '' + route.fullPath || '[unknown path]', + handler.methodName || '' ); return; }