Merge pull request #595 from dhermes/fix-496

Tracking status of Transaction as well as success / failure.

gcloud/datastore/test_transaction.py

@@ -47,6 +47,7 @@ def test_ctor(self):
         self.assertEqual(xact.dataset_id, _DATASET)
         self.assertEqual(xact.connection, connection)
         self.assertEqual(xact.id, None)
+        self.assertEqual(xact._status, self._getTargetClass()._INITIAL)
         self.assertTrue(isinstance(xact.mutation, Mutation))
         self.assertEqual(len(xact._auto_id_entities), 0)
 
@@ -64,6 +65,7 @@ def test_ctor_with_env(self):
         self.assertEqual(xact.id, None)
         self.assertEqual(xact.dataset_id, DATASET_ID)
         self.assertEqual(xact.connection, CONNECTION)
+        self.assertEqual(xact._status, self._getTargetClass()._INITIAL)
 
     def test_current(self):
         from gcloud.datastore.test_api import _NoCommitBatch
@@ -98,6 +100,19 @@ def test_begin(self):
         self.assertEqual(xact.id, 234)
         self.assertEqual(connection._begun, _DATASET)
 
+    def test_begin_tombstoned(self):
+        _DATASET = 'DATASET'
+        connection = _Connection(234)
+        xact = self._makeOne(dataset_id=_DATASET, connection=connection)
+        xact.begin()
+        self.assertEqual(xact.id, 234)
+        self.assertEqual(connection._begun, _DATASET)
+
+        xact.rollback()
+        self.assertEqual(xact.id, None)
+
+        self.assertRaises(ValueError, xact.begin)
+
     def test_rollback(self):
         _DATASET = 'DATASET'
         connection = _Connection(234)

gcloud/datastore/transaction.py

@@ -32,9 +32,8 @@ class Transaction(Batch):
 
       >>> datastore.set_defaults()
 
-      >>> with Transaction() as xact:
-      ...     datastore.put(entity1)
-      ...     datastore.put(entity2)
+      >>> with Transaction():
+      ...     datastore.put([entity1, entity2])
 
     Because it derives from :class:`Batch`, :class`Transaction` also provides
     :meth:`put` and :meth:`delete` methods::
@@ -46,7 +45,7 @@ class Transaction(Batch):
     By default, the transaction is rolled back if the transaction block
     exits with an error::
 
-      >>> with Transaction() as txn:
+      >>> with Transaction():
       ...     do_some_work()
       ...     raise SomeException()  # rolls back
 
@@ -71,16 +70,34 @@ class Transaction(Batch):
          ...     entity = Entity(key=Key('Thing'))
          ...     datastore.put([entity])
          ...     assert entity.key.is_partial  # There is no ID on this key.
+         ...
          >>> assert not entity.key.is_partial  # There *is* an ID.
 
+       After completion, you can determine if a commit succeeded or failed.
+       For example, trying to delete a key that doesn't exist::
+
+         >>> with Transaction() as xact:
+         ...     xact.delete(key)
+         ...
+         >>> xact.succeeded
+         False
+
+       or successfully storing two entities:
+
+         >>> with Transaction() as xact:
+         ...     datastore.put([entity1, entity2])
+         ...
+         >>> xact.succeeded
+         True
+
     If you don't want to use the context manager you can initialize a
     transaction manually::
 
       >>> transaction = Transaction()
       >>> transaction.begin()
 
       >>> entity = Entity(key=Key('Thing'))
-      >>> transaction.put([entity])
+      >>> transaction.put(entity)
 
       >>> if error:
       ...     transaction.rollback()
@@ -97,9 +114,22 @@ class Transaction(Batch):
              are not set.
     """
 
+    _INITIAL = 0
+    """Enum value for _INITIAL status of transaction."""
+
+    _IN_PROGRESS = 1
+    """Enum value for _IN_PROGRESS status of transaction."""
+
+    _ABORTED = 2
+    """Enum value for _ABORTED status of transaction."""
+
+    _FINISHED = 3
+    """Enum value for _FINISHED status of transaction."""
+
     def __init__(self, dataset_id=None, connection=None):
         super(Transaction, self).__init__(dataset_id, connection)
         self._id = None
+        self._status = self._INITIAL
 
     @property
     def id(self):
@@ -129,7 +159,12 @@ def begin(self):
         This method is called automatically when entering a with
         statement, however it can be called explicitly if you don't want
         to use a context manager.
+
+        :raises: :class:`ValueError` if the transaction has already begun.
         """
+        if self._status != self._INITIAL:
+            raise ValueError('Transaction already started previously.')
+        self._status = self._IN_PROGRESS
         self._id = self.connection.begin_transaction(self._dataset_id)
 
     def rollback(self):
@@ -140,8 +175,12 @@ def rollback(self):
         - Sets the current connection's transaction reference to None.
         - Sets the current transaction's ID to None.
         """
-        self.connection.rollback(self._dataset_id, self._id)
-        self._id = None
+        try:
+            self.connection.rollback(self._dataset_id, self._id)
+        finally:
+            self._status = self._ABORTED
+            # Clear our own ID in case this gets accidentally reused.
+            self._id = None
 
     def commit(self):
         """Commits the transaction.
@@ -154,7 +193,9 @@ def commit(self):
 
         - Sets the current transaction's ID to None.
         """
-        super(Transaction, self).commit()
-
-        # Clear our own ID in case this gets accidentally reused.
-        self._id = None
+        try:
+            super(Transaction, self).commit()
+        finally:
+            self._status = self._FINISHED
+            # Clear our own ID in case this gets accidentally reused.
+            self._id = None