Fix remoting metadata for "data" arguments

Fix the definition of "data" argument to

    { type: 'object', model: modelName, ... }

That way strong-remoting passed the request body directly to the model
method (does not create a new model instance), but the swagger will
still provide correct schema for these arguments.

This fixes a bug where upsert in relation methods was adding default
property values to request payload.
This commit is contained in:
Miroslav Bajtoš 2016-09-02 14:40:11 +02:00
parent 554ccbd035
commit f76edd5d61
5 changed files with 95 additions and 49 deletions

View File

@ -488,7 +488,7 @@ module.exports = function(registry) {
define('__create__' + relationName, { define('__create__' + relationName, {
isStatic: false, isStatic: false,
http: { verb: 'post', path: '/' + pathName }, http: { verb: 'post', path: '/' + pathName },
accepts: { arg: 'data', type: toModelName, http: { source: 'body' }}, accepts: { arg: 'data', type: 'object', model: toModelName, http: { source: 'body' }},
description: format('Creates a new instance in %s of this model.', relationName), description: format('Creates a new instance in %s of this model.', relationName),
accessType: 'WRITE', accessType: 'WRITE',
returns: { arg: 'data', type: toModelName, root: true }, returns: { arg: 'data', type: toModelName, root: true },
@ -497,7 +497,7 @@ module.exports = function(registry) {
define('__update__' + relationName, { define('__update__' + relationName, {
isStatic: false, isStatic: false,
http: { verb: 'put', path: '/' + pathName }, http: { verb: 'put', path: '/' + pathName },
accepts: { arg: 'data', type: toModelName, http: { source: 'body' }}, accepts: { arg: 'data', type: 'object', model: toModelName, http: { source: 'body' }},
description: format('Update %s of this model.', relationName), description: format('Update %s of this model.', relationName),
accessType: 'WRITE', accessType: 'WRITE',
returns: { arg: 'data', type: toModelName, root: true }, returns: { arg: 'data', type: toModelName, root: true },
@ -551,7 +551,7 @@ module.exports = function(registry) {
description: format('Foreign key for %s', relationName), description: format('Foreign key for %s', relationName),
required: true, required: true,
http: { source: 'path' }}, http: { source: 'path' }},
{ arg: 'data', type: toModelName, http: { source: 'body' }}, { arg: 'data', type: 'object', model: toModelName, http: { source: 'body' }},
], ],
description: format('Update a related item by id for %s.', relationName), description: format('Update a related item by id for %s.', relationName),
accessType: 'WRITE', accessType: 'WRITE',
@ -564,7 +564,10 @@ module.exports = function(registry) {
var accepts = []; var accepts = [];
if (relation.type === 'hasMany' && relation.modelThrough) { if (relation.type === 'hasMany' && relation.modelThrough) {
// Restrict: only hasManyThrough relation can have additional properties // Restrict: only hasManyThrough relation can have additional properties
accepts.push({ arg: 'data', type: modelThrough.modelName, http: { source: 'body' }}); accepts.push({
arg: 'data', type: 'object', model: modelThrough.modelName,
http: { source: 'body' },
});
} }
var addFunc = this.prototype['__link__' + relationName]; var addFunc = this.prototype['__link__' + relationName];
@ -654,7 +657,7 @@ module.exports = function(registry) {
define('__create__' + scopeName, { define('__create__' + scopeName, {
isStatic: isStatic, isStatic: isStatic,
http: { verb: 'post', path: '/' + pathName }, http: { verb: 'post', path: '/' + pathName },
accepts: { arg: 'data', type: toModelName, http: { source: 'body' }}, accepts: { arg: 'data', type: 'object', model: toModelName, http: { source: 'body' }},
description: format('Creates a new instance in %s of this model.', scopeName), description: format('Creates a new instance in %s of this model.', scopeName),
accessType: 'WRITE', accessType: 'WRITE',
returns: { arg: 'data', type: toModelName, root: true }, returns: { arg: 'data', type: toModelName, root: true },

View File

@ -639,8 +639,11 @@ module.exports = function(registry) {
setRemoting(PersistedModel, 'create', { setRemoting(PersistedModel, 'create', {
description: 'Create a new instance of the model and persist it into the data source.', description: 'Create a new instance of the model and persist it into the data source.',
accessType: 'WRITE', accessType: 'WRITE',
accepts: { arg: 'data', type: 'object', http: { source: 'body' }, description: accepts: {
'Model instance data' }, arg: 'data', type: 'object', model: typeName,
description: 'Model instance data',
http: { source: 'body' },
},
returns: { arg: 'data', type: typeName, root: true }, returns: { arg: 'data', type: typeName, root: true },
http: { verb: 'post', path: '/' }, http: { verb: 'post', path: '/' },
}); });
@ -650,8 +653,10 @@ module.exports = function(registry) {
description: 'Patch an existing model instance or insert a new one ' + description: 'Patch an existing model instance or insert a new one ' +
'into the data source.', 'into the data source.',
accessType: 'WRITE', accessType: 'WRITE',
accepts: { arg: 'data', type: 'object', http: { source: 'body' }, description: accepts: {
'Model instance data' }, arg: 'data', type: 'object', model: typeName, http: { source: 'body' },
description: 'Model instance data',
},
returns: { arg: 'data', type: typeName, root: true }, returns: { arg: 'data', type: typeName, root: true },
http: [{ verb: 'patch', path: '/' }], http: [{ verb: 'patch', path: '/' }],
}; };
@ -664,8 +669,11 @@ module.exports = function(registry) {
var replaceOrCreateOptions = { var replaceOrCreateOptions = {
description: 'Replace an existing model instance or insert a new one into the data source.', description: 'Replace an existing model instance or insert a new one into the data source.',
accessType: 'WRITE', accessType: 'WRITE',
accepts: { arg: 'data', type: 'object', http: { source: 'body' }, description: accepts: {
'Model instance data' }, arg: 'data', type: 'object', model: typeName,
http: { source: 'body' },
description: 'Model instance data',
},
returns: { arg: 'data', type: typeName, root: true }, returns: { arg: 'data', type: typeName, root: true },
http: [{ verb: 'post', path: '/replaceOrCreate' }], http: [{ verb: 'post', path: '/replaceOrCreate' }],
}; };
@ -684,7 +692,7 @@ module.exports = function(registry) {
accepts: [ accepts: [
{ arg: 'where', type: 'object', http: { source: 'query' }, { arg: 'where', type: 'object', http: { source: 'query' },
description: 'Criteria to match model instances' }, description: 'Criteria to match model instances' },
{ arg: 'data', type: 'object', http: { source: 'body' }, { arg: 'data', type: 'object', model: typeName, http: { source: 'body' },
description: 'An object of model property name/value pairs' }, description: 'An object of model property name/value pairs' },
], ],
returns: { arg: 'data', type: typeName, root: true }, returns: { arg: 'data', type: typeName, root: true },
@ -742,7 +750,7 @@ module.exports = function(registry) {
accepts: [ accepts: [
{ arg: 'id', type: 'any', description: 'Model id', required: true, { arg: 'id', type: 'any', description: 'Model id', required: true,
http: { source: 'path' }}, http: { source: 'path' }},
{ arg: 'data', type: 'object', http: { source: 'body' }, description: { arg: 'data', type: 'object', model: typeName, http: { source: 'body' }, description:
'Model instance data' }, 'Model instance data' },
], ],
returns: { arg: 'data', type: typeName, root: true }, returns: { arg: 'data', type: typeName, root: true },
@ -796,7 +804,7 @@ module.exports = function(registry) {
accepts: [ accepts: [
{ arg: 'where', type: 'object', http: { source: 'query' }, { arg: 'where', type: 'object', http: { source: 'query' },
description: 'Criteria to match model instances' }, description: 'Criteria to match model instances' },
{ arg: 'data', type: 'object', http: { source: 'body' }, { arg: 'data', type: 'object', model: typeName, http: { source: 'body' },
description: 'An object of model property name/value pairs' }, description: 'An object of model property name/value pairs' },
], ],
returns: { returns: {
@ -831,7 +839,11 @@ module.exports = function(registry) {
description: 'Patch attributes for a model instance and persist it into ' + description: 'Patch attributes for a model instance and persist it into ' +
'the data source.', 'the data source.',
accessType: 'WRITE', accessType: 'WRITE',
accepts: { arg: 'data', type: 'object', http: { source: 'body' }, description: 'An object of model property name/value pairs' }, 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 }, returns: { arg: 'data', type: typeName, root: true },
http: [{ verb: 'patch', path: '/' }], http: [{ verb: 'patch', path: '/' }],
}; };
@ -927,7 +939,7 @@ module.exports = function(registry) {
description: 'Model id', description: 'Model id',
}, },
{ {
arg: 'data', type: 'object', http: { source: 'body' }, arg: 'data', type: 'object', model: typeName, http: { source: 'body' },
description: 'An object of Change property name/value pairs', description: 'An object of Change property name/value pairs',
}, },
], ],

View File

@ -1,6 +1,11 @@
{ {
"name": "widget", "name": "widget",
"properties": {}, "properties": {
"name": {
"type": "string",
"default": "DefaultWidgetName"
}
},
"relations": { "relations": {
"store": { "store": {
"model": "store", "model": "store",

View File

@ -224,6 +224,32 @@ describe('relations - integration', function() {
}); });
}); });
}); });
describe('PUT /api/store/:id/widgets/:fk', function() {
beforeEach(function(done) {
var self = this;
this.store.widgets.create({
name: this.widgetName,
}, function(err, widget) {
self.widget = widget;
self.url = '/api/stores/' + self.store.id + '/widgets/' + widget.id;
done();
});
});
it('does not add default properties to request body', function(done) {
var self = this;
self.request.put(self.url)
.send({ active: true })
.end(function(err) {
if (err) return done(err);
app.models.Widget.findById(self.widget.id, function(err, w) {
if (err) return done(err);
expect(w.name).to.equal(self.widgetName);
done();
});
});
});
});
}); });
describe('/stores/:id/widgets/:fk - 200', function() { describe('/stores/:id/widgets/:fk - 200', function() {

View File

@ -81,20 +81,20 @@ describe('remoting - integration', function() {
var methods = getFormattedMethodsExcludingRelations(storeClass.methods); var methods = getFormattedMethodsExcludingRelations(storeClass.methods);
var expectedMethods = [ var expectedMethods = [
'create(data:object):store POST /stores', 'create(data:object:store):store POST /stores',
'patchOrCreate(data:object):store PATCH /stores', 'patchOrCreate(data:object:store):store PATCH /stores',
'replaceOrCreate(data:object):store PUT /stores', 'replaceOrCreate(data:object:store):store PUT /stores',
'replaceOrCreate(data:object):store POST /stores/replaceOrCreate', 'replaceOrCreate(data:object:store):store POST /stores/replaceOrCreate',
'exists(id:any):boolean GET /stores/:id/exists', 'exists(id:any):boolean GET /stores/:id/exists',
'findById(id:any,filter:object):store GET /stores/:id', 'findById(id:any,filter:object):store GET /stores/:id',
'replaceById(id:any,data:object):store PUT /stores/:id', 'replaceById(id:any,data:object:store):store PUT /stores/:id',
'replaceById(id:any,data:object):store POST /stores/:id/replace', 'replaceById(id:any,data:object:store):store POST /stores/:id/replace',
'find(filter:object):store GET /stores', 'find(filter:object):store GET /stores',
'findOne(filter:object):store GET /stores/findOne', 'findOne(filter:object):store GET /stores/findOne',
'updateAll(where:object,data:object):object POST /stores/update', 'updateAll(where:object,data:object:store):object POST /stores/update',
'deleteById(id:any):object DELETE /stores/:id', 'deleteById(id:any):object DELETE /stores/:id',
'count(where:object):number GET /stores/count', 'count(where:object):number GET /stores/count',
'prototype.patchAttributes(data:object):store PATCH /stores/:id', 'prototype.patchAttributes(data:object:store):store PATCH /stores/:id',
'createChangeStream(options:object):ReadableStream POST /stores/change-stream', 'createChangeStream(options:object):ReadableStream POST /stores/change-stream',
]; ];
@ -109,7 +109,7 @@ describe('remoting - integration', function() {
var expectedMethods = [ var expectedMethods = [
'__get__superStores(filter:object):store GET /stores/superStores', '__get__superStores(filter:object):store GET /stores/superStores',
'__create__superStores(data:store):store POST /stores/superStores', '__create__superStores(data:object:store):store POST /stores/superStores',
'__delete__superStores() DELETE /stores/superStores', '__delete__superStores() DELETE /stores/superStores',
'__count__superStores(where:object):number GET /stores/superStores/count', '__count__superStores(where:object):number GET /stores/superStores/count',
]; ];
@ -139,11 +139,11 @@ describe('remoting - integration', function() {
'GET /stores/:id/widgets/:fk', 'GET /stores/:id/widgets/:fk',
'prototype.__destroyById__widgets(fk:any) ' + 'prototype.__destroyById__widgets(fk:any) ' +
'DELETE /stores/:id/widgets/:fk', 'DELETE /stores/:id/widgets/:fk',
'prototype.__updateById__widgets(fk:any,data:widget):widget ' + 'prototype.__updateById__widgets(fk:any,data:object:widget):widget ' +
'PUT /stores/:id/widgets/:fk', 'PUT /stores/:id/widgets/:fk',
'prototype.__get__widgets(filter:object):widget ' + 'prototype.__get__widgets(filter:object):widget ' +
'GET /stores/:id/widgets', 'GET /stores/:id/widgets',
'prototype.__create__widgets(data:widget):widget ' + 'prototype.__create__widgets(data:object:widget):widget ' +
'POST /stores/:id/widgets', 'POST /stores/:id/widgets',
'prototype.__delete__widgets() ' + 'prototype.__delete__widgets() ' +
'DELETE /stores/:id/widgets', 'DELETE /stores/:id/widgets',
@ -163,9 +163,9 @@ describe('remoting - integration', function() {
'GET /physicians/:id/patients/:fk', 'GET /physicians/:id/patients/:fk',
'prototype.__destroyById__patients(fk:any) ' + 'prototype.__destroyById__patients(fk:any) ' +
'DELETE /physicians/:id/patients/:fk', 'DELETE /physicians/:id/patients/:fk',
'prototype.__updateById__patients(fk:any,data:patient):patient ' + 'prototype.__updateById__patients(fk:any,data:object:patient):patient ' +
'PUT /physicians/:id/patients/:fk', 'PUT /physicians/:id/patients/:fk',
'prototype.__link__patients(fk:any,data:appointment):appointment ' + 'prototype.__link__patients(fk:any,data:object:appointment):appointment ' +
'PUT /physicians/:id/patients/rel/:fk', 'PUT /physicians/:id/patients/rel/:fk',
'prototype.__unlink__patients(fk:any) ' + 'prototype.__unlink__patients(fk:any) ' +
'DELETE /physicians/:id/patients/rel/:fk', 'DELETE /physicians/:id/patients/rel/:fk',
@ -173,7 +173,7 @@ describe('remoting - integration', function() {
'HEAD /physicians/:id/patients/rel/:fk', 'HEAD /physicians/:id/patients/rel/:fk',
'prototype.__get__patients(filter:object):patient ' + 'prototype.__get__patients(filter:object):patient ' +
'GET /physicians/:id/patients', 'GET /physicians/:id/patients',
'prototype.__create__patients(data:patient):patient ' + 'prototype.__create__patients(data:object:patient):patient ' +
'POST /physicians/:id/patients', 'POST /physicians/:id/patients',
'prototype.__delete__patients() ' + 'prototype.__delete__patients() ' +
'DELETE /physicians/:id/patients', 'DELETE /physicians/:id/patients',
@ -188,7 +188,7 @@ describe('remoting - integration', function() {
var storeClass = findClass('store'); var storeClass = findClass('store');
var methods = getFormattedMethodsExcludingRelations(storeClass.methods); var methods = getFormattedMethodsExcludingRelations(storeClass.methods);
var expectedMethods = [ var expectedMethods = [
'upsertWithWhere(where:object,data:object):store POST /stores/upsertWithWhere', 'upsertWithWhere(where:object,data:object:store):store POST /stores/upsertWithWhere',
]; ];
expect(methods).to.include.members(expectedMethods); expect(methods).to.include.members(expectedMethods);
}); });
@ -207,22 +207,22 @@ describe('With model.settings.replaceOnPUT false', function() {
var methods = getFormattedMethodsExcludingRelations(storeClass.methods); var methods = getFormattedMethodsExcludingRelations(storeClass.methods);
var expectedMethods = [ var expectedMethods = [
'create(data:object):storeWithReplaceOnPUTfalse POST /stores-updating', 'create(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse POST /stores-updating',
'patchOrCreate(data:object):storeWithReplaceOnPUTfalse PUT /stores-updating', 'patchOrCreate(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse PUT /stores-updating',
'patchOrCreate(data:object):storeWithReplaceOnPUTfalse PATCH /stores-updating', 'patchOrCreate(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse PATCH /stores-updating',
'replaceOrCreate(data:object):storeWithReplaceOnPUTfalse POST /stores-updating/replaceOrCreate', 'replaceOrCreate(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse POST /stores-updating/replaceOrCreate',
'upsertWithWhere(where:object,data:object):storeWithReplaceOnPUTfalse POST /stores-updating/upsertWithWhere', 'upsertWithWhere(where:object,data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse POST /stores-updating/upsertWithWhere',
'exists(id:any):boolean GET /stores-updating/:id/exists', 'exists(id:any):boolean GET /stores-updating/:id/exists',
'exists(id:any):boolean HEAD /stores-updating/:id', 'exists(id:any):boolean HEAD /stores-updating/:id',
'findById(id:any,filter:object):storeWithReplaceOnPUTfalse GET /stores-updating/:id', 'findById(id:any,filter:object):storeWithReplaceOnPUTfalse GET /stores-updating/:id',
'replaceById(id:any,data:object):storeWithReplaceOnPUTfalse POST /stores-updating/:id/replace', 'replaceById(id:any,data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse POST /stores-updating/:id/replace',
'find(filter:object):storeWithReplaceOnPUTfalse GET /stores-updating', 'find(filter:object):storeWithReplaceOnPUTfalse GET /stores-updating',
'findOne(filter:object):storeWithReplaceOnPUTfalse GET /stores-updating/findOne', 'findOne(filter:object):storeWithReplaceOnPUTfalse GET /stores-updating/findOne',
'updateAll(where:object,data:object):object POST /stores-updating/update', 'updateAll(where:object,data:object:storeWithReplaceOnPUTfalse):object POST /stores-updating/update',
'deleteById(id:any):object DELETE /stores-updating/:id', 'deleteById(id:any):object DELETE /stores-updating/:id',
'count(where:object):number GET /stores-updating/count', 'count(where:object):number GET /stores-updating/count',
'prototype.patchAttributes(data:object):storeWithReplaceOnPUTfalse PUT /stores-updating/:id', 'prototype.patchAttributes(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse PUT /stores-updating/:id',
'prototype.patchAttributes(data:object):storeWithReplaceOnPUTfalse PATCH /stores-updating/:id', 'prototype.patchAttributes(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse PATCH /stores-updating/:id',
'createChangeStream(options:object):ReadableStream POST /stores-updating/change-stream', 'createChangeStream(options:object):ReadableStream POST /stores-updating/change-stream',
'createChangeStream(options:object):ReadableStream GET /stores-updating/change-stream', 'createChangeStream(options:object):ReadableStream GET /stores-updating/change-stream',
]; ];
@ -244,12 +244,12 @@ describe('With model.settings.replaceOnPUT true', function() {
var methods = getFormattedMethodsExcludingRelations(storeClass.methods); var methods = getFormattedMethodsExcludingRelations(storeClass.methods);
var expectedMethods = [ var expectedMethods = [
'patchOrCreate(data:object):storeWithReplaceOnPUTtrue PATCH /stores-replacing', 'patchOrCreate(data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue PATCH /stores-replacing',
'replaceOrCreate(data:object):storeWithReplaceOnPUTtrue POST /stores-replacing/replaceOrCreate', 'replaceOrCreate(data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue POST /stores-replacing/replaceOrCreate',
'replaceOrCreate(data:object):storeWithReplaceOnPUTtrue PUT /stores-replacing', 'replaceOrCreate(data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue PUT /stores-replacing',
'replaceById(id:any,data:object):storeWithReplaceOnPUTtrue POST /stores-replacing/:id/replace', 'replaceById(id:any,data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue POST /stores-replacing/:id/replace',
'replaceById(id:any,data:object):storeWithReplaceOnPUTtrue PUT /stores-replacing/:id', 'replaceById(id:any,data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue PUT /stores-replacing/:id',
'prototype.patchAttributes(data:object):storeWithReplaceOnPUTtrue PATCH /stores-replacing/:id', 'prototype.patchAttributes(data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue PATCH /stores-replacing/:id',
]; ];
expect(methods).to.include.members(expectedMethods); expect(methods).to.include.members(expectedMethods);
@ -273,7 +273,7 @@ function formatMethod(m) {
m.name, m.name,
'(', '(',
m.accepts.map(function(a) { m.accepts.map(function(a) {
return a.arg + ':' + a.type; return a.arg + ':' + a.type + (a.model ? ':' + a.model : '');
}).join(','), }).join(','),
')', ')',
formatReturns(m), formatReturns(m),