Merge pull request #180 from tseaver/129-rollback_using_connection_xact

Fix #129: use transaction ID from connection's own transaction.

Author: silvolu

gcloud/datastore/connection.py

@@ -165,23 +165,20 @@ def begin_transaction(self, dataset_id, serializable=False):
 
     return response.transaction
 
-  def rollback_transaction(self, dataset_id, transaction_id):
-    """Rollback an existing transaction.
+  def rollback_transaction(self, dataset_id):
+    """Rollback the connection's existing transaction.
 
     Raises a ``ValueError``
     if the connection isn't currently in a transaction.
 
     :type dataset_id: string
     :param dataset_id: The dataset to which the transaction belongs.
-
-    :type transaction_id: string
-    :param transaction_id: The ID of the transaction to roll back.
     """
     if not self.transaction() or not self.transaction().id():
       raise ValueError('No transaction to rollback.')
 
     request = datastore_pb.RollbackRequest()
-    request.transaction = transaction_id
+    request.transaction = self.transaction().id()
     # Nothing to do with this response, so just execute the method.
     self._rpc(dataset_id, 'rollback', request,
               datastore_pb.RollbackResponse)

gcloud/datastore/test_connection.py

@@ -244,31 +244,27 @@ def test_begin_transaction_explicit_serialize(self):
 
     def test_rollback_transaction_wo_existing_transaction(self):
         DATASET_ID = 'DATASET'
-        TRANSACTION_ID = 'TRANSACTION'
         conn = self._makeOne()
         self.assertRaises(ValueError,
-                          conn.rollback_transaction, DATASET_ID, TRANSACTION_ID)
+                          conn.rollback_transaction, DATASET_ID)
 
     def test_rollback_transaction_w_existing_transaction_no_id(self):
         class Xact(object):
             def id(self):
                 return None
         DATASET_ID = 'DATASET'
-        TRANSACTION_ID = 'TRANSACTION'
         conn = self._makeOne()
         conn.transaction(Xact())
         self.assertRaises(ValueError,
-                          conn.rollback_transaction, DATASET_ID, TRANSACTION_ID)
+                          conn.rollback_transaction, DATASET_ID)
 
     def test_rollback_transaction_ok(self):
         from gcloud.datastore.connection import datastore_pb
+        DATASET_ID = 'DATASET'
+        TRANSACTION = 'xact'
         class Xact(object):
             def id(self):
-                return 'xact'
-        xact = object()
-
-        DATASET_ID = 'DATASET'
-        TRANSACTION = 'TRANSACTION'
+                return TRANSACTION
         rsp_pb = datastore_pb.RollbackResponse()
         conn = self._makeOne()
         conn.transaction(Xact())
@@ -280,14 +276,13 @@ def id(self):
                         'rollback',
                        ])
         http = conn._http = Http({'status': '200'}, rsp_pb.SerializeToString())
-        self.assertEqual(conn.rollback_transaction(DATASET_ID, TRANSACTION),
-                         None)
+        self.assertEqual(conn.rollback_transaction(DATASET_ID), None)
         cw = http._called_with
         self.assertEqual(cw['uri'], URI)
         self.assertEqual(cw['method'], 'POST')
         self.assertEqual(cw['headers'],
                             {'Content-Type': 'application/x-protobuf',
-                             'Content-Length': '13',
+                             'Content-Length': '6',
                             })
         rq_class = datastore_pb.RollbackRequest
         request = rq_class()

gcloud/datastore/test_transaction.py

@@ -49,7 +49,7 @@ def test_rollback(self):
         xact.begin()
         xact.rollback()
         self.assertEqual(xact.id(), None)
-        self.assertEqual(connection._rolled_back, (_DATASET, 234))
+        self.assertEqual(connection._rolled_back, _DATASET)
         self.assertEqual(connection._xact, None)
 
     def test_commit_no_auto_ids(self):
@@ -127,7 +127,7 @@ class Foo(Exception):
         except Foo:
             pass # XXX
             #self.assertEqual(xact.id(), None)
-            #self.assertEqual(connection._rolled_back, (_DATASET, 234))
+            #self.assertEqual(connection._rolled_back, _DATASET))
             #self.assertEqual(connection._xact, None)
         # XXX should *not* have committed
         self.assertEqual(connection._committed, (_DATASET, mutation))
@@ -167,8 +167,8 @@ def transaction(self, xact=_marker):
     def begin_transaction(self, dataset_id):
         self._begun = dataset_id
         return self._xact_id
-    def rollback_transaction(self, dataset_id, xact_id):
-        self._rolled_back = (dataset_id, xact_id)
+    def rollback_transaction(self, dataset_id):
+        self._rolled_back = dataset_id
     def commit(self, dataset_id, mutation):
         self._committed = (dataset_id, mutation)
         return self._commit_result

gcloud/datastore/transaction.py

@@ -210,7 +210,7 @@ def rollback(self):
     - Sets the current connection's transaction reference to None.
     - Sets the current transaction's ID to None.
     """
-    self.connection().rollback_transaction(self.dataset().id(), self.id())
+    self.connection().rollback_transaction(self.dataset().id())
     self.connection().transaction(None)
     self._id = None