Merge pull request #150 from strongloop/feature/fix-auto-attach

Prevent autoAttach from overriding existing data source

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));
 }
 
@@ -82,11 +82,10 @@ MailConnector.prototype.transportForName = function(name) {
  * @return {Transport} transport
  */
 
-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) {

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
@@ -243,7 +260,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);
     }
   });

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).
@@ -365,6 +324,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 +349,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);
@@ -451,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;

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;
+    var userModel = this.constructor.User
-    loopback.User.findById(this.principalId, callback);
+      || 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 () {
@@ -110,11 +112,9 @@ 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 || 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) {
@@ -336,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);
@@ -411,7 +411,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;

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) {

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');

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});
@@ -58,5 +84,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);
+      });
+    });
   });
 });

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,

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
+  beforeEach(function() {
-  User.setMaxListeners(0);
+    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);  
   });