spanner: properly capture readOnly option (#2561)

callmehiphop · stephenplusplus · commit 51df1020f353 · 2017-08-29T11:28:08.000-04:00
diff --git a/packages/spanner/src/database.js b/packages/spanner/src/database.js
@@ -460,9 +460,6 @@ Database.prototype.getTransaction = function(options, callback) {
       return;
     }
 
-    options = extend({}, options);
-    delete options.readOnly;
-
     var transaction = self.pool_.createTransaction_(session, options);
 
     transaction.begin(function(err) {
diff --git a/packages/spanner/src/transaction-request.js b/packages/spanner/src/transaction-request.js
@@ -51,7 +51,14 @@ var PartialResultStream = require('./partial-result-stream.js');
  * @param {object=} options - Timestamp options.
  */
 function TransactionRequest(options) {
+  this.readOnly = false;
+
   if (options && !is.empty(options)) {
+    options = extend({}, options);
+
+    this.readOnly = !!options.readOnly;
+    delete options.readOnly;
+
     this.options = TransactionRequest.formatTimestampOptions_(options);
   }
 }
diff --git a/packages/spanner/src/transaction.js b/packages/spanner/src/transaction.js
@@ -164,7 +164,7 @@ Transaction.prototype.begin = function(callback) {
   var self = this;
   var options;
 
-  if (!this.options) {
+  if (!this.readOnly) {
     options = {
       readWrite: {}
     };
diff --git a/packages/spanner/test/database.js b/packages/spanner/test/database.js
@@ -1126,12 +1126,7 @@ describe('Database', function() {
           },
           createTransaction_: function(session, options) {
             assert.strictEqual(session, SESSION);
-
-            var expectedOptions = extend({}, OPTIONS);
-            delete expectedOptions.readOnly;
-
-            assert.notStrictEqual(options, OPTIONS);
-            assert.deepEqual(options, expectedOptions);
+            assert.strictEqual(options, OPTIONS);
 
             setImmediate(done);
             return TRANSACTION;
diff --git a/packages/spanner/test/transaction-request.js b/packages/spanner/test/transaction-request.js
@@ -80,6 +80,24 @@ describe('TransactionRequest', function() {
   });
 
   describe('instantiation', function() {
+    var formatTimestamp;
+
+    before(function() {
+      formatTimestamp = TransactionRequest.formatTimestampOptions_;
+    });
+
+    beforeEach(function() {
+      TransactionRequest.formatTimestampOptions_ = function() {};
+    });
+
+    after(function() {
+      TransactionRequest.formatTimestampOptions_ = formatTimestamp;
+    });
+
+    it('should default readOnly to false', function() {
+      assert.strictEqual(transactionRequest.readOnly, false);
+    });
+
     it('should localize the transaction options', function() {
       var UNFORMATTED_OPTIONS = {
         b: 'b'
@@ -89,10 +107,9 @@ describe('TransactionRequest', function() {
         a: 'a'
       };
 
-      var formatTimestamp = TransactionRequest.formatTimestampOptions_;
-
       TransactionRequest.formatTimestampOptions_ = function(options) {
-        assert.strictEqual(options, UNFORMATTED_OPTIONS);
+        assert.deepEqual(options, UNFORMATTED_OPTIONS);
+        assert.notStrictEqual(options, UNFORMATTED_OPTIONS);
         return FORMATTED_OPTIONS;
       };
 
@@ -115,6 +132,18 @@ describe('TransactionRequest', function() {
       TransactionRequest.formatTimestampOptions_ = formatTimestamp;
     });
 
+    it('should capture the readOnly option', function() {
+      TransactionRequest.formatTimestampOptions_ = function(options) {
+        assert.strictEqual(options.readOnly, undefined);
+      };
+
+      var transaction = new TransactionRequest({
+        readOnly: true
+      });
+
+      assert.strictEqual(transaction.readOnly, true);
+    });
+
     it('should promisify all the things', function() {
       assert(promisified);
     });
diff --git a/packages/spanner/test/transaction.js b/packages/spanner/test/transaction.js
@@ -180,6 +180,7 @@ describe('Transaction', function() {
 
   describe('begin', function() {
     var OPTIONS = {
+      readOnly: true,
       boundOptions: true,
       returnReadTimestamp: true
     };
@@ -193,6 +194,7 @@ describe('Transaction', function() {
     it('should make the correct request', function(done) {
       var transaction = new Transaction(SESSION, OPTIONS);
 
+      transaction.readOnly = true;
       transaction.request = function(config) {
         assert.deepEqual(config.reqOpts, EXPECTED_REQ_OPTS);
         assert.strictEqual(config.method(), util.noop);
@@ -203,6 +205,7 @@ describe('Transaction', function() {
     });
 
     it('should not require options', function(done) {
+      transaction.readOnly = false;
       transaction.request = function(config) {
         assert.deepEqual(config.reqOpts, {
           options: {

Original file line number	Diff line number	Diff line change
`@@ -460,9 +460,6 @@ Database.prototype.getTransaction = function(options, callback) {`
`460`	`460`	`return;`
`461`	`461`	`}`
`462`	`462`
`463`		`- options = extend({}, options);`
`464`		`- delete options.readOnly;`
`465`		`-`
`466`	`463`	`var transaction = self.pool_.createTransaction_(session, options);`
`467`	`464`
`468`	`465`	`transaction.begin(function(err) {`