From 88cb1623621a587932047ec3f11ca38decef9109 Mon Sep 17 00:00:00 2001
From: Danny Hermes <daniel.j.hermes@gmail.com>
Date: Fri, 6 Feb 2015 13:16:56 -0800
Subject: [PATCH] Removing Transaction.succeeded and using separate enum for
 failure.

---
 gcloud/datastore/test_transaction.py | 45 ++--------------------------
 gcloud/datastore/transaction.py      | 28 +++++++----------
 2 files changed, 12 insertions(+), 61 deletions(-)

diff --git a/gcloud/datastore/test_transaction.py b/gcloud/datastore/test_transaction.py
index eef44fef19126..27a7d6d06bc8a 100644
--- a/gcloud/datastore/test_transaction.py
+++ b/gcloud/datastore/test_transaction.py
@@ -47,7 +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, None)
+        self.assertEqual(xact._status, self._getTargetClass()._INITIAL)
         self.assertTrue(xact._commit_success is False)
         self.assertTrue(isinstance(xact.mutation, Mutation))
         self.assertEqual(len(xact._auto_id_entities), 0)
@@ -66,7 +66,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, None)
+        self.assertEqual(xact._status, self._getTargetClass()._INITIAL)
         self.assertTrue(xact._commit_success is False)
 
     def test_current(self):
@@ -94,47 +94,6 @@ def test_current(self):
         self.assertTrue(xact1.current() is None)
         self.assertTrue(xact2.current() is None)
 
-    def test_succeeded_fresh_transaction(self):
-        _DATASET = 'DATASET'
-        connection = _Connection()
-        xact = self._makeOne(dataset_id=_DATASET, connection=connection)
-        self.assertEqual(xact._status, None)
-
-        success = marker = object()
-        with self.assertRaises(ValueError):
-            success = xact.succeeded
-        self.assertTrue(success is marker)
-
-    def test_succeeded_in_progress(self):
-        _DATASET = 'DATASET'
-        connection = _Connection()
-        xact = self._makeOne(dataset_id=_DATASET, connection=connection)
-        xact.begin()
-        self.assertEqual(xact._status, self._getTargetClass()._IN_PROGRESS)
-
-        success = marker = object()
-        with self.assertRaises(ValueError):
-            success = xact.succeeded
-        self.assertTrue(success is marker)
-
-    def test_succeeded_on_success(self):
-        _DATASET = 'DATASET'
-        connection = _Connection()
-        xact = self._makeOne(dataset_id=_DATASET, connection=connection)
-        xact.begin()
-        xact.commit()
-        self.assertEqual(xact._status, self._getTargetClass()._FINISHED)
-        self.assertTrue(xact.succeeded is True)
-
-    def test_succeeded_on_failure(self):
-        _DATASET = 'DATASET'
-        connection = _Connection()
-        xact = self._makeOne(dataset_id=_DATASET, connection=connection)
-        xact.begin()
-        xact.rollback()
-        self.assertEqual(xact._status, self._getTargetClass()._FINISHED)
-        self.assertTrue(xact.succeeded is False)
-
     def test_begin(self):
         _DATASET = 'DATASET'
         connection = _Connection(234)
diff --git a/gcloud/datastore/transaction.py b/gcloud/datastore/transaction.py
index eca370dbb5bce..ee25d7cff83c0 100644
--- a/gcloud/datastore/transaction.py
+++ b/gcloud/datastore/transaction.py
@@ -114,16 +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."""
 
-    _FINISHED = 2
+    _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 = None
+        self._status = self._INITIAL
         self._commit_success = False
 
     @property
@@ -148,20 +154,6 @@ def current():
         if isinstance(top, Transaction):
             return top
 
-    @property
-    def succeeded(self):
-        """Determines if transaction has succeeded or failed.
-
-        :rtype: boolean
-        :returns: Boolean indicating successful commit.
-        :raises: :class:`ValueError` if the transaction is still in progress.
-        """
-        if self._status != self._FINISHED:
-            raise ValueError('Transaction not yet finished. '
-                             'Success not known.')
-
-        return self._commit_success
-
     def begin(self):
         """Begins a transaction.
 
@@ -171,7 +163,7 @@ def begin(self):
 
         :raises: :class:`ValueError` if the transaction has already begun.
         """
-        if self._status is not None:
+        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)
@@ -187,7 +179,7 @@ def rollback(self):
         try:
             self.connection.rollback(self._dataset_id, self._id)
         finally:
-            self._status = self._FINISHED
+            self._status = self._ABORTED
             # Clear our own ID in case this gets accidentally reused.
             self._id = None