fix(associations): correctly parse include options (#10580)

lib/model.js

@@ -90,7 +90,7 @@ class Model {
     }
 
     if (!options.includeValidated) {
-      this.constructor._conformOptions(options, this.constructor);
+      this.constructor._conformIncludes(options, this.constructor);
       if (options.include) {
         this.constructor._expandIncludeAll(options);
         this.constructor._validateIncludedElements(options);
@@ -306,14 +306,9 @@ class Model {
     }
   }
 
-  static _conformOptions(options, self) {
-    if (self) {
-      self._expandAttributes(options);
-    }
+  static _conformIncludes(options, self) {
+    if (!options.include) return;
 
-    if (!options.include) {
-      return;
-    }
     // if include is not an array, wrap in an array
     if (!Array.isArray(options.include)) {
       options.include = [options.include];
@@ -371,17 +366,17 @@ class Model {
           if (!include.model) include.model = model;
           if (!include.as) include.as = include.association.as;
 
-          this._conformOptions(include, model);
+          this._conformIncludes(include, model);
           return include;
         }
 
         if (include.model) {
-          this._conformOptions(include, include.model);
+          this._conformIncludes(include, include.model);
           return include;
         }
 
         if (include.all) {
-          this._conformOptions(include);
+          this._conformIncludes(include);
           return include;
         }
       }
@@ -789,7 +784,7 @@ class Model {
 
   static _baseMerge(...args) {
     _.assignWith(...args);
-    this._conformOptions(args[0]);
+    this._conformIncludes(args[0], this);
     this._uniqIncludes(args[0]);
     return args[0];
   }
@@ -1700,7 +1695,8 @@ class Model {
         return this.runHooks('beforeFind', options);
       }
     }).then(() => {
-      this._conformOptions(options, this);
+      this._conformIncludes(options, this);
+      this._expandAttributes(options);
       this._expandIncludeAll(options);
 
       if (options.hooks) {
@@ -1936,7 +1932,7 @@ class Model {
     options = Utils.cloneDeep(options);
 
     this._injectScope(options);
-    this._conformOptions(options, this);
+    this._conformIncludes(options, this);
 
     if (options.include) {
       this._expandIncludeAll(options);
@@ -2149,7 +2145,7 @@ class Model {
     }, options || {});
 
     if (!options.includeValidated) {
-      this._conformOptions(options, this);
+      this._conformIncludes(options, this);
       if (options.include) {
         this._expandIncludeAll(options);
         this._validateIncludedElements(options);

test/integration/model/scope/associations.test.js

@@ -418,6 +418,46 @@ describe(Support.getTestDialectTeaser('Model'), () => {
               expect(user.dataValues).not.to.have.property('secret');
             });
         });
+
+        it('should not throw error', function() {
+          const Clientfile = this.sequelize.define('clientfile');
+          const Mission = this.sequelize.define('mission', { secret: Sequelize.STRING });
+          const Building = this.sequelize.define('building');
+          const MissionAssociation = Clientfile.hasOne(Mission);
+          const BuildingAssociation = Clientfile.hasOne(Building);
+
+          return this.sequelize.sync({ force: true })
+            .then(() => {
+              return Clientfile.findAll({
+                include: [
+                  {
+                    association: 'mission',
+                    where: {
+                      secret: 'foo'
+                    }
+                  },
+                  {
+                    association: 'building'
+                  }
+                ]
+              });
+            })
+            .then(() => {
+              return Clientfile.findAll({
+                include: [
+                  {
+                    association: MissionAssociation,
+                    where: {
+                      secret: 'foo'
+                    }
+                  },
+                  {
+                    association: BuildingAssociation
+                  }
+                ]
+              });
+            });
+        });
       });
     });
   });

test/unit/model/include.test.js

@@ -256,7 +256,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
         const options = {
           include: ['Owner']
         };
-        Sequelize.Model._conformOptions(options, this.Company);
+        Sequelize.Model._conformIncludes(options, this.Company);
 
         expect(options.include[0]).to.deep.equal({
           model: this.User,
@@ -272,7 +272,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
             attributes: ['id']
           }]
         };
-        Sequelize.Model._conformOptions(options, this.Company);
+        Sequelize.Model._conformIncludes(options, this.Company);
 
         expect(options.include[0]).to.deep.equal({
           model: this.User,
@@ -290,7 +290,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
         };
 
         expect(() => {
-          Sequelize.Model._conformOptions(options, this.Company);
+          Sequelize.Model._conformIncludes(options, this.Company);
         }).to.throw('Include unexpected. Element has to be either a Model, an Association or an object.');
       });
 
@@ -302,7 +302,7 @@ describe(Support.getTestDialectTeaser('Model'), () => {
         };
 
         expect(() => {
-          Sequelize.Model._conformOptions(options, this.Company);
+          Sequelize.Model._conformIncludes(options, this.Company);
         }).to.throw('Include unexpected. Element has to be either a Model, an Association or an object.');
       });
     });

test/unit/sql/generateJoin.test.js

@@ -18,7 +18,7 @@ describe(Support.getTestDialectTeaser('SQL'), () => {
 
       const name = `${path}, ${util.inspect(options, { depth: 10 })}`;
 
-      Sequelize.Model._conformOptions(options);
+      Sequelize.Model._conformIncludes(options);
       options = Sequelize.Model._validateIncludedElements(options);
 
       const include = _.at(options, path)[0];