verdnatura-mirror
/
loopback
mirror of https://github.com/strongloop/loopback.git
0
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Miroslav Bajtoš 2017-01-02 15:28:47 +01:00
parent 693d52fc59
commit 74bb1daf8a
5 changed files with 162 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

105
lib/model.js
View File

@ -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.
*

56
lib/persisted-model.js
View File

@ -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: '/'}],
};

51
test/context-options.test.js
View File

@ -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'};

2
test/remote-connector.test.js
View File

@ -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);

28
test/remoting.integration.js
View File

@ -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) {