From ee106e4e15f9471345f9f488ea1ec89377439dcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 20 Sep 2016 09:51:50 +0200 Subject: [PATCH 1/3] Implement new http arg mapping optionsFromRequest Define a new Model method "createOptionsFromRemotingContext" that allows models to define what "options" should be passed to methods invoked via strong-remoting (e.g. REST). Define a new http mapping `http: 'optionsFromRequest'` that invokes `Model.createOptionsFromRemotingContext` to build the value from remoting context. This should provide enough infrastructure for components and applications to implement their own ways of building the "options" object. --- lib/model.js | 70 ++++++++++++++++++++++++++++++ test/model.test.js | 104 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 174 insertions(+) diff --git a/lib/model.js b/lib/model.js index a85f4e2f..15bc18a8 100644 --- a/lib/model.js +++ b/lib/model.js @@ -428,9 +428,39 @@ module.exports = function(registry) { if (options.isStatic === undefined) { options.isStatic = true; } + + if (options.accepts) { + options = extend({}, options); + options.accepts = setupOptionsArgs(options.accepts); + } + this.sharedClass.defineMethod(name, options); }; + function setupOptionsArgs(accepts) { + if (!Array.isArray(accepts)) + accepts = [accepts]; + + return accepts.map(function(arg) { + if (arg.http && arg.http === 'optionsFromRequest') { + // deep clone to preserve the input value + arg = extend({}, arg); + arg.http = createOptionsViaModelMethod; + } + return arg; + }); + } + + function createOptionsViaModelMethod(ctx) { + var EMPTY_OPTIONS = {}; + var ModelCtor = ctx.method && ctx.method.ctor; + if (!ModelCtor) + return EMPTY_OPTIONS; + if (typeof ModelCtor.createOptionsFromRemotingContext !== 'function') + return EMPTY_OPTIONS; + return ModelCtor.createOptionsFromRemotingContext(ctx); + } + /** * Disable remote invocation for the method with the given name. * @@ -869,6 +899,46 @@ module.exports = function(registry) { Model.ValidationError = require('loopback-datasource-juggler').ValidationError; + /** + * Create "options" value to use when invoking model methods + * via strong-remoting (e.g. REST). + * + * Example + * + * ```js + * MyModel.myMethod = function(options, cb) { + * // by default, options contains only one property "accessToken" + * var accessToken = options && options.accessToken; + * var userId = accessToken && accessToken.userId; + * var message = 'Hello ' + (userId ? 'user #' + userId : 'anonymous'); + * cb(null, message); + * }); + * + * MyModel.remoteMethod('myMethod', { + * accepts: { + * arg: 'options', + * type: 'object', + * // "optionsFromRequest" is a loopback-specific HTTP mapping that + * // calls Model's createOptionsFromRemotingContext + * // to build the argument value + * http: 'optionsFromRequest' + * }, + * returns: { + * arg: 'message', + * type: 'string' + * } + * }); + * ``` + * + * @param {Object} ctx A strong-remoting Context instance + * @returns {Object} The value to pass to "options" argument. + */ + Model.createOptionsFromRemotingContext = function(ctx) { + return { + accessToken: ctx.req.accessToken, + }; + }; + // setup the initial model Model.setup(); diff --git a/test/model.test.js b/test/model.test.js index 87d1c5fb..2ed1f0f2 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -786,4 +786,108 @@ describe.onServer('Remote Methods', function() { // fails on time-out when not implemented correctly }); }); + + describe('Model.createOptionsFromRemotingContext', function() { + var app, TestModel, accessToken, userId, actualOptions; + + before(setupAppAndRequest); + before(createUserAndAccessToken); + + it('sets empty options.accessToken for anonymous requests', function(done) { + request(app).get('/TestModels/saveOptions') + .expect(204, function(err) { + if (err) return done(err); + expect(actualOptions).to.eql({accessToken: null}); + done(); + }); + }); + + it('sets options.accessToken for authorized requests', function(done) { + request(app).get('/TestModels/saveOptions') + .set('Authorization', accessToken.id) + .expect(204, function(err) { + if (err) return done(err); + expect(actualOptions).to.have.property('accessToken'); + expect(actualOptions.accessToken.toObject()) + .to.eql(accessToken.toObject()); + done(); + }); + }); + + it('allows "beforeRemote" hooks to contribute options', function(done) { + TestModel.beforeRemote('saveOptions', function(ctx, unused, next) { + ctx.args.options.hooked = true; + next(); + }); + + request(app).get('/TestModels/saveOptions') + .expect(204, function(err) { + if (err) return done(err); + expect(actualOptions).to.have.property('hooked', true); + done(); + }); + }); + + it('allows apps to add options before remoting hooks', function(done) { + TestModel.createOptionsFromRemotingContext = function(ctx) { + return {hooks: []}; + }; + + TestModel.beforeRemote('saveOptions', function(ctx, unused, next) { + ctx.args.options.hooks.push('beforeRemote'); + next(); + }); + + // In real apps, this code can live in a component or in a boot script + app.remotes().phases + .addBefore('invoke', 'options-from-request') + .use(function(ctx, next) { + ctx.args.options.hooks.push('custom'); + next(); + }); + + request(app).get('/TestModels/saveOptions') + .expect(204, function(err) { + if (err) return done(err); + expect(actualOptions.hooks).to.eql(['custom', 'beforeRemote']); + done(); + }); + }); + + function setupAppAndRequest() { + app = loopback({localRegistry: true, loadBuiltinModels: true}); + + app.dataSource('db', {connector: 'memory'}); + + TestModel = app.registry.createModel('TestModel', {base: 'Model'}); + TestModel.saveOptions = function(options, cb) { + actualOptions = options; + cb(); + }; + + TestModel.remoteMethod('saveOptions', { + accepts: {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + http: {verb: 'GET', path: '/saveOptions'}, + }); + + app.model(TestModel, {dataSource: null}); + + app.enableAuth({dataSource: 'db'}); + + app.use(loopback.token()); + app.use(loopback.rest()); + } + + function createUserAndAccessToken() { + var CREDENTIALS = {email: 'context@example.com', password: 'pass'}; + var User = app.registry.getModel('User'); + return User.create(CREDENTIALS) + .then(function(u) { + return User.login(CREDENTIALS); + }).then(function(token) { + accessToken = token; + userId = token.userId; + }); + } + }); }); From 693d52fc59583c5f9e0750b27051ddcccdc7238a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Tue, 20 Sep 2016 10:54:41 +0200 Subject: [PATCH 2/3] Contextify DAO and relation methods Modify remoting metadata of data-access methods in PersistedModel and relation method in Model and add an "options" argument to "accepts" list. --- lib/model.js | 157 ++++++++++--- lib/persisted-model.js | 112 ++++++--- package.json | 3 +- test/context-options.test.js | 411 ++++++++++++++++++++++++++++++++++ test/remote-connector.test.js | 2 +- test/remoting.integration.js | 4 +- 6 files changed, 622 insertions(+), 67 deletions(-) create mode 100644 test/context-options.test.js diff --git a/lib/model.js b/lib/model.js index 15bc18a8..05d11493 100644 --- a/lib/model.js +++ b/lib/model.js @@ -10,6 +10,7 @@ var g = require('strong-globalize')(); var assert = require('assert'); +var debug = require('debug')('loopback:model'); var RemoteObjects = require('strong-remoting'); var SharedClass = require('strong-remoting').SharedClass; var extend = require('util')._extend; @@ -141,15 +142,29 @@ module.exports = function(registry) { }); // support remoting prototype methods - ModelCtor.sharedCtor = function(data, id, fn) { + ModelCtor.sharedCtor = function(data, id, options, fn) { var ModelCtor = this; - if (typeof data === 'function') { + var isRemoteInvocationWithOptions = typeof data !== 'object' && + typeof id === 'object' && + typeof options === 'function'; + if (isRemoteInvocationWithOptions) { + // sharedCtor(id, options, fn) + fn = options; + options = id; + id = data; + data = null; + } else if (typeof data === 'function') { + // sharedCtor(fn) fn = data; data = null; id = null; + options = null; } else if (typeof id === 'function') { + // sharedCtor(data, fn) + // sharedCtor(id, fn) fn = id; + options = null; if (typeof data !== 'object') { id = data; @@ -166,7 +181,8 @@ module.exports = function(registry) { } else if (data) { fn(null, new ModelCtor(data)); } else if (id) { - ModelCtor.findById(id, function(err, model) { + var filter = {}; + ModelCtor.findById(id, filter, options, function(err, model) { if (err) { fn(err); } else if (model) { @@ -186,8 +202,9 @@ module.exports = function(registry) { var idDesc = ModelCtor.modelName + ' id'; ModelCtor.sharedCtor.accepts = [ {arg: 'id', type: 'any', required: true, http: {source: 'path'}, - description: idDesc} + description: idDesc}, // {arg: 'instance', type: 'object', http: {source: 'body'}} + {arg: 'options', type: 'object', http: createOptionsViaModelMethod}, ]; ModelCtor.sharedCtor.http = [ @@ -238,6 +255,14 @@ module.exports = function(registry) { sharedClass.resolve(function resolver(define) { var relations = ModelCtor.relations || {}; + var defineRaw = define; + define = function(name, options, fn) { + if (options.accepts) { + options = extend({}, options); + options.accepts = setupOptionsArgs(options.accepts); + } + defineRaw(name, options, fn); + }; // get the relations for (var relationName in relations) { @@ -443,7 +468,7 @@ module.exports = function(registry) { return accepts.map(function(arg) { if (arg.http && arg.http === 'optionsFromRequest') { - // deep clone to preserve the input value + // clone to preserve the input value arg = extend({}, arg); arg.http = createOptionsViaModelMethod; } @@ -458,6 +483,7 @@ module.exports = function(registry) { return EMPTY_OPTIONS; if (typeof ModelCtor.createOptionsFromRemotingContext !== 'function') return EMPTY_OPTIONS; + debug('createOptionsFromRemotingContext for %s', ctx.method.stringName); return ModelCtor.createOptionsFromRemotingContext(ctx); } @@ -496,7 +522,10 @@ module.exports = function(registry) { define('__get__' + relationName, { isStatic: false, http: {verb: 'get', path: '/' + pathName}, - accepts: {arg: 'refresh', type: 'boolean', http: {source: 'query'}}, + accepts: [ + {arg: 'refresh', type: 'boolean', http: {source: 'query'}}, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], accessType: 'READ', description: format('Fetches belongsTo relation %s.', relationName), returns: {arg: relationName, type: modelName, root: true}, @@ -521,7 +550,10 @@ module.exports = function(registry) { define('__get__' + relationName, { isStatic: false, http: {verb: 'get', path: '/' + pathName}, - accepts: {arg: 'refresh', type: 'boolean', http: {source: 'query'}}, + accepts: [ + {arg: 'refresh', type: 'boolean', http: {source: 'query'}}, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], description: format('Fetches hasOne relation %s.', relationName), accessType: 'READ', returns: {arg: relationName, type: relation.modelTo.modelName, root: true}, @@ -531,7 +563,13 @@ module.exports = function(registry) { define('__create__' + relationName, { isStatic: false, http: {verb: 'post', path: '/' + pathName}, - accepts: {arg: 'data', type: 'object', model: toModelName, http: {source: 'body'}}, + accepts: [ + { + arg: 'data', type: 'object', model: toModelName, + http: {source: 'body'}, + }, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], description: format('Creates a new instance in %s of this model.', relationName), accessType: 'WRITE', returns: {arg: 'data', type: toModelName, root: true} @@ -540,7 +578,13 @@ module.exports = function(registry) { define('__update__' + relationName, { isStatic: false, http: {verb: 'put', path: '/' + pathName}, - accepts: {arg: 'data', type: 'object', model: toModelName, http: {source: 'body'}}, + accepts: [ + { + arg: 'data', type: 'object', model: toModelName, + http: {source: 'body'}, + }, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], description: format('Update %s of this model.', relationName), accessType: 'WRITE', returns: {arg: 'data', type: toModelName, root: true} @@ -549,6 +593,9 @@ module.exports = function(registry) { define('__destroy__' + relationName, { isStatic: false, http: {verb: 'delete', path: '/' + pathName}, + accepts: [ + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], description: format('Deletes %s of this model.', relationName), accessType: 'WRITE', }); @@ -562,10 +609,15 @@ module.exports = function(registry) { define('__findById__' + relationName, { isStatic: false, http: {verb: 'get', path: '/' + pathName + '/:fk'}, - accepts: {arg: 'fk', type: 'any', - description: format('Foreign key for %s', relationName), - required: true, - http: {source: 'path'}}, + accepts: [ + { + arg: 'fk', type: 'any', + description: format('Foreign key for %s', relationName), + required: true, + http: {source: 'path'}, + }, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], description: format('Find a related item by id for %s.', relationName), accessType: 'READ', returns: {arg: 'result', type: toModelName, root: true}, @@ -576,10 +628,15 @@ module.exports = function(registry) { define('__destroyById__' + relationName, { isStatic: false, http: {verb: 'delete', path: '/' + pathName + '/:fk'}, - accepts: { arg: 'fk', type: 'any', - description: format('Foreign key for %s', relationName), - required: true, - http: {source: 'path'}}, + accepts: [ + { + arg: 'fk', type: 'any', + description: format('Foreign key for %s', relationName), + required: true, + http: {source: 'path'}, + }, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], description: format('Delete a related item by id for %s.', relationName), accessType: 'WRITE', returns: [] @@ -595,6 +652,7 @@ module.exports = function(registry) { required: true, http: { source: 'path' }}, {arg: 'data', type: 'object', model: toModelName, http: {source: 'body'}}, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, ], description: format('Update a related item by id for %s.', relationName), accessType: 'WRITE', @@ -617,7 +675,10 @@ module.exports = function(registry) { accepts: [{ arg: 'fk', type: 'any', description: format('Foreign key for %s', relationName), required: true, - http: {source: 'path'}}].concat(accepts), + http: {source: 'path'}}, + ].concat(accepts).concat([ + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ]), description: format('Add a related item by id for %s.', relationName), accessType: 'WRITE', returns: {arg: relationName, type: modelThrough.modelName, root: true} @@ -627,10 +688,15 @@ module.exports = function(registry) { define('__unlink__' + relationName, { isStatic: false, http: {verb: 'delete', path: '/' + pathName + '/rel/:fk'}, - accepts: {arg: 'fk', type: 'any', - description: format('Foreign key for %s', relationName), - required: true, - http: {source: 'path'}}, + accepts: [ + { + arg: 'fk', type: 'any', + description: format('Foreign key for %s', relationName), + required: true, + http: {source: 'path'}, + }, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], description: format('Remove the %s relation to an item by id.', relationName), accessType: 'WRITE', returns: [] @@ -642,10 +708,15 @@ module.exports = function(registry) { define('__exists__' + relationName, { isStatic: false, http: {verb: 'head', path: '/' + pathName + '/rel/:fk'}, - accepts: {arg: 'fk', type: 'any', - description: format('Foreign key for %s', relationName), - required: true, - http: {source: 'path'}}, + accepts: [ + { + arg: 'fk', type: 'any', + description: format('Foreign key for %s', relationName), + required: true, + http: {source: 'path'}, + }, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], description: format('Check the existence of %s relation to an item by id.', relationName), accessType: 'READ', returns: {arg: 'exists', type: 'boolean', root: true}, @@ -688,7 +759,10 @@ module.exports = function(registry) { define('__get__' + scopeName, { isStatic: isStatic, http: {verb: 'get', path: '/' + pathName}, - accepts: {arg: 'filter', type: 'object'}, + accepts: [ + {arg: 'filter', type: 'object'}, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], description: format('Queries %s of %s.', scopeName, this.modelName), accessType: 'READ', returns: {arg: scopeName, type: [toModelName], root: true} @@ -697,7 +771,15 @@ module.exports = function(registry) { define('__create__' + scopeName, { isStatic: isStatic, http: {verb: 'post', path: '/' + pathName}, - accepts: {arg: 'data', type: 'object', model: toModelName, http: {source: 'body'}}, + accepts: [ + { + arg: 'data', + type: 'object', + model: toModelName, + http: {source: 'body'}, + }, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], description: format('Creates a new instance in %s of this model.', scopeName), accessType: 'WRITE', returns: {arg: 'data', type: toModelName, root: true} @@ -706,6 +788,16 @@ module.exports = function(registry) { define('__delete__' + scopeName, { isStatic: isStatic, http: {verb: 'delete', path: '/' + pathName}, + accepts: [ + { + arg: 'where', type: 'object', + // The "where" argument is not exposed in the REST API + // but we need to provide a value so that we can pass "options" + // as the third argument. + http: function(ctx) { return undefined; }, + }, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], description: format('Deletes all %s of this model.', scopeName), accessType: 'WRITE', }); @@ -713,8 +805,13 @@ module.exports = function(registry) { define('__count__' + scopeName, { isStatic: isStatic, http: {verb: 'get', path: '/' + pathName + '/count'}, - accepts: {arg: 'where', type: 'object', - description: 'Criteria to match model instances'}, + accepts: [ + { + arg: 'where', type: 'object', + description: 'Criteria to match model instances', + }, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], description: format('Counts %s of %s.', scopeName, this.modelName), accessType: 'READ', returns: {arg: 'count', type: 'number'} diff --git a/lib/persisted-model.js b/lib/persisted-model.js index feb23112..e8e18348 100644 --- a/lib/persisted-model.js +++ b/lib/persisted-model.js @@ -639,7 +639,14 @@ module.exports = function(registry) { setRemoting(PersistedModel, 'create', { description: 'Create a new instance of the model and persist it into the data source.', accessType: 'WRITE', - accepts: {arg: 'data', type: 'object', model: typeName, description: 'Model instance data', http: {source: 'body'}}, + accepts: [ + { + arg: 'data', type: 'object', model: typeName, allowArray: true, + description: 'Model instance data', + http: {source: 'body'}, + }, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], returns: {arg: 'data', type: typeName, root: true}, http: {verb: 'post', path: '/'} }); @@ -648,10 +655,15 @@ module.exports = function(registry) { aliases: ['patchOrCreate', 'updateOrCreate'], description: 'Patch an existing model instance or insert a new one into the data source.', accessType: 'WRITE', - accepts: { arg: 'data', type: 'object', model: typeName, http: { source: 'body' }, description: - 'Model instance data' }, - returns: { arg: 'data', type: typeName, root: true }, - http: [{ verb: 'patch', path: '/' }], + accepts: [ + { + arg: 'data', type: 'object', model: typeName, http: {source: 'body'}, + description: 'Model instance data', + }, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], + returns: {arg: 'data', type: typeName, root: true}, + http: [{verb: 'patch', path: '/'}], }; if (!options.replaceOnPUT) { @@ -662,10 +674,16 @@ module.exports = function(registry) { var replaceOrCreateOptions = { description: 'Replace an existing model instance or insert a new one into the data source.', accessType: 'WRITE', - accepts: { arg: 'data', type: 'object', model: typeName, http: { source: 'body' }, description: - 'Model instance data' }, - returns: { arg: 'data', type: typeName, root: true }, - http: [{ verb: 'post', path: '/replaceOrCreate' }], + accepts: [ + { + arg: 'data', type: 'object', model: typeName, + http: {source: 'body'}, + description: 'Model instance data', + }, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], + returns: {arg: 'data', type: typeName, root: true}, + http: [{verb: 'post', path: '/replaceOrCreate'}], }; if (options.replaceOnPUT) { @@ -680,10 +698,11 @@ module.exports = function(registry) { 'the data source based on the where criteria.', accessType: 'WRITE', accepts: [ - { arg: 'where', type: 'object', http: { source: 'query' }, - description: 'Criteria to match model instances' }, - { arg: 'data', type: 'object', model: typeName, http: { source: 'body' }, - description: 'An object of model property name/value pairs' }, + {arg: 'where', type: 'object', http: {source: 'query'}, + description: 'Criteria to match model instances'}, + {arg: 'data', type: 'object', model: typeName, http: {source: 'body'}, + description: 'An object of model property name/value pairs'}, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, ], returns: { arg: 'data', type: typeName, root: true }, http: { verb: 'post', path: '/upsertWithWhere' }, @@ -692,7 +711,10 @@ module.exports = function(registry) { setRemoting(PersistedModel, 'exists', { description: 'Check whether a model instance exists in the data source.', accessType: 'READ', - accepts: {arg: 'id', type: 'any', description: 'Model id', required: true}, + accepts: [ + {arg: 'id', type: 'any', description: 'Model id', required: true}, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], returns: {arg: 'exists', type: 'boolean'}, http: [ {verb: 'get', path: '/:id/exists'}, @@ -726,8 +748,9 @@ module.exports = function(registry) { accepts: [ { arg: 'id', type: 'any', description: 'Model id', required: true, http: {source: 'path'}}, - { arg: 'filter', type: 'object', - description: 'Filter defining fields and include' }, + {arg: 'filter', type: 'object', + description: 'Filter defining fields and include'}, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, ], returns: {arg: 'data', type: typeName, root: true}, http: {verb: 'get', path: '/:id'}, @@ -738,10 +761,11 @@ module.exports = function(registry) { description: 'Replace attributes for a model instance and persist it into the data source.', accessType: 'WRITE', accepts: [ - { arg: 'id', type: 'any', description: 'Model id', required: true, - http: { source: 'path' }}, - { arg: 'data', type: 'object', model: typeName, http: { source: 'body' }, description: - 'Model instance data' }, + {arg: 'id', type: 'any', description: 'Model id', required: true, + http: {source: 'path'}}, + {arg: 'data', type: 'object', model: typeName, http: {source: 'body'}, description: + 'Model instance data'}, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, ], returns: { arg: 'data', type: typeName, root: true }, http: [{ verb: 'post', path: '/:id/replace' }], @@ -756,7 +780,11 @@ module.exports = function(registry) { setRemoting(PersistedModel, 'find', { description: 'Find all instances of the model matched by filter from the data source.', accessType: 'READ', - accepts: {arg: 'filter', type: 'object', description: 'Filter defining fields, where, include, order, offset, and limit'}, + accepts: [ + {arg: 'filter', type: 'object', description: + 'Filter defining fields, where, include, order, offset, and limit'}, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], returns: {arg: 'data', type: [typeName], root: true}, http: {verb: 'get', path: '/'} }); @@ -764,7 +792,11 @@ module.exports = function(registry) { setRemoting(PersistedModel, 'findOne', { description: 'Find first instance of the model matched by filter from the data source.', accessType: 'READ', - accepts: {arg: 'filter', type: 'object', description: 'Filter defining fields, where, include, order, offset, and limit'}, + accepts: [ + {arg: 'filter', type: 'object', description: + 'Filter defining fields, where, include, order, offset, and limit'}, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], returns: {arg: 'data', type: typeName, root: true}, http: {verb: 'get', path: '/findOne'}, rest: {after: convertNullToNotFoundError} @@ -773,7 +805,10 @@ module.exports = function(registry) { setRemoting(PersistedModel, 'destroyAll', { description: 'Delete all matching records.', accessType: 'WRITE', - accepts: {arg: 'where', type: 'object', description: 'filter.where object'}, + accepts: [ + {arg: 'where', type: 'object', description: 'filter.where object'}, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], returns: { arg: 'count', type: 'object', @@ -793,6 +828,7 @@ module.exports = function(registry) { description: 'Criteria to match model instances'}, {arg: 'data', type: 'object', model: typeName, http: {source: 'body'}, description: 'An object of model property name/value pairs'}, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, ], returns: { arg: 'info', @@ -812,8 +848,11 @@ module.exports = function(registry) { aliases: ['destroyById', 'removeById'], description: 'Delete a model instance by {{id}} from the data source.', accessType: 'WRITE', - accepts: {arg: 'id', type: 'any', description: 'Model id', required: true, - http: {source: 'path'}}, + accepts: [ + {arg: 'id', type: 'any', description: 'Model id', required: true, + http: {source: 'path'}}, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], http: {verb: 'del', path: '/:id'}, returns: {arg: 'count', type: 'object', root: true} }); @@ -821,7 +860,10 @@ module.exports = function(registry) { setRemoting(PersistedModel, 'count', { description: 'Count instances of the model matched by where from the data source.', accessType: 'READ', - accepts: {arg: 'where', type: 'object', description: 'Criteria to match model instances'}, + accepts: [ + {arg: 'where', type: 'object', description: 'Criteria to match model instances'}, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], returns: {arg: 'count', type: 'number'}, http: {verb: 'get', path: '/count'} }); @@ -830,14 +872,16 @@ module.exports = function(registry) { aliases: ['patchAttributes'], description: 'Patch attributes for a model instance and persist it into the data source.', accessType: 'WRITE', - - accepts: { - arg: 'data', type: 'object', model: typeName, - http: { source: 'body' }, - description: 'An object of model property name/value pairs' - }, - returns: { arg: 'data', type: typeName, root: true }, - http: [{ verb: 'patch', path: '/' }], + accepts: [ + { + arg: 'data', type: 'object', model: typeName, + http: {source: 'body'}, + description: 'An object of model property name/value pairs', + }, + {arg: 'options', type: 'object', http: 'optionsFromRequest'}, + ], + returns: {arg: 'data', type: typeName, root: true}, + http: [{verb: 'patch', path: '/'}], }; setRemoting(PersistedModel.prototype, 'updateAttributes', updateAttributesOptions); diff --git a/package.json b/package.json index 70ad8dc1..44f5f36f 100644 --- a/package.json +++ b/package.json @@ -96,7 +96,8 @@ "sinon-chai": "^2.8.0", "strong-error-handler": "^1.0.1", "strong-task-emitter": "^0.0.6", - "supertest": "^2.0.0" + "supertest": "^2.0.0", + "supertest-as-promised": "^4.0.2" }, "repository": { "type": "git", diff --git a/test/context-options.test.js b/test/context-options.test.js new file mode 100644 index 00000000..762f320b --- /dev/null +++ b/test/context-options.test.js @@ -0,0 +1,411 @@ +// Copyright IBM Corp. 2013,2016. All Rights Reserved. +// Node module: loopback +// This file is licensed under the MIT License. +// License text available at https://opensource.org/licenses/MIT + +'use strict'; + +var expect = require('chai').expect; +var loopback = require('..'); +var supertest = require('supertest-as-promised')(require('bluebird')); + +describe('OptionsFromRemotingContext', function() { + var app, request, accessToken, userId, Product, actualOptions; + + beforeEach(setupAppAndRequest); + beforeEach(resetActualOptions); + + context('when making updates via REST', function() { + beforeEach(observeOptionsBeforeSave); + + it('injects options to create()', function() { + return request.post('/products') + .send({name: 'Pen'}) + .expect(200) + .then(expectInjectedOptions); + }); + + it('injects options to patchOrCreate()', function() { + return request.patch('/products') + .send({id: 1, name: 'Pen'}) + .expect(200) + .then(expectInjectedOptions); + }); + + it('injects options to replaceOrCreate()', function() { + return request.put('/products') + .send({id: 1, name: 'Pen'}) + .expect(200) + .then(expectInjectedOptions); + }); + + it('injects options to patchOrCreateWithWhere()', function() { + return request.post('/products/upsertWithWhere?where[name]=Pen') + .send({name: 'Pencil'}) + .expect(200) + .then(expectInjectedOptions); + }); + + it('injects options to replaceById()', function() { + return Product.create({id: 1, name: 'Pen'}) + .then(function(p) { + return request.put('/products/1') + .send({name: 'Pencil'}) + .expect(200); + }) + .then(expectInjectedOptions); + }); + + it('injects options to prototype.patchAttributes()', function() { + return Product.create({id: 1, name: 'Pen'}) + .then(function(p) { + return request.patch('/products/1') + .send({name: 'Pencil'}) + .expect(200); + }) + .then(expectInjectedOptions); + }); + + it('injects options to updateAll()', function() { + return request.post('/products/update?where[name]=Pen') + .send({name: 'Pencil'}) + .expect(200) + .then(expectInjectedOptions); + }); + }); + + context('when deleting via REST', function() { + beforeEach(observeOptionsBeforeDelete); + + it('injects options to deleteById()', function() { + return Product.create({id: 1, name: 'Pen'}) + .then(function(p) { + return request.delete('/products/1').expect(200); + }) + .then(expectInjectedOptions); + }); + }); + + context('when querying via REST', function() { + beforeEach(observeOptionsOnAccess); + beforeEach(givenProductId1); + + it('injects options to find()', function() { + return request.get('/products').expect(200) + .then(expectInjectedOptions); + }); + + it('injects options to findById()', function() { + return request.get('/products/1').expect(200) + .then(expectInjectedOptions); + }); + + it('injects options to findOne()', function() { + return request.get('/products/findOne?where[id]=1').expect(200) + .then(expectInjectedOptions); + }); + + it('injects options to exists()', function() { + return request.head('/products/1').expect(200) + .then(expectInjectedOptions); + }); + + it('injects options to count()', function() { + return request.get('/products/count').expect(200) + .then(expectInjectedOptions); + }); + }); + + context('when invoking prototype methods', function() { + beforeEach(observeOptionsOnAccess); + beforeEach(givenProductId1); + + it('injects options to sharedCtor', function() { + Product.prototype.dummy = function(cb) { cb(); }; + Product.remoteMethod('dummy', {isStatic: false}); + return request.post('/products/1/dummy').expect(204) + .then(expectInjectedOptions); + }); + }); + + // Catch: because relations methods are defined on "modelFrom", + // they will invoke createOptionsFromRemotingContext on "modelFrom" too, + // despite the fact that under the hood a method on "modelTo" is called. + + context('hasManyThrough', function() { + var Category, ThroughModel; + + beforeEach(givenCategoryHasManyProductsThroughAnotherModel); + beforeEach(givenCategoryAndProduct); + + it('injects options to findById', function() { + observeOptionsOnAccess(Product); + return request.get('/categories/1/products/1').expect(200) + .then(expectOptionsInjectedFromCategory); + }); + + it('injects options to destroyById', function() { + observeOptionsBeforeDelete(Product); + return request.del('/categories/1/products/1').expect(204) + .then(expectOptionsInjectedFromCategory); + }); + + it('injects options to updateById', function() { + observeOptionsBeforeSave(Product); + return request.put('/categories/1/products/1') + .send({description: 'a description'}) + .expect(200) + .then(expectInjectedOptions); + }); + + context('through-model operations', function() { + it('injects options to link', function() { + observeOptionsBeforeSave(ThroughModel); + return Product.create({id: 2, name: 'Car2'}) + .then(function() { + return request.put('/categories/1/products/rel/2') + .send({description: 'a description'}) + .expect(200); + }) + .then(expectOptionsInjectedFromCategory); + }); + + it('injects options to unlink', function() { + observeOptionsBeforeDelete(ThroughModel); + return request.del('/categories/1/products/rel/1').expect(204) + .then(expectOptionsInjectedFromCategory); + }); + + it('injects options to exists', function() { + observeOptionsOnAccess(ThroughModel); + return request.head('/categories/1/products/rel/1').expect(200) + .then(expectOptionsInjectedFromCategory); + }); + }); + + context('scope operations', function() { + it('injects options to get', function() { + observeOptionsOnAccess(Product); + return request.get('/categories/1/products').expect(200) + .then(expectOptionsInjectedFromCategory); + }); + + it('injects options to create', function() { + observeOptionsBeforeSave(Product); + return request.post('/categories/1/products') + .send({name: 'Pen'}) + .expect(200) + .then(expectOptionsInjectedFromCategory); + }); + + it('injects options to delete', function() { + observeOptionsBeforeDelete(ThroughModel); + return request.del('/categories/1/products').expect(204) + .then(expectOptionsInjectedFromCategory); + }); + + it('injects options to count', function() { + observeOptionsOnAccess(ThroughModel); + return request.get('/categories/1/products/count').expect(200) + .then(expectOptionsInjectedFromCategory); + }); + }); + + function givenCategoryHasManyProductsThroughAnotherModel() { + Category = app.registry.createModel( + 'Category', + {name: String}, + {forceId: false, replaceOnPUT: true}); + + app.model(Category, {dataSource: 'db'}); + // This is a shortcut for creating CategoryProduct "through" model + Category.hasAndBelongsToMany(Product); + + Category.createOptionsFromRemotingContext = function(ctx) { + return {injectedFrom: 'Category'}; + }; + + ThroughModel = app.registry.getModel('CategoryProduct'); + } + + function givenCategoryAndProduct() { + return Category.create({id: 1, name: 'First Category'}) + .then(function(cat) { + return cat.products.create({id: 1, name: 'Pen'}); + }); + } + + function expectOptionsInjectedFromCategory() { + expect(actualOptions).to.have.property('injectedFrom', 'Category'); + } + }); + + context('hasOne', function() { + var Category; + + beforeEach(givenCategoryHasOneProduct); + beforeEach(givenCategoryId1); + + it('injects options to get', function() { + observeOptionsOnAccess(Product); + return givenProductInCategory1() + .then(function() { + return request.get('/categories/1/product').expect(200); + }) + .then(expectOptionsInjectedFromCategory); + }); + + it('injects options to create', function() { + observeOptionsBeforeSave(Product); + return request.post('/categories/1/product') + .send({name: 'Pen'}) + .expect(200) + .then(expectOptionsInjectedFromCategory); + }); + + it('injects options to update', function() { + return givenProductInCategory1() + .then(function() { + observeOptionsBeforeSave(Product); + return request.put('/categories/1/product') + .send({description: 'a description'}) + .expect(200); + }) + .then(expectInjectedOptions); + }); + + it('injects options to destroy', function() { + observeOptionsBeforeDelete(Product); + return givenProductInCategory1() + .then(function() { + return request.del('/categories/1/product').expect(204); + }) + .then(expectOptionsInjectedFromCategory); + }); + + function givenCategoryHasOneProduct() { + Category = app.registry.createModel( + 'Category', + {name: String}, + {forceId: false, replaceOnPUT: true}); + + app.model(Category, {dataSource: 'db'}); + Category.hasOne(Product); + + Category.createOptionsFromRemotingContext = function(ctx) { + return {injectedFrom: 'Category'}; + }; + } + + function givenCategoryId1() { + return Category.create({id: 1, name: 'First Category'}); + } + + function givenProductInCategory1() { + return Product.create({id: 1, name: 'Pen', categoryId: 1}); + } + + function expectOptionsInjectedFromCategory() { + expect(actualOptions).to.have.property('injectedFrom', 'Category'); + } + }); + + context('belongsTo', function() { + var Category; + + beforeEach(givenCategoryBelongsToProduct); + + it('injects options to get', function() { + observeOptionsOnAccess(Product); + return Product.create({id: 1, name: 'Pen'}) + .then(function() { + return Category.create({id: 1, name: 'a name', productId: 1}); + }) + .then(function() { + return request.get('/categories/1/product').expect(200); + }) + .then(expectOptionsInjectedFromCategory); + }); + + function givenCategoryBelongsToProduct() { + Category = app.registry.createModel( + 'Category', + {name: String}, + {forceId: false, replaceOnPUT: true}); + + app.model(Category, {dataSource: 'db'}); + Category.belongsTo(Product); + + Category.createOptionsFromRemotingContext = function(ctx) { + return {injectedFrom: 'Category'}; + }; + } + + function givenCategoryId1() { + return Category.create({id: 1, name: 'First Category'}); + } + + function givenProductInCategory1() { + return Product.create({id: 1, name: 'Pen', categoryId: 1}); + } + + function expectOptionsInjectedFromCategory() { + expect(actualOptions).to.have.property('injectedFrom', 'Category'); + } + }); + + function setupAppAndRequest() { + app = loopback({localRegistry: true}); + app.dataSource('db', {connector: 'memory'}); + + Product = app.registry.createModel( + 'Product', + {name: String}, + {forceId: false, replaceOnPUT: true}); + + Product.createOptionsFromRemotingContext = function(ctx) { + return {injectedFrom: 'Product'}; + }; + + app.model(Product, {dataSource: 'db'}); + + app.use(loopback.rest()); + request = supertest(app); + } + + function resetActualOptions() { + actualOptions = undefined; + } + + function observeOptionsBeforeSave() { + var Model = arguments[0] || Product; + Model.observe('before save', function(ctx, next) { + actualOptions = ctx.options; + next(); + }); + } + + function observeOptionsBeforeDelete() { + var Model = arguments[0] || Product; + Model.observe('before delete', function(ctx, next) { + actualOptions = ctx.options; + next(); + }); + } + + function observeOptionsOnAccess() { + var Model = arguments[0] || Product; + Model.observe('access', function(ctx, next) { + actualOptions = ctx.options; + next(); + }); + } + + function givenProductId1() { + return Product.create({id: 1, name: 'Pen'}); + } + + function expectInjectedOptions(name) { + expect(actualOptions).to.have.property('injectedFrom'); + } +}); diff --git a/test/remote-connector.test.js b/test/remote-connector.test.js index dd058da6..bc92d37e 100644 --- a/test/remote-connector.test.js +++ b/test/remote-connector.test.js @@ -74,7 +74,7 @@ describe('RemoteConnector', function() { var ServerModel = this.ServerModel; - ServerModel.create = function(data, cb) { + ServerModel.create = function(data, options, cb) { calledServerCreate = true; data.id = 1; cb(null, data); diff --git a/test/remoting.integration.js b/test/remoting.integration.js index f881ca4e..c4bdd7ff 100644 --- a/test/remoting.integration.js +++ b/test/remoting.integration.js @@ -285,7 +285,9 @@ function formatMethod(m) { arr.push([ m.name, '(', - m.accepts.map(function(a) { + m.accepts.filter(function(a) { + return !(a.http && typeof a.http === 'function'); + }).map(function(a) { return a.arg + ':' + a.type + (a.model ? ':' + a.model : ''); }).join(','), ')', From 74bb1daf8a4cd147b695940a2b55f7708bc82a34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miroslav=20Bajto=C5=A1?= Date: Mon, 2 Jan 2017 15:28:47 +0100 Subject: [PATCH 3/3] Add new flag injectOptionsFromRemoteContext Hide the new "options" arguments behind a feature flag injectOptionsFromRemoteContext that is disabled by default for backwards compatibility. Fix construction of sharedCtor remoting metadata to prevent the situation when we are configuring remoting metadata after strong-remoting has already picked up data from our parent (base) model. --- lib/model.js | 105 +++++++++++++++++++--------------- lib/persisted-model.js | 56 +++++++++--------- test/context-options.test.js | 51 +++++++++++++++-- test/remote-connector.test.js | 2 +- test/remoting.integration.js | 28 +++++++++ 5 files changed, 162 insertions(+), 80 deletions(-) diff --git a/lib/model.js b/lib/model.js index 05d11493..a37dd40a 100644 --- a/lib/model.js +++ b/lib/model.js @@ -126,22 +126,9 @@ module.exports = function(registry) { var options = this.settings; var typeName = this.modelName; - var remotingOptions = {}; - extend(remotingOptions, options.remoting || {}); - - // create a sharedClass - var sharedClass = ModelCtor.sharedClass = new SharedClass( - ModelCtor.modelName, - ModelCtor, - remotingOptions - ); - - // setup a remoting type converter for this model - RemoteObjects.convert(typeName, function(val) { - return val ? new ModelCtor(val) : val; - }); - // support remoting prototype methods + // it's important to setup this function *before* calling `new SharedClass` + // otherwise remoting metadata from our base model is picked up ModelCtor.sharedCtor = function(data, id, options, fn) { var ModelCtor = this; @@ -200,12 +187,12 @@ module.exports = function(registry) { }; var idDesc = ModelCtor.modelName + ' id'; - ModelCtor.sharedCtor.accepts = [ + ModelCtor.sharedCtor.accepts = this._removeOptionsArgIfDisabled([ {arg: 'id', type: 'any', required: true, http: {source: 'path'}, description: idDesc}, // {arg: 'instance', type: 'object', http: {source: 'body'}} {arg: 'options', type: 'object', http: createOptionsViaModelMethod}, - ]; + ]); ModelCtor.sharedCtor.http = [ {path: '/:id'} @@ -213,6 +200,21 @@ module.exports = function(registry) { ModelCtor.sharedCtor.returns = {root: true}; + var remotingOptions = {}; + extend(remotingOptions, options.remoting || {}); + + // create a sharedClass + var sharedClass = ModelCtor.sharedClass = new SharedClass( + ModelCtor.modelName, + ModelCtor, + remotingOptions + ); + + // setup a remoting type converter for this model + RemoteObjects.convert(typeName, function(val) { + return val ? new ModelCtor(val) : val; + }); + // before remote hook ModelCtor.beforeRemote = function(name, fn) { var className = this.modelName; @@ -522,10 +524,10 @@ module.exports = function(registry) { define('__get__' + relationName, { isStatic: false, http: {verb: 'get', path: '/' + pathName}, - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ {arg: 'refresh', type: 'boolean', http: {source: 'query'}}, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), accessType: 'READ', description: format('Fetches belongsTo relation %s.', relationName), returns: {arg: relationName, type: modelName, root: true}, @@ -550,10 +552,10 @@ module.exports = function(registry) { define('__get__' + relationName, { isStatic: false, http: {verb: 'get', path: '/' + pathName}, - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ {arg: 'refresh', type: 'boolean', http: {source: 'query'}}, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), description: format('Fetches hasOne relation %s.', relationName), accessType: 'READ', returns: {arg: relationName, type: relation.modelTo.modelName, root: true}, @@ -563,13 +565,13 @@ module.exports = function(registry) { define('__create__' + relationName, { isStatic: false, http: {verb: 'post', path: '/' + pathName}, - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ { arg: 'data', type: 'object', model: toModelName, http: {source: 'body'}, }, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), description: format('Creates a new instance in %s of this model.', relationName), accessType: 'WRITE', returns: {arg: 'data', type: toModelName, root: true} @@ -578,13 +580,13 @@ module.exports = function(registry) { define('__update__' + relationName, { isStatic: false, http: {verb: 'put', path: '/' + pathName}, - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ { arg: 'data', type: 'object', model: toModelName, http: {source: 'body'}, }, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), description: format('Update %s of this model.', relationName), accessType: 'WRITE', returns: {arg: 'data', type: toModelName, root: true} @@ -593,9 +595,9 @@ module.exports = function(registry) { define('__destroy__' + relationName, { isStatic: false, http: {verb: 'delete', path: '/' + pathName}, - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), description: format('Deletes %s of this model.', relationName), accessType: 'WRITE', }); @@ -609,7 +611,7 @@ module.exports = function(registry) { define('__findById__' + relationName, { isStatic: false, http: {verb: 'get', path: '/' + pathName + '/:fk'}, - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ { arg: 'fk', type: 'any', description: format('Foreign key for %s', relationName), @@ -617,7 +619,7 @@ module.exports = function(registry) { http: {source: 'path'}, }, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), description: format('Find a related item by id for %s.', relationName), accessType: 'READ', returns: {arg: 'result', type: toModelName, root: true}, @@ -628,7 +630,7 @@ module.exports = function(registry) { define('__destroyById__' + relationName, { isStatic: false, http: {verb: 'delete', path: '/' + pathName + '/:fk'}, - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ { arg: 'fk', type: 'any', description: format('Foreign key for %s', relationName), @@ -636,7 +638,7 @@ module.exports = function(registry) { http: {source: 'path'}, }, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), description: format('Delete a related item by id for %s.', relationName), accessType: 'WRITE', returns: [] @@ -646,14 +648,14 @@ module.exports = function(registry) { define('__updateById__' + relationName, { isStatic: false, http: {verb: 'put', path: '/' + pathName + '/:fk'}, - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ {arg: 'fk', type: 'any', description: format('Foreign key for %s', relationName), required: true, http: { source: 'path' }}, {arg: 'data', type: 'object', model: toModelName, http: {source: 'body'}}, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), description: format('Update a related item by id for %s.', relationName), accessType: 'WRITE', returns: {arg: 'result', type: toModelName, root: true} @@ -676,9 +678,9 @@ module.exports = function(registry) { description: format('Foreign key for %s', relationName), required: true, http: {source: 'path'}}, - ].concat(accepts).concat([ + ].concat(accepts).concat(this._removeOptionsArgIfDisabled([ {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ]), + ])), description: format('Add a related item by id for %s.', relationName), accessType: 'WRITE', returns: {arg: relationName, type: modelThrough.modelName, root: true} @@ -688,7 +690,7 @@ module.exports = function(registry) { define('__unlink__' + relationName, { isStatic: false, http: {verb: 'delete', path: '/' + pathName + '/rel/:fk'}, - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ { arg: 'fk', type: 'any', description: format('Foreign key for %s', relationName), @@ -696,7 +698,7 @@ module.exports = function(registry) { http: {source: 'path'}, }, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), description: format('Remove the %s relation to an item by id.', relationName), accessType: 'WRITE', returns: [] @@ -708,7 +710,7 @@ module.exports = function(registry) { define('__exists__' + relationName, { isStatic: false, http: {verb: 'head', path: '/' + pathName + '/rel/:fk'}, - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ { arg: 'fk', type: 'any', description: format('Foreign key for %s', relationName), @@ -716,7 +718,7 @@ module.exports = function(registry) { http: {source: 'path'}, }, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), description: format('Check the existence of %s relation to an item by id.', relationName), accessType: 'READ', returns: {arg: 'exists', type: 'boolean', root: true}, @@ -759,10 +761,10 @@ module.exports = function(registry) { define('__get__' + scopeName, { isStatic: isStatic, http: {verb: 'get', path: '/' + pathName}, - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ {arg: 'filter', type: 'object'}, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), description: format('Queries %s of %s.', scopeName, this.modelName), accessType: 'READ', returns: {arg: scopeName, type: [toModelName], root: true} @@ -771,7 +773,7 @@ module.exports = function(registry) { define('__create__' + scopeName, { isStatic: isStatic, http: {verb: 'post', path: '/' + pathName}, - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ { arg: 'data', type: 'object', @@ -779,7 +781,7 @@ module.exports = function(registry) { http: {source: 'body'}, }, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), description: format('Creates a new instance in %s of this model.', scopeName), accessType: 'WRITE', returns: {arg: 'data', type: toModelName, root: true} @@ -788,7 +790,7 @@ module.exports = function(registry) { define('__delete__' + scopeName, { isStatic: isStatic, http: {verb: 'delete', path: '/' + pathName}, - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ { arg: 'where', type: 'object', // The "where" argument is not exposed in the REST API @@ -797,7 +799,7 @@ module.exports = function(registry) { http: function(ctx) { return undefined; }, }, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), description: format('Deletes all %s of this model.', scopeName), accessType: 'WRITE', }); @@ -805,13 +807,13 @@ module.exports = function(registry) { define('__count__' + scopeName, { isStatic: isStatic, http: {verb: 'get', path: '/' + pathName + '/count'}, - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ { arg: 'where', type: 'object', description: 'Criteria to match model instances', }, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), description: format('Counts %s of %s.', scopeName, this.modelName), accessType: 'READ', returns: {arg: 'count', type: 'number'} @@ -819,6 +821,15 @@ module.exports = function(registry) { }; + Model._removeOptionsArgIfDisabled = function(accepts) { + if (this.settings.injectOptionsFromRemoteContext) + return accepts; + var lastArg = accepts[accepts.length - 1]; + var hasOptions = lastArg.arg === 'options' && lastArg.type === 'object'; + assert(hasOptions, 'last accepts argument is "options" arg'); + return accepts.slice(0, -1); + }; + /** * Enabled deeply-nested queries of related models via REST API. * diff --git a/lib/persisted-model.js b/lib/persisted-model.js index e8e18348..c19520ee 100644 --- a/lib/persisted-model.js +++ b/lib/persisted-model.js @@ -639,14 +639,14 @@ module.exports = function(registry) { setRemoting(PersistedModel, 'create', { description: 'Create a new instance of the model and persist it into the data source.', accessType: 'WRITE', - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ { arg: 'data', type: 'object', model: typeName, allowArray: true, description: 'Model instance data', http: {source: 'body'}, }, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), returns: {arg: 'data', type: typeName, root: true}, http: {verb: 'post', path: '/'} }); @@ -655,13 +655,13 @@ module.exports = function(registry) { aliases: ['patchOrCreate', 'updateOrCreate'], description: 'Patch an existing model instance or insert a new one into the data source.', accessType: 'WRITE', - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ { arg: 'data', type: 'object', model: typeName, http: {source: 'body'}, description: 'Model instance data', }, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), returns: {arg: 'data', type: typeName, root: true}, http: [{verb: 'patch', path: '/'}], }; @@ -674,14 +674,14 @@ module.exports = function(registry) { var replaceOrCreateOptions = { description: 'Replace an existing model instance or insert a new one into the data source.', accessType: 'WRITE', - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ { arg: 'data', type: 'object', model: typeName, http: {source: 'body'}, description: 'Model instance data', }, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), returns: {arg: 'data', type: typeName, root: true}, http: [{verb: 'post', path: '/replaceOrCreate'}], }; @@ -697,13 +697,13 @@ module.exports = function(registry) { description: 'Update an existing model instance or insert a new one into ' + 'the data source based on the where criteria.', accessType: 'WRITE', - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ {arg: 'where', type: 'object', http: {source: 'query'}, description: 'Criteria to match model instances'}, {arg: 'data', type: 'object', model: typeName, http: {source: 'body'}, description: 'An object of model property name/value pairs'}, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), returns: { arg: 'data', type: typeName, root: true }, http: { verb: 'post', path: '/upsertWithWhere' }, }); @@ -711,10 +711,10 @@ module.exports = function(registry) { setRemoting(PersistedModel, 'exists', { description: 'Check whether a model instance exists in the data source.', accessType: 'READ', - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ {arg: 'id', type: 'any', description: 'Model id', required: true}, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), returns: {arg: 'exists', type: 'boolean'}, http: [ {verb: 'get', path: '/:id/exists'}, @@ -745,13 +745,13 @@ module.exports = function(registry) { setRemoting(PersistedModel, 'findById', { description: 'Find a model instance by {{id}} from the data source.', accessType: 'READ', - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ { arg: 'id', type: 'any', description: 'Model id', required: true, http: {source: 'path'}}, {arg: 'filter', type: 'object', description: 'Filter defining fields and include'}, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), returns: {arg: 'data', type: typeName, root: true}, http: {verb: 'get', path: '/:id'}, rest: {after: convertNullToNotFoundError} @@ -760,13 +760,13 @@ module.exports = function(registry) { var replaceByIdOptions = { description: 'Replace attributes for a model instance and persist it into the data source.', accessType: 'WRITE', - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ {arg: 'id', type: 'any', description: 'Model id', required: true, http: {source: 'path'}}, {arg: 'data', type: 'object', model: typeName, http: {source: 'body'}, description: 'Model instance data'}, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), returns: { arg: 'data', type: typeName, root: true }, http: [{ verb: 'post', path: '/:id/replace' }], }; @@ -780,11 +780,11 @@ module.exports = function(registry) { setRemoting(PersistedModel, 'find', { description: 'Find all instances of the model matched by filter from the data source.', accessType: 'READ', - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ {arg: 'filter', type: 'object', description: 'Filter defining fields, where, include, order, offset, and limit'}, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), returns: {arg: 'data', type: [typeName], root: true}, http: {verb: 'get', path: '/'} }); @@ -792,11 +792,11 @@ module.exports = function(registry) { setRemoting(PersistedModel, 'findOne', { description: 'Find first instance of the model matched by filter from the data source.', accessType: 'READ', - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ {arg: 'filter', type: 'object', description: 'Filter defining fields, where, include, order, offset, and limit'}, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), returns: {arg: 'data', type: typeName, root: true}, http: {verb: 'get', path: '/findOne'}, rest: {after: convertNullToNotFoundError} @@ -805,10 +805,10 @@ module.exports = function(registry) { setRemoting(PersistedModel, 'destroyAll', { description: 'Delete all matching records.', accessType: 'WRITE', - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ {arg: 'where', type: 'object', description: 'filter.where object'}, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), returns: { arg: 'count', type: 'object', @@ -823,13 +823,13 @@ module.exports = function(registry) { aliases: ['update'], description: 'Update instances of the model matched by {{where}} from the data source.', accessType: 'WRITE', - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ {arg: 'where', type: 'object', http: { source: 'query'}, description: 'Criteria to match model instances'}, {arg: 'data', type: 'object', model: typeName, http: {source: 'body'}, description: 'An object of model property name/value pairs'}, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), returns: { arg: 'info', description: 'Information related to the outcome of the operation', @@ -848,11 +848,11 @@ module.exports = function(registry) { aliases: ['destroyById', 'removeById'], description: 'Delete a model instance by {{id}} from the data source.', accessType: 'WRITE', - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ {arg: 'id', type: 'any', description: 'Model id', required: true, http: {source: 'path'}}, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), http: {verb: 'del', path: '/:id'}, returns: {arg: 'count', type: 'object', root: true} }); @@ -860,10 +860,10 @@ module.exports = function(registry) { setRemoting(PersistedModel, 'count', { description: 'Count instances of the model matched by where from the data source.', accessType: 'READ', - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ {arg: 'where', type: 'object', description: 'Criteria to match model instances'}, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), returns: {arg: 'count', type: 'number'}, http: {verb: 'get', path: '/count'} }); @@ -872,14 +872,14 @@ module.exports = function(registry) { aliases: ['patchAttributes'], description: 'Patch attributes for a model instance and persist it into the data source.', accessType: 'WRITE', - accepts: [ + accepts: this._removeOptionsArgIfDisabled([ { arg: 'data', type: 'object', model: typeName, http: {source: 'body'}, description: 'An object of model property name/value pairs', }, {arg: 'options', type: 'object', http: 'optionsFromRequest'}, - ], + ]), returns: {arg: 'data', type: typeName, root: true}, http: [{verb: 'patch', path: '/'}], }; diff --git a/test/context-options.test.js b/test/context-options.test.js index 762f320b..ac8f5041 100644 --- a/test/context-options.test.js +++ b/test/context-options.test.js @@ -128,6 +128,28 @@ describe('OptionsFromRemotingContext', function() { }); }); + it('honours injectOptionsFromRemoteContext in sharedCtor', function() { + var settings = { + forceId: false, + injectOptionsFromRemoteContext: false, + }; + var TestModel = app.registry.createModel('TestModel', {}, settings); + app.model(TestModel, {dataSource: 'db'}); + + TestModel.prototype.dummy = function(cb) { cb(); }; + TestModel.remoteMethod('dummy', {isStatic: false}); + + observeOptionsOnAccess(TestModel); + + return TestModel.create({id: 1}) + .then(function() { + return request.post('/TestModels/1/dummy').expect(204); + }) + .then(function() { + expect(actualOptions).to.eql({}); + }); + }); + // Catch: because relations methods are defined on "modelFrom", // they will invoke createOptionsFromRemotingContext on "modelFrom" too, // despite the fact that under the hood a method on "modelTo" is called. @@ -212,10 +234,15 @@ describe('OptionsFromRemotingContext', function() { }); function givenCategoryHasManyProductsThroughAnotherModel() { + var settings = { + forceId: false, + replaceOnPUT: true, + injectOptionsFromRemoteContext: true, + }; Category = app.registry.createModel( 'Category', {name: String}, - {forceId: false, replaceOnPUT: true}); + settings); app.model(Category, {dataSource: 'db'}); // This is a shortcut for creating CategoryProduct "through" model @@ -284,10 +311,15 @@ describe('OptionsFromRemotingContext', function() { }); function givenCategoryHasOneProduct() { + var settings = { + forceId: false, + replaceOnPUT: true, + injectOptionsFromRemoteContext: true, + }; Category = app.registry.createModel( 'Category', {name: String}, - {forceId: false, replaceOnPUT: true}); + settings); app.model(Category, {dataSource: 'db'}); Category.hasOne(Product); @@ -328,10 +360,15 @@ describe('OptionsFromRemotingContext', function() { }); function givenCategoryBelongsToProduct() { + var settings = { + forceId: false, + replaceOnPUT: true, + injectOptionsFromRemoteContext: true, + }; Category = app.registry.createModel( 'Category', {name: String}, - {forceId: false, replaceOnPUT: true}); + settings); app.model(Category, {dataSource: 'db'}); Category.belongsTo(Product); @@ -358,10 +395,16 @@ describe('OptionsFromRemotingContext', function() { app = loopback({localRegistry: true}); app.dataSource('db', {connector: 'memory'}); + var settings = { + forceId: false, + replaceOnPUT: true, + injectOptionsFromRemoteContext: true, + }; + Product = app.registry.createModel( 'Product', {name: String}, - {forceId: false, replaceOnPUT: true}); + settings); Product.createOptionsFromRemotingContext = function(ctx) { return {injectedFrom: 'Product'}; diff --git a/test/remote-connector.test.js b/test/remote-connector.test.js index bc92d37e..dd058da6 100644 --- a/test/remote-connector.test.js +++ b/test/remote-connector.test.js @@ -74,7 +74,7 @@ describe('RemoteConnector', function() { var ServerModel = this.ServerModel; - ServerModel.create = function(data, options, cb) { + ServerModel.create = function(data, cb) { calledServerCreate = true; data.id = 1; cb(null, data); diff --git a/test/remoting.integration.js b/test/remoting.integration.js index c4bdd7ff..dee8e99b 100644 --- a/test/remoting.integration.js +++ b/test/remoting.integration.js @@ -9,6 +9,7 @@ var path = require('path'); var SIMPLE_APP = path.join(__dirname, 'fixtures', 'simple-integration-app'); var app = require(path.join(SIMPLE_APP, 'server/server.js')); var assert = require('assert'); +var expect = require('chai').expect; describe('remoting - integration', function() { before(function(done) { @@ -263,6 +264,33 @@ describe('With model.settings.replaceOnPUT true', function() { }); }); +describe('injectContextFromRemotingContext', function() { + it('is disabled by default for DAO, scope and belongsTo methods', function() { + var storeClass = findClass('store'); + var violations = getMethodsAcceptingComputedOptions(storeClass.methods); + expect(violations).to.eql([]); + }); + + it('is disabled by default for belongsTo methods', function() { + var widgetClass = findClass('widget'); + var violations = getMethodsAcceptingComputedOptions(widgetClass.methods); + expect(violations).to.eql([]); + }); + + function getMethodsAcceptingComputedOptions(methods) { + return methods + .filter(function(m) { + return m.accepts.some(function(a) { + return a.arg === 'options' && a.type === 'object' && + a.http && typeof a.http === 'function'; + }); + }) + .map(function(m) { + return m.name; + }); + } +}); + function formatReturns(m) { var returns = m.returns; if (!returns || returns.length === 0) {