From 1cd53485a0f39ebe27196cfebb2d71e276f5008d Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Mon, 20 Mar 2017 22:11:36 +1030 Subject: [PATCH 1/7] Fix operations on ended transactions --- lib/postgresql.js | 9 +++++++-- lib/transaction.js | 9 ++++++++- package.json | 3 ++- test/postgresql.transaction.test.js | 25 +++++++++++++++++++++++++ 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/lib/postgresql.js b/lib/postgresql.js index 26f09e9c..7ffa23ff 100644 --- a/lib/postgresql.js +++ b/lib/postgresql.js @@ -194,8 +194,13 @@ PostgreSQL.prototype.executeSQL = function(sql, params, options, callback) { } var transaction = options.transaction; - if (transaction && transaction.connection && - transaction.connector === this) { + if (transaction && transaction.connector === this) { + if (!transaction.connection) { + return callback(new Error(g.f('Connection does not exist'))); + } + if (transaction.txId !== transaction.connection.txId) { + return callback(new Error(g.f('Transaction is not active'))); + } debug('Execute SQL within a transaction'); // Do not release the connection executeWithConnection(transaction.connection, null); diff --git a/lib/transaction.js b/lib/transaction.js index 9db790fb..03c79e2b 100644 --- a/lib/transaction.js +++ b/lib/transaction.js @@ -5,6 +5,8 @@ 'use strict'; var debug = require('debug')('loopback:connector:postgresql:transaction'); +var uuid = require('uuid'); +var Transaction = require('loopback-connector').Transaction; module.exports = mixinTransaction; @@ -18,6 +20,7 @@ function mixinTransaction(PostgreSQL) { * @param cb */ PostgreSQL.prototype.beginTransaction = function(isolationLevel, cb) { + var connector = this; debug('Begin a transaction with isolation level: %s', isolationLevel); this.pg.connect(function(err, connection, done) { if (err) return cb(err); @@ -25,7 +28,10 @@ function mixinTransaction(PostgreSQL) { connection.query('BEGIN TRANSACTION ISOLATION LEVEL ' + isolationLevel, function(err) { if (err) return cb(err); - cb(null, connection); + var tx = new Transaction(connector, connection); + tx.txId = uuid.v1(); + connection.txId = tx.txId; + cb(null, tx); }); }); }; @@ -65,6 +71,7 @@ function mixinTransaction(PostgreSQL) { PostgreSQL.prototype.releaseConnection = function(connection, err) { if (typeof connection.autorelease === 'function') { + connection.txId = null; connection.autorelease(err); connection.autorelease = null; } else { diff --git a/package.json b/package.json index 45d64eca..9a4b51a2 100644 --- a/package.json +++ b/package.json @@ -25,7 +25,8 @@ "debug": "^2.1.1", "loopback-connector": "^4.0.0", "pg": "^6.0.0", - "strong-globalize": "^2.6.2" + "strong-globalize": "^2.6.2", + "uuid": "^3.0.1" }, "devDependencies": { "eslint": "^2.13.1", diff --git a/test/postgresql.transaction.test.js b/test/postgresql.transaction.test.js index a9926498..8d731803 100644 --- a/test/postgresql.transaction.test.js +++ b/test/postgresql.transaction.test.js @@ -124,4 +124,29 @@ describe('transactions', function() { it('should not see the rolledback insert', expectToFindPosts(post, 0)); }); + + describe('finished', function() { + var post = {title: 't2', content: 'c2'}; + beforeEach(createPostInTx(post)); + + it('should throw an error when creating in a committed transaction', function(done) { + currentTx.commit(function(err) { + if(err) return done(err); + Post.create({title: 't4', content: 'c4'}, {transaction: currentTx}, function(err, post) { + if(!err) return done(new Error('should throw error')); + done(); + }); + }); + }); + + it('should throw an error when creating in a rolled back transaction', function(done) { + currentTx.rollback(function(err) { + if(err) return done(err); + Post.create({title: 't4', content: 'c4'}, {transaction: currentTx}, function(err, post) { + if(!err) return done(new Error('should throw error')); + done(); + }); + }); + }); + }); }); From c0ca09155e1f59feae506507da1068afa86baa99 Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Tue, 21 Mar 2017 08:29:47 +1030 Subject: [PATCH 2/7] Fix eslint errors in test --- test/postgresql.transaction.test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/postgresql.transaction.test.js b/test/postgresql.transaction.test.js index 8d731803..25247344 100644 --- a/test/postgresql.transaction.test.js +++ b/test/postgresql.transaction.test.js @@ -131,9 +131,9 @@ describe('transactions', function() { it('should throw an error when creating in a committed transaction', function(done) { currentTx.commit(function(err) { - if(err) return done(err); + if (err) return done(err); Post.create({title: 't4', content: 'c4'}, {transaction: currentTx}, function(err, post) { - if(!err) return done(new Error('should throw error')); + if (!err) return done(new Error('should throw error')); done(); }); }); @@ -141,9 +141,9 @@ describe('transactions', function() { it('should throw an error when creating in a rolled back transaction', function(done) { currentTx.rollback(function(err) { - if(err) return done(err); + if (err) return done(err); Post.create({title: 't4', content: 'c4'}, {transaction: currentTx}, function(err, post) { - if(!err) return done(new Error('should throw error')); + if (!err) return done(new Error('should throw error')); done(); }); }); From 6ebdf94b43a7c6a05f440c338e7210c599efb568 Mon Sep 17 00:00:00 2001 From: jannyHou Date: Fri, 24 Mar 2017 17:08:47 -0400 Subject: [PATCH 3/7] Increase poolSize --- test/init.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/init.js b/test/init.js index 9f4fe0ad..4f511b29 100644 --- a/test/init.js +++ b/test/init.js @@ -29,6 +29,7 @@ if (process.env.CI) { 'emptytest', username: process.env.PGUSER, password: process.env.PGPASSWORD, + poolSize: 100, }; } From 9de4d92627e314cb8115abc71f63a258bfd8013e Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Tue, 28 Mar 2017 08:03:15 +1030 Subject: [PATCH 4/7] Wrap transaction error callbacks in nextTick --- lib/postgresql.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/postgresql.js b/lib/postgresql.js index 7ffa23ff..e49b2e85 100644 --- a/lib/postgresql.js +++ b/lib/postgresql.js @@ -196,10 +196,14 @@ PostgreSQL.prototype.executeSQL = function(sql, params, options, callback) { var transaction = options.transaction; if (transaction && transaction.connector === this) { if (!transaction.connection) { - return callback(new Error(g.f('Connection does not exist'))); + return process.nextTick(function() { + callback(new Error(g.f('Connection does not exist'))) + }); } if (transaction.txId !== transaction.connection.txId) { - return callback(new Error(g.f('Transaction is not active'))); + return process.nextTick(function() { + callback(new Error(g.f('Transaction is not active'))) + }); } debug('Execute SQL within a transaction'); // Do not release the connection From adc7d2e4c1890df33e4fd61cccadd719d04aef49 Mon Sep 17 00:00:00 2001 From: jannyHou Date: Wed, 29 Mar 2017 13:57:51 -0400 Subject: [PATCH 5/7] Remove poolSize --- test/init.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/init.js b/test/init.js index 4f511b29..9f4fe0ad 100644 --- a/test/init.js +++ b/test/init.js @@ -29,7 +29,6 @@ if (process.env.CI) { 'emptytest', username: process.env.PGUSER, password: process.env.PGPASSWORD, - poolSize: 100, }; } From e744cdd2652021e13405dc2c4f227e7ce609cb7f Mon Sep 17 00:00:00 2001 From: jannyHou Date: Thu, 30 Mar 2017 14:50:09 -0400 Subject: [PATCH 6/7] Return done(err) --- test/postgresql.transaction.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/postgresql.transaction.test.js b/test/postgresql.transaction.test.js index 25247344..eb0d3f63 100644 --- a/test/postgresql.transaction.test.js +++ b/test/postgresql.transaction.test.js @@ -71,11 +71,11 @@ describe('transactions', function() { Post.create(post, {transaction: tx}, function(err, p) { if (err) { - done(err); + return done(err); } else { tx.commit(function(err) { if (err) { - done(err); + return done(err); } completed++; checkResults(); From 1361460cea86fadd14a68aa8fe9c55f14ecb9a6d Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Fri, 31 Mar 2017 08:28:59 +1030 Subject: [PATCH 7/7] Add missing semicolons --- lib/postgresql.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/postgresql.js b/lib/postgresql.js index e49b2e85..0a57ef90 100644 --- a/lib/postgresql.js +++ b/lib/postgresql.js @@ -197,12 +197,12 @@ PostgreSQL.prototype.executeSQL = function(sql, params, options, callback) { if (transaction && transaction.connector === this) { if (!transaction.connection) { return process.nextTick(function() { - callback(new Error(g.f('Connection does not exist'))) + callback(new Error(g.f('Connection does not exist'))); }); } if (transaction.txId !== transaction.connection.txId) { return process.nextTick(function() { - callback(new Error(g.f('Transaction is not active'))) + callback(new Error(g.f('Transaction is not active'))); }); } debug('Execute SQL within a transaction');