Extend decorations. Closes hapijs#3125

lupin72 · Nov 22, 2017 · 307a8d0 · 307a8d0
1 parent 6060fc9
commit 307a8d0
Show file tree

Hide file tree

Showing 8 changed files with 135 additions and 78 deletions.
diff --git a/API.md b/API.md
@@ -1,4 +1,4 @@
-# v17.0.x API Reference
+# v17.1.x API Reference
 
 <!-- toc -->
 
@@ -1448,6 +1448,11 @@ Extends various framework interfaces with custom methods where:
     - `apply` - when the `type` is `'request'`, if `true`, the `method` function is invoked using
       the signature `function(request)` where `request` is the current request object and the
       returned value is assigned as the decoration.
+    - `extend` - if `true`, overrides an existing decoration. The `method` must be a function with
+      the signature `function(existing)` where:
+        - `existing` - is the previously set decoration method value.
+        - must return the new decoration function or value.
+        - cannot be used to extend handler decorations.
 
 Return value: none.
 

diff --git a/README.md b/README.md
@@ -8,7 +8,7 @@ Lead Maintainer: [Eran Hammer](https://github.com/hueniverse)
 authentication, and other essential facilities for building web and services applications. **hapi** enables
 developers to focus on writing reusable application logic in a highly modular and prescriptive approach. 
 
-Development version: **17.0.x** ([release notes](https://github.com/hapijs/hapi/issues?labels=release+notes&page=1&state=closed)) 
+Development version: **17.1.x** ([release notes](https://github.com/hapijs/hapi/issues?labels=release+notes&page=1&state=closed)) 
 [![Build Status](https://secure.travis-ci.org/hapijs/hapi.svg?branch=master)](http://travis-ci.org/hapijs/hapi)
 
 For the latest updates, [change log](http://hapijs.com/updates), and release information visit [hapijs.com](http://hapijs.com) and follow [@hapijs](https://twitter.com/hapijs) on twitter. If you have questions, please open an issue in the

diff --git a/lib/core.js b/lib/core.js
@@ -62,24 +62,21 @@ exports = module.exports = internals.Core = class {
         this.auth = new Auth(this);
         this.caches = new Map();                                                        // Cache clients
         this.compression = new Compression();
-        this.decorations = { handler: [], request: [], server: [], toolkit: [] };
+        this.decorations = { handler: [], request: [], server: [], toolkit: [] };       // Public decoration names
         this.dependencies = [];                                                         // Plugin dependencies
         this.events = new Podium(internals.events);
-        this.handlers = {};                                                             // Registered handlers
         this.heavy = new Heavy(this.settings.load);
         this.instances = new Set();
         this.methods = new Methods(this);                                               // Server methods
         this.mime = new Mimos(this.settings.mime);
         this.registrations = {};                                                        // Tracks plugin for dependency validation { name -> { version } }
-        this.requestor = new Request();
         this.onConnection = null;                                                       // Used to remove event listener on stop
         this.plugins = {};                                                              // Exposed plugin properties by name
         this.app = {};
         this.registring = 0;                                                            // > 0 while register() is waiting for plugin callbacks
         this.requestCounter = { value: internals.counter.min, min: internals.counter.min, max: internals.counter.max };
         this.router = new Call.Router(this.settings.router);
         this.phase = 'stopped';                                                         // 'stopped', 'initializing', 'initialized', 'starting', 'started', 'stopping', 'invalid'
-        this.serverDecorations = {};
         this.sockets = new Set();                                                       // Track open sockets for graceful shutdown
         this.started = false;
         this.states = new Statehood.Definitions(this.settings.state);
@@ -105,6 +102,7 @@ exports = module.exports = internals.Core = class {
         };
 
         this._debug();
+        this._decorations = { handler: {}, request: {}, server: {}, toolkit: {}, requestApply: null };
         this._initializeCache();
 
         this.listener = this._createListener();
@@ -438,7 +436,7 @@ exports = module.exports = internals.Core = class {
 
             // Create request
 
-            const request = this.requestor.request(this.root, req, res, options);
+            const request = Request.generate(this.root, req, res, options);
 
             // Check load
 

diff --git a/lib/handler.js b/lib/handler.js
@@ -90,7 +90,7 @@ exports.defaults = function (method, handler, core) {
 
     if (typeof handler === 'object') {
         const type = Object.keys(handler)[0];
-        const serverHandler = core.handlers[type];
+        const serverHandler = core._decorations.handler[type];
 
         Hoek.assert(serverHandler, 'Unknown handler:', type);
 
@@ -107,7 +107,7 @@ exports.configure = function (handler, route) {
 
     if (typeof handler === 'object') {
         const type = Object.keys(handler)[0];
-        const serverHandler = route._core.handlers[type];
+        const serverHandler = route._core._decorations.handler[type];
 
         Hoek.assert(serverHandler, 'Unknown handler:', type);
 

diff --git a/lib/request.js b/lib/request.js
@@ -17,47 +17,12 @@ const Transmit = require('./transmit');
 // Declare internals
 
 const internals = {
-    properties: ['server', 'url', 'query', 'path', 'method', 'mime', 'setUrl', 'setMethod', 'headers', 'id', 'app', 'plugins', 'route', 'auth', 'pre', 'preResponses', 'info', 'orig', 'params', 'paramsArray', 'payload', 'state', 'jsonp', 'response', 'raw', 'domain', 'log', 'logs', 'generateResponse'],
-    events: Podium.validate(['finish', { name: 'peek', spread: true }, 'disconnect'])
+    events: Podium.validate(['finish', { name: 'peek', spread: true }, 'disconnect']),
+    reserved: ['server', 'url', 'query', 'path', 'method', 'mime', 'setUrl', 'setMethod', 'headers', 'id', 'app', 'plugins', 'route', 'auth', 'pre', 'preResponses', 'info', 'orig', 'params', 'paramsArray', 'payload', 'state', 'jsonp', 'response', 'raw', 'domain', 'log', 'logs', 'generateResponse']
 };
 
 
-exports = module.exports = internals.Generator = class {
-
-    constructor() {
-
-        this._decorations = null;
-    }
-
-    request(server, req, res, options) {
-
-        const request = new internals.Request(server, req, res, options);
-
-        // Decorate
-
-        if (this._decorations) {
-            const properties = Object.keys(this._decorations);
-            for (let i = 0; i < properties.length; ++i) {
-                const property = properties[i];
-                const assignment = this._decorations[property];
-                request[property] = (assignment.apply ? assignment.method(request) : assignment.method);
-            }
-        }
-
-        return request;
-    }
-
-    decorate(property, method, options) {
-
-        Hoek.assert(internals.properties.indexOf(property) === -1, 'Cannot override built-in request interface decoration:', property);
-
-        this._decorations = this._decorations || {};
-        this._decorations[property] = { method, apply: options.apply };
-    }
-};
-
-
-internals.Request = class {
+exports = module.exports = internals.Request = class {
 
     constructor(server, req, res, options) {
 
@@ -112,6 +77,24 @@ internals.Request = class {
         this.setUrl(req.url, this._core.settings.router.stripTrailingSlash);
     }
 
+    static generate(server, req, res, options) {
+
+        const request = new internals.Request(server, req, res, options);
+
+        // Decorate
+
+        if (server._core._decorations.requestApply) {
+            const properties = Object.keys(server._core._decorations.requestApply);
+            for (let i = 0; i < properties.length; ++i) {
+                const property = properties[i];
+                const assignment = server._core._decorations.requestApply[property];
+                request[property] = assignment(request);
+            }
+        }
+
+        return request;
+    }
+
     get events() {
 
         if (!this._events) {
@@ -492,6 +475,9 @@ internals.Request = class {
 };
 
 
+internals.Request.reserved = internals.reserved;
+
+
 internals.info = function (core, req) {
 
     const host = req.headers.host ? req.headers.host.replace(/\s/g, '') : '';

diff --git a/lib/server.js b/lib/server.js
@@ -10,6 +10,7 @@ const Core = require('./core');
 const Cors = require('./cors');
 const Ext = require('./ext');
 const Package = require('../package.json');
+const Request = require('./request');
 const Route = require('./route');
 
 
@@ -77,10 +78,9 @@ internals.Server = class {
 
         // Decorations
 
-        const methods = Object.keys(core.serverDecorations);
-        for (let i = 0; i < methods.length; ++i) {
-            const method = methods[i];
-            this[method] = core.serverDecorations[method];
+        for (let i = 0; i < core.decorations.server.length; ++i) {
+            const method = core.decorations.server[i];
+            this[method] = core._decorations.server[method];
         }
 
         core.registerServer(this);
@@ -108,42 +108,60 @@ internals.Server = class {
         Hoek.assert(property, 'Missing decoration property name');
         Hoek.assert(typeof property === 'string', 'Decoration property must be a string');
         Hoek.assert(property[0] !== '_', 'Property name cannot begin with an underscore:', property);
-        Hoek.assert(this._core.decorations[type].indexOf(property) === -1, `${type[0].toUpperCase() + type.slice(1)} decoration already defined: ${property}`);
+
+        const existing = this._core._decorations[type][property];
+        if (options.extend) {
+            Hoek.assert(type !== 'handler', 'Cannot extent handler decoration:', property);
+            Hoek.assert(existing, `Cannot extend missing ${type} decoration: ${property}`);
+            Hoek.assert(typeof method === 'function', `Extended ${type} decoration method must be a function: ${property}`);
+
+            method = method(existing);
+        }
+        else {
+            Hoek.assert(existing === undefined, `${type[0].toUpperCase() + type.slice(1)} decoration already defined: ${property}`);
+        }
 
         if (type === 'handler') {
 
             // Handler
 
             Hoek.assert(typeof method === 'function', 'Handler must be a function:', property);
             Hoek.assert(!method.defaults || typeof method.defaults === 'object' || typeof method.defaults === 'function', 'Handler defaults property must be an object or function');
-
-            this._core.handlers[property] = method;
+            Hoek.assert(!options.extend, 'Cannot extend handler decoration:', property);
         }
         else if (type === 'request') {
 
             // Request
 
-            this._core.requestor.decorate(property, method, options);
+            Hoek.assert(Request.reserved.indexOf(property) === -1, 'Cannot override built-in request interface decoration:', property);
+
+            if (options.apply) {
+                this._core._decorations.requestApply = this._core._decorations.requestApply || {};
+                this._core._decorations.requestApply[property] = method;
+            }
+            else {
+                Request.prototype[property] = method;
+            }
         }
         else if (type === 'toolkit') {
 
             // Toolkit
 
-            this._core.toolkit.decorate(property, method);
+            Hoek.assert(this._core.toolkit.reserved.indexOf(property) === -1, 'Cannot override built-in toolkit decoration:', property);
         }
         else {
 
             // Server
 
-            Hoek.assert(this[property] === undefined && this._core[property] === undefined, 'Cannot override the built-in server interface method:', property);
+            Hoek.assert(Object.getOwnPropertyNames(internals.Server.prototype).indexOf(property) === -1, 'Cannot override the built-in server interface method:', property);
 
-            this._core.serverDecorations[property] = method;
             this._core.instances.forEach((server) => {
 
                 server[property] = method;
             });
         }
 
+        this._core._decorations[type][property] = method;
         this._core.decorations[type].push(property);
     }
 
@@ -335,7 +353,7 @@ internals.Server = class {
 
         try {
             const items = [].concat(plugins);
-            for (let i = 0; i < items.length; ++i){
+            for (let i = 0; i < items.length; ++i) {
                 let item = items[i];
 
                 /*

diff --git a/lib/toolkit.js b/lib/toolkit.js
@@ -11,26 +11,19 @@ const Response = require('./response');
 
 // Declare internals
 
-const internals = {};
+const internals = {
+    reserved: ['abandon', 'authenticated', 'close', 'context', 'continue', 'entity', 'redirect', 'realm', 'request', 'response', 'state', 'unauthenticated', 'unstate']
+};
 
 
 exports = module.exports = internals.Manager = class {
 
     constructor() {
 
-        this._decorations = null;
-
         this.abandon = Symbol('abandon');
         this.close = Symbol('close');
         this.continue = Symbol('continue');
-    }
-
-    decorate(property, method) {
-
-        Hoek.assert(['abandon', 'authenticated', 'close', 'context', 'continue', 'entity', 'redirect', 'realm', 'request', 'response', 'state', 'unauthenticated', 'unstate'].indexOf(property) === -1, 'Cannot override built-in toolkit decoration:', property);
-
-        this._decorations = this._decorations || {};
-        this._decorations[property] = method;
+        this.reserved = internals.reserved;
     }
 
     async execute(method, request, options) {
@@ -129,7 +122,6 @@ internals.Toolkit = class {
 
     constructor(request, manager, options) {
 
-
         this.abandon = manager.abandon;
         this.close = manager.close;
         this.continue = manager.continue;
@@ -142,12 +134,9 @@ internals.Toolkit = class {
             this.unauthenticated = internals.unauthenticated;
         }
 
-        if (manager._decorations) {
-            const methods = Object.keys(manager._decorations);
-            for (let i = 0; i < methods.length; ++i) {
-                const method = methods[i];
-                this[method] = manager._decorations[method];
-            }
+        for (let i = 0; i < request._core.decorations.toolkit.length; ++i) {
+            const method = request._core.decorations.toolkit[i];
+            this[method] = request._core._decorations.toolkit[method];
         }
     }