From c09e84644e9e7b0bfba2725d8240520e9a9b4f6f Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Tue, 27 Jun 2017 07:59:28 +0930 Subject: [PATCH 1/9] Add test for #258 --- test/postgresql.transaction.test.js | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/postgresql.transaction.test.js b/test/postgresql.transaction.test.js index eb0d3f63..d0edc475 100644 --- a/test/postgresql.transaction.test.js +++ b/test/postgresql.transaction.test.js @@ -109,6 +109,33 @@ describe('transactions', function() { it('should see the committed insert', expectToFindPosts(post, 1)); }); + describe('update all', function() { + var p1Content = {title: 'p1', content: 'post-a'}; + var p2Content = {title: 'p2', content: 'post-a'}; + + before(function(done) { + Post.create(p1Content, function(err, p1) { + Post.create(p1Content, function(err, p2) { + done(); + }); + }); + }); + + it('should work with update all', function(done) { + Transaction.begin(db.connector, Transaction.READ_COMMITTED, function(err, tx) { + Post.updateAll({content: 'post-a'}, {content: 'post-b'}, {transaction: tx}, + function(err, changes) { + tx.commit(function(err) { + if (err) { + done(err); + } + done(); + }); + }); + }); + }); + }); + describe('rollback', function() { var post = {title: 't2', content: 'c2'}; before(createPostInTx(post)); From 1e8a2c52c935374557018e947cc34936684d3660 Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Tue, 27 Jun 2017 08:06:47 +0930 Subject: [PATCH 2/9] Add missing return --- 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 d0edc475..a5e93943 100644 --- a/test/postgresql.transaction.test.js +++ b/test/postgresql.transaction.test.js @@ -127,7 +127,7 @@ describe('transactions', function() { function(err, changes) { tx.commit(function(err) { if (err) { - done(err); + return done(err); } done(); }); From 0557e463e684c5ea6f6bd2540cfe22e1c9d1afc9 Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Tue, 27 Jun 2017 08:27:02 +0930 Subject: [PATCH 3/9] Fix operations directly on model #258 --- lib/transaction.js | 8 ++++++++ test/postgresql.transaction.test.js | 6 +++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/lib/transaction.js b/lib/transaction.js index 03c79e2b..66cf9924 100644 --- a/lib/transaction.js +++ b/lib/transaction.js @@ -44,6 +44,10 @@ function mixinTransaction(PostgreSQL) { PostgreSQL.prototype.commit = function(connection, cb) { debug('Commit a transaction'); var self = this; + if (connection instanceof Transaction) { + // If called directly on the model rather than through Transaction + connection = connection.connection; + } connection.query('COMMIT', function(err) { self.releaseConnection(connection, err); cb(err); @@ -58,6 +62,10 @@ function mixinTransaction(PostgreSQL) { PostgreSQL.prototype.rollback = function(connection, cb) { debug('Rollback a transaction'); var self = this; + if (connection instanceof Transaction) { + // If called directly on the model rather than through Transaction + connection = connection.connection; + } connection.query('ROLLBACK', function(err) { //if there was a problem rolling back the query //something is seriously messed up. Return the error diff --git a/test/postgresql.transaction.test.js b/test/postgresql.transaction.test.js index a5e93943..6ebdf057 100644 --- a/test/postgresql.transaction.test.js +++ b/test/postgresql.transaction.test.js @@ -109,7 +109,7 @@ describe('transactions', function() { it('should see the committed insert', expectToFindPosts(post, 1)); }); - describe('update all', function() { + describe('on the model', function() { var p1Content = {title: 'p1', content: 'post-a'}; var p2Content = {title: 'p2', content: 'post-a'}; @@ -121,8 +121,8 @@ describe('transactions', function() { }); }); - it('should work with update all', function(done) { - Transaction.begin(db.connector, Transaction.READ_COMMITTED, function(err, tx) { + it('should work when operating directly on the model', function(done) { + Post.beginTransaction(Transaction.READ_COMMITTED, function(err, tx) { Post.updateAll({content: 'post-a'}, {content: 'post-b'}, {transaction: tx}, function(err, changes) { tx.commit(function(err) { From b6fb9db7bdafe2dfbe040a65cc9b3f665d6f1c6d Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Wed, 28 Jun 2017 08:32:22 +0930 Subject: [PATCH 4/9] Avoid connector for transaction tracking Resolves #258 --- lib/postgresql.js | 3 ++- lib/transaction.js | 17 +++++------------ 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/lib/postgresql.js b/lib/postgresql.js index 5cfddcb7..95778bcd 100644 --- a/lib/postgresql.js +++ b/lib/postgresql.js @@ -37,6 +37,7 @@ exports.initialize = function initializeDataSource(dataSource, callback) { dataSource.connector = new PostgreSQL(postgresql, dbSettings); dataSource.connector.dataSource = dataSource; + dataSource.connector.activeTransactions = {}; if (callback) { if (dbSettings.lazyConnect) { @@ -188,7 +189,7 @@ PostgreSQL.prototype.executeSQL = function(sql, params, options, callback) { callback(new Error(g.f('Connection does not exist'))); }); } - if (transaction.txId !== transaction.connection.txId) { + if (!transaction.connection.txId || !self.activeTransactions[transaction.connection.txId]) { return process.nextTick(function() { callback(new Error(g.f('Transaction is not active'))); }); diff --git a/lib/transaction.js b/lib/transaction.js index 66cf9924..1e05969b 100644 --- a/lib/transaction.js +++ b/lib/transaction.js @@ -28,10 +28,9 @@ function mixinTransaction(PostgreSQL) { connection.query('BEGIN TRANSACTION ISOLATION LEVEL ' + isolationLevel, function(err) { if (err) return cb(err); - var tx = new Transaction(connector, connection); - tx.txId = uuid.v1(); - connection.txId = tx.txId; - cb(null, tx); + connection.txId = uuid.v1(); + connector.activeTransactions[connection.txId] = true; + cb(null, connection); }); }); }; @@ -44,10 +43,6 @@ function mixinTransaction(PostgreSQL) { PostgreSQL.prototype.commit = function(connection, cb) { debug('Commit a transaction'); var self = this; - if (connection instanceof Transaction) { - // If called directly on the model rather than through Transaction - connection = connection.connection; - } connection.query('COMMIT', function(err) { self.releaseConnection(connection, err); cb(err); @@ -62,10 +57,6 @@ function mixinTransaction(PostgreSQL) { PostgreSQL.prototype.rollback = function(connection, cb) { debug('Rollback a transaction'); var self = this; - if (connection instanceof Transaction) { - // If called directly on the model rather than through Transaction - connection = connection.connection; - } connection.query('ROLLBACK', function(err) { //if there was a problem rolling back the query //something is seriously messed up. Return the error @@ -78,7 +69,9 @@ function mixinTransaction(PostgreSQL) { }; PostgreSQL.prototype.releaseConnection = function(connection, err) { + var self = this; if (typeof connection.autorelease === 'function') { + delete self.activeTransactions[connection.txId]; connection.txId = null; connection.autorelease(err); connection.autorelease = null; From cfd40defb67b47d28d77f03c6f49d197f9787494 Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Wed, 28 Jun 2017 08:44:27 +0930 Subject: [PATCH 5/9] Remove require of Transaction --- lib/transaction.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/transaction.js b/lib/transaction.js index 1e05969b..33c74742 100644 --- a/lib/transaction.js +++ b/lib/transaction.js @@ -6,7 +6,6 @@ 'use strict'; var debug = require('debug')('loopback:connector:postgresql:transaction'); var uuid = require('uuid'); -var Transaction = require('loopback-connector').Transaction; module.exports = mixinTransaction; From bee6bb2746573fe8a9d6132a051b266e1407cac6 Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Mon, 17 Jul 2017 08:03:05 +0930 Subject: [PATCH 6/9] Update transaction test --- 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 6ebdf057..bffc3cd8 100644 --- a/test/postgresql.transaction.test.js +++ b/test/postgresql.transaction.test.js @@ -115,7 +115,7 @@ describe('transactions', function() { before(function(done) { Post.create(p1Content, function(err, p1) { - Post.create(p1Content, function(err, p2) { + Post.create(p2Content, function(err, p2) { done(); }); }); From d001396e5b6c7892bdada4ab66650f3f1c50c6db Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Mon, 17 Jul 2017 08:06:28 +0930 Subject: [PATCH 7/9] Revert transaction tracking This reverts commit b1a76b3b54eeb48fb816d67f8f7b99e3de022b65. --- lib/postgresql.js | 3 +-- lib/transaction.js | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/lib/postgresql.js b/lib/postgresql.js index 95778bcd..5cfddcb7 100644 --- a/lib/postgresql.js +++ b/lib/postgresql.js @@ -37,7 +37,6 @@ exports.initialize = function initializeDataSource(dataSource, callback) { dataSource.connector = new PostgreSQL(postgresql, dbSettings); dataSource.connector.dataSource = dataSource; - dataSource.connector.activeTransactions = {}; if (callback) { if (dbSettings.lazyConnect) { @@ -189,7 +188,7 @@ PostgreSQL.prototype.executeSQL = function(sql, params, options, callback) { callback(new Error(g.f('Connection does not exist'))); }); } - if (!transaction.connection.txId || !self.activeTransactions[transaction.connection.txId]) { + if (transaction.txId !== transaction.connection.txId) { return process.nextTick(function() { callback(new Error(g.f('Transaction is not active'))); }); diff --git a/lib/transaction.js b/lib/transaction.js index 33c74742..5bc914c4 100644 --- a/lib/transaction.js +++ b/lib/transaction.js @@ -27,9 +27,10 @@ function mixinTransaction(PostgreSQL) { connection.query('BEGIN TRANSACTION ISOLATION LEVEL ' + isolationLevel, function(err) { if (err) return cb(err); - connection.txId = uuid.v1(); - connector.activeTransactions[connection.txId] = true; - cb(null, connection); + var tx = new Transaction(connector, connection); + tx.txId = uuid.v1(); + connection.txId = tx.txId; + cb(null, tx); }); }); }; @@ -42,6 +43,10 @@ function mixinTransaction(PostgreSQL) { PostgreSQL.prototype.commit = function(connection, cb) { debug('Commit a transaction'); var self = this; + if (connection instanceof Transaction) { + // If called directly on the model rather than through Transaction + connection = connection.connection; + } connection.query('COMMIT', function(err) { self.releaseConnection(connection, err); cb(err); @@ -56,6 +61,10 @@ function mixinTransaction(PostgreSQL) { PostgreSQL.prototype.rollback = function(connection, cb) { debug('Rollback a transaction'); var self = this; + if (connection instanceof Transaction) { + // If called directly on the model rather than through Transaction + connection = connection.connection; + } connection.query('ROLLBACK', function(err) { //if there was a problem rolling back the query //something is seriously messed up. Return the error @@ -68,9 +77,7 @@ function mixinTransaction(PostgreSQL) { }; PostgreSQL.prototype.releaseConnection = function(connection, err) { - var self = this; if (typeof connection.autorelease === 'function') { - delete self.activeTransactions[connection.txId]; connection.txId = null; connection.autorelease(err); connection.autorelease = null; From f81e2068ee73d3f7f235c0031244196923bd6d35 Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Tue, 18 Jul 2017 08:15:03 +0930 Subject: [PATCH 8/9] Revert change to commit and rollback --- lib/transaction.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/transaction.js b/lib/transaction.js index 5bc914c4..e3f34f8c 100644 --- a/lib/transaction.js +++ b/lib/transaction.js @@ -43,10 +43,6 @@ function mixinTransaction(PostgreSQL) { PostgreSQL.prototype.commit = function(connection, cb) { debug('Commit a transaction'); var self = this; - if (connection instanceof Transaction) { - // If called directly on the model rather than through Transaction - connection = connection.connection; - } connection.query('COMMIT', function(err) { self.releaseConnection(connection, err); cb(err); @@ -61,10 +57,6 @@ function mixinTransaction(PostgreSQL) { PostgreSQL.prototype.rollback = function(connection, cb) { debug('Rollback a transaction'); var self = this; - if (connection instanceof Transaction) { - // If called directly on the model rather than through Transaction - connection = connection.connection; - } connection.query('ROLLBACK', function(err) { //if there was a problem rolling back the query //something is seriously messed up. Return the error From 8583c8cffb35905007d96a04a1b8771a839a2907 Mon Sep 17 00:00:00 2001 From: Zak Barbuto Date: Wed, 19 Jul 2017 08:24:45 +0930 Subject: [PATCH 9/9] Add missing require --- lib/transaction.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/transaction.js b/lib/transaction.js index e3f34f8c..03c79e2b 100644 --- a/lib/transaction.js +++ b/lib/transaction.js @@ -6,6 +6,7 @@ 'use strict'; var debug = require('debug')('loopback:connector:postgresql:transaction'); var uuid = require('uuid'); +var Transaction = require('loopback-connector').Transaction; module.exports = mixinTransaction;