Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor router #592

Merged
merged 1 commit into from
Feb 27, 2013
Merged

refactor router #592

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
127 changes: 127 additions & 0 deletions lib/router.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
// Load modules

var Route = require('./route');
var Utils = require('./utils');


// Declare internals

var internals = {};


module.exports = internals.Router = function (server) {

this.server = server;
this.table = {}; // Array per HTTP method, including * for catch-all

this.notfound = new Route({
method: 'notfound',
path: '/{p*}',
config: {
auth: { mode: 'none' }, // In case defaults are set otherwise
handler: 'notFound'
}
}, server);

if (server.settings.cors) {
this.cors = new Route({
path: '/{p*}',
method: 'options',
config: {
auth: { mode: 'none' }, // In case defaults are set otherwise
handler: function (request) {

request.reply({});
}
}
}, server);
}
};


internals.Router.prototype.route = function (request) {

// Lookup route

var method = (request.method === 'head' ? 'get' : request.method);

var routes = this.table[method] || [];
for (var i = 0, il = routes.length; i<il;++i) {
var route = routes[i];
if (route.match(request)) {
return route;
}
}

// CORS

if (method === 'options' &&
this.cors) {

return this.cors;
}

// *

routes = this.table['*'] || [];
for (i = 0, il = routes.length; i < il; ++i) {
var route = routes[i];
if (route.match(request)) {
return route;
}
}

// Not found

return this.notfound;
};


internals.Router.prototype.add = function (configs) {

var self = this;

Utils.assert(configs, 'Routes configs must exist');

configs = (configs instanceof Array ? configs : [configs]);

var methods = {};
configs.forEach(function (config) {

var route = new Route(config, self.server); // Do no use config beyond this point, use route members

self.table[route.method] = self.table[route.method] || [];

// Check for existing route with same fingerprint

methods[route.method] = true;
self.table[route.method].forEach(function (existing) {

Utils.assert(route.fingerprint !== existing.fingerprint, 'New route: ' + route.path + ' conflicts with existing: ' + existing.path);
});

self.table[route.method].push(route);
});

Object.keys(methods).forEach(function (method) {

self.table[method].sort(Route.sort);
});
};


internals.Router.prototype.routingTable = function () {

var self = this;

var table = [];
Object.keys(this.table).forEach(function (method) {

self.table[method].forEach(function (route) {

table.push(route);
});
});

return table;
};
138 changes: 6 additions & 132 deletions lib/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ var Catbox = require('catbox');
var Defaults = require('./defaults');
var Boom = require('boom');
var Request = require('./request');
var Route = require('./route');
var Router = require('./router');
var Schema = require('./schema');
var Views = require('./views');
var Utils = require('./utils');
Expand All @@ -23,8 +23,6 @@ var Utils = require('./utils');
var internals = {};


// Create and configure server instance

module.exports = internals.Server = function (/* host, port, options */) { // all optional

var self = this;
Expand Down Expand Up @@ -113,18 +111,7 @@ module.exports = internals.Server = function (/* host, port, options */) {

Utils.assert(!this.settings.router.routeDefaults || !this.settings.router.routeDefaults.handler, 'Route defaults cannot include a handler');

this._router = {
table: {} // Array per HTTP method, including * for catch-all
};

this._router.notfound = new Route({
method: 'notfound',
path: '/{p*}',
config: {
auth: { mode: 'none' }, // In case defaults are set otherwise
handler: 'notFound'
}
}, this);
this._router = new Router(this);

// Plugin interface (pack)

Expand Down Expand Up @@ -161,20 +148,6 @@ module.exports = internals.Server = function (/* host, port, options */) {
if (this.settings.cors) {
this.settings.cors._headers = (this.settings.cors.headers || []).concat(this.settings.cors.additionalHeaders || []).join(', ');
this.settings.cors._methods = (this.settings.cors.methods || []).concat(this.settings.cors.additionalMethods || []).join(', ');

var optionsConfig = {
path: '/{p*}',
method: 'options',
config: {
auth: { mode: 'none' }, // In case defaults are set otherwise
handler: function (request) {

request.reply({});
}
}
};

this._router.cors = new Route(optionsConfig, this);
}

// Initialize cache engine
Expand Down Expand Up @@ -232,87 +205,15 @@ internals.Server.prototype._dispatch = function (options) {

// Lookup route

var method = (request.method === 'head' ? 'get' : request.method);

var routes = self._router.table[method];
if (routes) {
for (var i = 0, il = routes.length; i < il; ++i) {
var route = routes[i];
if (route.match(request)) {
return request._execute(route);
}
}
}

// CORS

if (method === 'options' &&
self.settings.cors) {

return request._execute(self._router.cors);
}

// *

routes = self._router.table['*'];
if (routes) {
for (i = 0, il = routes.length; i < il; ++i) {
var route = routes[i];
if (route.match(request)) {
return request._execute(route);
}
}
}

// Not found

return request._execute(self._router.notfound);
request._execute(self._router.route(request));
});
};
};


// Find a route match

internals.Server.prototype._match = function (method, path) {

Utils.assert(method, 'The method parameter must be provided');
Utils.assert(path, 'The path parameter must be provided');

// Lookup route
internals.Server.prototype.routingTable = function () {

method = method.toLowerCase();
method = (method === 'head' ? 'get' : method);
var routes = this._router.table[method];

if (routes) {
for (var i = 0, il = routes.length; i < il; ++i) {
var route = routes[i];
if (route.test(path)) {
return route;
}
}
}

return null;
};


internals.Server.prototype._routeTable = function () {

var self = this;

var flattenedRoutes = [];

Object.keys(this._router.table).forEach(function (method) {

self._router.table[method].forEach(function (route) {

flattenedRoutes.push(route);
});
});

return flattenedRoutes;
return this._router.routingTable();
};


Expand Down Expand Up @@ -408,34 +309,7 @@ internals.Server.prototype.ext = function (event, func) {

internals.Server.prototype.route = internals.Server.prototype.addRoute = internals.Server.prototype.addRoutes = function (configs) {

var self = this;

Utils.assert(configs, 'Routes configs must exist');

configs = (configs instanceof Array ? configs : [configs]);

var methods = {};
configs.forEach(function (config) {

var route = new Route(config, self); // Do no use config beyond this point, use route members

self._router.table[route.method] = self._router.table[route.method] || [];

// Check for existing route with same fingerprint

methods[route.method] = true;
var routes = self._router.table[route.method];
for (var ri = 0, rl = routes.length; ri < rl; ++ri) {
Utils.assert(route.fingerprint !== routes[ri].fingerprint, 'New route: ' + route.path + ' conflicts with existing: ' + routes[ri].path);
}

routes.push(route);
});

Object.keys(methods).forEach(function (method) {

self._router.table[method].sort(Route.sort);
});
this._router.add(configs);
};


Expand Down
2 changes: 1 addition & 1 deletion test/integration/proxy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ describe('Proxy', function () {
callback(new Error());
};

var route = _server._match('get', '/proxyerror');
var route = _server._router.route({ method: 'get', path: '/proxyerror' });
route.proxy.httpClient = requestStub;

makeRequest({ path: '/proxyerror', method: 'get' }, function (rawRes) {
Expand Down
53 changes: 2 additions & 51 deletions test/unit/server.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,55 +162,6 @@ describe('Server', function () {
done();
});

describe('#_match', function () {

it('throws an error when the method parameter is null', function (done) {

var fn = function () {

var server = new Hapi.Server('0.0.0.0', 0);
server._match(null, '/test');
};
expect(fn).to.throw(Error, 'The method parameter must be provided');
done();
});

it('throws an error when the path parameter is null', function (done) {

var fn = function () {

var server = new Hapi.Server('0.0.0.0', 0);
server._match('POST', null);
};
expect(fn).to.throw(Error, 'The path parameter must be provided');
done();
});

it('returns null when no routes are added', function (done) {

var server = new Hapi.Server('0.0.0.0', 0);
var result = server._match('GET', '/test');

expect(result).to.not.exist;
done();
});

it('returns the route when there is a match', function (done) {

var server = new Hapi.Server('0.0.0.0', 0);
server.route({
method: 'GET',
path: '/test',
handler: function () { }
});
var result = server._match('GET', '/test');

expect(result).exist;
expect(result.path).to.equal('/test');
done();
});
});

describe('#start', function () {

it('doesn\'t throw an error', function (done) {
Expand Down Expand Up @@ -558,7 +509,7 @@ describe('Server', function () {
});
});

describe('#_routeTable', function () {
describe('#routingTable', function () {

it('returns an array of the current routes', function (done) {

Expand All @@ -567,7 +518,7 @@ describe('Server', function () {
server.route({ path: '/test/', method: 'get', handler: function () { } });
server.route({ path: '/test/{p}/end', method: 'get', handler: function () { } });

var routes = server._routeTable();
var routes = server.routingTable();

expect(routes.length).to.equal(2);
expect(routes[0].path).to.equal('/test/');
Expand Down