fix: sequelize should take transaction from CLS everywhere

sequelize · Jun 21, 2022 · 67f8553 · 67f8553
1 parent 1a22466
commit 67f8553
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 21 deletions.
diff --git a/src/model-internals.ts b/src/model-internals.ts
@@ -1,7 +1,9 @@
 import NodeUtil from 'util';
 import { EagerLoadingError } from './errors';
+import type { Transactionable } from './model';
+import type { Sequelize } from './sequelize';
 import { isModelStatic } from './utils/model-utils.js';
-
+import type { Transaction } from './index';
 // TODO: strictly type this file during the TS migration of model.js
 
 // The goal of this file is to include the different private methods that are currently present on the Model class.
@@ -146,11 +148,11 @@ export function throwInvalidInclude(include: any): never {
 Got ${NodeUtil.inspect(include)} instead`);
 }
 
-export function setTransactionFromCls(options: any, sequelize: any) {
-  if (options.transaction === undefined && sequelize.constructor._cls) {
-    const t = sequelize.constructor._cls.get('transaction');
+export function setTransactionFromCls(options: Transactionable, sequelize: Sequelize): void {
+  if (options.transaction === undefined && sequelize.Sequelize._cls) {
+    const t = sequelize.Sequelize._cls.get('transaction');
     if (t) {
-      options.transaction = t;
+      options.transaction = t as Transaction;
     }
   }
 }
diff --git a/src/model.js b/src/model.js
@@ -1366,7 +1366,7 @@ Specify a different name for either index to resolve this issue.`);
 
     indexes = this.getIndexes().filter(item1 => !indexes.some(item2 => item1.name === item2.name)).sort((index1, index2) => {
       if (this.sequelize.options.dialect === 'postgres') {
-        // move concurrent indexes to the bottom to avoid weird deadlocks
+      // move concurrent indexes to the bottom to avoid weird deadlocks
         if (index1.concurrently === true) {
           return 1;
         }
@@ -1942,7 +1942,7 @@ Specify a different name for either index to resolve this issue.`);
 
       // Don't add limit if querying directly on the pk or a unique column
       if (!options.where || !_.some(options.where, (value, key) => (key === this.primaryKeyAttribute || uniqueSingleColumns.includes(key))
-        && (Utils.isPrimitive(value) || Buffer.isBuffer(value)))) {
+          && (Utils.isPrimitive(value) || Buffer.isBuffer(value)))) {
         options.limit = 1;
       }
     }
@@ -2834,9 +2834,9 @@ Specify a different name for either index to resolve this issue.`);
               if (associationInstance[include.association.through.model.name]) {
                 for (const attr of Object.keys(include.association.through.model.rawAttributes)) {
                   if (include.association.through.model.rawAttributes[attr]._autoGenerated
-                      || attr === include.association.foreignKey
-                      || attr === include.association.otherKey
-                      || typeof associationInstance[include.association.through.model.name][attr] === 'undefined') {
+                    || attr === include.association.foreignKey
+                    || attr === include.association.otherKey
+                    || typeof associationInstance[include.association.through.model.name][attr] === 'undefined') {
                     continue;
                   }
 
@@ -2866,8 +2866,8 @@ Specify a different name for either index to resolve this issue.`);
       for (const instance of instances) {
         for (const attr in model.rawAttributes) {
           if (model.rawAttributes[attr].field
-            && instance.dataValues[model.rawAttributes[attr].field] !== undefined
-            && model.rawAttributes[attr].field !== attr
+              && instance.dataValues[model.rawAttributes[attr].field] !== undefined
+              && model.rawAttributes[attr].field !== attr
           ) {
             instance.dataValues[attr] = instance.dataValues[model.rawAttributes[attr].field];
             delete instance.dataValues[model.rawAttributes[attr].field];
@@ -3214,7 +3214,7 @@ Specify a different name for either index to resolve this issue.`);
     if (updateDoneRowByRow) {
       result = [instances.length, instances];
     } else if (_.isEmpty(valuesUse)
-      || Object.keys(valuesUse).length === 1 && valuesUse[this._timestampAttributes.updatedAt]) {
+       || Object.keys(valuesUse).length === 1 && valuesUse[this._timestampAttributes.updatedAt]) {
       // only updatedAt is being passed, then skip update
       result = [0];
     } else {
@@ -4117,8 +4117,8 @@ Instead of specifying a Model, either:
     // Transfer database generated values (defaults, autoincrement, etc)
     for (const attr of Object.keys(this.constructor.rawAttributes)) {
       if (this.constructor.rawAttributes[attr].field
-        && values[this.constructor.rawAttributes[attr].field] !== undefined
-        && this.constructor.rawAttributes[attr].field !== attr
+          && values[this.constructor.rawAttributes[attr].field] !== undefined
+          && this.constructor.rawAttributes[attr].field !== attr
       ) {
         values[attr] = values[this.constructor.rawAttributes[attr].field];
         delete values[this.constructor.rawAttributes[attr].field];
@@ -4165,9 +4165,9 @@ Instead of specifying a Model, either:
               if (instance[include.association.through.model.name]) {
                 for (const attr of Object.keys(include.association.through.model.rawAttributes)) {
                   if (include.association.through.model.rawAttributes[attr]._autoGenerated
-                      || attr === include.association.foreignKey
-                      || attr === include.association.otherKey
-                      || typeof instance[include.association.through.model.name][attr] === 'undefined') {
+                    || attr === include.association.foreignKey
+                    || attr === include.association.otherKey
+                    || typeof instance[include.association.through.model.name][attr] === 'undefined') {
                     continue;
                   }
 

diff --git a/test/integration/cls.test.js b/test/integration/cls.test.js
@@ -186,16 +186,24 @@ if (current.dialect.supports.transactions) {
         },
       });
 
-      // hooks: ['afterFind'], failing in this but the transaction is coming
       testHooks({
         method: 'Model.findAll',
-        hooks: ['beforeFind', 'beforeFindAfterExpandIncludeAll', 'beforeFindAfterOptions', 'afterFind'],
+        hooks: ['beforeFind', 'beforeFindAfterExpandIncludeAll', 'beforeFindAfterOptions'],
         optionPos: 0,
         async execute(User) {
           await User.findAll();
         },
       });
 
+      testHooks({
+        method: 'Model.findAll',
+        hooks: ['afterFind'],
+        optionPos: 1,
+        async execute(User) {
+          await User.findAll();
+        },
+      });
+
       testHooks({
         method: 'Model.count',
         hooks: ['beforeCount'],
@@ -286,14 +294,14 @@ if (current.dialect.supports.transactions) {
         },
       });
 
-      // these paranoid test getting failed the transaction is going in the runhook function but still the test is failing
       describe('paranoid restore', () => {
         beforeEach(async function () {
           this.ParanoidUser = this.sequelize.define('paranoid_user', {
             name: Sequelize.STRING,
           }, { paranoid: true });
 
           await this.ParanoidUser.sync({ force: true });
+          this.User = this.ParanoidUser;
         });
 
         testHooks({