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.

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

Log method pre string notation #1919

Merged
merged 1 commit into from
Sep 8, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion docs/Reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
16 changes: 11 additions & 5 deletions lib/handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}
Expand All @@ -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*)\))?$/);
Expand All @@ -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 = [];
Expand All @@ -310,7 +316,7 @@ internals.fromString = function (notation, server) {
}
}

args.push(reply);
args.push(finalize);
method.apply(null, args);
};

Expand Down
42 changes: 18 additions & 24 deletions lib/methods.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Load modules

var Boom = require('boom');
var Catbox = require('catbox');
var Hoek = require('hoek');
var Schema = require('./schema');

Expand Down Expand Up @@ -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];

Expand All @@ -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

Expand Down
4 changes: 1 addition & 3 deletions lib/pack.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};


Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
31 changes: 31 additions & 0 deletions test/handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
});

29 changes: 19 additions & 10 deletions test/pack.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
};
Expand All @@ -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();
});
});
});

Expand All @@ -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();
});
});
});

Expand All @@ -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();
});
});
});

Expand Down