From c9c96b155e524dd08cc802a8d0036aa7356d763f Mon Sep 17 00:00:00 2001 From: Gajus Kuizinas Date: Mon, 18 Feb 2019 13:40:53 +0000 Subject: [PATCH] feat: add guards against starting parallel transactions using the same connection --- package.json | 1 + src/binders/bindTransactionConnection.js | 31 +++++++++++----- src/connectionMethods/nestedTransaction.js | 4 +++ src/connectionMethods/transaction.js | 10 +++++- test/slonik/binders/bindPool/transaction.js | 37 +++++++++++++++++++- test/slonik/connectionMethods/transaction.js | 10 ++++-- 6 files changed, 80 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index c652c1ef..02575c41 100644 --- a/package.json +++ b/package.json @@ -15,6 +15,7 @@ "boolean": "^0.2.0", "camelcase": "^5.0.0", "crack-json": "^1.1.0", + "delay": "^4.1.0", "es6-error": "^4.1.1", "get-stack-trace": "^2.0.1", "pg": "^7.8.0", diff --git a/src/binders/bindTransactionConnection.js b/src/binders/bindTransactionConnection.js index c76d87dd..e3733288 100644 --- a/src/binders/bindTransactionConnection.js +++ b/src/binders/bindTransactionConnection.js @@ -20,6 +20,7 @@ import type { DatabaseTransactionConnectionType, InternalDatabaseConnectionType, LoggerType, + TaggedTemplateLiteralInvocationType, TransactionFunctionType } from '../types'; @@ -29,16 +30,28 @@ export default ( clientConfiguration: ClientConfigurationType, transactionDepth: number ): DatabaseTransactionConnectionType => { + const mapInvocation = (fn) => { + const bound = mapTaggedTemplateLiteralInvocation(fn); + + return (taggedQuery: TaggedTemplateLiteralInvocationType) => { + if (transactionDepth !== connection.slonik.transactionDepth) { + return Promise.reject(new Error('Cannot run a query using parent transaction.')); + } + + return bound(taggedQuery); + }; + }; + return { - any: mapTaggedTemplateLiteralInvocation(any.bind(null, parentLog, connection, clientConfiguration)), - anyFirst: mapTaggedTemplateLiteralInvocation(anyFirst.bind(null, parentLog, connection, clientConfiguration)), - many: mapTaggedTemplateLiteralInvocation(many.bind(null, parentLog, connection, clientConfiguration)), - manyFirst: mapTaggedTemplateLiteralInvocation(manyFirst.bind(null, parentLog, connection, clientConfiguration)), - maybeOne: mapTaggedTemplateLiteralInvocation(maybeOne.bind(null, parentLog, connection, clientConfiguration)), - maybeOneFirst: mapTaggedTemplateLiteralInvocation(maybeOneFirst.bind(null, parentLog, connection, clientConfiguration)), - one: mapTaggedTemplateLiteralInvocation(one.bind(null, parentLog, connection, clientConfiguration)), - oneFirst: mapTaggedTemplateLiteralInvocation(oneFirst.bind(null, parentLog, connection, clientConfiguration)), - query: mapTaggedTemplateLiteralInvocation(query.bind(null, parentLog, connection, clientConfiguration)), + any: mapInvocation(any.bind(null, parentLog, connection, clientConfiguration)), + anyFirst: mapInvocation(anyFirst.bind(null, parentLog, connection, clientConfiguration)), + many: mapInvocation(many.bind(null, parentLog, connection, clientConfiguration)), + manyFirst: mapInvocation(manyFirst.bind(null, parentLog, connection, clientConfiguration)), + maybeOne: mapInvocation(maybeOne.bind(null, parentLog, connection, clientConfiguration)), + maybeOneFirst: mapInvocation(maybeOneFirst.bind(null, parentLog, connection, clientConfiguration)), + one: mapInvocation(one.bind(null, parentLog, connection, clientConfiguration)), + oneFirst: mapInvocation(oneFirst.bind(null, parentLog, connection, clientConfiguration)), + query: mapInvocation(query.bind(null, parentLog, connection, clientConfiguration)), transaction: (handler: TransactionFunctionType) => { return nestedTransaction(parentLog, connection, clientConfiguration, handler, transactionDepth); } diff --git a/src/connectionMethods/nestedTransaction.js b/src/connectionMethods/nestedTransaction.js index 89d327fb..cac6a786 100644 --- a/src/connectionMethods/nestedTransaction.js +++ b/src/connectionMethods/nestedTransaction.js @@ -21,6 +21,8 @@ const nestedTransaction: InternalNestedTransactionFunctionType = async (parentLo }); try { + connection.slonik.transactionDepth = newTransactionDepth; + const result = await handler(bindTransactionConnection(log, connection, clientConfiguration, newTransactionDepth)); return result; @@ -32,6 +34,8 @@ const nestedTransaction: InternalNestedTransactionFunctionType = async (parentLo }, 'rolling back transaction due to an error'); throw error; + } finally { + connection.slonik.transactionDepth = newTransactionDepth - 1; } }; diff --git a/src/connectionMethods/transaction.js b/src/connectionMethods/transaction.js index b2884822..d5581668 100644 --- a/src/connectionMethods/transaction.js +++ b/src/connectionMethods/transaction.js @@ -12,6 +12,12 @@ import type { } from '../types'; const transaction: InternalTransactionFunctionType = async (parentLog, connection, clientConfiguration, handler) => { + if (connection.slonik.transactionDepth !== null) { + throw new Error('Cannot use the same connection to start a new transaction before completing the last transaction.'); + } + + connection.slonik.transactionDepth = 0; + await connection.query('START TRANSACTION'); const log = parentLog.child({ @@ -19,7 +25,7 @@ const transaction: InternalTransactionFunctionType = async (parentLog, connectio }); try { - const result = await handler(bindTransactionConnection(log, connection, clientConfiguration, 0)); + const result = await handler(bindTransactionConnection(log, connection, clientConfiguration, connection.slonik.transactionDepth)); await connection.query('COMMIT'); @@ -32,6 +38,8 @@ const transaction: InternalTransactionFunctionType = async (parentLog, connectio }, 'rolling back transaction due to an error'); throw error; + } finally { + connection.slonik.transactionDepth = null; } }; diff --git a/test/slonik/binders/bindPool/transaction.js b/test/slonik/binders/bindPool/transaction.js index dbb2c989..0ed82d25 100644 --- a/test/slonik/binders/bindPool/transaction.js +++ b/test/slonik/binders/bindPool/transaction.js @@ -2,6 +2,7 @@ import test from 'ava'; import sinon from 'sinon'; +import delay from 'delay'; import sql from '../../../../src/templateTags/sql'; import bindPool from '../../../../src/binders/bindPool'; import log from '../../../helpers/Logger'; @@ -30,7 +31,10 @@ const createPool = () => { connect: () => { return connection; }, - query: () => {} + query: () => {}, + slonik: { + transactionDepth: null + } }; const querySpy = sinon.spy(internalPool, 'query'); @@ -183,3 +187,34 @@ test('rollsback the entire transaction with multiple savepoints (multiple depth 'ROLLBACK' ]); }); + +test('throws an error if an attempt is made to create a new transaction before the last transaction is completed', async (t) => { + const pool = createPool(); + + const connection = pool.connect((c1) => { + return Promise.race([ + c1.transaction(() => { + return delay(1000); + }), + c1.transaction(() => { + return delay(1000); + }) + ]); + }); + + await t.throwsAsync(connection, 'Cannot use the same connection to start a new transaction before completing the last transaction.'); +}); + +test('throws an error if an attempt is made to execute a query using the parent transaction before the current transaction is completed', async (t) => { + const pool = createPool(); + + const connection = pool.connect((c1) => { + return c1.transaction((t1) => { + return t1.transaction(() => { + return t1.query(sql`SELECT 1`); + }); + }); + }); + + await t.throwsAsync(connection, 'Cannot run a query using parent transaction.'); +}); diff --git a/test/slonik/connectionMethods/transaction.js b/test/slonik/connectionMethods/transaction.js index cdd6095f..9c646929 100644 --- a/test/slonik/connectionMethods/transaction.js +++ b/test/slonik/connectionMethods/transaction.js @@ -12,7 +12,10 @@ test('commits successful transaction', async (t) => { query.returns({}); const connection = { - query + query, + slonik: { + transactionDepth: null + } }; const result = await transaction(log, connection, createClientConfiguration(), async () => { @@ -39,7 +42,10 @@ test('rollbacks unsuccessful transaction', async (t) => { query.returns({}); const connection = { - query + query, + slonik: { + transactionDepth: null + } }; const transactionExecution = transaction(log, connection, createClientConfiguration(), async () => {