Skip to content

Commit f76edd5

Browse files
bajtosbajtos
committed
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.
1 parent 554ccbd commit f76edd5

File tree

5 files changed

+95
-49
lines changed

5 files changed

+95
-49
lines changed

‎lib/model.js‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ module.exports = function(registry) {
488488
define('__create__' + relationName, {
489489
isStatic: false,
490490
http: { verb: 'post', path: '/' + pathName },
491-
accepts: { arg: 'data', type: toModelName, http: { source: 'body' }},
491+
accepts: { arg: 'data', type: 'object', model: toModelName, http: { source: 'body' }},
492492
description: format('Creates a new instance in %s of this model.', relationName),
493493
accessType: 'WRITE',
494494
returns: { arg: 'data', type: toModelName, root: true },
@@ -497,7 +497,7 @@ module.exports = function(registry) {
497497
define('__update__' + relationName, {
498498
isStatic: false,
499499
http: { verb: 'put', path: '/' + pathName },
500-
accepts: { arg: 'data', type: toModelName, http: { source: 'body' }},
500+
accepts: { arg: 'data', type: 'object', model: toModelName, http: { source: 'body' }},
501501
description: format('Update %s of this model.', relationName),
502502
accessType: 'WRITE',
503503
returns: { arg: 'data', type: toModelName, root: true },
@@ -551,7 +551,7 @@ module.exports = function(registry) {
551551
description: format('Foreign key for %s', relationName),
552552
required: true,
553553
http: { source: 'path' }},
554-
{ arg: 'data', type: toModelName, http: { source: 'body' }},
554+
{ arg: 'data', type: 'object', model: toModelName, http: { source: 'body' }},
555555
],
556556
description: format('Update a related item by id for %s.', relationName),
557557
accessType: 'WRITE',
@@ -564,7 +564,10 @@ module.exports = function(registry) {
564564
var accepts = [];
565565
if (relation.type === 'hasMany' && relation.modelThrough) {
566566
// Restrict: only hasManyThrough relation can have additional properties
567-
accepts.push({ arg: 'data', type: modelThrough.modelName, http: { source: 'body' }});
567+
accepts.push({
568+
arg: 'data', type: 'object', model: modelThrough.modelName,
569+
http: { source: 'body' },
570+
});
568571
}
569572

570573
var addFunc = this.prototype['__link__' + relationName];
@@ -654,7 +657,7 @@ module.exports = function(registry) {
654657
define('__create__' + scopeName, {
655658
isStatic: isStatic,
656659
http: { verb: 'post', path: '/' + pathName },
657-
accepts: { arg: 'data', type: toModelName, http: { source: 'body' }},
660+
accepts: { arg: 'data', type: 'object', model: toModelName, http: { source: 'body' }},
658661
description: format('Creates a new instance in %s of this model.', scopeName),
659662
accessType: 'WRITE',
660663
returns: { arg: 'data', type: toModelName, root: true },

‎lib/persisted-model.js‎

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -639,8 +639,11 @@ module.exports = function(registry) {
639639
setRemoting(PersistedModel, 'create', {
640640
description: 'Create a new instance of the model and persist it into the data source.',
641641
accessType: 'WRITE',
642-
accepts: { arg: 'data', type: 'object', http: { source: 'body' }, description:
643-
'Model instance data' },
642+
accepts: {
643+
arg: 'data', type: 'object', model: typeName,
644+
description: 'Model instance data',
645+
http: { source: 'body' },
646+
},
644647
returns: { arg: 'data', type: typeName, root: true },
645648
http: { verb: 'post', path: '/' },
646649
});
@@ -650,8 +653,10 @@ module.exports = function(registry) {
650653
description: 'Patch an existing model instance or insert a new one ' +
651654
'into the data source.',
652655
accessType: 'WRITE',
653-
accepts: { arg: 'data', type: 'object', http: { source: 'body' }, description:
654-
'Model instance data' },
656+
accepts: {
657+
arg: 'data', type: 'object', model: typeName, http: { source: 'body' },
658+
description: 'Model instance data',
659+
},
655660
returns: { arg: 'data', type: typeName, root: true },
656661
http: [{ verb: 'patch', path: '/' }],
657662
};
@@ -664,8 +669,11 @@ module.exports = function(registry) {
664669
var replaceOrCreateOptions = {
665670
description: 'Replace an existing model instance or insert a new one into the data source.',
666671
accessType: 'WRITE',
667-
accepts: { arg: 'data', type: 'object', http: { source: 'body' }, description:
668-
'Model instance data' },
672+
accepts: {
673+
arg: 'data', type: 'object', model: typeName,
674+
http: { source: 'body' },
675+
description: 'Model instance data',
676+
},
669677
returns: { arg: 'data', type: typeName, root: true },
670678
http: [{ verb: 'post', path: '/replaceOrCreate' }],
671679
};
@@ -684,7 +692,7 @@ module.exports = function(registry) {
684692
accepts: [
685693
{ arg: 'where', type: 'object', http: { source: 'query' },
686694
description: 'Criteria to match model instances' },
687-
{ arg: 'data', type: 'object', http: { source: 'body' },
695+
{ arg: 'data', type: 'object', model: typeName, http: { source: 'body' },
688696
description: 'An object of model property name/value pairs' },
689697
],
690698
returns: { arg: 'data', type: typeName, root: true },
@@ -742,7 +750,7 @@ module.exports = function(registry) {
742750
accepts: [
743751
{ arg: 'id', type: 'any', description: 'Model id', required: true,
744752
http: { source: 'path' }},
745-
{ arg: 'data', type: 'object', http: { source: 'body' }, description:
753+
{ arg: 'data', type: 'object', model: typeName, http: { source: 'body' }, description:
746754
'Model instance data' },
747755
],
748756
returns: { arg: 'data', type: typeName, root: true },
@@ -796,7 +804,7 @@ module.exports = function(registry) {
796804
accepts: [
797805
{ arg: 'where', type: 'object', http: { source: 'query' },
798806
description: 'Criteria to match model instances' },
799-
{ arg: 'data', type: 'object', http: { source: 'body' },
807+
{ arg: 'data', type: 'object', model: typeName, http: { source: 'body' },
800808
description: 'An object of model property name/value pairs' },
801809
],
802810
returns: {
@@ -831,7 +839,11 @@ module.exports = function(registry) {
831839
description: 'Patch attributes for a model instance and persist it into ' +
832840
'the data source.',
833841
accessType: 'WRITE',
834-
accepts: { arg: 'data', type: 'object', http: { source: 'body' }, description: 'An object of model property name/value pairs' },
842+
accepts: {
843+
arg: 'data', type: 'object', model: typeName,
844+
http: { source: 'body' },
845+
description: 'An object of model property name/value pairs',
846+
},
835847
returns: { arg: 'data', type: typeName, root: true },
836848
http: [{ verb: 'patch', path: '/' }],
837849
};
@@ -927,7 +939,7 @@ module.exports = function(registry) {
927939
description: 'Model id',
928940
},
929941
{
930-
arg: 'data', type: 'object', http: { source: 'body' },
942+
arg: 'data', type: 'object', model: typeName, http: { source: 'body' },
931943
description: 'An object of Change property name/value pairs',
932944
},
933945
],

‎test/fixtures/simple-integration-app/common/models/widget.json‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
{
22
"name": "widget",
3-
"properties": {},
3+
"properties": {
4+
"name": {
5+
"type": "string",
6+
"default": "DefaultWidgetName"
7+
}
8+
},
49
"relations": {
510
"store": {
611
"model": "store",
712
"type": "belongsTo"
813
}
914
}
10-
}
15+
}

‎test/relations.integration.js‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,32 @@ describe('relations - integration', function() {
224224
});
225225
});
226226
});
227+
228+
describe('PUT /api/store/:id/widgets/:fk', function() {
229+
beforeEach(function(done) {
230+
var self = this;
231+
this.store.widgets.create({
232+
name: this.widgetName,
233+
}, function(err, widget) {
234+
self.widget = widget;
235+
self.url = '/api/stores/' + self.store.id + '/widgets/' + widget.id;
236+
done();
237+
});
238+
});
239+
it('does not add default properties to request body', function(done) {
240+
var self = this;
241+
self.request.put(self.url)
242+
.send({ active: true })
243+
.end(function(err) {
244+
if (err) return done(err);
245+
app.models.Widget.findById(self.widget.id, function(err, w) {
246+
if (err) return done(err);
247+
expect(w.name).to.equal(self.widgetName);
248+
done();
249+
});
250+
});
251+
});
252+
});
227253
});
228254

229255
describe('/stores/:id/widgets/:fk - 200', function() {

‎test/remoting.integration.js‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -81,20 +81,20 @@ describe('remoting - integration', function() {
8181
var methods = getFormattedMethodsExcludingRelations(storeClass.methods);
8282

8383
var expectedMethods = [
84-
'create(data:object):store POST /stores',
85-
'patchOrCreate(data:object):store PATCH /stores',
86-
'replaceOrCreate(data:object):store PUT /stores',
87-
'replaceOrCreate(data:object):store POST /stores/replaceOrCreate',
84+
'create(data:object:store):store POST /stores',
85+
'patchOrCreate(data:object:store):store PATCH /stores',
86+
'replaceOrCreate(data:object:store):store PUT /stores',
87+
'replaceOrCreate(data:object:store):store POST /stores/replaceOrCreate',
8888
'exists(id:any):boolean GET /stores/:id/exists',
8989
'findById(id:any,filter:object):store GET /stores/:id',
90-
'replaceById(id:any,data:object):store PUT /stores/:id',
91-
'replaceById(id:any,data:object):store POST /stores/:id/replace',
90+
'replaceById(id:any,data:object:store):store PUT /stores/:id',
91+
'replaceById(id:any,data:object:store):store POST /stores/:id/replace',
9292
'find(filter:object):store GET /stores',
9393
'findOne(filter:object):store GET /stores/findOne',
94-
'updateAll(where:object,data:object):object POST /stores/update',
94+
'updateAll(where:object,data:object:store):object POST /stores/update',
9595
'deleteById(id:any):object DELETE /stores/:id',
9696
'count(where:object):number GET /stores/count',
97-
'prototype.patchAttributes(data:object):store PATCH /stores/:id',
97+
'prototype.patchAttributes(data:object:store):store PATCH /stores/:id',
9898
'createChangeStream(options:object):ReadableStream POST /stores/change-stream',
9999
];
100100

@@ -109,7 +109,7 @@ describe('remoting - integration', function() {
109109

110110
var expectedMethods = [
111111
'__get__superStores(filter:object):store GET /stores/superStores',
112-
'__create__superStores(data:store):store POST /stores/superStores',
112+
'__create__superStores(data:object:store):store POST /stores/superStores',
113113
'__delete__superStores() DELETE /stores/superStores',
114114
'__count__superStores(where:object):number GET /stores/superStores/count',
115115
];
@@ -139,11 +139,11 @@ describe('remoting - integration', function() {
139139
'GET /stores/:id/widgets/:fk',
140140
'prototype.__destroyById__widgets(fk:any) ' +
141141
'DELETE /stores/:id/widgets/:fk',
142-
'prototype.__updateById__widgets(fk:any,data:widget):widget ' +
142+
'prototype.__updateById__widgets(fk:any,data:object:widget):widget ' +
143143
'PUT /stores/:id/widgets/:fk',
144144
'prototype.__get__widgets(filter:object):widget ' +
145145
'GET /stores/:id/widgets',
146-
'prototype.__create__widgets(data:widget):widget ' +
146+
'prototype.__create__widgets(data:object:widget):widget ' +
147147
'POST /stores/:id/widgets',
148148
'prototype.__delete__widgets() ' +
149149
'DELETE /stores/:id/widgets',
@@ -163,17 +163,17 @@ describe('remoting - integration', function() {
163163
'GET /physicians/:id/patients/:fk',
164164
'prototype.__destroyById__patients(fk:any) ' +
165165
'DELETE /physicians/:id/patients/:fk',
166-
'prototype.__updateById__patients(fk:any,data:patient):patient ' +
166+
'prototype.__updateById__patients(fk:any,data:object:patient):patient ' +
167167
'PUT /physicians/:id/patients/:fk',
168-
'prototype.__link__patients(fk:any,data:appointment):appointment ' +
168+
'prototype.__link__patients(fk:any,data:object:appointment):appointment ' +
169169
'PUT /physicians/:id/patients/rel/:fk',
170170
'prototype.__unlink__patients(fk:any) ' +
171171
'DELETE /physicians/:id/patients/rel/:fk',
172172
'prototype.__exists__patients(fk:any):boolean ' +
173173
'HEAD /physicians/:id/patients/rel/:fk',
174174
'prototype.__get__patients(filter:object):patient ' +
175175
'GET /physicians/:id/patients',
176-
'prototype.__create__patients(data:patient):patient ' +
176+
'prototype.__create__patients(data:object:patient):patient ' +
177177
'POST /physicians/:id/patients',
178178
'prototype.__delete__patients() ' +
179179
'DELETE /physicians/:id/patients',
@@ -188,7 +188,7 @@ describe('remoting - integration', function() {
188188
var storeClass = findClass('store');
189189
var methods = getFormattedMethodsExcludingRelations(storeClass.methods);
190190
var expectedMethods = [
191-
'upsertWithWhere(where:object,data:object):store POST /stores/upsertWithWhere',
191+
'upsertWithWhere(where:object,data:object:store):store POST /stores/upsertWithWhere',
192192
];
193193
expect(methods).to.include.members(expectedMethods);
194194
});
@@ -207,22 +207,22 @@ describe('With model.settings.replaceOnPUT false', function() {
207207
var methods = getFormattedMethodsExcludingRelations(storeClass.methods);
208208

209209
var expectedMethods = [
210-
'create(data:object):storeWithReplaceOnPUTfalse POST /stores-updating',
211-
'patchOrCreate(data:object):storeWithReplaceOnPUTfalse PUT /stores-updating',
212-
'patchOrCreate(data:object):storeWithReplaceOnPUTfalse PATCH /stores-updating',
213-
'replaceOrCreate(data:object):storeWithReplaceOnPUTfalse POST /stores-updating/replaceOrCreate',
214-
'upsertWithWhere(where:object,data:object):storeWithReplaceOnPUTfalse POST /stores-updating/upsertWithWhere',
210+
'create(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse POST /stores-updating',
211+
'patchOrCreate(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse PUT /stores-updating',
212+
'patchOrCreate(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse PATCH /stores-updating',
213+
'replaceOrCreate(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse POST /stores-updating/replaceOrCreate',
214+
'upsertWithWhere(where:object,data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse POST /stores-updating/upsertWithWhere',
215215
'exists(id:any):boolean GET /stores-updating/:id/exists',
216216
'exists(id:any):boolean HEAD /stores-updating/:id',
217217
'findById(id:any,filter:object):storeWithReplaceOnPUTfalse GET /stores-updating/:id',
218-
'replaceById(id:any,data:object):storeWithReplaceOnPUTfalse POST /stores-updating/:id/replace',
218+
'replaceById(id:any,data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse POST /stores-updating/:id/replace',
219219
'find(filter:object):storeWithReplaceOnPUTfalse GET /stores-updating',
220220
'findOne(filter:object):storeWithReplaceOnPUTfalse GET /stores-updating/findOne',
221-
'updateAll(where:object,data:object):object POST /stores-updating/update',
221+
'updateAll(where:object,data:object:storeWithReplaceOnPUTfalse):object POST /stores-updating/update',
222222
'deleteById(id:any):object DELETE /stores-updating/:id',
223223
'count(where:object):number GET /stores-updating/count',
224-
'prototype.patchAttributes(data:object):storeWithReplaceOnPUTfalse PUT /stores-updating/:id',
225-
'prototype.patchAttributes(data:object):storeWithReplaceOnPUTfalse PATCH /stores-updating/:id',
224+
'prototype.patchAttributes(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse PUT /stores-updating/:id',
225+
'prototype.patchAttributes(data:object:storeWithReplaceOnPUTfalse):storeWithReplaceOnPUTfalse PATCH /stores-updating/:id',
226226
'createChangeStream(options:object):ReadableStream POST /stores-updating/change-stream',
227227
'createChangeStream(options:object):ReadableStream GET /stores-updating/change-stream',
228228
];
@@ -244,12 +244,12 @@ describe('With model.settings.replaceOnPUT true', function() {
244244
var methods = getFormattedMethodsExcludingRelations(storeClass.methods);
245245

246246
var expectedMethods = [
247-
'patchOrCreate(data:object):storeWithReplaceOnPUTtrue PATCH /stores-replacing',
248-
'replaceOrCreate(data:object):storeWithReplaceOnPUTtrue POST /stores-replacing/replaceOrCreate',
249-
'replaceOrCreate(data:object):storeWithReplaceOnPUTtrue PUT /stores-replacing',
250-
'replaceById(id:any,data:object):storeWithReplaceOnPUTtrue POST /stores-replacing/:id/replace',
251-
'replaceById(id:any,data:object):storeWithReplaceOnPUTtrue PUT /stores-replacing/:id',
252-
'prototype.patchAttributes(data:object):storeWithReplaceOnPUTtrue PATCH /stores-replacing/:id',
247+
'patchOrCreate(data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue PATCH /stores-replacing',
248+
'replaceOrCreate(data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue POST /stores-replacing/replaceOrCreate',
249+
'replaceOrCreate(data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue PUT /stores-replacing',
250+
'replaceById(id:any,data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue POST /stores-replacing/:id/replace',
251+
'replaceById(id:any,data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue PUT /stores-replacing/:id',
252+
'prototype.patchAttributes(data:object:storeWithReplaceOnPUTtrue):storeWithReplaceOnPUTtrue PATCH /stores-replacing/:id',
253253
];
254254

255255
expect(methods).to.include.members(expectedMethods);
@@ -273,7 +273,7 @@ function formatMethod(m) {
273273
m.name,
274274
'(',
275275
m.accepts.map(function(a) {
276-
return a.arg + ':' + a.type;
276+
return a.arg + ':' + a.type + (a.model ? ':' + a.model : '');
277277
}).join(','),
278278
')',
279279
formatReturns(m),

0 commit comments

Comments
 (0)