diff --git a/common/models/user.json b/common/models/user.json index 16545ab4b..8ff1847b9 100644 --- a/common/models/user.json +++ b/common/models/user.json @@ -87,6 +87,12 @@ "permission": "ALLOW", "property": "resetPassword", "accessType": "EXECUTE" + }, + { + "principalType": "ROLE", + "principalId": "$owner", + "permission": "ALLOW", + "property": "replaceById" } ], "relations": { diff --git a/lib/persisted-model.js b/lib/persisted-model.js index 734992795..5e0ffd6ef 100644 --- a/lib/persisted-model.js +++ b/lib/persisted-model.js @@ -115,6 +115,18 @@ module.exports = function(registry) { throwNotAttached(this.modelName, 'upsert'); }; + /** + * Replace or insert a model instance + * @param {Object} data The model instance data to insert. + * @callback {Function} callback Callback function called with `cb(err, obj)` signature. + * @param {Error} err Error object; see [Error object](http://docs.strongloop.com/display/LB/Error+object). + * @param {Object} model Replaced model instance. + */ + + PersistedModel.replaceOrCreate = function replaceOrCreate(data, cb) { + throwNotAttached(this.modelName, 'replaceOrCreate'); + }; + /** * Finds one record matching the optional filter object. If not found, creates * the object using the data provided as second argument. In this sense it is @@ -482,6 +494,21 @@ module.exports = function(registry) { throwNotAttached(this.modelName, 'updateAttributes'); }; + /** + * Replace all attributes of a model instance identified by the given ID. + * Also performs validation before replacing. + * + * @param {id} id Primary key value of the instance. + * @param {Object} data Data to replace. + * @callback {Function} callback Callback function called with `(err, instance)` arguments. Required. + * @param {Error} err Error object; see [Error object](http://docs.strongloop.com/display/LB/Error+object). + * @param {Object} instance Replaced instance. + */ + + PersistedModel.replaceById = function replaceById(id, data, cb) { + throwNotAttached(this.modelName, 'replaceById'); + }; + /** * Reload object from persistence. Requires `id` member of `object` to be able to call `find`. * @callback {Function} callback Callback function called with `(err, instance)` arguments. Required. @@ -565,15 +592,43 @@ module.exports = function(registry) { http: {verb: 'post', path: '/'} }); + // forward compatibility setRemoting(PersistedModel, 'upsert', { aliases: ['updateOrCreate'], description: 'Update an existing model instance or insert a new one into the data source.', accessType: 'WRITE', accepts: {arg: 'data', type: 'object', description: 'Model instance data', http: {source: 'body'}}, returns: {arg: 'data', type: typeName, root: true}, - http: {verb: 'put', path: '/'} + http: { verb: 'patch', path: '/' }, }); + if (!options.replaceOnPUT) { + setRemoting(PersistedModel, 'upsert', { + aliases: ['updateOrCreate'], + description: 'Update an existing model instance or insert a new one into the data source.', + accessType: 'WRITE', + accepts: {arg: 'data', type: 'object', description: 'Model instance data', http: {source: 'body'}}, + returns: {arg: 'data', type: typeName, root: true}, + http: { verb: 'put', path: '/' }, + }); + } + + 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' }, + returns: { arg: 'data', type: typeName, root: true }, + http: { verb: 'post', path: '/replaceOrCreate' }, + }; + + if (options.replaceOnPUT) { + replaceOrCreateOptions.http.verb = 'put'; + replaceOrCreateOptions.http.path = '/'; + } + + setRemoting(PersistedModel, 'replaceOrCreate', replaceOrCreateOptions); + setRemoting(PersistedModel, 'exists', { description: 'Check whether a model instance exists in the data source.', accessType: 'READ', @@ -619,6 +674,27 @@ module.exports = function(registry) { rest: {after: convertNullToNotFoundError} }); + var replaceByIdOptions = { + 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', http: { source: 'body' }, description: + 'Model instance data' }, + ], + returns: { arg: 'data', type: typeName }, + http: { verb: 'post', path: '/:id/replace' }, + rest: { after: convertNullToNotFoundError }, + }; + + if (options.replaceOnPUT) { + replaceByIdOptions.http.verb = 'put'; + replaceOrCreateOptions.http.path = '/:id'; + } + + setRemoting(PersistedModel, 'replaceById', replaceByIdOptions); + setRemoting(PersistedModel, 'find', { description: 'Find all instances of the model matched by filter from the data source.', accessType: 'READ', @@ -692,9 +768,19 @@ module.exports = function(registry) { accessType: 'WRITE', accepts: {arg: 'data', type: 'object', http: {source: 'body'}, description: 'An object of model property name/value pairs'}, returns: {arg: 'data', type: typeName, root: true}, - http: {verb: 'put', path: '/'} + http: {verb: 'patch', path: '/'} }); + if (!options.replaceOnPUT) { + setRemoting(PersistedModel.prototype, 'updateAttributes', { + description: 'Update 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'}, + returns: {arg: 'data', type: typeName, root: true}, + http: {verb: 'put', path: '/'} + }); + } + if (options.trackChanges || options.enableRemoteReplication) { setRemoting(PersistedModel, 'diff', { description: 'Get a set of deltas and conflicts since the given checkpoint.', diff --git a/test/access-control.integration.js b/test/access-control.integration.js index b32152573..bbc5a192d 100644 --- a/test/access-control.integration.js +++ b/test/access-control.integration.js @@ -122,6 +122,11 @@ describe('access control - integration', function() { }); }); lt.describe.whenCalledRemotely('PUT', '/api/users/:id', function() { + beforeEach(function(done) { + app.models.user.settings.replaceOnPUT = false; + app.models.user.setup(); + done(); + }); lt.it.shouldBeAllowed(); }); }); @@ -208,7 +213,8 @@ describe('access control - integration', function() { lt.describe.whenLoggedInAsUser(CURRENT_USER, function() { beforeEach(function(done) { var self = this; - + app.models.account.settings.replaceOnPUT = true; + app.models.account.setup(); // Create an account under the given user app.models.account.create({ userId: self.user.id, @@ -220,6 +226,10 @@ describe('access control - integration', function() { }); }); + // + // TODO: How to check Model.settings.options.replaceOnPUT + // to decide whether the following test should be for + // (POST for replace and (PATCH AND PUT) for update) OR (PUT for replace and PATCH for update) lt.describe.whenCalledRemotely('PUT', '/api/accounts/:id', function() { lt.it.shouldBeAllowed(); }); diff --git a/test/model.test.js b/test/model.test.js index a02690011..2991f98e3 100644 --- a/test/model.test.js +++ b/test/model.test.js @@ -645,6 +645,8 @@ describe.onServer('Remote Methods', function() { 'upsert', 'updateOrCreate', 'exists', 'findById', + 'replaceById', + 'replaceOrCreate', 'find', 'findOne', 'updateAll', 'update', diff --git a/test/remoting.integration.js b/test/remoting.integration.js index 5154bb473..9e41b6e13 100644 --- a/test/remoting.integration.js +++ b/test/remoting.integration.js @@ -113,7 +113,7 @@ describe('remoting - integration', function() { })[0]; } - it('has expected remote methods', function() { + it('has expected remote methods without model.settings.replaceOnPUT', function() { var storeClass = findClass('store'); var methods = storeClass.methods .filter(function(m) { @@ -126,8 +126,11 @@ describe('remoting - integration', function() { var expectedMethods = [ 'create(data:object):store POST /stores', 'upsert(data:object):store PUT /stores', + 'upsert(data:object):store PATCH /stores', + 'replaceOrCreate(data:object):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 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', @@ -142,6 +145,42 @@ describe('remoting - integration', function() { expect(methods).to.include.members(expectedMethods); }); + // TODO: apparently the way I set up model settings is not right? + it.skip('has expected remote methods with model.settings.replaceOnPUT', function() { + app.models.store.settings.replaceOnPUT = true; + app.models.store.setup(); + var storeClass = findClass('store'); + var methods = storeClass.methods + .filter(function(m) { + return m.name.indexOf('__') === -1; + }) + .map(function(m) { + return formatMethod(m); + }); + + var expectedMethods = [ + 'create(data:object):store POST /stores', + 'upsert(data:object):store PUT /stores', + 'upsert(data:object):store PATCH /stores', + 'replaceOrCreate(data:object):store PUT /stores', + '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', + 'find(filter:object):store GET /stores', + 'findOne(filter:object):store GET /stores/findOne', + 'updateAll(where:object,data:object):object POST /stores/update', + 'deleteById(id:any):object DELETE /stores/:id', + 'count(where:object):number GET /stores/count', + 'prototype.updateAttributes(data:object):store PUT /stores/:id', + 'prototype.updateAttributes(data:object):store PATCH /stores/:id', + 'createChangeStream(options:object):ReadableStream POST /stores/change-stream', + ]; + + // TODO: update the doc for list of methods accordingly + // https://docs.strongloop.com/display/public/LB/Exposing+models+over+REST + expect(methods).to.include.members(expectedMethods); + }); + it('has expected remote methods for scopes', function() { var storeClass = findClass('store'); var methods = storeClass.methods