Skip to content

Commit

Permalink
Fix: Transaction_OracleDB can use config.connection (#3731)
Browse files Browse the repository at this point in the history
  • Loading branch information
briandamaged committed Mar 15, 2020
1 parent 37d9c30 commit 31c5b86
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 1 deletion.
3 changes: 2 additions & 1 deletion lib/dialects/oracledb/transaction.js
Expand Up @@ -53,7 +53,8 @@ module.exports = class Oracle_Transaction extends Transaction {
async acquireConnection(config, cb) {
const configConnection = config && config.connection;

const connection = await this.client.acquireConnection();
const connection =
configConnection || (await this.client.acquireConnection());
try {
connection.__knexTxId = this.txid;
connection.isTransaction = true;
Expand Down
37 changes: 37 additions & 0 deletions test/integration/builder/transaction.js
Expand Up @@ -749,4 +749,41 @@ module.exports = function(knex) {
process.removeListener('unhandledRejection', fn);
}
});

context('when a `connection` is passed in explicitly', function() {
beforeEach(function() {
this.sandbox = sinon.createSandbox();
});

afterEach(function() {
this.sandbox.restore();
});

it('assumes the caller will release the connection', async function() {
this.sandbox.spy(knex.client, 'releaseConnection');
const conn = await knex.client.acquireConnection();
try {
await knex.transaction(
async function(trx) {
// Do nothing!
},
{ connection: conn }
);
} catch (err) {
// Do nothing. The transaction could have failed due to some other
// bug, and it might have still released the connection in the process.
}

expect(knex.client.releaseConnection).to.have.not.been.calledWith(conn);

// By design, this line will only be reached if the connection
// was never released.
knex.client.releaseConnection(conn);

// Note: It's still possible that the test might fail due to a Timeout
// even after concluding. This is because the underlying implementation
// might have opened another connection by mistake, but never actually
// closed it. (Ex: this was the case for OracleDB before fixing #3721)
});
});
};

0 comments on commit 31c5b86

Please sign in to comment.