fix(model): ignore VIRTUAL/getters with attributes.exclude (#9568)

lib/dialects/abstract/query.js

@@ -226,6 +226,7 @@ class AbstractQuery {
 
   handleSelectQuery(results) {
     let result = null;
+
     // Map raw fields to names if a mapping is provided
     if (this.options.fieldMap) {
       const fieldMap = this.options.fieldMap;
@@ -237,6 +238,7 @@ class AbstractQuery {
         return result;
       }, result));
     }
+    
     // Raw queries
     if (this.options.raw) {
       result = results.map(result => {
@@ -278,7 +280,7 @@ class AbstractQuery {
       result = this.model.bulkBuild(results, {
         isNewRecord: false,
         raw: true,
-        attributes: this.options.attributes
+        attributes: this.options.originalAttributes || this.options.attributes
       });
     }
 

lib/model.js

@@ -1664,7 +1664,10 @@ class Model {
     tableNames[this.getTableName(options)] = true;
     options = Utils.cloneDeep(options);
 
-    _.defaults(options, { hooks: true, rejectOnEmpty: this.options.rejectOnEmpty });
+    _.defaults(options, {
+      hooks: true,
+      rejectOnEmpty: this.options.rejectOnEmpty
+    });
 
     // set rejectOnEmpty option from model config
     options.rejectOnEmpty = options.rejectOnEmpty || this.options.rejectOnEmpty;
@@ -1683,22 +1686,28 @@ class Model {
         return this.runHooks('beforeFindAfterExpandIncludeAll', options);
       }
     }).then(() => {
+      options.originalAttributes = this._injectDependentVirtualAttributes(options.attributes);
+
       if (options.include) {
         options.hasJoin = true;
         
         this._validateIncludedElements(options, tableNames);
         
-        // If we're not raw, we have to make sure we include the primary key for deduplication
-        if (options.attributes && !options.raw && this.primaryKeyAttribute && options.attributes.indexOf(this.primaryKeyAttribute) === -1) {
-          options.originalAttributes = options.attributes;
-          if (!options.group || !options.hasSingleAssociation || options.hasMultiAssociation) {
+        // If we're not raw, we have to make sure we include the primary key for de-duplication
+        if (
+          options.attributes
+          && !options.raw
+          && this.primaryKeyAttribute
+          && !options.attributes.includes(this.primaryKeyAttribute)
+          && (!options.group || !options.hasSingleAssociation || options.hasMultiAssociation)
+        ) {
           options.attributes = [this.primaryKeyAttribute].concat(options.attributes);
         }
       }
-      }
       
       if (!options.attributes) {
-        options.attributes = Object.keys(this.tableAttributes);
+        options.attributes = Object.keys(this.rawAttributes);
+        options.originalAttributes = this._injectDependentVirtualAttributes(options.attributes);
       }
       
       // whereCollection is used for non-primary key updates
@@ -1756,6 +1765,24 @@ class Model {
     }
   }
 
+  static _injectDependentVirtualAttributes(attributes) {
+    if (!this._hasVirtualAttributes) return attributes;
+    if (!attributes || !Array.isArray(attributes)) return attributes;
+
+    for (const attribute of attributes) {
+      if (
+        this._isVirtualAttribute(attribute)
+        && this.rawAttributes[attribute].type.fields
+      ) {
+        attributes = attributes.concat(this.rawAttributes[attribute].type.fields);
+      }
+    }
+    
+    attributes = _.uniq(attributes);
+    
+    return attributes;
+  }
+
   static _findSeparate(results, options) {
     if (!options.include || options.raw || !results) return Promise.resolve(results);
 
@@ -3011,10 +3038,9 @@ class Model {
       let attributes = Object.keys(this.rawAttributes);
 
       if (options.attributes.exclude) {
-        attributes = attributes.filter(elem => {
-          return options.attributes.exclude.indexOf(elem) === -1;
-        });
+        attributes = attributes.filter(elem => !options.attributes.exclude.includes(elem));
       }
+      
       if (options.attributes.include) {
         attributes = attributes.concat(options.attributes.include);
       }
@@ -3265,6 +3291,7 @@ class Model {
       if (this._customGetters.hasOwnProperty(key) && !options.raw) {
         return this._customGetters[key].call(this, key, options);
       }
+
       if (options.plain && this._options.include && this._options.includeNames.indexOf(key) !== -1) {
         if (Array.isArray(this.dataValues[key])) {
           return this.dataValues[key].map(instance => instance.get(options));
@@ -3274,15 +3301,27 @@ class Model {
           return this.dataValues[key];
         }
       }
+
       return this.dataValues[key];
     }
 
-    if (this._hasCustomGetters || options.plain && this._options.include || options.clone) {
+    if (
+      this._hasCustomGetters
+      || options.plain && this._options.include
+      || options.clone
+    ) {
       const values = {};
       let _key;
 
       if (this._hasCustomGetters) {
         for (_key in this._customGetters) {
+          if (
+            this._options.attributes
+            && !this._options.attributes.includes(_key)
+          ) {
+            continue;
+          }
+
           if (this._customGetters.hasOwnProperty(_key)) {
             values[_key] = this.get(_key, options);
           }
@@ -3294,8 +3333,10 @@ class Model {
           values[_key] = this.get(_key, options);
         }
       }
+
       return values;
     }
+
     return this.dataValues;
   }
 

lib/utils.js

@@ -150,14 +150,9 @@ exports.cloneDeep = cloneDeep;
 
 /* Expand and normalize finder options */
 function mapFinderOptions(options, Model) {
-  if (Model._hasVirtualAttributes && Array.isArray(options.attributes)) {
-    for (const attribute of options.attributes) {
-      if (Model._isVirtualAttribute(attribute) && Model.rawAttributes[attribute].type.fields) {
-        options.attributes = options.attributes.concat(Model.rawAttributes[attribute].type.fields);
-      }
-    }
+  if (options.attributes && Array.isArray(options.attributes)) {
+    options.attributes = Model._injectDependentVirtualAttributes(options.attributes);
     options.attributes = _.without.apply(_, [options.attributes].concat(Model._virtualAttributes));
-    options.attributes = _.uniq(options.attributes);
   }
 
   mapOptionFieldNames(options, Model);

test/integration/model/attributes/types.test.js

@@ -116,7 +116,8 @@ describe(Support.getTestDialectTeaser('Model'), () => {
           return this.User.create({
             field1: 'something'
           }).then(user => {
-            // We already verified that the virtual is not added to the table definition, so if this succeeds, were good
+            // We already verified that the virtual is not added to the table definition,
+            // so if this succeeds, were good
 
             expect(user.virtualWithDefault).to.equal('cake');
             expect(user.storage).to.equal('something');
@@ -132,17 +133,46 @@ describe(Support.getTestDialectTeaser('Model'), () => {
         });
 
         it('should be ignored in bulkCreate and and bulkUpdate', function() {
-          const self = this;
           return this.User.bulkCreate([{
             field1: 'something'
           }], {
             logging: this.sqlAssert
           }).then(() => {
-            return self.User.findAll();
+            return this.User.findAll();
           }).then(users => {
             expect(users[0].storage).to.equal('something');
           });
         });
+
+        it('should be able to exclude with attributes', function() {
+          return this.User.bulkCreate([{
+            field1: 'something'
+          }], {
+            logging: this.sqlAssert
+          }).then(() => {
+            return this.User.findAll({
+              logging: this.sqlAssert
+            });
+          }).then(users => {
+            const user = users[0].get();
+
+            expect(user.storage).to.equal('something');
+            expect(user).to.include.all.keys(['field1', 'field2']);
+
+            return this.User.findAll({
+              attributes: {
+                exclude: ['field1']
+              },
+              logging: this.sqlAssert
+            });
+          }).then(users => {
+            const user = users[0].get();
+
+            expect(user.storage).to.equal('something');
+            expect(user).not.to.include.all.keys(['field1']);
+            expect(user).to.include.all.keys(['field2']);
+          });
+        });
       });
     });
   });