不要怂,就是干,撸起袖子干!

Commit c28e034e by Simon Schick

refactor(cls): remove cls support (#10817)

BREAKING CHANGE:
All CLS support has been removed.

Migration guide:
If you do not use CLS, you can ignore this.
Check all your usage of the `.transaction` method and make you sure explicitly pass the `transaction` object for each subsequent query.
1 parent a1c6f16d
...@@ -52,68 +52,16 @@ return sequelize.transaction(t => { ...@@ -52,68 +52,16 @@ return sequelize.transaction(t => {
}); });
``` ```
### Automatically pass transactions to all queries ### Example
In the examples above, the transaction is still manually passed, by passing `{ transaction: t }` as the second argument. To automatically pass the transaction to all queries you must install the [continuation local storage](https://github.com/othiym23/node-continuation-local-storage) (CLS) module and instantiate a namespace in your own code:
```js
const cls = require('continuation-local-storage'),
namespace = cls.createNamespace('my-very-own-namespace');
```
To enable CLS you must tell sequelize which namespace to use by using a static method of the sequelize constructor:
```js
const Sequelize = require('sequelize');
Sequelize.useCLS(namespace);
new Sequelize(....);
```
Notice, that the `useCLS()` method is on the *constructor*, not on an instance of sequelize. This means that all instances will share the same namespace, and that CLS is all-or-nothing - you cannot enable it only for some instances.
CLS works like a thread-local storage for callbacks. What this means in practice is that different callback chains can access local variables by using the CLS namespace. When CLS is enabled sequelize will set the `transaction` property on the namespace when a new transaction is created. Since variables set within a callback chain are private to that chain several concurrent transactions can exist at the same time:
```js
sequelize.transaction((t1) => {
namespace.get('transaction') === t1; // true
});
sequelize.transaction((t2) => {
namespace.get('transaction') === t2; // true
});
```
In most case you won't need to access `namespace.get('transaction')` directly, since all queries will automatically look for a transaction on the namespace:
```js
sequelize.transaction((t1) => {
// With CLS enabled, the user will be created inside the transaction
return User.create({ name: 'Alice' });
});
```
After you've used `Sequelize.useCLS()` all promises returned from sequelize will be patched to maintain CLS context. CLS is a complicated subject - more details in the docs for [cls-bluebird](https://www.npmjs.com/package/cls-bluebird), the patch used to make bluebird promises work with CLS.
**Note:** _[CLS only supports async/await, at the moment, when using cls-hooked package](https://github.com/othiym23/node-continuation-local-storage/issues/98#issuecomment-323503807). Although, [cls-hooked](https://github.com/Jeff-Lewis/cls-hooked/blob/master/README.md) relies on *experimental API* [async_hooks](https://github.com/nodejs/node/blob/master/doc/api/async_hooks.md)_
## Concurrent/Partial transactions
You can have concurrent transactions within a sequence of queries or have some of them excluded from any transactions. Use the `{transaction: }` option to control which transaction a query belong to:
**Warning:** _SQLite does not support more than one transaction at the same time._
### Without CLS enabled
```js ```js
sequelize.transaction((t1) => { sequelize.transaction((t1) => {
return sequelize.transaction((t2) => { return sequelize.transaction((t2) => {
// With CLS enable, queries here will by default use t2
// Pass in the `transaction` option to define/alter the transaction they belong to. // Pass in the `transaction` option to define/alter the transaction they belong to.
return Promise.all([ return Promise.all([
User.create({ name: 'Bob' }, { transaction: null }), User.create({ name: 'Bob' }, { transaction: null }),
User.create({ name: 'Mallory' }, { transaction: t1 }), User.create({ name: 'Mallory' }, { transaction: t1 }),
User.create({ name: 'John' }) // this would default to t2 User.create({ name: 'John' }) // No transaction
]); ]);
}); });
}); });
......
...@@ -24,3 +24,36 @@ If you have relied on accessing sequelize operators via `Symbol.for('gt')` etc. ...@@ -24,3 +24,36 @@ If you have relied on accessing sequelize operators via `Symbol.for('gt')` etc.
`Model.build` has been acting as proxy for `bulkBuild` and `new Model` for a while. `Model.build` has been acting as proxy for `bulkBuild` and `new Model` for a while.
Use `Model.bulkBuild` or `new Model` instead. Use `Model.bulkBuild` or `new Model` instead.
### Removal of CLS
CLS allowed implicit passing of the `transaction` property to query options inside of a transaction.
This feature was removed for 3 reasons.
- It required hooking the promise implementation which is not sustainable for the future of sequelize.
- It's generally unsafe due to it's implicit nature.
- It wasn't always reliable when mixed promise implementations were used.
Check all your usage of the `.transaction` method and make sure to explicitly pass the `transaction` object for each subsequent query.
#### Example
```js
db.transaction(async transaction => {
const mdl = await myModel.findByPk(1);
await mdl.update({
a: 1;
});
});
```
should now be:
```js
db.transaction(async transaction => {
const mdl = await myModel.findByPk(1, { transaction });
await mdl.update({
a: 1;
}, { transaction });
});
```
...@@ -2286,13 +2286,6 @@ class Model { ...@@ -2286,13 +2286,6 @@ class Model {
} }
} }
if (options.transaction === undefined && this.sequelize.constructor._cls) {
const t = this.sequelize.constructor._cls.get('transaction');
if (t) {
options.transaction = t;
}
}
const internalTransaction = !options.transaction; const internalTransaction = !options.transaction;
let values; let values;
let transaction; let transaction;
......
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
const url = require('url'); const url = require('url');
const path = require('path'); const path = require('path');
const retry = require('retry-as-promised'); const retry = require('retry-as-promised');
const clsBluebird = require('cls-bluebird');
const _ = require('lodash'); const _ = require('lodash');
const Utils = require('./utils'); const Utils = require('./utils');
...@@ -614,9 +613,6 @@ class Sequelize { ...@@ -614,9 +613,6 @@ class Sequelize {
const retryOptions = Object.assign({}, this.options.retry, options.retry || {}); const retryOptions = Object.assign({}, this.options.retry, options.retry || {});
return Promise.resolve(retry(() => Promise.try(() => { return Promise.resolve(retry(() => Promise.try(() => {
if (options.transaction === undefined && Sequelize._cls) {
options.transaction = Sequelize._cls.get('transaction');
}
if (options.transaction && options.transaction.finished) { if (options.transaction && options.transaction.finished) {
const error = new Error(`${options.transaction.finished} has been called on this transaction(${options.transaction.id}), you can no longer use it. (The rejected query is attached as the 'sql' property of this error)`); const error = new Error(`${options.transaction.finished} has been called on this transaction(${options.transaction.id}), you can no longer use it. (The rejected query is attached as the 'sql' property of this error)`);
error.sql = sql; error.sql = sql;
...@@ -1040,8 +1036,6 @@ class Sequelize { ...@@ -1040,8 +1036,6 @@ class Sequelize {
/** /**
* Start a transaction. When using transactions, you should pass the transaction in the options argument in order for the query to happen under that transaction @see {@link Transaction} * Start a transaction. When using transactions, you should pass the transaction in the options argument in order for the query to happen under that transaction @see {@link Transaction}
* *
* If you have [CLS](https://github.com/othiym23/node-continuation-local-storage) enabled, the transaction will automatically be passed to any query that runs within the callback
*
* @example * @example
* sequelize.transaction().then(transaction => { * sequelize.transaction().then(transaction => {
* return User.findOne(..., {transaction}) * return User.findOne(..., {transaction})
...@@ -1050,27 +1044,6 @@ class Sequelize { ...@@ -1050,27 +1044,6 @@ class Sequelize {
* .catch(() => transaction.rollback()); * .catch(() => transaction.rollback());
* }) * })
* *
* @example <caption>A syntax for automatically committing or rolling back based on the promise chain resolution is also supported</caption>
*
* sequelize.transaction(transaction => { // Note that we use a callback rather than a promise.then()
* return User.findOne(..., {transaction})
* .then(user => user.update(..., {transaction}))
* }).then(() => {
* // Committed
* }).catch(err => {
* // Rolled back
* console.error(err);
* });
*
* @example <caption>To enable CLS, add it do your project, create a namespace and set it on the sequelize constructor:</caption>
*
* const cls = require('continuation-local-storage');
* const ns = cls.createNamespace('....');
* const Sequelize = require('sequelize');
* Sequelize.useCLS(ns);
*
* // Note, that CLS is enabled for all sequelize instances, and all instances will share the same namespace
*
* @param {Object} [options] Transaction options * @param {Object} [options] Transaction options
* @param {string} [options.type='DEFERRED'] See `Sequelize.Transaction.TYPES` for possible options. Sqlite only. * @param {string} [options.type='DEFERRED'] See `Sequelize.Transaction.TYPES` for possible options. Sqlite only.
* @param {string} [options.isolationLevel] See `Sequelize.Transaction.ISOLATION_LEVELS` for possible options * @param {string} [options.isolationLevel] See `Sequelize.Transaction.ISOLATION_LEVELS` for possible options
...@@ -1090,7 +1063,6 @@ class Sequelize { ...@@ -1090,7 +1063,6 @@ class Sequelize {
if (!autoCallback) return transaction.prepareEnvironment(false).return(transaction); if (!autoCallback) return transaction.prepareEnvironment(false).return(transaction);
// autoCallback provided // autoCallback provided
return Sequelize._clsRun(() => {
return transaction.prepareEnvironment() return transaction.prepareEnvironment()
.then(() => autoCallback(transaction)) .then(() => autoCallback(transaction))
.tap(() => transaction.commit()) .tap(() => transaction.commit())
...@@ -1101,46 +1073,6 @@ class Sequelize { ...@@ -1101,46 +1073,6 @@ class Sequelize {
if (!transaction.finished) return transaction.rollback().catch(() => {}); if (!transaction.finished) return transaction.rollback().catch(() => {});
}).throw(err); }).throw(err);
}); });
});
}
/**
* Use CLS with Sequelize.
* CLS namespace provided is stored as `Sequelize._cls`
* and bluebird Promise is patched to use the namespace, using `cls-bluebird` module.
*
* @param {Object} ns CLS namespace
* @returns {Object} Sequelize constructor
*/
static useCLS(ns) {
// check `ns` is valid CLS namespace
if (!ns || typeof ns !== 'object' || typeof ns.bind !== 'function' || typeof ns.run !== 'function') throw new Error('Must provide CLS namespace');
// save namespace as `Sequelize._cls`
this._cls = ns;
// patch bluebird to bind all promise callbacks to CLS namespace
clsBluebird(ns, Promise);
// return Sequelize for chaining
return this;
}
/**
* Run function in CLS context.
* If no CLS context in use, just runs the function normally
*
* @private
* @param {Function} fn Function to run
* @returns {*} Return value of function
*/
static _clsRun(fn) {
const ns = Sequelize._cls;
if (!ns) return fn();
let res;
ns.run(context => res = fn(context));
return res;
} }
log(...args) { log(...args) {
......
...@@ -58,8 +58,6 @@ class Transaction { ...@@ -58,8 +58,6 @@ class Transaction {
return Promise.reject(new Error(`Transaction cannot be committed because it has been finished with state: ${this.finished}`)); return Promise.reject(new Error(`Transaction cannot be committed because it has been finished with state: ${this.finished}`));
} }
this._clearCls();
return this return this
.sequelize .sequelize
.getQueryInterface() .getQueryInterface()
...@@ -91,8 +89,6 @@ class Transaction { ...@@ -91,8 +89,6 @@ class Transaction {
return Promise.reject(new Error('Transaction cannot be rolled back because it never started')); return Promise.reject(new Error('Transaction cannot be rolled back because it never started'));
} }
this._clearCls();
return this return this
.sequelize .sequelize
.getQueryInterface() .getQueryInterface()
...@@ -105,13 +101,9 @@ class Transaction { ...@@ -105,13 +101,9 @@ class Transaction {
}); });
} }
prepareEnvironment(useCLS) { prepareEnvironment() {
let connectionPromise; let connectionPromise;
if (useCLS === undefined) {
useCLS = true;
}
if (this.parent) { if (this.parent) {
connectionPromise = Promise.resolve(this.parent.connection); connectionPromise = Promise.resolve(this.parent.connection);
} else { } else {
...@@ -134,12 +126,6 @@ class Transaction { ...@@ -134,12 +126,6 @@ class Transaction {
.catch(setupErr => this.rollback().finally(() => { .catch(setupErr => this.rollback().finally(() => {
throw setupErr; throw setupErr;
})); }));
})
.tap(() => {
if (useCLS && this.sequelize.constructor._cls) {
this.sequelize.constructor._cls.set('transaction', this);
}
return null;
}); });
} }
...@@ -172,16 +158,6 @@ class Transaction { ...@@ -172,16 +158,6 @@ class Transaction {
return res; return res;
} }
_clearCls() {
const cls = this.sequelize.constructor._cls;
if (cls) {
if (cls.get('transaction') === this) {
cls.set('transaction', null);
}
}
}
/** /**
* A hook that is run after a transaction is committed * A hook that is run after a transaction is committed
* *
......
...@@ -31,7 +31,6 @@ ...@@ -31,7 +31,6 @@
"license": "MIT", "license": "MIT",
"dependencies": { "dependencies": {
"bluebird": "^3.5.0", "bluebird": "^3.5.0",
"cls-bluebird": "^2.1.0",
"debug": "^4.1.1", "debug": "^4.1.1",
"dottie": "^2.0.0", "dottie": "^2.0.0",
"inflection": "1.12.0", "inflection": "1.12.0",
...@@ -57,7 +56,6 @@ ...@@ -57,7 +56,6 @@
"chai-as-promised": "^7.x", "chai-as-promised": "^7.x",
"chai-datetime": "^1.x", "chai-datetime": "^1.x",
"chai-spies": "^1.x", "chai-spies": "^1.x",
"continuation-local-storage": "^3.x",
"cross-env": "^5.2.0", "cross-env": "^5.2.0",
"env-cmd": "^8.0.0", "env-cmd": "^8.0.0",
"esdoc": "^1.1.0", "esdoc": "^1.1.0",
......
'use strict';
const chai = require('chai'),
expect = chai.expect,
Support = require('./support'),
Sequelize = Support.Sequelize,
Promise = Sequelize.Promise,
cls = require('continuation-local-storage'),
current = Support.sequelize;
if (current.dialect.supports.transactions) {
describe(Support.getTestDialectTeaser('Continuation local storage'), () => {
before(function() {
this.thenOriginal = Promise.prototype.then;
Sequelize.useCLS(cls.createNamespace('sequelize'));
});
after(() => {
delete Sequelize._cls;
});
beforeEach(function() {
return Support.prepareTransactionTest(this.sequelize).then(sequelize => {
this.sequelize = sequelize;
this.ns = cls.getNamespace('sequelize');
this.User = this.sequelize.define('user', {
name: Sequelize.STRING
});
return this.sequelize.sync({ force: true });
});
});
describe('context', () => {
it('does not use continuation storage on manually managed transactions', function() {
return Sequelize._clsRun(() => {
return this.sequelize.transaction().then(transaction => {
expect(this.ns.get('transaction')).to.be.undefined;
return transaction.rollback();
});
});
});
it('supports several concurrent transactions', function() {
let t1id, t2id;
return Promise.join(
this.sequelize.transaction(() => {
t1id = this.ns.get('transaction').id;
return Promise.resolve();
}),
this.sequelize.transaction(() => {
t2id = this.ns.get('transaction').id;
return Promise.resolve();
}),
() => {
expect(t1id).to.be.ok;
expect(t2id).to.be.ok;
expect(t1id).not.to.equal(t2id);
}
);
});
it('supports nested promise chains', function() {
return this.sequelize.transaction(() => {
const tid = this.ns.get('transaction').id;
return this.User.findAll().then(() => {
expect(this.ns.get('transaction').id).to.be.ok;
expect(this.ns.get('transaction').id).to.equal(tid);
});
});
});
it('does not leak variables to the outer scope', function() {
// This is a little tricky. We want to check the values in the outer scope, when the transaction has been successfully set up, but before it has been comitted.
// We can't just call another function from inside that transaction, since that would transfer the context to that function - exactly what we are trying to prevent;
let transactionSetup = false,
transactionEnded = false;
this.sequelize.transaction(() => {
transactionSetup = true;
return Promise.delay(500).then(() => {
expect(this.ns.get('transaction')).to.be.ok;
transactionEnded = true;
});
});
return new Promise(resolve => {
// Wait for the transaction to be setup
const interval = setInterval(() => {
if (transactionSetup) {
clearInterval(interval);
resolve();
}
}, 200);
}).then(() => {
expect(transactionEnded).not.to.be.ok;
expect(this.ns.get('transaction')).not.to.be.ok;
// Just to make sure it didn't change between our last check and the assertion
expect(transactionEnded).not.to.be.ok;
});
});
it('does not leak variables to the following promise chain', function() {
return this.sequelize.transaction(() => {
return Promise.resolve();
}).then(() => {
expect(this.ns.get('transaction')).not.to.be.ok;
});
});
it('does not leak outside findOrCreate', function() {
return this.User.findOrCreate({
where: {
name: 'Kafka'
},
logging(sql) {
if (/default/.test(sql)) {
throw new Error('The transaction was not properly assigned');
}
}
}).then(() => {
return this.User.findAll();
});
});
});
describe('sequelize.query integration', () => {
it('automagically uses the transaction in all calls', function() {
return this.sequelize.transaction(() => {
return this.User.create({ name: 'bob' }).then(() => {
return Promise.all([
expect(this.User.findAll({ transaction: null })).to.eventually.have.length(0),
expect(this.User.findAll({})).to.eventually.have.length(1)
]);
});
});
});
});
it('bluebird patch is applied', function() {
expect(Promise.prototype.then).to.be.a('function');
expect(this.thenOriginal).to.be.a('function');
expect(Promise.prototype.then).not.to.equal(this.thenOriginal);
});
it('CLS namespace is stored in Sequelize._cls', function() {
expect(Sequelize._cls).to.equal(this.ns);
});
it('promises returned by sequelize.query are correctly patched', function() {
return this.sequelize.transaction(t =>
this.sequelize.query('select 1', { type: Sequelize.QueryTypes.SELECT })
.then(() => expect(this.ns.get('transaction')).to.equal(t))
);
});
});
}
'use strict';
const chai = require('chai'),
expect = chai.expect,
Support = require('../support'),
current = Support.sequelize,
cls = require('continuation-local-storage'),
sinon = require('sinon'),
stub = sinon.stub;
describe(Support.getTestDialectTeaser('Model'), () => {
describe('method findOrCreate', () => {
before(() => {
current.constructor.useCLS(cls.createNamespace('sequelize'));
});
after(() => {
delete current.constructor._cls;
});
beforeEach(function() {
this.User = current.define('User', {}, {
name: 'John'
});
this.transactionStub = stub(this.User.sequelize, 'transaction').rejects(new Error('abort'));
this.clsStub = stub(current.constructor._cls, 'get').returns({ id: 123 });
});
afterEach(function() {
this.transactionStub.restore();
this.clsStub.restore();
});
it('should use transaction from cls if available', function() {
const options = {
where: {
name: 'John'
}
};
return this.User.findOrCreate(options)
.then(() => {
expect.fail('expected to fail');
})
.catch(/abort/, () => {
expect(this.clsStub.calledOnce).to.equal(true, 'expected to ask for transaction');
});
});
it('should not use transaction from cls if provided as argument', function() {
const options = {
where: {
name: 'John'
},
transaction: { id: 123 }
};
return this.User.findOrCreate(options)
.then(() => {
expect.fail('expected to fail');
})
.catch(/abort/, () => {
expect(this.clsStub.called).to.equal(false);
});
});
});
});
...@@ -706,14 +706,6 @@ export class Sequelize extends Hooks { ...@@ -706,14 +706,6 @@ export class Sequelize extends Hooks {
public static afterSync(name: string, fn: (options: SyncOptions) => HookReturn): void; public static afterSync(name: string, fn: (options: SyncOptions) => HookReturn): void;
public static afterSync(fn: (options: SyncOptions) => HookReturn): void; public static afterSync(fn: (options: SyncOptions) => HookReturn): void;
/**
* Use CLS with Sequelize.
* CLS namespace provided is stored as `Sequelize._cls`
* and bluebird Promise is patched to use the namespace, using `cls-bluebird` module.
*
* @param namespace
*/
public static useCLS(namespace: object): typeof Sequelize;
/** /**
* A reference to Sequelize constructor from sequelize. Useful for accessing DataTypes, Errors etc. * A reference to Sequelize constructor from sequelize. Useful for accessing DataTypes, Errors etc.
...@@ -1280,18 +1272,6 @@ export class Sequelize extends Hooks { ...@@ -1280,18 +1272,6 @@ export class Sequelize extends Hooks {
* }); * });
* ``` * ```
* *
* If you have [CLS](https://github.com/othiym23/node-continuation-local-storage) enabled, the transaction
* will automatically be passed to any query that runs witin the callback. To enable CLS, add it do your
* project, create a namespace and set it on the sequelize constructor:
*
* ```js
* const cls = require('continuation-local-storage'),
* ns = cls.createNamespace('....');
* const Sequelize = require('sequelize');
* Sequelize.cls = ns;
* ```
* Note, that CLS is enabled for all sequelize instances, and all instances will share the same namespace
*
* @param options Transaction Options * @param options Transaction Options
* @param autoCallback Callback for the transaction * @param autoCallback Callback for the transaction
*/ */
......
import { Config, Sequelize, Model } from 'sequelize'; import { Config, Sequelize, Model } from 'sequelize';
import { Fn } from '../lib/utils'; import { Fn } from '../lib/utils';
Sequelize.useCLS({
});
export const sequelize = new Sequelize({ export const sequelize = new Sequelize({
hooks: { hooks: {
afterConnect: (connection, config: Config) => { afterConnect: (connection, config: Config) => {
......
Markdown is supported
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!