Fix $injectScope to not insert include twice

lib/model.js

@@ -236,23 +236,21 @@ function conformOptions(options, self) {
     self.$expandAttributes(options);
   }
 
-  if (!options.include) {
-    return;
-  }
-  // if include is not an array, wrap in an array
-  if (!Array.isArray(options.include)) {
-    options.include = [options.include];
-  } else if (!options.include.length) {
-    delete options.include;
-    return;
-  }
+  if (options.include) {
+    // if include is not an array, wrap in an array
+    if (!Array.isArray(options.include)) {
+      options.include = [options.include];
+    }
 
-  // convert all included elements to { model: Model } form
-  options.include = options.include.map(function(include) {
-    include = conformInclude(include, self);
+    // convert all included elements to { model: Model } form
+    options.include = options.include.map(function(include) {
+      return conformInclude(include, self);
+    });
 
-    return include;
-  });
+    if (!options.include.length) {
+      delete options.include;
+    }
+  }
 }
 
 Model.$conformOptions = conformOptions;
@@ -601,7 +599,7 @@ validateIncludedElement = function(include, tableNames, options) {
     model = include.association.target.name === include.model.name ? include.association.target : include.association.source;
   }
 
-  Model.$injectScope(model.$scope, include);
+  model.$injectScope(include);
 
   // This check should happen after injecting the scope, since the scope may contain a .attributes
   if (!include.attributes) {
@@ -1328,7 +1326,7 @@ Model.prototype.findAll = function(options) {
 
   return Promise.bind(this).then(function() {
     conformOptions(options, this);
-    Model.$injectScope(this.$scope, options);
+    this.$injectScope(options);
 
     if (options.hooks) {
       return this.runHooks('beforeFind', options);
@@ -1511,7 +1509,7 @@ Model.prototype.find = Model.prototype.findOne;
 Model.prototype.aggregate = function(attribute, aggregateFunction, options) {
   options = Utils._.extend({ attributes: [] }, options || {});
   conformOptions(options, this);
-  Model.$injectScope(this.$scope, options);
+  this.$injectScope(options);
 
   var attrOptions = this.rawAttributes[attribute]
     , field = attrOptions && attrOptions.field || attribute
@@ -1559,7 +1557,7 @@ Model.prototype.aggregate = function(attribute, aggregateFunction, options) {
 Model.prototype.count = function(options) {
   options = Utils._.clone(options || {});
   conformOptions(options, this);
-  Model.$injectScope(this.$scope, options);
+  this.$injectScope(options);
 
   var col = '*';
 
@@ -2242,7 +2240,7 @@ Model.prototype.destroy = function(options) {
   }, options || {});
 
   options.type = QueryTypes.BULKDELETE;
-  Model.$injectScope(this.$scope, options);
+  this.$injectScope(options);
 
   Utils.mapOptionFieldNames(options, this);
 
@@ -2402,7 +2400,7 @@ Model.prototype.update = function(values, options) {
 
   options.type = QueryTypes.BULKUPDATE;
 
-  Model.$injectScope(this.$scope, options);
+  this.$injectScope(options);
 
   // Clone values so it doesn't get modified for caller scope
   values = _.clone(values);
@@ -2606,13 +2604,10 @@ Model.prototype.$expandAttributes = function (options) {
   }
 };
 
-// Inject current scope into options. Includes should have been conformed (conformOptions) before calling this
-Model.$injectScope = function (scope, options) {
-  // Do not inject scope more than once.
-  if (options._scopeInjected) return;
-  options._scopeInjected = true;
-
-  scope = optClone(scope);
+// Inject $scope into options. Includes should have been conformed (conformOptions) before calling this
+Model.prototype.$injectScope = function (options) {
+  var self = this;
+  var scope = optClone(this.$scope);
 
   var filteredScope = _.omit(scope, 'include'); // Includes need special treatment
 
@@ -2623,15 +2618,18 @@ Model.$injectScope = function (scope, options) {
     options.include = options.include || [];
 
     // Reverse so we consider the latest include first.
-    // This is used if several scopes specify the same include - the last scope should take precendence
+    // This is used if several scopes specify the same include - the last scope should take precedence
     scope.include.reverse().forEach(function (scopeInclude) {
-      if (scopeInclude.all || !_.any(options.include, function matchesModelAndAlias(item) {
-        var sameModel = item.model && item.model.name === scopeInclude.model.name;
+      if (scopeInclude.all || !options.include.some(function matchesModelAndAlias(item) {
+        var isSameModel = item.model && item.model.name === scopeInclude.model.name;
+        if (!isSameModel || !item.as) return isSameModel;
 
-        if (sameModel && item.as) {
+        if (scopeInclude.as) {
           return item.as === scopeInclude.as;
+        } else {
+          var association = scopeInclude.association || self.getAssociation(scopeInclude.model, scopeInclude.as);
+          return association ? item.as === association.as : false;
         }
-        return sameModel;
       })) {
         options.include.push(scopeInclude);
       }

test/unit/model/scope.test.js

@@ -253,8 +253,9 @@ describe(Support.getTestDialectTeaser('Model'), function() {
         limit: 9
       };
 
-      current.Model.$injectScope(scope, options);
-      delete options._scopeInjected;
+      current.Model.prototype.$injectScope.call({
+        $scope: scope
+      }, options);
 
       expect(options).to.deep.equal({
         where: {
@@ -276,7 +277,9 @@ describe(Support.getTestDialectTeaser('Model'), function() {
 
       var options = {};
 
-      current.Model.$injectScope(scope, options);
+      current.Model.prototype.$injectScope.call({
+        $scope: scope
+      }, options);
 
       expect(options.include).to.have.length(1);
       expect(options.include[0]).to.deep.equal({ model: Project, where: { something: true }});
@@ -291,7 +294,9 @@ describe(Support.getTestDialectTeaser('Model'), function() {
         include: [{ model: Project, where: { something: true }}]
       };
 
-      current.Model.$injectScope(scope, options);
+      current.Model.prototype.$injectScope.call({
+        $scope: scope
+      }, options);
 
       expect(options.include).to.have.length(1);
       expect(options.include[0]).to.deep.equal({ model: Project, where: { something: true }});
@@ -306,7 +311,9 @@ describe(Support.getTestDialectTeaser('Model'), function() {
         include: [{model: User, as: 'otherUser'}]
       };
 
-      current.Model.$injectScope(scope, options);
+      current.Model.prototype.$injectScope.call({
+        $scope: scope
+      }, options);
 
       expect(options.include).to.have.length(2);
       expect(options.include[0]).to.deep.equal({model: User, as: 'otherUser'});
@@ -326,7 +333,9 @@ describe(Support.getTestDialectTeaser('Model'), function() {
         ]
       };
 
-      current.Model.$injectScope(scope, options);
+      current.Model.prototype.$injectScope.call({
+        $scope: scope
+      }, options);
 
       expect(options.include).to.have.length(2);
       expect(options.include[0]).to.deep.equal({ model: User, where: { something: true }});
@@ -347,7 +356,9 @@ describe(Support.getTestDialectTeaser('Model'), function() {
           ]
         };
 
-        current.Model.$injectScope(scope, options);
+        current.Model.prototype.$injectScope.call({
+          $scope: scope
+        }, options);
 
         expect(options.include).to.have.length(2);
         expect(options.include[0]).to.deep.equal({ model: User, where: { something: true }});
@@ -368,7 +379,9 @@ describe(Support.getTestDialectTeaser('Model'), function() {
           ]
         };
 
-        current.Model.$injectScope(scope, options);
+        current.Model.prototype.$injectScope.call({
+          $scope: scope
+        }, options);
 
         expect(options.include).to.have.length(2);
         expect(options.include[0]).to.deep.equal({ all: true });