Merge pull request #1919 from hapijs/issue/1917

Log method pre string notation

docs/Reference.md

@@ -1166,7 +1166,7 @@ Methods are registered via `server.method(name, fn, [options])` where:
   for the previous example via `server.methods.utils.users.get`.
 - `fn` - the method function with the signature is `function(arg1, arg2, ..., argn, next)` where:
     - `arg1`, `arg2`, etc. - the method function arguments.
-    - `next` - the function called when the method is done with the signature `function(err, result, isUncacheable)` where:
+    - `next` - the function called when the method is done with the signature `function(err, result, ttl)` where:
         - `err` - error response if the method failed.
         - `result` - the return value.
         - `ttl` - `0` if result is valid but cannot be cached. Defaults to cache policy.

lib/handler.js

@@ -206,7 +206,7 @@ exports.configure = function (handler, route) {
     }
 
     if (typeof handler === 'string') {
-        var parsed = internals.fromString(handler, route.server);
+        var parsed = internals.fromString('handler', handler, route.server);
         return parsed.method;
     }
 
@@ -265,7 +265,7 @@ exports.prerequisites = function (config, server) {
             };
 
             if (typeof item.method === 'string') {
-                var parsed = internals.fromString(item.method, server);
+                var parsed = internals.fromString('pre', item.method, server);
                 item.method = parsed.method;
                 item.assign = item.assign || parsed.name;
             }
@@ -280,7 +280,7 @@ exports.prerequisites = function (config, server) {
 };
 
 
-internals.fromString = function (notation, server) {
+internals.fromString = function (type, notation, server) {
 
     //                                  1:name            2:(        3:arguments
     var methodParts = notation.match(/^([\w\.]+)(?:\s*)(?:(\()(?:\s*)(\w+(?:\.\w+)*(?:\s*\,\s*\w+(?:\.\w+)*)*)?(?:\s*)\))?$/);
@@ -298,8 +298,14 @@ internals.fromString = function (notation, server) {
 
     result.method = function (request, reply) {
 
+        var finalize = function (err, value, cached, report) {
+
+            request.log(['hapi', type, 'method', name], report);
+            return reply(err, value);
+        };
+
         if (!argsNotation) {
-            return method.call(null, request, reply);
+            return method.call(null, request, finalize);
         }
 
         var args = [];
@@ -310,7 +316,7 @@ internals.fromString = function (notation, server) {
             }
         }
 
-        args.push(reply);
+        args.push(finalize);
         method.apply(null, args);
     };
 

lib/methods.js

@@ -1,6 +1,7 @@
 // Load modules
 
 var Boom = require('boom');
+var Catbox = require('catbox');
 var Hoek = require('hoek');
 var Schema = require('./schema');
 
@@ -54,23 +55,18 @@ internals.Methods.prototype._add = function (name, fn, options, env) {
 
     // Create method
 
-    var cache = null;
-    if (settings.cache) {
-        settings.cache.generateFunc = function (id, next) {
+    settings.cache = settings.cache || {};
+    settings.cache.generateFunc = function (id, next) {
 
-            id.args[id.args.length - 1] = next;                 // function (err, result, ttl)
-            fn.apply(bind, id.args);
-        };
+        id.args[id.args.length - 1] = next;                 // function (err, result, ttl)
+        fn.apply(bind, id.args);
+    };
 
-        cache = this.pack._provisionCache(settings.cache, 'method', name, settings.cache.segment);
-    }
+    var cache = (settings.cache.expiresIn || settings.cache.expiresAt ? this.pack._provisionCache(settings.cache, 'method', name, settings.cache.segment)
+                                                                      : new Catbox.Policy(settings.cache));
 
     var method = function (/* arguments, methodNext */) {
 
-        if (!cache) {
-            return fn.apply(bind, arguments);
-        }
-
         var args = arguments;
         var methodNext = args[args.length - 1];
 
@@ -83,21 +79,19 @@ internals.Methods.prototype._add = function (name, fn, options, env) {
         cache.get({ id: key, args: args }, methodNext);
     };
 
-    if (cache) {
-        method.cache = {
-            drop: function (/* arguments, callback */) {
-
-                var dropCallback = arguments[arguments.length - 1];
+    method.cache = {
+        drop: function (/* arguments, callback */) {
 
-                var key = settings.generateKey.apply(null, arguments);
-                if (key === null) {                             // Value can be ''
-                    return Hoek.nextTick(dropCallback)(Boom.badImplementation('Invalid method key'));
-                }
+            var dropCallback = arguments[arguments.length - 1];
 
-                return cache.drop(key, dropCallback);
+            var key = settings.generateKey.apply(null, arguments);
+            if (key === null) {                             // Value can be ''
+                return Hoek.nextTick(dropCallback)(Boom.badImplementation('Invalid method key'));
             }
-        };
-    }
+
+            return cache.drop(key, dropCallback);
+        }
+    };
 
     // create method path
 

lib/pack.js

@@ -604,9 +604,7 @@ internals.Pack.prototype._provisionCache = function (options, type, name, segmen
     Hoek.assert(cache.shared || options.shared || !cache.segments[segment], 'Cannot provision the same cache segment more than once');
     cache.segments[segment] = true;
 
-    var settings = (options.expiresIn || options.expiresAt ? options : {});
-    var policy = new Catbox.Policy(settings, cache.client, segment);
-    return policy;
+    return new Catbox.Policy(options, cache.client, segment);
 };
 
 

package.json

@@ -19,7 +19,7 @@
   },
   "dependencies": {
     "boom": "^2.4.x",
-    "catbox": "^3.2.x",
+    "catbox": "^3.3.x",
     "catbox-memory": "1.x.x",
     "cryptiles": "2.x.x",
     "hoek": "^2.3.x",

test/handler.js

@@ -753,5 +753,36 @@ describe('Handler', function () {
             done();
         });
     });
+
+    it('logs server method using string notation', function (done) {
+
+        var server = new Hapi.Server();
+
+        server.method('user', function (id, next) {
+
+            return next(null, { id: id, name: 'Bob' });
+        });
+
+        server.route({
+            method: 'GET',
+            path: '/user/{id}',
+            config: {
+                pre: [
+                    'user(params.id)'
+                ],
+                handler: function (request, reply) {
+
+                    return reply(request.getLog('method'));
+                }
+            }
+        });
+
+        server.inject('/user/5', function (res) {
+
+            expect(res.result[0].tags).to.deep.equal(['hapi', 'pre', 'method', 'user']);
+            expect(res.result[0].data.msec).to.exist;
+            done();
+        });
+    });
 });
 

test/pack.js

@@ -1279,7 +1279,7 @@ describe('Pack', function () {
             name: 'test',
             register: function (plugin, options, next) {
 
-                plugin.method('log', function () { });
+                plugin.method('log', function (methodNext) { return methodNext(null); });
                 return next();
             }
         };
@@ -1300,16 +1300,19 @@ describe('Pack', function () {
             register: function (plugin, options, next) {
 
                 plugin.bind({ x: 1 });
-                plugin.method('log', function () { return this.x; });
+                plugin.method('log', function (methodNext) { return methodNext(null, this.x); });
                 return next();
             }
         };
 
         server.pack.register(plugin, function (err) {
 
             expect(err).to.not.exist;
-            expect(server.methods.log()).to.equal(1);
-            done();
+            server.methods.log(function (err, result) {
+
+                expect(result).to.equal(1);
+                done();
+            });
         });
     });
 
@@ -1321,16 +1324,19 @@ describe('Pack', function () {
             name: 'test',
             register: function (plugin, options, next) {
 
-                plugin.method('log', function () { return this.x; }, { bind: { x: 2 } });
+                plugin.method('log', function (methodNext) { return methodNext(null, this.x); }, { bind: { x: 2 } });
                 return next();
             }
         };
 
         server.pack.register(plugin, function (err) {
 
             expect(err).to.not.exist;
-            expect(server.methods.log()).to.equal(2);
-            done();
+            server.methods.log(function (err, result) {
+
+                expect(result).to.equal(2);
+                done();
+            });
         });
     });
 
@@ -1343,16 +1349,19 @@ describe('Pack', function () {
             register: function (plugin, options, next) {
 
                 plugin.bind({ x: 1 });
-                plugin.method('log', function () { return this.x; }, { bind: { x: 2 } });
+                plugin.method('log', function (methodNext) { return methodNext(null, this.x); }, { bind: { x: 2 } });
                 return next();
             }
         };
 
         server.pack.register(plugin, function (err) {
 
             expect(err).to.not.exist;
-            expect(server.methods.log()).to.equal(2);
-            done();
+            server.methods.log(function (err, result) {
+
+                expect(result).to.equal(2);
+                done();
+            });
         });
     });