refactor: replace bluebird .tap (#12070)

lib/dialects/abstract/connection-manager.js

@@ -129,7 +129,9 @@ class ConnectionManager {
         create: () => this._connect(config),
         destroy: connection => {
           return this._disconnect(connection)
-            .tap(() => { debug('connection destroy'); });
+            .then(result => {
+              debug('connection destroy');return result;
+            });
         },
         validate: config.pool.validate,
         max: config.pool.max,
@@ -194,8 +196,9 @@ class ConnectionManager {
         create: () => {
           // round robin config
           const nextRead = reads++ % config.replication.read.length;
-          return this._connect(config.replication.read[nextRead]).tap(connection => {
+          return this._connect(config.replication.read[nextRead]).then(connection => {
             connection.queryType = 'read';
+            return connection;
           });
         },
         destroy: connection => this._disconnect(connection),
@@ -209,8 +212,9 @@ class ConnectionManager {
       write: new Pool({
         name: 'sequelize:write',
         create: () => {
-          return this._connect(config.replication.write).tap(connection => {
+          return this._connect(config.replication.write).then(connection => {
             connection.queryType = 'write';
+            return connection;
           });
         },
         destroy: connection => this._disconnect(connection),
@@ -282,7 +286,9 @@ class ConnectionManager {
           if (error instanceof TimeoutError) throw new errors.ConnectionAcquireTimeoutError(error);
           throw error;
         });
-    }).tap(() => { debug('connection acquired'); });
+    }).then(result => {
+      debug('connection acquired');return result;
+    });
   }
 
   /**

lib/dialects/mysql/connection-manager.js

@@ -90,7 +90,9 @@ class ConnectionManager extends AbstractConnectionManager {
       connection.on('error', errorHandler);
       connection.once('connect', connectHandler);
     })
-      .tap(() => { debug('connection acquired'); })
+      .then(result => {
+        debug('connection acquired');return result;
+      })
       .then(connection => {
         connection.on('error', error => {
           switch (error.code) {

lib/dialects/postgres/connection-manager.js

@@ -193,7 +193,7 @@ class ConnectionManager extends AbstractConnectionManager {
           resolve(connection);
         }
       });
-    }).tap(connection => {
+    }).then(connection => {
       let query = '';
 
       if (this.sequelize.options.standardConformingStrings !== false && connection['standard_conforming_strings'] !== 'on') {
@@ -217,21 +217,24 @@ class ConnectionManager extends AbstractConnectionManager {
       }
 
       if (query) {
-        return connection.query(query);
+        return Promise.resolve(connection.query(query)).then(() => connection);
       }
-    }).tap(connection => {
+      return connection;
+    }).then(connection => {
       if (Object.keys(this.nameOidMap).length === 0 &&
         this.enumOids.oids.length === 0 &&
         this.enumOids.arrayOids.length === 0) {
-        return this._refreshDynamicOIDs(connection);
+        return Promise.resolve(this._refreshDynamicOIDs(connection)).then(() => connection);
       }
-    }).tap(connection => {
+      return connection;
+    }).then(connection => {
       // Don't let a Postgres restart (or error) to take down the whole app
       connection.on('error', error => {
         connection._invalid = true;
         debug(`connection error ${error.code || error.message}`);
         this.pool.destroy(connection);
       });
+      return connection;
     });
   }
 

lib/dialects/postgres/query-interface.js

@@ -144,11 +144,12 @@ function ensureEnums(qi, tableName, attributes, options, model) {
 
     return promises
       .reduce((promise, asyncFunction) => promise.then(asyncFunction), Promise.resolve())
-      .tap(() => {
+      .then(result => {
       // If ENUM processed, then refresh OIDs
         if (promises.length) {
-          return qi.sequelize.dialect.connectionManager._refreshDynamicOIDs();
+          return Promise.resolve(qi.sequelize.dialect.connectionManager._refreshDynamicOIDs()).then(() => result);
         }
+        return result;
       });
   });
 }

lib/dialects/sqlite/connection-manager.js

@@ -73,7 +73,7 @@ class ConnectionManager extends AbstractConnectionManager {
           resolve(this.connections[options.inMemory || options.uuid]);
         }
       );
-    }).tap(connection => {
+    }).then(connection => {
       if (this.sequelize.config.password) {
         // Make it possible to define and use password for sqlite encryption plugin like sqlcipher
         connection.run(`PRAGMA KEY=${this.sequelize.escape(this.sequelize.config.password)}`);
@@ -83,6 +83,7 @@ class ConnectionManager extends AbstractConnectionManager {
         // explicitly disallowed. It's still opt-in per relation
         connection.run('PRAGMA FOREIGN_KEYS=ON');
       }
+      return connection;
     });
   }
 

lib/model.js

@@ -1759,10 +1759,11 @@ class Model {
     }).then(() => {
       const selectOptions = Object.assign({}, options, { tableNames: Object.keys(tableNames) });
       return this.QueryInterface.select(this, this.getTableName(selectOptions), selectOptions);
-    }).tap(results => {
+    }).then(results => {
       if (options.hooks) {
-        return this.runHooks('afterFind', results, options);
+        return Promise.resolve(this.runHooks('afterFind', results, options)).then(() => results);
       }
+      return results;
     }).then(results => {
 
       //rejectOnEmpty mode
@@ -2502,10 +2503,11 @@ class Model {
 
           return created;
         })
-        .tap(result => {
+        .then(result => {
           if (options.hooks) {
-            return this.runHooks('afterUpsert', result, options);
+            return Promise.resolve(this.runHooks('afterUpsert', result, options)).then(() => result);
           }
+          return result;
         });
     });
   }
@@ -2947,16 +2949,18 @@ class Model {
         return this.QueryInterface.bulkUpdate(this.getTableName(options), attrValueHash, Object.assign(where, options.where), options, this.rawAttributes);
       }
       return this.QueryInterface.bulkDelete(this.getTableName(options), options.where, options, this);
-    }).tap(() => {
+    }).then(result => {
       // Run afterDestroy hook on each record individually
       if (options.individualHooks) {
-        return Promise.all(instances.map(instance => this.runHooks('afterDestroy', instance, options)));
+        return Promise.resolve(Promise.all(instances.map(instance => this.runHooks('afterDestroy', instance, options)))).then(() => result);
       }
-    }).tap(() => {
+      return result;
+    }).then(result => {
       // Run after hook
       if (options.hooks) {
-        return this.runHooks('afterBulkDestroy', options);
+        return Promise.resolve(this.runHooks('afterBulkDestroy', options)).then(() => result);
       }
+      return result;
     });
   }
 
@@ -3012,16 +3016,18 @@ class Model {
       attrValueHash[deletedAtAttribute.field || deletedAtCol] = deletedAtDefaultValue;
       options.omitNull = false;
       return this.QueryInterface.bulkUpdate(this.getTableName(options), attrValueHash, options.where, options, this.rawAttributes);
-    }).tap(() => {
+    }).then(result => {
       // Run afterDestroy hook on each record individually
       if (options.individualHooks) {
-        return Promise.all(instances.map(instance => this.runHooks('afterRestore', instance, options)));
+        return Promise.resolve(Promise.all(instances.map(instance => this.runHooks('afterRestore', instance, options)))).then(() => result);
       }
-    }).tap(() => {
+      return result;
+    }).then(result => {
       // Run after hook
       if (options.hooks) {
-        return this.runHooks('afterBulkRestore', options);
+        return Promise.resolve(this.runHooks('afterBulkRestore', options)).then(() => result);
       }
+      return result;
     });
   }
 
@@ -3196,8 +3202,9 @@ class Model {
               individualOptions.validate = false;
 
               return instance.save(individualOptions);
-            })).tap(_instances => {
+            })).then(_instances => {
               instances = _instances;
+              return _instances;
             });
           });
         });
@@ -3229,22 +3236,24 @@ class Model {
 
         return [affectedRows];
       });
-    }).tap(result => {
+    }).then(result => {
       if (options.individualHooks) {
-        return Promise.all(instances.map(instance => {
-          return this.runHooks('afterUpdate', instance, options);
+        return Promise.resolve(Promise.all(instances.map(instance => {
+          return Promise.resolve(this.runHooks('afterUpdate', instance, options)).then(() => result);
         })).then(() => {
           result[1] = instances;
-        });
+        })).then(() => result);
       }
-    }).tap(() => {
+      return result;
+    }).then(result => {
       // Run after hook
       if (options.hooks) {
         options.attributes = values;
-        return this.runHooks('afterBulkUpdate', options).then(() => {
+        return Promise.resolve(this.runHooks('afterBulkUpdate', options).then(() => {
           delete options.attributes;
-        });
+        })).then(() => result);
       }
+      return result;
     });
   }
 
@@ -4049,18 +4058,18 @@ class Model {
           result.dataValues = Object.assign(result.dataValues, values);
           return result;
         })
-        .tap(() => {
-          if (!wasNewRecord) return this;
-          if (!this._options.include || !this._options.include.length) return this;
+        .then(result => {
+          if (!wasNewRecord) return Promise.resolve(this).then(() => result);
+          if (!this._options.include || !this._options.include.length) return Promise.resolve(this).then(() => result);
 
           // Nested creation for HasOne/HasMany/BelongsToMany relations
-          return Promise.all(this._options.include.filter(include => !(include.association instanceof BelongsTo ||
+          return Promise.resolve(Promise.all(this._options.include.filter(include => !(include.association instanceof BelongsTo ||
             include.parent && include.parent.association instanceof BelongsToMany)).map(include => {
             let instances = this.get(include.as);
 
-            if (!instances) return Promise.resolve();
+            if (!instances) return Promise.resolve(Promise.resolve()).then(() => result);
             if (!Array.isArray(instances)) instances = [instances];
-            if (!instances.length) return Promise.resolve();
+            if (!instances.length) return Promise.resolve(Promise.resolve()).then(() => result);
 
             const includeOptions = _(Utils.cloneDeep(include))
               .omit(['association'])
@@ -4071,9 +4080,9 @@ class Model {
               }).value();
 
             // Instances will be updated in place so we can safely treat HasOne like a HasMany
-            return Promise.all(instances.map(instance => {
+            return Promise.resolve(Promise.all(instances.map(instance => {
               if (include.association instanceof BelongsToMany) {
-                return instance.save(includeOptions).then(() => {
+                return Promise.resolve(instance.save(includeOptions).then(() => {
                   const values = {};
                   values[include.association.foreignKey] = this.get(this.constructor.primaryKeyAttribute, { raw: true });
                   values[include.association.otherKey] = instance.get(instance.constructor.primaryKeyAttribute, { raw: true });
@@ -4092,20 +4101,21 @@ class Model {
                     }
                   }
 
-                  return include.association.throughModel.create(values, includeOptions);
-                });
+                  return Promise.resolve(include.association.throughModel.create(values, includeOptions)).then(() => result);
+                })).then(() => result);
               }
               instance.set(include.association.foreignKey, this.get(include.association.sourceKey || this.constructor.primaryKeyAttribute, { raw: true }), { raw: true });
               Object.assign(instance, include.association.scope);
-              return instance.save(includeOptions);
-            }));
-          }));
+              return Promise.resolve(instance.save(includeOptions)).then(() => result);
+            }))).then(() => result);
+          }))).then(() => result);
         })
-        .tap(result => {
+        .then(result => {
         // Run after hook
           if (options.hooks) {
-            return this.constructor.runHooks(`after${hook}`, result, options);
+            return Promise.resolve(this.constructor.runHooks(`after${hook}`, result, options)).then(() => result);
           }
+          return result;
         })
         .then(result => {
           for (const field of options.fields) {
@@ -4138,12 +4148,13 @@ class Model {
     });
 
     return this.constructor.findOne(options)
-      .tap(reload => {
+      .then(reload => {
         if (!reload) {
           throw new sequelizeErrors.InstanceError(
             'Instance could not be reloaded because it does not exist anymore (find call returned null)'
           );
         }
+        return reload;
       })
       .then(reload => {
       // update the internal options of the instance
@@ -4254,11 +4265,12 @@ class Model {
         return this.save(_.defaults({ hooks: false }, options));
       }
       return this.constructor.QueryInterface.delete(this, this.constructor.getTableName(options), where, Object.assign({ type: QueryTypes.DELETE, limit: null }, options));
-    }).tap(() => {
+    }).then(result => {
       // Run after hook
       if (options.hooks) {
-        return this.constructor.runHooks('afterDestroy', this, options);
+        return Promise.resolve(this.constructor.runHooks('afterDestroy', this, options)).then(() => result);
       }
+      return result;
     });
   }
 
@@ -4311,11 +4323,12 @@ class Model {
 
       this.setDataValue(deletedAtCol, deletedAtDefaultValue);
       return this.save(Object.assign({}, options, { hooks: false, omitNull: false }));
-    }).tap(() => {
+    }).then(result => {
       // Run after hook
       if (options.hooks) {
-        return this.constructor.runHooks('afterRestore', this, options);
+        return Promise.resolve(this.constructor.runHooks('afterRestore', this, options)).then(() => result);
       }
+      return result;
     });
   }
 

lib/sequelize.js

@@ -1110,7 +1110,7 @@ class Sequelize {
     return Sequelize._clsRun(() => {
       return transaction.prepareEnvironment()
         .then(() => autoCallback(transaction))
-        .tap(() => transaction.commit())
+        .then(result => Promise.resolve(transaction.commit()).then(() => result))
         .catch(err => {
           // Rollback transaction if not already finished (commit, rollback, etc)
           // and reject with original error (ignore any error in rollback)

lib/transaction.js

@@ -69,10 +69,10 @@ class Transaction {
           return this.cleanup();
         }
         return null;
-      }).tap(
-        () => Promise.each(
+      }).then(
+        result => Promise.resolve(Promise.each(
           this._afterCommitHooks,
-          hook => Promise.resolve(hook.apply(this, [this])))
+          hook => Promise.resolve(hook.apply(this, [this])))).then(() => result)
       );
   }
 
@@ -133,11 +133,11 @@ class Transaction {
             throw setupErr;
           }));
       })
-      .tap(() => {
+      .then(result => {
         if (useCLS && this.sequelize.constructor._cls) {
           this.sequelize.constructor._cls.set('transaction', this);
         }
-        return null;
+        return Promise.resolve(null).then(() => result);
       });
   }
 

test/integration/dialects/postgres/dao.test.js

@@ -72,9 +72,10 @@ if (dialect.match(/^postgres/)) {
           }]
         });
       }).get('friends')
-        .tap(friends => {
+        .then(friends => {
           expect(friends).to.have.length(1);
           expect(friends[0].name).to.equal('John Smythe');
+          return friends;
         });
     });
 

test/integration/include/findAndCountAll.test.js

@@ -231,13 +231,13 @@ describe(Support.getTestDialectTeaser('Include'), () => {
         return Foo.findAndCountAll({
           include: [{ model: Bar, required: true }],
           limit: 2
-        }).tap(() => {
-          return Foo.findAll({
+        }).then(result => {
+          return Promise.resolve(Foo.findAll({
             include: [{ model: Bar, required: true }],
             limit: 2
           }).then(items => {
             expect(items.length).to.equal(2);
-          });
+          })).then(() => result);
         });
       }).then(result => {
         expect(result.count).to.equal(4);

test/integration/instance/destroy.test.js

@@ -247,11 +247,12 @@ describe(Support.getTestDialectTeaser('Instance'), () => {
             username: 'foo'
           }
         });
-      }).tap(user => {
+      }).then(user => {
         expect(user).to.be.ok;
         expect(moment.utc(user.deletedAt).startOf('second').toISOString())
           .to.equal(moment.utc(deletedAt).startOf('second').toISOString());
         expect(user.username).to.equal('foo');
+        return user;
       }).then(user => {
         // update model and delete again
         user.username = 'bar';
@@ -345,15 +346,17 @@ describe(Support.getTestDialectTeaser('Instance'), () => {
         afterSave.resetHistory();
 
         return user.destroy();
-      }).tap(() => {
+      }).then(result => {
         expect(beforeSave.callCount).to.equal(0, 'should not call beforeSave');
         expect(afterSave.callCount).to.equal(0, 'should not call afterSave');
+        return result;
       }).then(user => {
         // now try with `hooks: true`
         return user.destroy({ hooks: true });
-      }).tap(() => {
+      }).then(result => {
         expect(beforeSave.callCount).to.equal(0, 'should not call beforeSave even if `hooks: true`');
         expect(afterSave.callCount).to.equal(0, 'should not call afterSave even if `hooks: true`');
+        return result;
       });
     });
 

test/integration/instance/reload.test.js

@@ -110,14 +110,14 @@ describe(Support.getTestDialectTeaser('Instance'), () => {
       return this.User.create({
         aNumber: 1,
         bNumber: 1
-      }).tap(user => {
-        return this.User.update({
+      }).then(user => {
+        return Promise.resolve(this.User.update({
           bNumber: 2
         }, {
           where: {
             id: user.get('id')
           }
-        });
+        })).then(() => user);
       }).then(user => {
         return user.reload({
           attributes: ['bNumber']