From 6412993f8d523fa6435d76a3e70e63d3712e6158 Mon Sep 17 00:00:00 2001 From: David Luecke Date: Fri, 6 Mar 2015 16:49:37 -0700 Subject: [PATCH 1/2] Start for normalizer. --- lib/mixins/index.js | 3 ++- lib/mixins/normalizer.js | 21 +++++++++++++++++++++ test/application.test.js | 4 ++-- test/mixins/normalizer.test.js | 26 ++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 3 deletions(-) create mode 100644 lib/mixins/normalizer.js create mode 100644 test/mixins/normalizer.test.js diff --git a/lib/mixins/index.js b/lib/mixins/index.js index 1e3eff11a6..b59a7e7af4 100644 --- a/lib/mixins/index.js +++ b/lib/mixins/index.js @@ -3,6 +3,7 @@ module.exports = function() { return [ require('./promise'), - require('./event') + require('./event'), + require('./normalizer') ]; }; diff --git a/lib/mixins/normalizer.js b/lib/mixins/normalizer.js new file mode 100644 index 0000000000..e52adae11a --- /dev/null +++ b/lib/mixins/normalizer.js @@ -0,0 +1,21 @@ +var _ = require('lodash'); +var commons = require('feathers-commons'); + +function makeNormalizer(method) { + return function() { + var args = commons.getArguments(method, arguments); + return this._super.apply(this, args); + }; +} + +module.exports = function (service) { + if (typeof service.mixin === 'function') { + var mixin = _.transform(_.pick(service, this.methods), function(result, value, key) { + if(typeof value === 'function') { + result[key] = makeNormalizer(key); + } + }); + + service.mixin(mixin); + } +}; \ No newline at end of file diff --git a/test/application.test.js b/test/application.test.js index 0dff4714ed..6ac603bb77 100644 --- a/test/application.test.js +++ b/test/application.test.js @@ -296,10 +296,10 @@ describe('Feathers application', function () { it('mixins are unique to one application', function() { var app = feathers(); app.mixins.push(function() {}); - assert.equal(app.mixins.length, 3); + assert.equal(app.mixins.length, 4); var otherApp = feathers(); otherApp.mixins.push(function() {}); - assert.equal(otherApp.mixins.length, 3); + assert.equal(otherApp.mixins.length, 4); }); }); diff --git a/test/mixins/normalizer.test.js b/test/mixins/normalizer.test.js new file mode 100644 index 0000000000..2d424216e9 --- /dev/null +++ b/test/mixins/normalizer.test.js @@ -0,0 +1,26 @@ +'use strict'; + +var assert = require('assert'); +var Proto = require('uberproto'); +var normalizer = require('../../lib/mixins/normalizer'); + +describe('Argument normalizer mixin', function () { + it('normalizes findAll', function (done) { + var context = { + methods: ['find'] + }; + var FixtureService = Proto.extend({ + find: function(params, callback) { + assert.ok(typeof callback === 'function'); + assert.equal(params.test, 'Here'); + done(); + } + }); + + normalizer.call(context, FixtureService); + + var instance = Proto.create.call(FixtureService); + + instance.find({ test: 'Here' }); + }); +}); From 4fbf8080576726ceac816a253b5b3452b4b23b5d Mon Sep 17 00:00:00 2001 From: David Luecke Date: Sat, 7 Mar 2015 12:14:55 -0700 Subject: [PATCH 2/2] Adding normalizer mixin to always normalize service method calls. --- lib/mixins/index.js | 13 +++++++++++- lib/mixins/normalizer.js | 22 +++++++++---------- lib/mixins/promise.js | 33 ++++++++++++++-------------- test/mixins/normalizer.test.js | 39 +++++++++++++++++++++++++++++++++- 4 files changed, 77 insertions(+), 30 deletions(-) diff --git a/lib/mixins/index.js b/lib/mixins/index.js index b59a7e7af4..acc65f178e 100644 --- a/lib/mixins/index.js +++ b/lib/mixins/index.js @@ -1,9 +1,20 @@ 'use strict'; +var _ = require('lodash'); + module.exports = function() { - return [ + var mixins = [ require('./promise'), require('./event'), require('./normalizer') ]; + + // Override push to make sure that normalize is always the last + mixins.push = function() { + var args = [ this.length - 1, 0].concat(_.toArray(arguments)); + this.splice.apply(this, args); + return this.length; + }; + + return mixins; }; diff --git a/lib/mixins/normalizer.js b/lib/mixins/normalizer.js index e52adae11a..aaa6b01c7a 100644 --- a/lib/mixins/normalizer.js +++ b/lib/mixins/normalizer.js @@ -1,21 +1,19 @@ var _ = require('lodash'); -var commons = require('feathers-commons'); - -function makeNormalizer(method) { - return function() { - var args = commons.getArguments(method, arguments); - return this._super.apply(this, args); - }; -} +var getArguments = require('feathers-commons').getArguments; module.exports = function (service) { if (typeof service.mixin === 'function') { - var mixin = _.transform(_.pick(service, this.methods), function(result, value, key) { - if(typeof value === 'function') { - result[key] = makeNormalizer(key); + var mixin = {}; + + _.each(this.methods, function(method) { + if(typeof service[method] === 'function') { + mixin[method] = function() { + var args = getArguments(method, arguments); + return this._super.apply(this, args); + }; } }); service.mixin(mixin); } -}; \ No newline at end of file +}; diff --git a/lib/mixins/promise.js b/lib/mixins/promise.js index 4449250614..71a2551763 100644 --- a/lib/mixins/promise.js +++ b/lib/mixins/promise.js @@ -1,27 +1,28 @@ 'use strict'; var _ = require('lodash'); -var makeWrapper = function() { - return function() { - var result = this._super.apply(this, arguments); - var callback = arguments[arguments.length - 1]; - if(typeof result !== 'undefined' && _.isFunction(result.then) && _.isFunction(callback)) { - result.then(function(data) { - callback(null, data); - }, function(error) { - callback(error); - }); - } - return result; - }; +var wrapper = function () { + var result = this._super.apply(this, arguments); + var callback = arguments[arguments.length - 1]; + + if(typeof result !== 'undefined' && _.isFunction(result.then) && _.isFunction(callback)) { + result.then(function(data) { + callback(null, data); + }, function(error) { + callback(error); + }); + } + return result; }; module.exports = function (service) { if (typeof service.mixin === 'function') { - var mixin = _.transform(_.pick(service, this.methods), function(result, value, key) { - if(typeof value === 'function') { - result[key] = makeWrapper(); + var mixin = {}; + + _.each(this.methods, function(method) { + if(typeof service[method] === 'function') { + mixin[method] = wrapper; } }); diff --git a/test/mixins/normalizer.test.js b/test/mixins/normalizer.test.js index 2d424216e9..e4a6c37c4d 100644 --- a/test/mixins/normalizer.test.js +++ b/test/mixins/normalizer.test.js @@ -3,9 +3,25 @@ var assert = require('assert'); var Proto = require('uberproto'); var normalizer = require('../../lib/mixins/normalizer'); +var mixins = require('../../lib/mixins'); describe('Argument normalizer mixin', function () { - it('normalizes findAll', function (done) { + it('normalizer mixin is always the last to run', function() { + var arr = mixins(); + var dummy = function() { }; + + assert.equal(arr.length, 3); + + arr.push(dummy); + + assert.equal(arr[arr.length - 1], normalizer, 'Last mixin is still the normalizer'); + assert.equal(arr[arr.length - 2], dummy, 'Dummy got added before last'); + }); + + // The normalization is already tests in all variations in `getArguments` + // so we just so we only test two random samples + + it('normalizes .find without a callback', function (done) { var context = { methods: ['find'] }; @@ -23,4 +39,25 @@ describe('Argument normalizer mixin', function () { instance.find({ test: 'Here' }); }); + + it('normalizes .update without params and callback', function (done) { + var context = { + methods: ['update'] + }; + var FixtureService = Proto.extend({ + update: function(id, data, params, callback) { + assert.equal(id, 1); + assert.ok(typeof callback === 'function'); + assert.deepEqual(data, { test: 'Here' }); + assert.deepEqual(params, {}); + done(); + } + }); + + normalizer.call(context, FixtureService); + + var instance = Proto.create.call(FixtureService); + + instance.update(1, { test: 'Here' }); + }); });