From 74018019b48295075ce5a0a7fd69e0b8c3ffc184 Mon Sep 17 00:00:00 2001
From: Esco Obong <menzoic@gmail.com>
Date: Thu, 12 Mar 2015 14:55:39 -0400
Subject: [PATCH 1/6] fix implementation of Role methods: users,roles, and
applications
---
common/models/role.js | 49 +++++++++++++++++++++++++++----------------
test/role.test.js | 46 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 77 insertions(+), 18 deletions(-)
diff --git a/common/models/role.js b/common/models/role.js
index 2c005cde..c3b996a1 100644
--- a/common/models/role.js
+++ b/common/models/role.js
@@ -29,42 +29,55 @@ module.exports = function(Role) {
// Set up the connection to users/applications/roles once the model
Role.once('dataSourceAttached', function() {
var roleMappingModel = this.RoleMapping || loopback.getModelByType(RoleMapping);
+
+ /**
+ * Fetch the ids of all users assigned to this role
+ * @param {Function} callback
+ */
Role.prototype.users = function(callback) {
- roleMappingModel.find({where: {roleId: this.id,
- principalType: RoleMapping.USER}}, function(err, mappings) {
+ roleMappingModel.find({
+ where: {roleId: this.id, principalType: RoleMapping.USER}
+ }, function(err, mappings) {
if (err) {
- if (callback) callback(err);
- return;
+ return callback(err);
}
- return mappings.map(function(m) {
+ callback(null, mappings.map(function(m) {
return m.principalId;
- });
+ }));
});
};
+ /**
+ * Fetch the ids of all applications assigned to this role
+ * @param {Function} callback
+ */
Role.prototype.applications = function(callback) {
- roleMappingModel.find({where: {roleId: this.id,
- principalType: RoleMapping.APPLICATION}}, function(err, mappings) {
+ roleMappingModel.find({
+ where: {roleId: this.id, principalType: RoleMapping.APPLICATION}
+ }, function(err, mappings) {
if (err) {
- if (callback) callback(err);
- return;
+ return callback(err);
}
- return mappings.map(function(m) {
+ callback(null, mappings.map(function(m) {
return m.principalId;
- });
+ }));
});
};
+ /**
+ * Fetch the ids of all roles assigned to this role
+ * @param {Function} callback
+ */
Role.prototype.roles = function(callback) {
- roleMappingModel.find({where: {roleId: this.id,
- principalType: RoleMapping.ROLE}}, function(err, mappings) {
+ roleMappingModel.find({
+ where: {roleId: this.id, principalType: RoleMapping.ROLE}
+ }, function(err, mappings) {
if (err) {
- if (callback) callback(err);
- return;
+ return callback(err);
}
- return mappings.map(function(m) {
+ callback(null, mappings.map(function(m) {
return m.principalId;
- });
+ }));
});
};
diff --git a/test/role.test.js b/test/role.test.js
index dfedc6cc..f10a1c33 100644
--- a/test/role.test.js
+++ b/test/role.test.js
@@ -3,6 +3,7 @@ var loopback = require('../index');
var Role = loopback.Role;
var RoleMapping = loopback.RoleMapping;
var User = loopback.User;
+var Application = loopback.Application;
var ACL = loopback.ACL;
function checkResult(err, result) {
@@ -208,4 +209,49 @@ describe('role model', function() {
});
+ it('should fetch all user ids assigned to the role', function(done) {
+ User.create({name: 'Raymond', email: 'x@y.com', password: 'foobar'}, function(err, user) {
+ Role.create({name: 'userRole'}, function(err, role) {
+ role.principals.create({principalType: RoleMapping.USER, principalId: user.id}, function(err, p) {
+ role.users(function(err, users) {
+ assert(!err);
+ assert.equal(users.length, 1);
+ assert.equal(users[0], user.id);
+ done();
+ });
+ });
+ });
+ });
+ });
+
+ it('should fetch all application ids assigned to the role', function(done) {
+ Application.create({name: 'New App'}, function(err, application) {
+ Role.create({name: 'applicationRole'}, function(err, role) {
+ role.principals.create({principalType: RoleMapping.APPLICATION, principalId: application.id}, function(err, p) {
+ role.applications(function(err, applications) {
+ assert(!err);
+ assert.equal(applications.length, 1);
+ assert.equal(applications[0], application.id);
+ done();
+ });
+ });
+ });
+ });
+ });
+
+ it('should fetch all role ids assigned to the role', function(done) {
+ Role.create({name: 'New Role'}, function(err, newRole) {
+ Role.create({name: 'applicationRole'}, function(err, role) {
+ role.principals.create({principalType: RoleMapping.ROLE, principalId: newRole.id}, function(err, p) {
+ role.roles(function(err, roles) {
+ assert(!err);
+ assert.equal(roles.length, 1);
+ assert.equal(roles[0], newRole.id);
+ done();
+ });
+ });
+ });
+ });
+ });
+
});
From 8cc558a991d915be8483c7c61abc7d91cf1d53bc Mon Sep 17 00:00:00 2001
From: Esco Obong <menzoic@gmail.com>
Date: Fri, 13 Mar 2015 11:50:30 -0400
Subject: [PATCH 2/6] consolidate Role methods roles, applications, and users
into one, add query param to allow for pagination and restricting fields
---
common/models/role.js | 95 ++++++++++++++++++++++++-------------------
lib/builtin-models.js | 8 ++--
package.json | 1 +
test/role.test.js | 90 +++++++++++++++++++++++-----------------
4 files changed, 112 insertions(+), 82 deletions(-)
diff --git a/common/models/role.js b/common/models/role.js
index c3b996a1..b8f36317 100644
--- a/common/models/role.js
+++ b/common/models/role.js
@@ -6,6 +6,10 @@ var async = require('async');
var AccessContext = require('../../lib/access-context').AccessContext;
var RoleMapping = loopback.RoleMapping;
+var Role = loopback.Role;
+var User = loopback.User;
+var Application = loopback.Application;
+
assert(RoleMapping, 'RoleMapping model must be defined before Role model');
/**
@@ -29,57 +33,66 @@ module.exports = function(Role) {
// Set up the connection to users/applications/roles once the model
Role.once('dataSourceAttached', function() {
var roleMappingModel = this.RoleMapping || loopback.getModelByType(RoleMapping);
+ var principalTypesToModels = {};
+
+ principalTypesToModels[RoleMapping.USER] = User;
+ principalTypesToModels[RoleMapping.APPLICATION] = Application;
+ principalTypesToModels[RoleMapping.ROLE] = Role;
+
+ Object.keys(principalTypesToModels).forEach(function(principalType){
+ var model = principalTypesToModels[principalType];
+ var pluralName = model.pluralModelName.toLowerCase();
+ /**
+ * Fetch all users assigned to this role
+ * @function Role.prototype#users
+ * @param {object} [query] query object passed to model find call
+ * @param {Function} [callback]
+ */
+ /**
+ * Fetch all applications assigned to this role
+ * @function Role.prototype#applications
+ * @param {object} [query] query object passed to model find call
+ * @param {Function} [callback]
+ */
+ /**
+ * Fetch all roles assigned to this role
+ * @function Role.prototype#roles
+ * @param {object} [query] query object passed to model find call
+ * @param {Function} [callback]
+ */
+ Role.prototype[pluralName] = function(query, callback) {
+ listByPrincipalType(model, principalType, query, callback);
+ }
+ });
/**
- * Fetch the ids of all users assigned to this role
- * @param {Function} callback
+ * Fetch all models assigned to this role
+ * @param {*} model model type to fetch
+ * @param {String} [principalType] principalType used in the rolemapping for model
+ * @param {object} [query] query object passed to model find call
+ * @param {Function} [callback]
*/
- Role.prototype.users = function(callback) {
+ function listByPrincipalType(model, principalType, query, callback) {
+ if (callback === undefined) {
+ callback = query;
+ query = {};
+ }
+
roleMappingModel.find({
- where: {roleId: this.id, principalType: RoleMapping.USER}
+ where: {roleId: this.id, principalType: principalType}
}, function(err, mappings) {
+ var ids;
if (err) {
return callback(err);
}
- callback(null, mappings.map(function(m) {
+ ids = mappings.map(function(m) {
return m.principalId;
- }));
+ });
+ query.where = query.where || {};
+ query.where.id = {inq: ids};
+ model.find(query, callback);
});
- };
-
- /**
- * Fetch the ids of all applications assigned to this role
- * @param {Function} callback
- */
- Role.prototype.applications = function(callback) {
- roleMappingModel.find({
- where: {roleId: this.id, principalType: RoleMapping.APPLICATION}
- }, function(err, mappings) {
- if (err) {
- return callback(err);
- }
- callback(null, mappings.map(function(m) {
- return m.principalId;
- }));
- });
- };
-
- /**
- * Fetch the ids of all roles assigned to this role
- * @param {Function} callback
- */
- Role.prototype.roles = function(callback) {
- roleMappingModel.find({
- where: {roleId: this.id, principalType: RoleMapping.ROLE}
- }, function(err, mappings) {
- if (err) {
- return callback(err);
- }
- callback(null, mappings.map(function(m) {
- return m.principalId;
- }));
- });
- };
+ }
});
diff --git a/lib/builtin-models.js b/lib/builtin-models.js
index 2a346d3e..4a986e8f 100644
--- a/lib/builtin-models.js
+++ b/lib/builtin-models.js
@@ -13,6 +13,10 @@ module.exports = function(loopback) {
require('../common/models/access-token.json'),
require('../common/models/access-token.js'));
+ loopback.User = createModel(
+ require('../common/models/user.json'),
+ require('../common/models/user.js'));
+
loopback.RoleMapping = createModel(
require('../common/models/role-mapping.json'),
require('../common/models/role-mapping.js'));
@@ -29,10 +33,6 @@ module.exports = function(loopback) {
require('../common/models/scope.json'),
require('../common/models/scope.js'));
- loopback.User = createModel(
- require('../common/models/user.json'),
- require('../common/models/user.js'));
-
loopback.Change = createModel(
require('../common/models/change.json'),
require('../common/models/change.js'));
diff --git a/package.json b/package.json
index 2e065bad..b932f7f8 100644
--- a/package.json
+++ b/package.json
@@ -83,6 +83,7 @@
"loopback-datasource-juggler": "^2.19.1",
"loopback-testing": "^1.1.0",
"mocha": "^2.1.0",
+ "sinon": "^1.13.0",
"strong-task-emitter": "^0.0.6",
"supertest": "^0.15.0"
},
diff --git a/test/role.test.js b/test/role.test.js
index f10a1c33..d0cc3586 100644
--- a/test/role.test.js
+++ b/test/role.test.js
@@ -1,4 +1,5 @@
var assert = require('assert');
+var sinon = require('sinon');
var loopback = require('../index');
var Role = loopback.Role;
var RoleMapping = loopback.RoleMapping;
@@ -209,49 +210,64 @@ describe('role model', function() {
});
- it('should fetch all user ids assigned to the role', function(done) {
- User.create({name: 'Raymond', email: 'x@y.com', password: 'foobar'}, function(err, user) {
- Role.create({name: 'userRole'}, function(err, role) {
- role.principals.create({principalType: RoleMapping.USER, principalId: user.id}, function(err, p) {
- role.users(function(err, users) {
- assert(!err);
- assert.equal(users.length, 1);
- assert.equal(users[0], user.id);
- done();
- });
- });
- });
- });
- });
+ describe('listByPrincipalType', function(){
+ var sandbox;
- it('should fetch all application ids assigned to the role', function(done) {
- Application.create({name: 'New App'}, function(err, application) {
- Role.create({name: 'applicationRole'}, function(err, role) {
- role.principals.create({principalType: RoleMapping.APPLICATION, principalId: application.id}, function(err, p) {
- role.applications(function(err, applications) {
- assert(!err);
- assert.equal(applications.length, 1);
- assert.equal(applications[0], application.id);
- done();
- });
- });
- });
+ beforeEach(function(){
+ sandbox = sinon.sandbox.create();
});
- });
- it('should fetch all role ids assigned to the role', function(done) {
- Role.create({name: 'New Role'}, function(err, newRole) {
- Role.create({name: 'applicationRole'}, function(err, role) {
- role.principals.create({principalType: RoleMapping.ROLE, principalId: newRole.id}, function(err, p) {
- role.roles(function(err, roles) {
- assert(!err);
- assert.equal(roles.length, 1);
- assert.equal(roles[0], newRole.id);
- done();
+ afterEach(function(){
+ sandbox.restore();
+ });
+
+ it('should fetch all models assigned to the role', function(done) {
+ var principalTypesToModels = {};
+ var runs = 0;
+ var mappings;
+
+ principalTypesToModels[RoleMapping.USER] = User;
+ principalTypesToModels[RoleMapping.APPLICATION] = Application;
+ principalTypesToModels[RoleMapping.ROLE] = Role;
+
+ mappings = Object.keys(principalTypesToModels);
+
+ mappings.forEach(function(principalType){
+ var Model = principalTypesToModels[principalType];
+ Model.create({name:"test", email:'x@y.com', password: 'foobar'}, function(err, model){
+ Role.create({name:'testRole'}, function(err, role){
+ role.principals.create({principalType: principalType, principalId: model.id}, function(err, p){
+ var pluralName = Model.pluralModelName.toLowerCase();
+ role[pluralName](function(err, models){
+ assert(!err);
+ assert.equal(models.length, 1);
+ if (++runs === mappings.length) {
+ done();
+ }
+ });
+ });
+ })
+ })
+ });
+ });
+
+ it('should apply query', function(done) {
+ User.create({name: 'Raymond', email: 'x@y.com', password: 'foobar'}, function(err, user) {
+ Role.create({name: 'userRole'}, function(err, role) {
+ role.principals.create({principalType: RoleMapping.USER, principalId: user.id}, function(err, p) {
+ var query = {fields:['id','name']};
+ sandbox.spy(User, 'find');
+ role.users(query, function(err, users) {
+ assert(!err);
+ assert.equal(users.length, 1);
+ assert.equal(users[0].id, user.id);
+ assert(User.find.calledWith(query));
+ done();
+ });
});
});
});
});
- });
+ })
});
From c764c09837b1a71d6ef107e34244f4406ea4399e Mon Sep 17 00:00:00 2001
From: Esco Obong <menzoic@gmail.com>
Date: Fri, 13 Mar 2015 16:53:26 -0400
Subject: [PATCH 3/6] fix lint erros
---
common/models/role.js | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/common/models/role.js b/common/models/role.js
index daa7505f..1108060f 100644
--- a/common/models/role.js
+++ b/common/models/role.js
@@ -40,27 +40,27 @@ module.exports = function(Role) {
principalTypesToModels[RoleMapping.APPLICATION] = Application;
principalTypesToModels[RoleMapping.ROLE] = Role;
- Object.keys(principalTypesToModels).forEach(function(principalType){
+ Object.keys(principalTypesToModels).forEach(function(principalType) {
var model = principalTypesToModels[principalType];
var pluralName = model.pluralModelName.toLowerCase();
- /**
+ /**
* Fetch all users assigned to this role
* @function Role.prototype#users
* @param {object} [query] query object passed to model find call
* @param {Function} [callback]
- */
- /**
+ */
+ /**
* Fetch all applications assigned to this role
* @function Role.prototype#applications
* @param {object} [query] query object passed to model find call
* @param {Function} [callback]
*/
- /**
+ /**
* Fetch all roles assigned to this role
* @function Role.prototype#roles
* @param {object} [query] query object passed to model find call
* @param {Function} [callback]
- */
+ */
Role.prototype[pluralName] = function(query, callback) {
listByPrincipalType(model, principalType, query, callback);
}
From 551261ec16c3fd2bee3226b27c67f56327d0f3e5 Mon Sep 17 00:00:00 2001
From: Esco Obong <menzoic@gmail.com>
Date: Fri, 13 Mar 2015 18:30:53 -0400
Subject: [PATCH 4/6] fix linting errors
---
common/models/role.js | 2 +-
test/role.test.js | 24 ++++++++++++------------
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/common/models/role.js b/common/models/role.js
index 1108060f..0d8880fa 100644
--- a/common/models/role.js
+++ b/common/models/role.js
@@ -63,7 +63,7 @@ module.exports = function(Role) {
*/
Role.prototype[pluralName] = function(query, callback) {
listByPrincipalType(model, principalType, query, callback);
- }
+ };
});
/**
diff --git a/test/role.test.js b/test/role.test.js
index d0cc3586..882bf0f4 100644
--- a/test/role.test.js
+++ b/test/role.test.js
@@ -210,14 +210,14 @@ describe('role model', function() {
});
- describe('listByPrincipalType', function(){
+ describe('listByPrincipalType', function() {
var sandbox;
- beforeEach(function(){
+ beforeEach(function() {
sandbox = sinon.sandbox.create();
});
- afterEach(function(){
+ afterEach(function() {
sandbox.restore();
});
@@ -232,13 +232,13 @@ describe('role model', function() {
mappings = Object.keys(principalTypesToModels);
- mappings.forEach(function(principalType){
+ mappings.forEach(function(principalType) {
var Model = principalTypesToModels[principalType];
- Model.create({name:"test", email:'x@y.com', password: 'foobar'}, function(err, model){
- Role.create({name:'testRole'}, function(err, role){
- role.principals.create({principalType: principalType, principalId: model.id}, function(err, p){
+ Model.create({name:'test', email:'x@y.com', password: 'foobar'}, function(err, model) {
+ Role.create({name:'testRole'}, function(err, role) {
+ role.principals.create({principalType: principalType, principalId: model.id}, function(err, p) {
var pluralName = Model.pluralModelName.toLowerCase();
- role[pluralName](function(err, models){
+ role[pluralName](function(err, models) {
assert(!err);
assert.equal(models.length, 1);
if (++runs === mappings.length) {
@@ -246,8 +246,8 @@ describe('role model', function() {
}
});
});
- })
- })
+ });
+ });
});
});
@@ -255,7 +255,7 @@ describe('role model', function() {
User.create({name: 'Raymond', email: 'x@y.com', password: 'foobar'}, function(err, user) {
Role.create({name: 'userRole'}, function(err, role) {
role.principals.create({principalType: RoleMapping.USER, principalId: user.id}, function(err, p) {
- var query = {fields:['id','name']};
+ var query = {fields:['id', 'name']};
sandbox.spy(User, 'find');
role.users(query, function(err, users) {
assert(!err);
@@ -268,6 +268,6 @@ describe('role model', function() {
});
});
});
- })
+ });
});
From 7923d036f89b8c6fd99766b5a593d2f6b9b93fef Mon Sep 17 00:00:00 2001
From: Esco Obong <menzoic@gmail.com>
Date: Wed, 25 Mar 2015 10:10:34 -0400
Subject: [PATCH 5/6] mark utiltiy function as private
---
common/models/role.js | 1 +
1 file changed, 1 insertion(+)
diff --git a/common/models/role.js b/common/models/role.js
index 0d8880fa..7cf3ed45 100644
--- a/common/models/role.js
+++ b/common/models/role.js
@@ -68,6 +68,7 @@ module.exports = function(Role) {
/**
* Fetch all models assigned to this role
+ * @private
* @param {*} model model type to fetch
* @param {String} [principalType] principalType used in the rolemapping for model
* @param {object} [query] query object passed to model find call
From 957f84e989c6a6540a96a23cf769ec901c3e5693 Mon Sep 17 00:00:00 2001
From: Esco Obong <menzoic@gmail.com>
Date: Thu, 26 Mar 2015 10:10:13 -0400
Subject: [PATCH 6/6] add callback args for listByPrincipalType to jsdoc
comment, pass explicit arguments to callback
---
common/models/role.js | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/common/models/role.js b/common/models/role.js
index 7cf3ed45..340874ff 100644
--- a/common/models/role.js
+++ b/common/models/role.js
@@ -72,7 +72,7 @@ module.exports = function(Role) {
* @param {*} model model type to fetch
* @param {String} [principalType] principalType used in the rolemapping for model
* @param {object} [query] query object passed to model find call
- * @param {Function} [callback]
+ * @param {Function} [callback] callback function called with `(err, models)` arguments.
*/
function listByPrincipalType(model, principalType, query, callback) {
if (callback === undefined) {
@@ -92,7 +92,9 @@ module.exports = function(Role) {
});
query.where = query.where || {};
query.where.id = {inq: ids};
- model.find(query, callback);
+ model.find(query, function(err, models) {
+ callback(err, models);
+ });
});
}