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.
weflex · Sep 7, 2016 · f76edd5 · f76edd5
1 parent 554ccbd
commit f76edd5
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 49 deletions.
diff --git a/lib/model.js b/lib/model.js
@@ -488,7 +488,7 @@ module.exports = function(registry) {
     define('__create__' + relationName, {
       isStatic: false,
       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),
       accessType: 'WRITE',
       returns: { arg: 'data', type: toModelName, root: true },
@@ -497,7 +497,7 @@ module.exports = function(registry) {
     define('__update__' + relationName, {
       isStatic: false,
       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),
       accessType: 'WRITE',
       returns: { arg: 'data', type: toModelName, root: true },
@@ -551,7 +551,7 @@ module.exports = function(registry) {
           description: format('Foreign key for %s', relationName),
           required: true,
           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),
       accessType: 'WRITE',
@@ -564,7 +564,10 @@ module.exports = function(registry) {
       var accepts = [];
       if (relation.type === 'hasMany' && relation.modelThrough) {
         // 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];
@@ -654,7 +657,7 @@ module.exports = function(registry) {
     define('__create__' + scopeName, {
       isStatic: isStatic,
       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),
       accessType: 'WRITE',
       returns: { arg: 'data', type: toModelName, root: true },

diff --git a/lib/persisted-model.js b/lib/persisted-model.js
@@ -639,8 +639,11 @@ 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', http: { source: 'body' }, description:
-        'Model instance data' },
+      accepts: {
+        arg: 'data', type: 'object', model: typeName,
+        description: 'Model instance data',
+        http: { source: 'body' },
+      },
       returns: { arg: 'data', type: typeName, root: true },
       http: { verb: 'post', path: '/' },
     });
@@ -650,8 +653,10 @@ module.exports = function(registry) {
       description: 'Patch an existing model instance or insert a new one ' +
         'into the data source.',
       accessType: 'WRITE',
-      accepts: { arg: 'data', type: 'object', http: { source: 'body' }, description:
-        'Model instance data' },
+      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: '/' }],
     };
@@ -664,8 +669,11 @@ 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', http: { source: 'body' }, description:
-        'Model instance data' },
+      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' }],
     };
@@ -684,7 +692,7 @@ module.exports = function(registry) {
       accepts: [
         { arg: 'where', type: 'object', http: { source: 'query' },
           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' },
       ],
       returns: { arg: 'data', type: typeName, root: true },
@@ -742,7 +750,7 @@ module.exports = function(registry) {
       accepts: [
         { arg: 'id', type: 'any', description: 'Model id', required: true,
           http: { source: 'path' }},
-          { arg: 'data', type: 'object', http: { source: 'body' }, description:
+          { arg: 'data', type: 'object', model: typeName, http: { source: 'body' }, description:
         'Model instance data' },
       ],
       returns: { arg: 'data', type: typeName, root: true },
@@ -796,7 +804,7 @@ module.exports = function(registry) {
       accepts: [
         { arg: 'where', type: 'object', http: { source: 'query' },
           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' },
       ],
       returns: {
@@ -831,7 +839,11 @@ module.exports = function(registry) {
       description: 'Patch attributes for a model instance and persist it into ' +
         'the data source.',
       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 },
       http: [{ verb: 'patch', path: '/' }],
     };
@@ -927,7 +939,7 @@ module.exports = function(registry) {
             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',
           },
         ],

diff --git a/test/fixtures/simple-integration-app/common/models/widget.json b/test/fixtures/simple-integration-app/common/models/widget.json
@@ -1,10 +1,15 @@
 {
   "name": "widget",
-  "properties": {},
+  "properties": {
+    "name": {
+      "type": "string",
+      "default": "DefaultWidgetName"
+    }
+  },
   "relations": {
     "store": {
       "model": "store",
       "type": "belongsTo"
     }
   }
-}
+}
diff --git a/test/relations.integration.js b/test/relations.integration.js
@@ -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() {

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