From 8a114c635f9b4458e48dbfa46271e8b45f29675e Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Mon, 20 Jan 2014 14:32:47 -0800 Subject: [PATCH 01/10] Prevent autoAttach from overriding existing data source --- lib/loopback.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/loopback.js b/lib/loopback.js index d5db06df..ab0a79e2 100644 --- a/lib/loopback.js +++ b/lib/loopback.js @@ -243,7 +243,8 @@ loopback.autoAttach = function() { Object.keys(models).forEach(function(modelName) { var ModelCtor = models[modelName]; - if(ModelCtor) { + // Only auto attach if the model doesn't have an explicit data source + if(ModelCtor && (!(ModelCtor.dataSource instanceof DataSource))) { loopback.autoAttachModel(ModelCtor); } }); From a5242a3b7708851d6dddb0c3b37234f8dac46256 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Tue, 21 Jan 2014 16:35:28 -0800 Subject: [PATCH 02/10] Fix the typo for the method name --- lib/connectors/mail.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/connectors/mail.js b/lib/connectors/mail.js index 4d3dccf3..0f20663a 100644 --- a/lib/connectors/mail.js +++ b/lib/connectors/mail.js @@ -82,7 +82,7 @@ MailConnector.prototype.transportForName = function(name) { * @return {Transport} transport */ -MailConnector.prototype.transportForName = function(name) { +MailConnector.prototype.defaultTransport = function(name) { return this.transports[0]; } From d6a4230aed6fdecb4d6c19b6f8bb9bde5f45cd62 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Tue, 21 Jan 2014 16:41:48 -0800 Subject: [PATCH 03/10] Fix the method args --- lib/connectors/mail.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/connectors/mail.js b/lib/connectors/mail.js index 0f20663a..1cdec0e5 100644 --- a/lib/connectors/mail.js +++ b/lib/connectors/mail.js @@ -82,7 +82,7 @@ MailConnector.prototype.transportForName = function(name) { * @return {Transport} transport */ -MailConnector.prototype.defaultTransport = function(name) { +MailConnector.prototype.defaultTransport = function() { return this.transports[0]; } From a43d6004c135da833f04770108242aff8544e615 Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Thu, 23 Jan 2014 14:25:54 -0800 Subject: [PATCH 04/10] Fix race condition where MyEmail model was not attached to the correct dataSource in tests --- lib/connectors/mail.js | 7 ++++--- test/email.test.js | 8 ++++++-- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/connectors/mail.js b/lib/connectors/mail.js index 1cdec0e5..cb260c72 100644 --- a/lib/connectors/mail.js +++ b/lib/connectors/mail.js @@ -22,7 +22,7 @@ function MailConnector(settings) { var transports = settings.transports || []; this.transportsIndex = {}; this.transports = []; - + transports.forEach(this.setupTransport.bind(this)); } @@ -83,10 +83,9 @@ MailConnector.prototype.transportForName = function(name) { */ MailConnector.prototype.defaultTransport = function() { - return this.transports[0]; + return this.transports[0] || this.stubTransport; } - /** * Send an email with the given `options`. * @@ -110,6 +109,8 @@ Mailer.send = function (options, fn) { var dataSource = this.dataSource; var settings = dataSource && dataSource.settings; var connector = dataSource.connector; + assert(connector, 'Cannot send mail without a connector!'); + var transport = connector.transportForName(options.transport); if(!transport) { diff --git a/test/email.test.js b/test/email.test.js index 91ff7650..9a23ecb8 100644 --- a/test/email.test.js +++ b/test/email.test.js @@ -1,9 +1,13 @@ var loopback = require('../'); -var MyEmail = loopback.Email.extend('my-email'); +var MyEmail; var assert = require('assert'); describe('Email and SMTP', function () { - + beforeEach(function() { + MyEmail = loopback.Email.extend('my-email'); + loopback.autoAttach(); + }); + it('should have a send method', function () { assert(typeof MyEmail.send === 'function'); assert(typeof MyEmail.prototype.send === 'function'); From 86a85291ac54ed11d181945040fe9f4d3fdc1fae Mon Sep 17 00:00:00 2001 From: Ritchie Martori Date: Thu, 23 Jan 2014 14:39:15 -0800 Subject: [PATCH 05/10] Fix user test race condition --- test/user.test.js | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/test/user.test.js b/test/user.test.js index fecc52d8..20271335 100644 --- a/test/user.test.js +++ b/test/user.test.js @@ -1,4 +1,4 @@ -var User = loopback.User.extend('user'); +var User; var AccessToken = loopback.AccessToken; var passport = require('passport'); var MailConnector = require('../lib/connectors/mail'); @@ -8,10 +8,14 @@ var userMemory = loopback.createDataSource({ }); describe('User', function(){ - // allow many User.afterRemote's to be called - User.setMaxListeners(0); + beforeEach(function() { + User = loopback.User.extend('user'); + User.email = loopback.Email.extend('email'); + loopback.autoAttach(); - before(function () { + // allow many User.afterRemote's to be called + User.setMaxListeners(0); + User.hasMany(AccessToken, {as: 'accessTokens', foreignKey: 'userId'}); AccessToken.belongsTo(User); }); From bcc7e68e0eeaae9127971ed527561d62b9a5b132 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 23 Jan 2014 14:26:45 -0800 Subject: [PATCH 06/10] Add lookback.getModelByType() and use it resolve model deps --- lib/loopback.js | 17 +++++++++++++++++ lib/models/acl.js | 3 ++- lib/models/role.js | 14 ++++++++------ test/loopback.test.js | 34 ++++++++++++++++++++++++++++++++++ test/role.test.js | 13 +++++++++---- 5 files changed, 70 insertions(+), 11 deletions(-) diff --git a/lib/loopback.js b/lib/loopback.js index ab0a79e2..724e9835 100644 --- a/lib/loopback.js +++ b/lib/loopback.js @@ -203,6 +203,23 @@ loopback.getModel = function(modelName) { return loopback.Model.modelBuilder.models[modelName]; }; +/** + * Look up a model class by the base model class. The method can be used by LoopBack + * to find configured models in models.json over the base model. + * @param {Model} The base model class + * @return {Model} The subclass if found or the base class + */ +loopback.getModelByType = function(modelType) { + assert(typeof modelType === 'function', 'The model type must be a constructor'); + var models = loopback.Model.modelBuilder.models; + for(var m in models) { + if(models[m].prototype instanceof modelType) { + return models[m]; + } + } + return modelType; +}; + /** * Set the default `dataSource` for a given `type`. * @param {String} type The datasource type diff --git a/lib/models/acl.js b/lib/models/acl.js index 4e4a161c..1bb5b921 100644 --- a/lib/models/acl.js +++ b/lib/models/acl.js @@ -365,6 +365,7 @@ ACL.checkAccess = function (context, callback) { var staticACLs = this.getStaticACLs(model.modelName, property); var self = this; + var roleModel = loopback.getModelByType(Role); this.find({where: {model: model.modelName, property: propertyQuery, accessType: accessTypeQuery}}, function (err, acls) { if (err) { @@ -389,7 +390,7 @@ ACL.checkAccess = function (context, callback) { // Check role matches if (acl.principalType === ACL.ROLE) { inRoleTasks.push(function (done) { - Role.isInRole(acl.principalId, context, + roleModel.isInRole(acl.principalId, context, function (err, inRole) { if (!err && inRole) { effectiveACLs.push(acl); diff --git a/lib/models/role.js b/lib/models/role.js index c9cee824..7d583043 100644 --- a/lib/models/role.js +++ b/lib/models/role.js @@ -53,7 +53,8 @@ RoleMapping.ROLE = 'ROLE'; */ RoleMapping.prototype.application = function (callback) { if (this.principalType === RoleMapping.APPLICATION) { - var applicationModel = this.constructor.Application || loopback.Application; + var applicationModel = this.constructor.Application + || loopback.getModelByType(loopback.Application); applicationModel.findById(this.principalId, callback); } else { process.nextTick(function () { @@ -70,8 +71,9 @@ RoleMapping.prototype.application = function (callback) { */ RoleMapping.prototype.user = function (callback) { if (this.principalType === RoleMapping.USER) { - var userModel = this.constructor.User || loopback.User; - loopback.User.findById(this.principalId, callback); + var userModel = this.constructor.User + || loopback.getModelByType(loopback.User); + userModel.findById(this.principalId, callback); } else { process.nextTick(function () { callback && callback(null, null); @@ -87,7 +89,7 @@ RoleMapping.prototype.user = function (callback) { */ RoleMapping.prototype.childRole = function (callback) { if (this.principalType === RoleMapping.ROLE) { - var roleModel = this.constructor.Role || Role; + var roleModel = this.constructor.Role || loopback.getModelByType(Role); roleModel.findById(this.principalId, callback); } else { process.nextTick(function () { @@ -114,7 +116,7 @@ Role.RoleMapping = RoleMapping; // Set up the connection to users/applications/roles once the model Role.once('dataSourceAttached', function () { - var roleMappingModel = this.RoleMapping || RoleMapping; + var roleMappingModel = this.RoleMapping || loopback.getModelByType(RoleMapping); Role.prototype.users = function (callback) { roleMappingModel.find({where: {roleId: this.id, principalType: RoleMapping.USER}}, function (err, mappings) { @@ -411,7 +413,7 @@ Role.getRoles = function (context, callback) { }); }); - var roleMappingModel = this.RoleMapping || RoleMapping; + var roleMappingModel = this.RoleMapping || loopback.getModelByType(RoleMapping); context.principals.forEach(function (p) { // Check against the role mappings var principalType = p.type || undefined; diff --git a/test/loopback.test.js b/test/loopback.test.js index d3650953..2fd4d363 100644 --- a/test/loopback.test.js +++ b/test/loopback.test.js @@ -58,5 +58,39 @@ describe('loopback', function() { assert(MyCustomModel.super_.modelName === MyModel.modelName); }); }); + + describe('loopback.getModel and getModelByType', function () { + it('should be able to get model by name', function () { + var MyModel = loopback.createModel('MyModel', {}, { + foo: { + bar: 'bat' + } + }); + var MyCustomModel = loopback.createModel('MyCustomModel', {}, { + base: 'MyModel', + foo: { + bat: 'baz' + } + }); + assert(loopback.getModel('MyModel') === MyModel); + assert(loopback.getModel('MyCustomModel') === MyCustomModel); + assert(loopback.getModel('Invalid') === undefined); + }); + it('should be able to get model by type', function () { + var MyModel = loopback.createModel('MyModel', {}, { + foo: { + bar: 'bat' + } + }); + var MyCustomModel = loopback.createModel('MyCustomModel', {}, { + base: 'MyModel', + foo: { + bat: 'baz' + } + }); + assert(loopback.getModelByType(MyModel) === MyCustomModel); + assert(loopback.getModelByType(MyCustomModel) === MyCustomModel); + }); + }); }); }); diff --git a/test/role.test.js b/test/role.test.js index 4417fd10..163e5610 100644 --- a/test/role.test.js +++ b/test/role.test.js @@ -12,6 +12,14 @@ function checkResult(err, result) { } describe('role model', function () { + var ds; + + beforeEach(function() { + ds = loopback.createDataSource({connector: 'memory'}); + User.attachTo(ds); + Role.attachTo(ds); + RoleMapping.attachTo(ds); + }); it("should define role/role relations", function () { Role.create({name: 'user'}, function (err, userRole) { @@ -38,6 +46,7 @@ describe('role model', function () { }); it("should define role/user relations", function () { + User.create({name: 'Raymond', email: 'x@y.com', password: 'foobar'}, function (err, user) { // console.log('User: ', user.id); Role.create({name: 'userRole'}, function (err, role) { @@ -113,10 +122,6 @@ describe('role model', function () { }); it("should support owner role resolver", function () { - var ds = loopback.createDataSource({connector: 'memory'}); - User.attachTo(ds); - Role.attachTo(ds); - RoleMapping.attachTo(ds); var Album = ds.createModel('Album', { name: String, From bb389ce20964a4ae7d9cfab25fba4d5a98e4114a Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 23 Jan 2014 14:40:27 -0800 Subject: [PATCH 07/10] Lookup the email model --- lib/models/user.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/models/user.js b/lib/models/user.js index 1f0a0d00..5bd2b102 100644 --- a/lib/models/user.js +++ b/lib/models/user.js @@ -246,7 +246,7 @@ User.prototype.verify = function (options, fn) { // Email model - var Email = options.mailer || this.constructor.email || loopback.Email; + var Email = options.mailer || this.constructor.email || loopback.getModelByType(loopback.Email); crypto.randomBytes(64, function(err, buf) { if(err) { From 224b500c3ac8e44a7d8110b380f799d24b162164 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 23 Jan 2014 14:46:02 -0800 Subject: [PATCH 08/10] Fix the Scope reference to models --- lib/models/acl.js | 82 +++++++++++++++++++++++------------------------ 1 file changed, 41 insertions(+), 41 deletions(-) diff --git a/lib/models/acl.js b/lib/models/acl.js index 1bb5b921..883661e7 100644 --- a/lib/models/acl.js +++ b/lib/models/acl.js @@ -44,47 +44,6 @@ var AccessRequest = ctx.AccessRequest; var role = require('./role'); var Role = role.Role; -/*! - * Schema for Scope which represents the permissions that are granted to client - * applications by the resource owner - */ -var ScopeSchema = { - name: {type: String, required: true}, - description: String -}; - -/** - * Resource owner grants/delegates permissions to client applications - * - * For a protected resource, does the client application have the authorization - * from the resource owner (user or system)? - * - * Scope has many resource access entries - * @class - */ -var Scope = loopback.createModel('Scope', ScopeSchema); - - -/** - * Check if the given scope is allowed to access the model/property - * @param {String} scope The scope name - * @param {String} model The model name - * @param {String} property The property/method/relation name - * @param {String} accessType The access type - * @callback {Function} callback - * @param {String|Error} err The error object - * @param {AccessRequest} result The access permission - */ -Scope.checkPermission = function (scope, model, property, accessType, callback) { - Scope.findOne({where: {name: scope}}, function (err, scope) { - if (err) { - callback && callback(err); - } else { - ACL.checkPermission(ACL.SCOPE, scope.id, model, property, accessType, callback); - } - }); -}; - /** * System grants permissions to principals (users/applications, can be grouped * into roles). @@ -452,6 +411,47 @@ ACL.checkAccessForToken = function (token, model, modelId, method, callback) { }); }; +/*! + * Schema for Scope which represents the permissions that are granted to client + * applications by the resource owner + */ +var ScopeSchema = { + name: {type: String, required: true}, + description: String +}; + +/** + * Resource owner grants/delegates permissions to client applications + * + * For a protected resource, does the client application have the authorization + * from the resource owner (user or system)? + * + * Scope has many resource access entries + * @class + */ +var Scope = loopback.createModel('Scope', ScopeSchema); + + +/** + * Check if the given scope is allowed to access the model/property + * @param {String} scope The scope name + * @param {String} model The model name + * @param {String} property The property/method/relation name + * @param {String} accessType The access type + * @callback {Function} callback + * @param {String|Error} err The error object + * @param {AccessRequest} result The access permission + */ +Scope.checkPermission = function (scope, model, property, accessType, callback) { + this.findOne({where: {name: scope}}, function (err, scope) { + if (err) { + callback && callback(err); + } else { + var aclModel = loopback.getModelByType(ACL); + aclModel.checkPermission(ACL.SCOPE, scope.id, model, property, accessType, callback); + } + }); +}; module.exports.ACL = ACL; module.exports.Scope = Scope; From 5586c54c49d72afa989b733bd1d88976e36def57 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 23 Jan 2014 15:04:48 -0800 Subject: [PATCH 09/10] Fix the Role ref to RoleMapping --- lib/models/role.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/models/role.js b/lib/models/role.js index 7d583043..e249f780 100644 --- a/lib/models/role.js +++ b/lib/models/role.js @@ -112,8 +112,6 @@ var Role = loopback.createModel('Role', RoleSchema, { } }); -Role.RoleMapping = RoleMapping; - // Set up the connection to users/applications/roles once the model Role.once('dataSourceAttached', function () { var roleMappingModel = this.RoleMapping || loopback.getModelByType(RoleMapping); @@ -338,7 +336,7 @@ Role.isInRole = function (role, context, callback) { return; } - var roleMappingModel = this.RoleMapping || RoleMapping; + var roleMappingModel = this.RoleMapping || loopback.getModelByType(RoleMapping); this.findOne({where: {name: role}}, function (err, result) { if (err) { callback && callback(err); From f08b9427ab9a460a61ddad9ec6ace3090aae85a2 Mon Sep 17 00:00:00 2001 From: Raymond Feng Date: Thu, 23 Jan 2014 15:27:31 -0800 Subject: [PATCH 10/10] Add a test for autoAttach --- test/loopback.test.js | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/test/loopback.test.js b/test/loopback.test.js index 2fd4d363..7988fdb4 100644 --- a/test/loopback.test.js +++ b/test/loopback.test.js @@ -14,7 +14,33 @@ describe('loopback', function() { assert(dataSource.connector); }); }); - + + describe('loopback.autoAttach', function () { + it('doesn\'t overwrite model with datasource configured', function () { + var ds1 = loopback.createDataSource('db1', { + connector: loopback.Memory + }); + + // setup default data sources + loopback.setDefaultDataSourceForType('db', ds1); + + var ds2 = loopback.createDataSource('db2', { + connector: loopback.Memory + }); + + var model1 = ds2.createModel('m1', {}); + + var model2 = loopback.createModel('m2'); + model2.autoAttach = 'db'; + + // auto attach data sources to models + loopback.autoAttach(); + + assert(model1.dataSource === ds2); + assert(model2.dataSource === ds1); + }); + }); + describe('loopback.remoteMethod(Model, fn, [options]);', function() { it("Setup a remote method.", function() { var Product = loopback.createModel('product', {price: Number});