From ca0d96cb94f7ef4e176edf221f9ee154dcf8c850 Mon Sep 17 00:00:00 2001 From: Dave Gramlich Date: Mon, 26 Jun 2017 13:59:41 -0400 Subject: [PATCH] spanner: make getTransaction public (#2344) --- packages/spanner/src/database.js | 133 ++++++++++++++------------ packages/spanner/src/transaction.js | 2 +- packages/spanner/test/database.js | 27 +++--- packages/spanner/test/session-pool.js | 2 +- packages/spanner/test/transaction.js | 13 +++ 5 files changed, 103 insertions(+), 74 deletions(-) diff --git a/packages/spanner/src/database.js b/packages/spanner/src/database.js index c166724e394..b79aa5c325c 100644 --- a/packages/spanner/src/database.js +++ b/packages/spanner/src/database.js @@ -399,6 +399,77 @@ Database.prototype.getSchema = function(callback) { }); }; +/** + * Get a read/write ready Transaction object. + * + * @param {object=} options - [Transaction options](https://cloud.google.com/spanner/docs/timestamp-bounds). + * @param {number} options.timeout - Specify a timeout for the transaction. The + * transaction will be ran in its entirety, however if an abort error is + * returned the transaction will be retried if the timeout has not been met. + * Default: `60000` (milliseconds) + * @param {boolean} options.readOnly - Specifies if the transaction is + * read-only. Default: `false`. + * @param {number} options.exactStaleness - Executes all reads at the timestamp + * that is `exactStaleness` old. + * @param {date} options.readTimestamp - Execute all reads at the given + * timestamp. + * @param {boolean} options.returnTimestamp - If `true`, returns the read + * timestamp. + * @param {boolean} options.strong - Read at the timestamp where all previously + * committed transactions are visible. + * @param {function} callback - The callback function. + * @param {?error} callback.err - An error returned while getting the + * transaction object. + * @param {module:spanner/transaction} transaction - The transaction object. + * + * @example + * database.getTransaction(function(err, transaction) {}); + * + * //- + * // If the callback is omitted, we'll return a Promise. + * //- + * database.getTransaction().then(function(data) { + * var transaction = data[0]; + * }); + */ +Database.prototype.getTransaction = function(options, callback) { + var self = this; + + if (is.fn(options)) { + callback = options; + options = null; + } + + if (!options || !options.readOnly) { + this.pool_.getWriteSession(function(err, session, transaction) { + callback(err, transaction); + }); + return; + } + + this.pool_.getSession(function(err, session) { + if (err) { + callback(err); + return; + } + + options = extend({}, options); + delete options.readOnly; + + var transaction = self.pool_.createTransaction_(session, options); + + transaction.begin(function(err) { + if (err) { + transaction.end(); + callback(err); + return; + } + + callback(null, transaction); + }); + }); +}; + /** * Execute a SQL statement on this database. * @@ -811,7 +882,7 @@ Database.prototype.runTransaction = function(options, runFn) { options = extend({}, options); - this.getTransaction_(options, function(err, transaction) { + this.getTransaction(options, function(err, transaction) { if (err) { runFn(err); return; @@ -1004,66 +1075,6 @@ Database.prototype.getSession_ = function(callback) { this.pool_.getSession(callback); }; -/** - * Get a read/write ready Transaction object. - * - * @private - * - * @param {object=} options - Transaction options. - * @param {boolean} options.readOnly - Specifies if the transaction is read - * only. - * @param {function} callback - The callback function. - * @param {?error} callback.err - An error returned while getting the - * transaction object. - * @param {module:spanner/transaction} transaction - The transaction object. - * - * @example - * database.getTransaction(function(err, transaction) {}); - * - * //- - * // If the callback is omitted, we'll return a Promise. - * //- - * database.getTransaction().then(function(data) { - * var transaction = data[0]; - * }); - */ -Database.prototype.getTransaction_ = function(options, callback) { - var self = this; - - if (is.fn(options)) { - callback = options; - options = null; - } - - if (!options || !options.readOnly) { - this.pool_.getWriteSession(function(err, session, transaction) { - callback(err, transaction); - }); - return; - } - - this.pool_.getSession(function(err, session) { - if (err) { - callback(err); - return; - } - - options = extend({}, options); - delete options.readOnly; - - var transaction = self.pool_.createTransaction_(session, options); - - transaction.begin(function(err) { - if (err) { - callback(err); - return; - } - - callback(null, transaction); - }); - }); -}; - /** * Create a Session object. * diff --git a/packages/spanner/src/transaction.js b/packages/spanner/src/transaction.js index 96a6b4944fb..b82fd1eeeb3 100644 --- a/packages/spanner/src/transaction.js +++ b/packages/spanner/src/transaction.js @@ -325,7 +325,7 @@ Transaction.prototype.request = function(config, callback) { delete reqOpts.gaxOptions; config.method(reqOpts, gaxOptions, function(err, resp) { - if (!err || err.code !== ABORTED) { + if (!self.runFn_ || !err || err.code !== ABORTED) { callback(err, resp); return; } diff --git a/packages/spanner/test/database.js b/packages/spanner/test/database.js index 81294ff1c27..546fd1c0942 100644 --- a/packages/spanner/test/database.js +++ b/packages/spanner/test/database.js @@ -798,7 +798,7 @@ describe('Database', function() { describe('runTransaction', function() { it('should get a Transaction object', function(done) { - database.getTransaction_ = function() { + database.getTransaction = function() { done(); }; @@ -808,7 +808,7 @@ describe('Database', function() { it('should execute callback with error', function(done) { var error = new Error('Error.'); - database.getTransaction_ = function(options, callback) { + database.getTransaction = function(options, callback) { callback(error); }; @@ -831,7 +831,7 @@ describe('Database', function() { return fakeDate; }; - database.getTransaction_ = function(options, callback) { + database.getTransaction = function(options, callback) { assert.deepEqual(options, OPTIONS); callback(null, TRANSACTION); }; @@ -855,7 +855,7 @@ describe('Database', function() { timeout: 1000 }; - database.getTransaction_ = function(options, callback) { + database.getTransaction = function(options, callback) { callback(null, TRANSACTION); }; @@ -1045,7 +1045,7 @@ describe('Database', function() { }); }); - describe('getTransaction_', function() { + describe('getTransaction', function() { describe('write mode', function() { it('should get a session from the pool', function(done) { var error = new Error('Error.'); @@ -1058,7 +1058,7 @@ describe('Database', function() { } }; - database.getTransaction_(function(err, transaction_) { + database.getTransaction(function(err, transaction_) { assert.strictEqual(err, error); assert.strictEqual(transaction_, transaction); done(); @@ -1079,7 +1079,7 @@ describe('Database', function() { } }; - database.getTransaction_(OPTIONS, assert.ifError); + database.getTransaction(OPTIONS, assert.ifError); }); it('should return an error if could not get session', function(done) { @@ -1091,7 +1091,7 @@ describe('Database', function() { } }; - database.getTransaction_(OPTIONS, function(err) { + database.getTransaction(OPTIONS, function(err) { assert.strictEqual(err, error); done(); }); @@ -1121,7 +1121,7 @@ describe('Database', function() { } }; - database.getTransaction_(OPTIONS, assert.ifError); + database.getTransaction(OPTIONS, assert.ifError); }); it('should begin a transaction', function(done) { @@ -1141,7 +1141,7 @@ describe('Database', function() { } }; - database.getTransaction_(OPTIONS, function(err, transaction) { + database.getTransaction(OPTIONS, function(err, transaction) { assert.ifError(err); assert.strictEqual(transaction, TRANSACTION); done(); @@ -1150,11 +1150,15 @@ describe('Database', function() { it('should return an error if transaction cannot begin', function(done) { var error = new Error('err'); + var endCalled = false; var SESSION = {}; var TRANSACTION = { begin: function(callback) { callback(error); + }, + end: function() { + endCalled = true; } }; @@ -1167,8 +1171,9 @@ describe('Database', function() { } }; - database.getTransaction_(OPTIONS, function(err) { + database.getTransaction(OPTIONS, function(err) { assert.strictEqual(err, error); + assert.strictEqual(endCalled, true); done(); }); }); diff --git a/packages/spanner/test/session-pool.js b/packages/spanner/test/session-pool.js index f2304da6e30..98b5ade8832 100644 --- a/packages/spanner/test/session-pool.js +++ b/packages/spanner/test/session-pool.js @@ -940,7 +940,7 @@ describe('SessionPool', function() { sessionPool.requestStream(CONFIG) .on('data', function(data) { - assert.strictEqual(data, responseData); + assert.deepEqual(data, responseData); done(); }); diff --git a/packages/spanner/test/transaction.js b/packages/spanner/test/transaction.js index b57644d3671..29aa83dca50 100644 --- a/packages/spanner/test/transaction.js +++ b/packages/spanner/test/transaction.js @@ -485,6 +485,10 @@ describe('Transaction', function() { return true; }; + transaction.runFn_ = function() { + done(new Error('Should not have been called.')); + }; + transaction.request(config, function() { done(new Error('Should not have been called.')); }); @@ -519,6 +523,15 @@ describe('Transaction', function() { done(new Error('Should not have been called.')); }); }); + + it('should return the aborted error if no runFn', function(done) { + transaction.runFn_ = null; + + transaction.request(config, function(err) { + assert.strictEqual(err, abortedError); + done(); + }); + }); }); });