Skip to content

Expose Replace* methods for 2.x #2435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Amir-61 merged 1 commit into from
Aug 15, 2016
Merged

Expose Replace* methods for 2.x #2435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions common/models/user.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,12 @@
"permission": "ALLOW",
"property": "updateAttributes"
},
{
"principalType": "ROLE",
"principalId": "$owner",
"permission": "ALLOW",
"property": "replaceById"
},
{
"principalType": "ROLE",
"principalId": "$everyone",
Expand Down
135 changes: 118 additions & 17 deletions lib/persisted-model.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,27 @@ module.exports = function(registry) {
* @param {Object} model Updated model instance.
*/

PersistedModel.upsert = PersistedModel.updateOrCreate = function upsert(data, callback) {
PersistedModel.upsert = PersistedModel.updateOrCreate = PersistedModel.patchOrCreate =
function upsert(data, callback) {
throwNotAttached(this.modelName, 'upsert');
};

/**
* Replace or insert a model instance; replace existing record if one is found,
* such that parameter `data.id` matches `id` of model instance; otherwise,
* insert a new record.
* @param {Object} data The model instance data.
* @options {Object} [options] Options for replaceOrCreate
* @property {Boolean} validate Perform validation before saving. Default is true.
* @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, callback) {
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
Expand Down Expand Up @@ -492,10 +509,45 @@ module.exports = function(registry) {
* @param {Object} instance Updated instance.
*/

PersistedModel.prototype.updateAttributes = function updateAttributes(data, cb) {
PersistedModel.prototype.updateAttributes = PersistedModel.prototype.patchAttributes =
function updateAttributes(data, cb) {
throwNotAttached(this.modelName, 'updateAttributes');
};

/**
* Replace attributes for a model instance and persist it into the datasource.
* Performs validation before replacing.
*
* @param {Object} data Data to replace.
* @options {Object} [options] Options for replace
* @property {Boolean} validate Perform validation before saving. Default is true.
* @callback {Function} callback Callback function called with `(err, instance)` arguments.
* @param {Error} err Error object; see [Error object](http://docs.strongloop.com/display/LB/Error+object).
* @param {Object} instance Replaced instance.
*/

PersistedModel.prototype.replaceAttributes = function replaceAttributes(data, cb) {
throwNotAttached(this.modelName, 'replaceAttributes');
};

/**
* Replace attributes for a model instance whose id is the first input
* argument and persist it into the datasource.
* Performs validation before replacing.
*
* @param {*} id The ID value of model instance to replace.
* @param {Object} data Data to replace.
* @options {Object} [options] Options for replace
* @property {Boolean} validate Perform validation before saving. Default is true.
* @callback {Function} callback Callback function called with `(err, instance)` arguments.
* @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.
Expand Down Expand Up @@ -564,6 +616,9 @@ module.exports = function(registry) {
var typeName = PersistedModel.modelName;
var options = PersistedModel.settings;

// This is just for LB 2.x
options.replaceOnPUT = options.replaceOnPUT === true;

@bajtos bajtos Jun 20, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be set to false by default, shouldn't it?

@Amir-61 Amir-61 Jun 20, 2016 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bajtos:

I believe options.replaceOnPUT = options.replaceOnPUT === true; by default sets to false, isn't it? Lets see:
If options.replaceOnPUT is undefined undefined === true => false
If options.replaceOnPUT is false false === true => false
If options.replaceOnPUT is true true === true => true

Am I missing something?

@bajtos bajtos Jun 22, 2016

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh sorry, this is my fault, I overlooked === and thought it's =.


function setRemoting(scope, name, options) {
var fn = scope[name];
fn._delegate = true;
Expand All @@ -579,15 +634,35 @@ module.exports = function(registry) {
http: {verb: 'post', path: '/'}
});

setRemoting(PersistedModel, 'upsert', {
aliases: ['updateOrCreate'],
description: g.s('Update an existing model instance or insert a new one ' +
'into the data source.'),
var upsertOptions = {
aliases: ['patchOrCreate', 'updateOrCreate'],
description: g.s('Patch 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: '/'}
});
accepts: { arg: 'data', type: 'object', http: { source: 'body' }, description:
'Model instance data' },
returns: { arg: 'data', type: typeName, root: true },
http: [{ verb: 'patch', path: '/' }],
};

if (!options.replaceOnPUT) {
upsertOptions.http.push({ verb: 'put', path: '/' });
}
setRemoting(PersistedModel, 'upsert', upsertOptions);

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.push({ verb: 'put', path: '/' });
}

setRemoting(PersistedModel, 'replaceOrCreate', replaceOrCreateOptions);

setRemoting(PersistedModel, 'exists', {
description: g.s('Check whether a model instance exists in the data source.'),
Expand Down Expand Up @@ -634,6 +709,25 @@ 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, root: true },
http: [{ verb: 'post', path: '/:id/replace' }],
};

if (options.replaceOnPUT) {
replaceByIdOptions.http.push({ verb: 'put', path: '/:id' });
}

setRemoting(PersistedModel, 'replaceById', replaceByIdOptions);

setRemoting(PersistedModel, 'find', {
description: g.s('Find all instances of the model matched by filter from the data source.'),
accessType: 'READ',
Expand Down Expand Up @@ -702,14 +796,21 @@ module.exports = function(registry) {
http: {verb: 'get', path: '/count'}
});

setRemoting(PersistedModel.prototype, 'updateAttributes', {
description: g.s('Update attributes for a model instance and persist it into ' +
'the data source.'),
var updateAttributesOptions = {
aliases: ['patchAttributes'],
description: g.s('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'},
returns: {arg: 'data', type: typeName, root: true},
http: {verb: 'put', path: '/'}
});

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

setRemoting(PersistedModel.prototype, 'updateAttributes', updateAttributesOptions);

if (!options.replaceOnPUT) {
updateAttributesOptions.http.push({ verb: 'put', path: '/' });
}

if (options.trackChanges || options.enableRemoteReplication) {
setRemoting(PersistedModel, 'diff', {
Expand Down
126 changes: 111 additions & 15 deletions test/access-control.integration.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,15 @@ describe('access control - integration', function() {
assert.equal(user.password, undefined);
});
});

// user has replaceOnPUT = false; so then both PUT and PATCH should be allowed for update
lt.describe.whenCalledRemotely('PUT', '/api/users/:id', function() {
lt.it.shouldBeAllowed();
});

lt.describe.whenCalledRemotely('PATCH', '/api/users/:id', function() {
lt.it.shouldBeAllowed();
});
});

lt.it.shouldBeDeniedWhenCalledAnonymously('DELETE', urlForUser);
Expand Down Expand Up @@ -173,7 +179,7 @@ describe('access control - integration', function() {
}
});

describe('/accounts', function() {
describe('/accounts with replaceOnPUT true', function() {
var count = 0;
before(function() {
var roleModel = loopback.getModelByType(loopback.Role);
Expand All @@ -187,56 +193,146 @@ describe('access control - integration', function() {
});
});

lt.beforeEach.givenModel('account');
lt.beforeEach.givenModel('accountWithReplaceOnPUTtrue');

lt.it.shouldBeDeniedWhenCalledAnonymously('GET', '/api/accounts');
lt.it.shouldBeDeniedWhenCalledUnauthenticated('GET', '/api/accounts');
lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'GET', '/api/accounts');
lt.it.shouldBeDeniedWhenCalledAnonymously('GET', '/api/accounts-replacing');
lt.it.shouldBeDeniedWhenCalledUnauthenticated('GET', '/api/accounts-replacing');
lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'GET', '/api/accounts-replacing');

lt.it.shouldBeDeniedWhenCalledAnonymously('GET', urlForAccount);
lt.it.shouldBeDeniedWhenCalledUnauthenticated('GET', urlForAccount);
lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'GET', urlForAccount);

lt.it.shouldBeDeniedWhenCalledAnonymously('POST', '/api/accounts');
lt.it.shouldBeDeniedWhenCalledUnauthenticated('POST', '/api/accounts');
lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'POST', '/api/accounts');
lt.it.shouldBeDeniedWhenCalledAnonymously('POST', '/api/accounts-replacing');
lt.it.shouldBeDeniedWhenCalledUnauthenticated('POST', '/api/accounts-replacing');
lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'POST', '/api/accounts-replacing');

lt.it.shouldBeDeniedWhenCalledAnonymously('POST', urlForReplaceAccountPOST);
lt.it.shouldBeDeniedWhenCalledUnauthenticated('POST', urlForReplaceAccountPOST);
lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'POST', urlForReplaceAccountPOST);

lt.it.shouldBeDeniedWhenCalledAnonymously('PUT', urlForAccount);
lt.it.shouldBeDeniedWhenCalledUnauthenticated('PUT', urlForAccount);
lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'PUT', urlForAccount);

lt.it.shouldBeDeniedWhenCalledAnonymously('PATCH', urlForAccount);
lt.it.shouldBeDeniedWhenCalledUnauthenticated('PATCH', urlForAccount);
lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'PATCH', urlForAccount);

lt.describe.whenLoggedInAsUser(CURRENT_USER, function() {
var actId;
beforeEach(function(done) {
var self = this;

// Create an account under the given user
app.models.account.create({
app.models.accountWithReplaceOnPUTtrue.create({
userId: self.user.id,
balance: 100
}, function(err, act) {
self.url = '/api/accounts/' + act.id;
actId = act.id;
self.url = '/api/accounts-replacing/' + actId;
done();
});
});

lt.describe.whenCalledRemotely('PATCH', '/api/accounts-replacing/:id', function() {
lt.it.shouldBeAllowed();
});
lt.describe.whenCalledRemotely('PUT', '/api/accounts-replacing/:id', function() {
lt.it.shouldBeAllowed();
});
lt.describe.whenCalledRemotely('GET', '/api/accounts-replacing/:id', function() {
lt.it.shouldBeAllowed();
});
lt.describe.whenCalledRemotely('DELETE', '/api/accounts-replacing/:id', function() {
lt.it.shouldBeDenied();
});
describe('replace on POST verb', function() {
beforeEach(function(done) {
this.url = '/api/accounts-replacing/' + actId + '/replace';
done();
});
lt.describe.whenCalledRemotely('POST', '/api/accounts-replacing/:id/replace', function() {
lt.it.shouldBeAllowed();
});
});
});

lt.it.shouldBeDeniedWhenCalledAnonymously('DELETE', urlForAccount);
lt.it.shouldBeDeniedWhenCalledUnauthenticated('DELETE', urlForAccount);
lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'DELETE', urlForAccount);

function urlForAccount() {
return '/api/accounts-replacing/' + this.accountWithReplaceOnPUTtrue.id;
}
function urlForReplaceAccountPOST() {
return '/api/accounts-replacing/' + this.accountWithReplaceOnPUTtrue.id + '/replace';
}
});

describe('/accounts with replaceOnPUT false', function() {
lt.beforeEach.givenModel('accountWithReplaceOnPUTfalse');
lt.it.shouldBeDeniedWhenCalledAnonymously('POST', urlForReplaceAccountPOST);
lt.it.shouldBeDeniedWhenCalledUnauthenticated('POST', urlForReplaceAccountPOST);
lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'POST', urlForReplaceAccountPOST);

lt.it.shouldBeDeniedWhenCalledAnonymously('PUT', urlForAccount);
lt.it.shouldBeDeniedWhenCalledUnauthenticated('PUT', urlForAccount);
lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'PUT', urlForAccount);

lt.it.shouldBeDeniedWhenCalledAnonymously('PATCH', urlForAccount);
lt.it.shouldBeDeniedWhenCalledUnauthenticated('PATCH', urlForAccount);
lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'PATCH', urlForAccount);

lt.describe.whenLoggedInAsUser(CURRENT_USER, function() {
var actId;
beforeEach(function(done) {
var self = this;
// Create an account under the given user
app.models.accountWithReplaceOnPUTfalse.create({
userId: self.user.id,
balance: 100,
}, function(err, act) {
actId = act.id;
self.url = '/api/accounts-updating/' + actId;
done();
});
});
lt.describe.whenCalledRemotely('PUT', '/api/accounts/:id', function() {

lt.describe.whenCalledRemotely('PATCH', '/api/accounts-updating/:id', function() {
lt.it.shouldBeAllowed();
});

lt.describe.whenCalledRemotely('PUT', '/api/accounts-updating/:id', function() {

lt.it.shouldBeAllowed();
});
lt.describe.whenCalledRemotely('GET', '/api/accounts/:id', function() {
lt.describe.whenCalledRemotely('GET', '/api/accounts-updating/:id', function() {
lt.it.shouldBeAllowed();
});
lt.describe.whenCalledRemotely('DELETE', '/api/accounts/:id', function() {
lt.describe.whenCalledRemotely('DELETE', '/api/accounts-updating/:id', function() {
lt.it.shouldBeDenied();
});

describe('replace on POST verb', function() {
beforeEach(function(done) {
this.url = '/api/accounts-updating/' + actId + '/replace';
done();
});
lt.describe.whenCalledRemotely('POST', '/api/accounts-updating/:id/replace', function() {
lt.it.shouldBeAllowed();
});
});
});

lt.it.shouldBeDeniedWhenCalledAnonymously('DELETE', urlForAccount);
lt.it.shouldBeDeniedWhenCalledUnauthenticated('DELETE', urlForAccount);
lt.it.shouldBeDeniedWhenCalledByUser(CURRENT_USER, 'DELETE', urlForAccount);

function urlForAccount() {
return '/api/accounts/' + this.account.id;
return '/api/accounts-updating/' + this.accountWithReplaceOnPUTfalse.id;
}
function urlForReplaceAccountPOST() {
return '/api/accounts-updating/' + this.accountWithReplaceOnPUTfalse.id + '/replace';
}
});

Expand Down
Loading