From 277f07c1015326b2a3bc2b3f509435c6a0274d08 Mon Sep 17 00:00:00 2001 From: Eran Hammer Date: Tue, 25 Mar 2014 18:57:49 -0700 Subject: [PATCH] Cleanup for #1521 --- docs/Reference.md | 18 ++++++------ lib/directory.js | 4 +-- lib/file.js | 4 +-- lib/handler.js | 22 ++++++--------- lib/index.js | 2 +- lib/methods.js | 3 +- lib/pack.js | 4 ++- lib/proxy.js | 4 +-- lib/route.js | 7 ++--- lib/schema.js | 64 ++++++++++--------------------------------- lib/server.js | 10 ++----- lib/views.js | 4 +-- test/schema.js | 70 ++++++++++++++++++++++++++++++++--------------- test/server.js | 2 +- 14 files changed, 94 insertions(+), 124 deletions(-) diff --git a/docs/Reference.md b/docs/Reference.md index 30efce6de..0f2b05669 100755 --- a/docs/Reference.md +++ b/docs/Reference.md @@ -1245,10 +1245,13 @@ server.inject('/', function (res) { #### `server.handler(name, method)` -Registers a new handler type. This allows you to define a new handler type which can then be used in routes. Overriding the built in handler types (`directory`, `file`, `proxy`, and `view`), or any previously registered type is not allowed. +Registers a new handler type which can then be used in routes. Overriding the built in handler types (`directory`, `file`, `proxy`, and `view`), +or any previously registered types is not allowed. -- `name` - String name for the handler that you want to register. -- `method` - The method that will be used to handle the requests routed to it. +- `name` - string name for the handler being registered. +- `method` - the function used to generate the route handler using the signature `function(route, options)` where: + - `route` - the internal route object. + - `options` - the configuration object provided in the handler config. ```javascript var Hapi = require('hapi'); @@ -1259,7 +1262,7 @@ server.handler('test', function (route, options) { return function (request, reply) { - reply ('new handler: ' + options.msg); + reply('new handler: ' + options.msg); } }); @@ -2744,17 +2747,14 @@ exports.register = function (plugin, options, next) { #### `plugin.handler(name, method)` -Registers a new handler type. This allows you to define a new handler type which can then be used in routes. Overriding the built in handler types (`directory`, `file`, `proxy`, and `view`), or any previously registered type is not allowed. - -- `name` - String name for the handler that you want to register. -- `method` - The method that will be used to handle the requests routed to it. +Registers a new handler type as describe in [`server.handler(name, method)`](#serverhandlername-method). ```javascript exports.register = function (plugin, options, next) { var handlerFunc = function (route, options) { - return function(request, reply) { + return function (request, reply) { reply('Message from plugin handler: ' + options.msg); } diff --git a/lib/directory.js b/lib/directory.js index f6258d2fa..e4a1023f7 100755 --- a/lib/directory.js +++ b/lib/directory.js @@ -3,7 +3,6 @@ var Fs = require('fs'); var Path = require('path'); var Boom = require('boom'); -var Joi = require('joi'); var Async = require('async'); var Response = require('./response'); var File = require('./file'); @@ -18,8 +17,7 @@ var internals = {}; exports.handler = function (route, options) { - var schemaError = Joi.validate(options, Schema.directoryHandlerSchema); - Utils.assert(!schemaError, 'Invalid handler options for directory:', schemaError); + Schema.assert('directory handler', options, route.path); Utils.assert(route.path[route.path.length - 1] === '}', 'The route path must end with a parameter:', route.path); Utils.assert(route.params.length >= 1, 'The route path must include at least one parameter:', route.path); diff --git a/lib/file.js b/lib/file.js index 4b982501a..eb5a21780 100755 --- a/lib/file.js +++ b/lib/file.js @@ -5,7 +5,6 @@ var Path = require('path'); var Crypto = require('crypto'); var Mime = require('mime'); var Boom = require('boom'); -var Joi = require('joi'); var Utils = require('./utils'); var Response = require('./response'); var Schema = require('./schema'); @@ -18,8 +17,7 @@ var internals = {}; exports.handler = function (route, options) { - var schemaError = Joi.validate(options, Schema.fileHandlerSchema); - Utils.assert(!schemaError, 'Invalid handler options for file:', schemaError); + Schema.assert('file handler', options, route.path); var settings = (typeof options !== 'object' ? { path: options } : Utils.clone(options)); // options can be reused Utils.assert(typeof settings.path !== 'string' || settings.path[settings.path.length - 1] !== '/', 'File path cannot end with a \'/\':', route.path); diff --git a/lib/handler.js b/lib/handler.js index 027d0255e..fc65b0931 100755 --- a/lib/handler.js +++ b/lib/handler.js @@ -214,6 +214,15 @@ exports.configure = function (handler, route) { }; +exports.register = function (pack) { + + pack._handler('proxy', Proxy.handler); + pack._handler('file', File.handler); + pack._handler('directory', Directory.handler); + pack._handler('view', Views.handler); +}; + + exports.prerequisites = function (config, server) { if (!config) { @@ -390,16 +399,3 @@ exports.invoke = function (request, event, callback) { }); }); }; - - -exports.register = function (server) { - // If handlers are already registered, just return - if (Object.keys(server.pack._handlers).length > 0) { - return; - } - - server.handler('proxy', Proxy.handler); - server.handler('file', File.handler); - server.handler('directory', Directory.handler); - server.handler('view', Views.handler); -}; diff --git a/lib/index.js b/lib/index.js index 4dd51dd34..f86df424a 100755 --- a/lib/index.js +++ b/lib/index.js @@ -3,7 +3,7 @@ exports.error = exports.Error = exports.boom = exports.Boom = require('boom'); exports.Server = require('./server'); exports.utils = require('./utils'); -exports.types = require('joi').types; +exports.types = require('joi'); exports.Pack = require('./pack'); exports.Composer = require('./composer'); diff --git a/lib/methods.js b/lib/methods.js index 77e2e90a3..f206bde36 100755 --- a/lib/methods.js +++ b/lib/methods.js @@ -45,8 +45,7 @@ internals.Methods.prototype._add = function (name, fn, options, env) { Utils.assert(!Utils.reach(this.methods, name, { functions: false }), 'Server method function name already exists'); var options = options || {}; - var schemaError = Schema.method(options); - Utils.assert(!schemaError, 'Invalid method options for', name, ':', schemaError); + Schema.assert('method', options, name); var settings = Utils.clone(options); settings.generateKey = settings.generateKey || internals.generateKey; diff --git a/lib/pack.js b/lib/pack.js index c02b869e5..37b000097 100755 --- a/lib/pack.js +++ b/lib/pack.js @@ -11,6 +11,7 @@ var Defaults = require('./defaults'); var Ext = require('./ext'); var Methods = require('./methods'); var Protect = require('./protect'); +var Handler = require('./handler'); // Declare internals @@ -58,6 +59,8 @@ exports = module.exports = internals.Pack = function (options) { } this._ext = new Ext(['onPreStart']); + + Handler.register(this); }; @@ -334,7 +337,6 @@ internals.Pack.prototype._register = function (plugin, options, callback, _depen root.handler = function (/* name, method */) { var args = Array.prototype.slice.call(arguments); - return self._handler.apply(self, args); }; diff --git a/lib/proxy.js b/lib/proxy.js index 1af2a4606..cb918607b 100755 --- a/lib/proxy.js +++ b/lib/proxy.js @@ -2,7 +2,6 @@ var Boom = require('boom'); var Nipple = require('nipple'); -var Joi = require('joi'); var Defaults = require('./defaults'); var Utils = require('./utils'); var Response = require('./response'); @@ -16,8 +15,7 @@ var internals = {}; exports.handler = function (route, options) { - var schemaError = Joi.validate(options, Schema.proxyHandlerSchema); - Utils.assert(!schemaError, 'Invalid handler options for proxy:', schemaError); + Schema.assert('proxy handler', options, route.path); Utils.assert(!route.settings.payload || ((route.settings.payload.output === 'data' || route.settings.payload.output === 'stream') && !route.settings.payload.parse), 'Cannot proxy if payload is parsed or if output is not stream or data'); var settings = Utils.applyToDefaults(Defaults.proxy, options); settings.mapUri = options.mapUri || internals.mapUri(options.protocol, options.host, options.port, options.uri); diff --git a/lib/route.js b/lib/route.js index 19b2037ae..e10b4465c 100755 --- a/lib/route.js +++ b/lib/route.js @@ -30,11 +30,8 @@ exports = module.exports = internals.Route = function (options, server, env) { this.settings = Utils.clone(options.config) || {}; this.settings.handler = this.settings.handler || options.handler; - var schemaError = Schema.routeOptions(options); - Utils.assert(!schemaError, 'Invalid route options for', options.path, ':', schemaError); - - schemaError = Schema.routeConfig(this.settings); - Utils.assert(!schemaError, 'Invalid route config for', options.path, ':', schemaError); + Schema.assert('route', options, options.path); + Schema.assert('routeConfig', this.settings, options.path); this.server = server; this.env = env || {}; // Plugin-specific environment diff --git a/lib/schema.js b/lib/schema.js index 41aa9821b..c2cba04d4 100755 --- a/lib/schema.js +++ b/lib/schema.js @@ -9,13 +9,11 @@ var Utils = require('./utils'); var internals = {}; -// Validate server options +exports.assert = function (type, options, message) { -exports.server = function (options) { - - var error = Joi.validate(options, internals.serverSchema); - return (error ? error.annotated() : null); -}; + var error = Joi.validate(options, internals[type]); + Utils.assert(!error, 'Invalid', type, 'options', message ? '(' + message + ')' : '', error && error.annotated()); +} internals.cache = Joi.object({ @@ -49,7 +47,7 @@ internals.viewSchema = function (base) { }; -internals.serverSchema = { +internals.server = { app: Joi.object().allow(null), cache: Joi.alternatives(Joi.string(), internals.cache, Joi.array().includes(internals.cache)).allow(null), cors: Joi.object({ @@ -115,16 +113,7 @@ internals.serverSchema = { }; -// Validate route options - -exports.routeOptions = function (options) { - - var error = Joi.validate(options, internals.routeOptionsSchema); - return (error ? error.annotated() : null); -}; - - -internals.routeOptionsSchema = { +internals.route = { method: Joi.alternatives(Joi.string(), Joi.array().includes(Joi.string()).min(1)).required(), path: Joi.string().required(), vhost: [Joi.string(), Joi.array()], @@ -133,15 +122,6 @@ internals.routeOptionsSchema = { }; -// Validate route config - -exports.routeConfig = function (config) { - - var error = Joi.validate(config, internals.routeConfigSchema); - return (error ? error.annotated() : null); -}; - - internals.pre = [ Joi.string(), Joi.func(), @@ -154,7 +134,7 @@ internals.pre = [ ]; -internals.routeConfigSchema = { +internals.routeConfig = { pre: Joi.array().includes(internals.pre.concat(Joi.array().includes(internals.pre).min(1))), handler: [ Joi.func(), @@ -211,9 +191,7 @@ internals.routeConfigSchema = { }; -// Built in route handler schemas - -exports.directoryHandlerSchema = Joi.object({ +internals['directory handler'] = Joi.object({ path: Joi.alternatives(Joi.string(), Joi.array().includes(Joi.string()), Joi.func()).required(), index: Joi.boolean(), listing: Joi.boolean(), @@ -224,7 +202,7 @@ exports.directoryHandlerSchema = Joi.object({ }); -exports.fileHandlerSchema = [ +internals['file handler'] = [ Joi.string(), Joi.func(), Joi.object({ @@ -236,7 +214,7 @@ exports.fileHandlerSchema = [ ]; -exports.proxyHandlerSchema = Joi.object({ +internals['proxy handler'] = Joi.object({ host: Joi.string().xor('mapUri', 'uri'), port: Joi.number().integer().without('mapUri', 'uri'), protocol: Joi.string().valid('http', 'https', 'http:', 'https:').without('mapUri', 'uri'), @@ -253,7 +231,7 @@ exports.proxyHandlerSchema = Joi.object({ }); -exports.viewHandlerSchema = [ +internals['view handler'] = [ Joi.string(), Joi.object({ template: Joi.string(), @@ -278,14 +256,7 @@ exports.view = function (options) { }; -exports.cache = function (options) { - - var error = Joi.validate(options, internals.cacheSchema); - return (error ? error.annotated() : null); -}; - - -internals.cacheSchema = { +internals.cachePolicy = { cache: Joi.string().allow(null).allow(''), segment: Joi.string(), shared: Joi.boolean(), @@ -296,15 +267,8 @@ internals.cacheSchema = { }; -exports.method = function (options) { - - var error = Joi.validate(options, internals.methodSchema); - return (error ? error.annotated() : null); -}; - - -internals.methodSchema = { +internals.method = { bind: Joi.object().allow(null), generateKey: Joi.func(), - cache: internals.cacheSchema + cache: internals.cachePolicy }; diff --git a/lib/server.js b/lib/server.js index ecf2bff84..c5e7916cd 100755 --- a/lib/server.js +++ b/lib/server.js @@ -15,7 +15,6 @@ var Router = require('./router'); var Schema = require('./schema'); var Views = require('./views'); var Ext = require('./ext'); -var Handler = require('./handler'); var Utils = require('./utils'); // Pack delayed required inline @@ -71,8 +70,7 @@ exports = module.exports = internals.Server = function (/* host, port, options * } this.settings = Utils.applyToDefaults(Defaults.server, args.options || {}); - var schemaError = Schema.server(this.settings); - Utils.assert(!schemaError, 'Invalid server options:', schemaError); + Schema.assert('server', this.settings); this.settings.labels = Utils.unique([].concat(this.settings.labels)); // Convert string to array and removes duplicates @@ -221,8 +219,6 @@ exports = module.exports = internals.Server = function (/* host, port, options * this.info.uri = this.info.protocol + '://' + (this._host || Os.hostname() || 'localhost') + ':' + this.info.port; } } - - Handler.register(this); }; Utils.inherits(internals.Server, Events.EventEmitter); @@ -440,9 +436,7 @@ internals.Server.prototype.views = function (options) { internals.Server.prototype.cache = function (name, options) { - var schemaError = Schema.cache(options); - Utils.assert(!schemaError, 'Invalid cache options for', name, ':', schemaError); - + Schema.assert('cachePolicy', options, name); Utils.assert(!options.segment, 'Cannot override segment name in server cache'); return this.pack._provisionCache(options, 'server', name); }; diff --git a/lib/views.js b/lib/views.js index 062b11939..a5caa968f 100755 --- a/lib/views.js +++ b/lib/views.js @@ -3,7 +3,6 @@ var Fs = require('fs'); var Path = require('path'); var Boom = require('boom'); -var Joi = require('joi'); var Defaults = require('./defaults'); var Schema = require('./schema'); var Utils = require('./utils'); @@ -360,8 +359,7 @@ internals.Manager.prototype._compile = function (template, engine, settings, cal exports.handler = function (route, options) { - var schemaError = Joi.validate(options, Schema.viewHandlerSchema); - Utils.assert(!schemaError, 'Invalid handler options for view:', schemaError); + Schema.assert('view handler', options, route.path); if (typeof options === 'string') { options = { template: options }; diff --git a/test/schema.js b/test/schema.js index d068eb589..ddbe08993 100755 --- a/test/schema.js +++ b/test/schema.js @@ -18,11 +18,6 @@ var after = Lab.after; var describe = Lab.experiment; var it = Lab.test; -var S = Hapi.types.String, - N = Hapi.types.Number, - O = Hapi.types.Object, - B = Hapi.types.Boolean; - describe('Schema', function () { @@ -32,7 +27,10 @@ describe('Schema', function () { var settings = { unknown: true, something: {} }; - expect(Schema.server(settings)).to.exist; + expect(function () { + + Schema.assert('server', settings); + }).to.throw('Invalid server options'); done(); }); @@ -41,7 +39,10 @@ describe('Schema', function () { var server = new Hapi.Server(); server.settings.unknown = true; - expect(Schema.server(server.settings)).to.exist; + expect(function () { + + Schema.assert('server', server.settings); + }).to.throw('Invalid server options'); done(); }); @@ -50,7 +51,10 @@ describe('Schema', function () { var server = new Hapi.Server(); server.settings.router = { unknown: true }; - expect(Schema.server(server.settings)).to.exist; + expect(function () { + + Schema.assert('server', server.settings); + }).to.throw('Invalid server options'); done(); }); @@ -58,18 +62,19 @@ describe('Schema', function () { var server = new Hapi.Server(); - expect(Schema.server(server.settings)).to.not.exist; + expect(function () { + + Schema.assert('server', server.settings); + }).to.not.throw(); done(); }); it('succeeds with only a couple of settings provided', function (done) { - var fn = function () { + expect(function () { var server = new Hapi.Server({ cache: 'catbox-memory' }); - }; - - expect(fn).to.not.throw(Error); + }).to.not.throw(); done(); }); @@ -80,31 +85,40 @@ describe('Schema', function () { var server = new Hapi.Server({ cache: { engine: {}, partition: 'gilden-yak' } }); }; - expect(fn).to.not.throw(Error); + expect(fn).to.not.throw(); done(); }); }); - describe('#routeOptions', function () { + describe('#route', function () { it('fails when unknown properties exist', function (done) { var options = { method: 'GET', path: '/', handler: function () { }, unknown: true }; - expect(Schema.routeOptions(options)).to.exist; + expect(function () { + + Schema.assert('route', options, '/'); + }).to.throw('Invalid route options (/)'); done(); }); it('fails when a required property is missing', function (done) { var options = { method: 'GET' }; - expect(Schema.routeOptions(options)).to.exist; + expect(function () { + + Schema.assert('route', options, '/'); + }).to.throw('Invalid route options (/)'); done(); }); it('succeeds when required fields are present', function (done) { var options = { method: 'GET', path: '/', handler: function () { } }; - expect(Schema.routeOptions(options)).to.not.exist; + expect(function () { + + Schema.assert('route', options, '/'); + }).to.not.throw(); done(); }); @@ -112,7 +126,10 @@ describe('Schema', function () { var options = { method: 'GET', path: '/', handler: function () { }, config: { description: 'here is my description' } }; - expect(Schema.routeOptions(options)).to.not.exist; + expect(function () { + + Schema.assert('route', options, '/'); + }).to.not.throw(); done(); }); }); @@ -122,20 +139,29 @@ describe('Schema', function () { it('fails when config payload has invalid value', function (done) { var config = { payload: 'something' }; - expect(Schema.routeConfig(config)).to.exist; + expect(function () { + + Schema.assert('routeConfig', config, '/'); + }).to.throw('Invalid routeConfig options (/)'); done(); }); it('succeeds when route config has a description', function (done) { var config = { description: 'here is my description' }; - expect(Schema.routeConfig(config)).to.not.exist; + expect(function () { + + Schema.assert('routeConfig', config, '/'); + }).to.not.throw(); done(); }); it('succeeds validating cache config', function (done) { var config = { handler: internals.item, cache: { expiresIn: 20000 } }; - expect(Schema.routeConfig(config)).to.not.exist; + expect(function () { + + Schema.assert('routeConfig', config, '/'); + }).to.not.throw(); done(); }); }); diff --git a/test/server.js b/test/server.js index 2770fec27..782e11187 100755 --- a/test/server.js +++ b/test/server.js @@ -365,7 +365,7 @@ describe('Server', function () { it('rejects request due to high event loop delay load', function (done) { - var server = new Hapi.Server(0, { load: { sampleInterval: 5, maxEventLoopDelay: 1 } }); + var server = new Hapi.Server(0, { load: { sampleInterval: 5, maxEventLoopDelay: 5 } }); var handler = function (request, reply) { var start = Date.now();