From df33c06b96d4505502459bf2b05ad93b22c3ffde Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Mon, 20 Mar 2017 22:11:36 +1030 Subject: [PATCH 01/11] 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 a819f4f91869b933ec3a7c226824be4334769901 Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Tue, 21 Mar 2017 08:29:47 +1030 Subject: [PATCH 02/11] 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 fa1de9c8e251c35cac76d66f63544626aa016eac Mon Sep 17 00:00:00 2001 From: jannyHou Date: Fri, 24 Mar 2017 17:08:47 -0400 Subject: [PATCH 03/11] Increase poolSize --- test/init.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/init.js b/test/init.js index e67b256b..349af157 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 bf08208ba098f05af48864462d5b329689289679 Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Tue, 28 Mar 2017 08:03:15 +1030 Subject: [PATCH 04/11] 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 7edbeafde5f1b796bc1582a882e45eac84ed2abb Mon Sep 17 00:00:00 2001 From: jannyHou Date: Wed, 29 Mar 2017 13:57:51 -0400 Subject: [PATCH 05/11] Remove poolSize --- test/init.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/init.js b/test/init.js index 349af157..e67b256b 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 c10c072ef39e0eb1200b4bd0bdbb8c58c2e7a4ac Mon Sep 17 00:00:00 2001 From: jannyHou Date: Thu, 30 Mar 2017 14:50:09 -0400 Subject: [PATCH 06/11] 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 b24a58004bcef5ec72290edd8c11ca51b1530849 Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Fri, 31 Mar 2017 08:28:59 +1030 Subject: [PATCH 07/11] 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'); From 4cf33d33da6e318e1cbb4fb8b8a0a533e9f76026 Mon Sep 17 00:00:00 2001 From: jannyHou Date: Sun, 9 Apr 2017 18:10:46 -0400 Subject: [PATCH 08/11] Add debug --- test/postgresql.transaction.test.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/postgresql.transaction.test.js b/test/postgresql.transaction.test.js index eb0d3f63..2c17d00b 100644 --- a/test/postgresql.transaction.test.js +++ b/test/postgresql.transaction.test.js @@ -59,7 +59,7 @@ describe('transactions', function() { } describe('bulk', function() { - it('should work with bulk transactions', function(done) { + it.only('should work with bulk transactions', function(done) { var completed = 0; var concurrent = 20; for (var i = 0; i <= concurrent; i++) { @@ -67,14 +67,19 @@ describe('transactions', function() { var create = createPostInTx(post); Transaction.begin(db.connector, Transaction.SERIALIZABLE, function(err, tx) { - if (err) return done(err); + if (err) { + console.log('beginTransaction ' + tx); + return done(err); + } Post.create(post, {transaction: tx}, function(err, p) { if (err) { + console.log('create ' + post + ' tx ' + tx); return done(err); } else { tx.commit(function(err) { if (err) { + console.log('commit tx ' + tx); return done(err); } completed++; From f0916aeafe05c17e9be8ecd8defc5347f25cde4f Mon Sep 17 00:00:00 2001 From: jannyHou Date: Sun, 9 Apr 2017 18:23:39 -0400 Subject: [PATCH 09/11] Test only one file --- 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 2c17d00b..a8fcfe7d 100644 --- a/test/postgresql.transaction.test.js +++ b/test/postgresql.transaction.test.js @@ -11,7 +11,7 @@ var Transaction = require('loopback-connector').Transaction; var db, Post; -describe('transactions', function() { +describe.only('transactions', function() { before(function(done) { db = getDataSource(true); Post = db.define('PostTX', { @@ -59,7 +59,7 @@ describe('transactions', function() { } describe('bulk', function() { - it.only('should work with bulk transactions', function(done) { + it('should work with bulk transactions', function(done) { var completed = 0; var concurrent = 20; for (var i = 0; i <= concurrent; i++) { From dfeedd9e62f95bf88e91848c8952ae34ec2e364c Mon Sep 17 00:00:00 2001 From: jannyHou Date: Sun, 9 Apr 2017 18:55:58 -0400 Subject: [PATCH 10/11] Test all files --- test/postgresql.transaction.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/postgresql.transaction.test.js b/test/postgresql.transaction.test.js index a8fcfe7d..173a57c1 100644 --- a/test/postgresql.transaction.test.js +++ b/test/postgresql.transaction.test.js @@ -11,7 +11,7 @@ var Transaction = require('loopback-connector').Transaction; var db, Post; -describe.only('transactions', function() { +describe('transactions', function() { before(function(done) { db = getDataSource(true); Post = db.define('PostTX', { From 46a03a99f9a1a86a25dc7686c63c06680557f8e3 Mon Sep 17 00:00:00 2001 From: jannyHou Date: Wed, 12 Apr 2017 00:39:08 -0400 Subject: [PATCH 11/11] Use connection pooling --- lib/postgresql.js | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/lib/postgresql.js b/lib/postgresql.js index 0a57ef90..deed5da4 100644 --- a/lib/postgresql.js +++ b/lib/postgresql.js @@ -139,6 +139,7 @@ PostgreSQL.prototype.connect = function(callback) { var self = this; self.pg.connect(function(err, client, done) { self.client = client; + self.finish = done; process.nextTick(done); callback && callback(err, client); }); @@ -173,6 +174,10 @@ PostgreSQL.prototype.executeSQL = function(sql, params, options, callback) { } if (self.settings.debug && data) self.debug('%j', data); if (done) { + if (err) { + self.client = undefined; + } + // done(err); process.nextTick(function() { // Release the connection in next tick done(err); @@ -209,10 +214,15 @@ PostgreSQL.prototype.executeSQL = function(sql, params, options, callback) { // Do not release the connection executeWithConnection(transaction.connection, null); } else { - self.pg.connect(function(err, connection, done) { - if (err) return callback(err); - executeWithConnection(connection, done); - }); + if (self.client) { + executeWithConnection(self.client, self.finish); + } else { + self.pg.connect(function(newConErr, client, newConDone) { + self.client = client; + self.finish = newConDone; + executeWithConnection(self.client, self.finish); + }); + } } };