refactor(has-many): move association methods to the association

To prepare for being able to run a association method on multiple source instances i've moved the association methods to the association prototype
instead of having them on the instance prototype, the instance prototype now only has proxy methods to the methods on the association.

lib/associations/has-many.js

@@ -221,11 +221,43 @@ HasMany.prototype.injectAttributes = function() {
   return this;
 };
 
-HasMany.prototype.injectGetter = function(obj) {
+HasMany.prototype.mixin = function(obj) {
   var association = this;
 
   obj[this.accessors.get] = function(options) {
-    var scopeWhere = association.scope ? {} : null
+    return association.get(this, options);
+  };
+
+  if (this.accessors.count) {
+    obj[this.accessors.count] = function(options) {
+      return association.count(this, options);
+    };
+  }
+
+  obj[this.accessors.hasSingle] = obj[this.accessors.hasAll] = function(instances, options) {
+    return association.has(this, instances, options);
+  };
+
+  obj[this.accessors.set] = function(instances, options) {
+    return association.set(this, instances, options);
+  };
+
+  obj[this.accessors.add] = obj[this.accessors.addMultiple] = function(instances, options) {
+    return association.add(this, instances, options);
+  };
+
+  obj[this.accessors.remove] = obj[this.accessors.removeMultiple] = function(instances, options) {
+    return association.add(this, instances, options);
+  };
+
+  obj[this.accessors.create] = function(values, options) {
+    return association.create(this, values, options);
+  };
+};
+
+HasMany.prototype.get = function(instance, options) {
+  var association = this
+    , scopeWhere = association.scope ? {} : null
     , Model = association.target;
 
   options = association.target.__optClone(options) || {};
@@ -238,7 +270,7 @@ HasMany.prototype.injectGetter = function(obj) {
     $and: [
       new Utils.where(
         association.target.rawAttributes[association.foreignKey],
-          this.get(association.source.primaryKeyAttribute, {raw: true})
+        instance.get(association.source.primaryKeyAttribute, {raw: true})
       ),
       scopeWhere,
       options.where
@@ -254,11 +286,11 @@ HasMany.prototype.injectGetter = function(obj) {
   }
 
   return Model.all(options);
-  };
+};
 
-  if (this.accessors.count) {
-    obj[this.accessors.count] = function(options) {
-      var model = association.target
+HasMany.prototype.count = function(instance, options) {
+  var association = this
+    , model = association.target
     , sequelize = model.sequelize;
 
   options = association.target.__optClone(options) || {};
@@ -268,23 +300,24 @@ HasMany.prototype.injectGetter = function(obj) {
   options.raw = true;
   options.plain = true;
 
-      return obj[association.accessors.get].call(this, options).then(function (result) {
+  return this.get(instance, options).then(function (result) {
     return parseInt(result.count, 10);
   });
-    };
-  }
+};
 
-  obj[this.accessors.hasSingle] = obj[this.accessors.hasAll] = function(instances, options) {
-    var where = {};
+HasMany.prototype.has = function(sourceInstance, targetInstances, options) {
+  var association = this
+    , where = {};
 
-    if (!Array.isArray(instances)) {
-      instances = [instances];
+  if (!Array.isArray(targetInstances)) {
+    targetInstances = [targetInstances];
   }
 
   options = options || {};
   options.scope = false;
+  options.raw = true;
 
-    where.$or = instances.map(function (instance) {
+  where.$or = targetInstances.map(function (instance) {
     if (instance instanceof association.target.Instance) {
       return instance.where();
     } else {
@@ -301,43 +334,34 @@ HasMany.prototype.injectGetter = function(obj) {
     ]
   };
 
-    return this[association.accessors.get](
-      options,
-      { raw: true }
+  return this.get(
+    sourceInstance,
+    options
   ).then(function(associatedObjects) {
-      return associatedObjects.length === instances.length;
+    return associatedObjects.length === targetInstances.length;
   });
-  };
-
-  return this;
 };
 
-HasMany.prototype.injectSetter = function(obj) {
+HasMany.prototype.set = function(sourceInstance, targetInstances, options) {
   var association = this;
 
-  obj[this.accessors.set] = function(newAssociatedObjects, additionalAttributes) {
-    var options = additionalAttributes || {};
-    additionalAttributes = additionalAttributes || {};
-
-    if (newAssociatedObjects === null) {
-      newAssociatedObjects = [];
+  if (targetInstances === null) {
+    targetInstances = [];
   } else {
-      newAssociatedObjects = association.toInstanceArray(newAssociatedObjects);
+    targetInstances = association.toInstanceArray(targetInstances);
   }
 
-    var instance = this;
-
-    return instance[association.accessors.get](_.defaults({
+  return association.get(sourceInstance, _.defaults({
     scope: false,
     raw: true
   }, options)).then(function(oldAssociations) {
     var promises = []
       , obsoleteAssociations = oldAssociations.filter(function(old) {
-            return !_.find(newAssociatedObjects, function(obj) {
+          return !_.find(targetInstances, function(obj) {
             return obj[association.target.primaryKeyAttribute] === old[association.target.primaryKeyAttribute];
           });
         })
-        , unassociatedObjects = newAssociatedObjects.filter(function(obj) {
+      , unassociatedObjects = targetInstances.filter(function(obj) {
           return !_.find(oldAssociations, function(old) {
             return obj[association.target.primaryKeyAttribute] === old[association.target.primaryKeyAttribute];
           });
@@ -367,7 +391,7 @@ HasMany.prototype.injectSetter = function(obj) {
       updateWhere = {};
 
       update = {};
-        update[association.foreignKey] = instance.get(association.source.primaryKeyAttribute);
+      update[association.foreignKey] = sourceInstance.get(association.source.primaryKeyAttribute);
 
       _.assign(update, association.scope);
       updateWhere[association.target.primaryKeyAttribute] = unassociatedObjects.map(function(unassociatedObject) {
@@ -382,23 +406,25 @@ HasMany.prototype.injectSetter = function(obj) {
       ));
     }
 
-      return Utils.Promise.all(promises).return(instance);
+    return Utils.Promise.all(promises).return(sourceInstance);
   });
-  };
+};
 
-  obj[this.accessors.addMultiple] = obj[this.accessors.add] = function(newInstances, options) {
-    // If newInstance is null or undefined, no-op
-    if (!newInstances) return Utils.Promise.resolve();
-    options = options || {};
+HasMany.prototype.add = function(sourceInstance, targetInstances, options) {
+  if (!targetInstances) return Utils.Promise.resolve();
 
-    var instance = this, update = {}, where = {};
+  var association = this
+    , update = {}
+    , where = {};
 
-    newInstances = association.toInstanceArray(newInstances);
+  options = options || {};
 
-    update[association.foreignKey] = instance.get(association.source.primaryKeyAttribute);
+  targetInstances = association.toInstanceArray(targetInstances);
+
+  update[association.foreignKey] = sourceInstance.get(association.source.primaryKeyAttribute);
   _.assign(update, association.scope);
 
-    where[association.target.primaryKeyAttribute] = newInstances.map(function (unassociatedObject) {
+  where[association.target.primaryKeyAttribute] = targetInstances.map(function (unassociatedObject) {
     return unassociatedObject.get(association.target.primaryKeyAttribute);
   });
 
@@ -407,19 +433,23 @@ HasMany.prototype.injectSetter = function(obj) {
     _.defaults({
       where: where
     }, options)
-    ).return(instance);
- };
+  ).return(sourceInstance);
+};
+
+HasMany.prototype.remove = function(sourceInstance, targetInstances, options) {
+  var association = this
+    , update = {}
+    , where = {};
 
-  obj[this.accessors.removeMultiple] = obj[this.accessors.remove] = function(oldAssociatedObjects, options) {
   options = options || {};
-    oldAssociatedObjects = association.toInstanceArray(oldAssociatedObjects);
+  targetInstances = association.toInstanceArray(targetInstances);
 
-    var update = {};
   update[association.foreignKey] = null;
 
-    var where = {};
   where[association.foreignKey] = this.get(association.source.primaryKeyAttribute);
-    where[association.target.primaryKeyAttribute] = oldAssociatedObjects.map(function (oldAssociatedObject) { return oldAssociatedObject.get(association.target.primaryKeyAttribute); });
+  where[association.target.primaryKeyAttribute] = targetInstances.map(function (targetInstance) {
+    return targetInstance.get(association.target.primaryKeyAttribute);
+  });
 
   return association.target.unscoped().update(
     update,
@@ -427,16 +457,11 @@ HasMany.prototype.injectSetter = function(obj) {
       where: where
     }, options)
   ).return(this);
-  };
-
-  return this;
 };
 
-HasMany.prototype.injectCreator = function(obj) {
+HasMany.prototype.create = function(sourceInstance, values, options) {
   var association = this;
 
-  obj[this.accessors.create] = function(values, options) {
-    var instance = this;
   options = options || {};
 
   if (Array.isArray(options)) {
@@ -456,12 +481,9 @@ HasMany.prototype.injectCreator = function(obj) {
     });
   }
 
-    values[association.foreignKey] = instance.get(association.source.primaryKeyAttribute);
+  values[association.foreignKey] = sourceInstance.get(association.source.primaryKeyAttribute);
   if (options.fields) options.fields.push(association.foreignKey);
   return association.target.create(values, options);
-  };
-
-  return this;
 };
 
 module.exports = HasMany;

lib/associations/mixin.js

@@ -205,27 +205,26 @@ Mixin.belongsTo = singleLinked(BelongsTo);
  * @param {string}              [options.onUpdate='CASCADE']
  * @param {boolean}             [options.constraints=true] Should on update and on delete constraints be enabled on the foreign key.
  */
-Mixin.hasMany = function(targetModel, options) { // testhint options:none
-  if (!(targetModel instanceof this.sequelize.Model)) {
+Mixin.hasMany = function(target, options) { // testhint options:none
+  if (!(target instanceof this.sequelize.Model)) {
     throw new Error(this.name + '.hasMany called with something that\'s not an instance of Sequelize.Model');
   }
 
-  var sourceModel = this;
+  var source = this;
 
   // Since this is a mixin, we'll need a unique variable name for hooks (since Model will override our hooks option)
   options = options || {};
   options.hooks = options.hooks === undefined ? false : Boolean(options.hooks);
   options.useHooks = options.hooks;
 
-  options = _.extend(options, _.omit(sourceModel.options, ['hooks']));
+  options = _.extend(options, _.omit(source.options, ['hooks']));
 
   // the id is in the foreign table or in a connecting table
-  var association = new HasMany(sourceModel, targetModel, options);
-  sourceModel.associations[association.associationAccessor] = association.injectAttributes();
+  var association = new HasMany(source, target, options);
+  source.associations[association.associationAccessor] = association;
 
-  association.injectGetter(sourceModel.Instance.prototype);
-  association.injectSetter(sourceModel.Instance.prototype);
-  association.injectCreator(sourceModel.Instance.prototype);
+  association.injectAttributes();
+  association.mixin(source.Instance.prototype);
 
   return association;
 };

test/unit/associations/has-many.test.js

@@ -7,6 +7,7 @@ var chai = require('chai')
   , stub = sinon.stub
   , Support   = require(__dirname + '/../support')
   , DataTypes = require(__dirname + '/../../../lib/data-types')
+  , HasMany   = require(__dirname + '/../../../lib/associations/has-many')
   , current   = Support.sequelize
   , Promise   = current.Promise;
 
@@ -61,4 +62,27 @@ describe(Support.getTestDialectTeaser('hasMany'), function() {
       });
     });
   });
+
+  describe('mixin', function () {
+    var User = current.define('User')
+      , Task = current.define('Task');
+
+    it('should mixin association methods', function () {
+      var as = Math.random().toString()
+        , association = new HasMany(User, Task, {as: as})
+        , obj = {};
+
+      association.mixin(obj);
+
+      expect(obj[association.accessors.get]).to.be.an('function');
+      expect(obj[association.accessors.set]).to.be.an('function');
+      expect(obj[association.accessors.addMultiple]).to.be.an('function');
+      expect(obj[association.accessors.add]).to.be.an('function');
+      expect(obj[association.accessors.remove]).to.be.an('function');
+      expect(obj[association.accessors.removeMultiple]).to.be.an('function');
+      expect(obj[association.accessors.hasSingle]).to.be.an('function');
+      expect(obj[association.accessors.hasAll]).to.be.an('function');
+      expect(obj[association.accessors.count]).to.be.an('function');
+    });
+  });
 });