pass transaction instance in run()

stephenplusplus · stephenplusplus · commit 3f8c632a9f48 · 2016-06-23T09:19:04.000-04:00
diff --git a/lib/datastore/transaction.js b/lib/datastore/transaction.js
@@ -201,8 +201,8 @@ Transaction.prototype.commit = function(callback) {
     if (err) {
       // Rollback automatically for the user.
       self.rollback(function() {
-        // Provide the response items from the failed commit to the user. Even
-        // a failed rollback should be transparent.
+        // Provide the error & API response from the failed commit to the user.
+        // Even a failed rollback should be transparent.
         // RE: https://github.com/GoogleCloudPlatform/gcloud-node/pull/1369#discussion_r66833976
         callback(err, resp);
       });
@@ -330,7 +330,7 @@ Transaction.prototype.rollback = function(callback) {
     method: 'rollback'
   };
 
-  this.request_(protoOpts, {}, function(err, resp) {
+  this.request_(protoOpts, function(err, resp) {
     self.skipCommit = true;
 
     callback(err || null, resp);
@@ -344,10 +344,12 @@ Transaction.prototype.rollback = function(callback) {
  * @param {function} callback - The function to execute within the context of
  *     a transaction.
  * @param {?error} callback.err - An error returned while making this request.
+ * @param {module:datastore/transaction} callback.transaction - This transaction
+ *     instance.
  * @param {object} callback.apiResponse - The full API response.
  *
  * @example
- * transaction.run(function(err) {
+ * transaction.run(function(err, transaction) {
  *   // Perform Datastore transactional operations.
  *   var key = datastore.key(['Company', 123]);
  *
@@ -377,15 +379,15 @@ Transaction.prototype.run = function(callback) {
     method: 'beginTransaction'
   };
 
-  this.request_(protoOpts, {}, function(err, resp) {
+  this.request_(protoOpts, function(err, resp) {
     if (err) {
-      callback(err, resp);
+      callback(err, null, resp);
       return;
     }
 
     self.id = resp.transaction;
 
-    callback(null, resp);
+    callback(null, self, resp);
   });
 };
 
diff --git a/test/datastore/transaction.js b/test/datastore/transaction.js
@@ -119,25 +119,6 @@ describe('Transaction', function() {
     });
   });
 
-  describe('createQuery', function() {
-    it('should return query from datastore.createQuery', function() {
-      var args = [0, 1, 2, 3];
-      var createQueryReturnValue = {};
-
-      transaction.datastore.createQuery = function() {
-        assert.strictEqual(this, transaction);
-        assert.strictEqual(arguments[0], args[0]);
-        assert.strictEqual(arguments[1], args[1]);
-        assert.strictEqual(arguments[2], args[2]);
-        assert.strictEqual(arguments[3], args[3]);
-        return createQueryReturnValue;
-      };
-
-      var query = transaction.createQuery.apply(transaction, args);
-      assert.strictEqual(query, createQueryReturnValue);
-    });
-  });
-
   describe('commit', function() {
     beforeEach(function() {
       transaction.id = TRANSACTION_ID;
@@ -165,9 +146,12 @@ describe('Transaction', function() {
       var error = new Error('Error.');
       var apiResponse = {};
 
+      var rollbackError = new Error('Error.');
+      var rollbackApiResponse = {};
+
       beforeEach(function() {
         transaction.rollback = function(callback) {
-          callback();
+          callback(rollbackError, rollbackApiResponse);
         };
 
         transaction.request_ = function(protoOpts, reqOpts, callback) {
@@ -176,7 +160,7 @@ describe('Transaction', function() {
         };
       });
 
-      it('should pass the error to the callback', function(done) {
+      it('should pass the commit error to the callback', function(done) {
         transaction.commit(function(err, resp) {
           assert.strictEqual(err, error);
           assert.strictEqual(resp, apiResponse);
@@ -314,6 +298,25 @@ describe('Transaction', function() {
     });
   });
 
+  describe('createQuery', function() {
+    it('should return query from datastore.createQuery', function() {
+      var args = [0, 1, 2, 3];
+      var createQueryReturnValue = {};
+
+      transaction.datastore.createQuery = function() {
+        assert.strictEqual(this, transaction);
+        assert.strictEqual(arguments[0], args[0]);
+        assert.strictEqual(arguments[1], args[1]);
+        assert.strictEqual(arguments[2], args[2]);
+        assert.strictEqual(arguments[3], args[3]);
+        return createQueryReturnValue;
+      };
+
+      var query = transaction.createQuery.apply(transaction, args);
+      assert.strictEqual(query, createQueryReturnValue);
+    });
+  });
+
   describe('delete', function() {
     it('should push entities into a queue', function() {
       var keys = [
@@ -397,51 +400,67 @@ describe('Transaction', function() {
   });
 
   describe('run', function() {
-    it('should begin', function(done) {
-      transaction.request_ = function(protoOpts, reqOpts, callback) {
-        callback = callback || reqOpts;
-        assert.strictEqual(protoOpts.service, 'Datastore');
-        assert.equal(protoOpts.method, 'beginTransaction');
+    it('should make the correct API request', function(done) {
+      transaction.request_ = function(protoOpts) {
+        assert.deepEqual(protoOpts, {
+          service: 'Datastore',
+          method: 'beginTransaction'
+        });
+
         done();
       };
 
-      transaction.run();
+      transaction.run(assert.ifError);
     });
 
-    it('should set transaction id', function(done) {
-      transaction.request_ = function(protoOpts, reqOpts, callback) {
-        callback = callback || reqOpts;
-        callback(null, { transaction: TRANSACTION_ID });
-      };
-      transaction.run(function(err) {
-        assert.ifError(err);
-        assert.equal(transaction.id, TRANSACTION_ID);
-        done();
+    describe('error', function() {
+      var error = new Error('Error.');
+      var apiResponse = {};
+
+      beforeEach(function() {
+        transaction.request_ = function(protoOpts, callback) {
+          callback(error, apiResponse);
+        };
       });
-    });
 
-    it('should pass error to callback', function(done) {
-      var error = new Error('Error.');
-      transaction.request_ = function(protoOpts, reqOpts, callback) {
-        callback = callback || reqOpts;
-        callback(error);
-      };
-      transaction.run(function(err) {
-        assert.deepEqual(err, error);
-        done();
+      it('should pass error & API response to callback', function(done) {
+        transaction.run(function(err, transaction, apiResponse_) {
+          assert.strictEqual(err, error);
+          assert.strictEqual(transaction, null);
+          assert.strictEqual(apiResponse_, apiResponse);
+          done();
+        });
       });
     });
 
-    it('should pass apiResponse to callback', function(done) {
-      var resp = { success: true };
-      transaction.request_ = function(protoOpts, reqOpts, callback) {
-        callback = callback || reqOpts;
-        callback(null, resp);
+    describe('success', function() {
+      var apiResponse = {
+        transaction: TRANSACTION_ID
       };
-      transaction.run(function(err, apiResponse) {
-        assert.ifError(err);
-        assert.deepEqual(resp, apiResponse);
-        done();
+
+      beforeEach(function() {
+        transaction.request_ = function(protoOpts, callback) {
+          callback(null, apiResponse);
+        };
+      });
+
+      it('should set transaction id', function(done) {
+        delete transaction.id;
+
+        transaction.run(function(err) {
+          assert.ifError(err);
+          assert.equal(transaction.id, TRANSACTION_ID);
+          done();
+        });
+      });
+
+      it('should exec callback with Transaction & apiResponse', function(done) {
+        transaction.run(function(err, transaction_, apiResponse_) {
+          assert.ifError(err);
+          assert.strictEqual(transaction_, transaction);
+          assert.deepEqual(apiResponse_, apiResponse);
+          done();
+        });
       });
     });
   });
