Fix unlean stopping of Transaction

Avoid unlean stop of transaction which occurs when an exception occurs in the start or stop method.
issue2256
review2256

trytond/tests/test_transaction.py

@@ -0,0 +1,63 @@
+#!/usr/bin/env python
+#This file is part of Tryton.  The COPYRIGHT file at the top level of
+#this repository contains the full copyright notices and license terms.
+import unittest
+from trytond.tests.test_tryton import DB_NAME, USER, CONTEXT 
+from trytond.transaction import Transaction
+
+
+def empty_transaction(*args):
+    '''
+    Just starts a transaction in the context manager and returns `True`
+    and stops transaction for the given arguments.
+
+    All positional arguments are passed to `start` method of transaction
+    '''
+    with Transaction().start(*args):
+        return True
+
+def manipulate_cursor(*args):
+    '''
+    Just start a transaction in the context manager and close the cursor
+    during the transaction so that the cursor.close in the stop fails
+    '''
+    with Transaction().start(*args) as transaction:
+        transaction.cursor.close()
+        transaction.cursor = None
+        return True
+
+
+class TransactionTestCase(unittest.TestCase):
+    '''
+    Test the Transaction Context manager
+    '''
+
+    def test0010nonexistdb(self):
+        '''
+        Attempt opening a transaction with a non existant DB and ensure that
+        it stops cleanly and allows starting of next transaction
+        '''
+        self.assertRaises(
+            Exception, empty_transaction, "Non existant DB", USER, CONTEXT
+        )
+        self.assertTrue(empty_transaction(DB_NAME, USER, CONTEXT))
+
+    def test0020cursorclose(self):
+        '''
+        Manipulate the cursor during the transaction so that the close in 
+        transaction stop fails. Ensure that this does not affect opening of
+        another transaction
+        '''
+        self.assertRaises(
+            Exception, manipulate_cursor, DB_NAME, USER, CONTEXT
+        )
+        self.assertTrue(empty_transaction(DB_NAME, USER, CONTEXT))
+
+
+def suite():
+    return unittest.TestLoader().loadTestsFromTestCase(TransactionTestCase)
+
+if __name__ == '__main__':
+    suite = suite()
+    unittest.TextTestRunner(verbosity=2).run(suite)
+

trytond/tests/test_tryton.py

@@ -231,6 +231,8 @@ def all_suite():
     suite_.addTests(test_tools.suite())
     import trytond.tests.test_pyson as test_pyson
     suite_.addTests(test_pyson.suite())
+    import trytond.tests.test_transaction as test_transaction
+    suite_.addTests(test_transaction.suite())
     import trytond.tests.test_fields as test_fields
     suite_.addTests(test_fields.suite())
     import trytond.tests.test_modelsingleton as test_modelsingleton

trytond/transaction.py

@@ -70,9 +70,9 @@ class Transaction(local):
         assert self.user is None
         assert self.cursor is None
         assert self.context is None
-        self.user = user
         database = Database(database_name).connect()
         self.cursor = database.cursor(readonly=readonly)
+        self.user = user
         self.context = context or {}
         self.create_records = {}
         self.delete_records = {}
@@ -84,14 +84,16 @@ class Transaction(local):
         '''
         Stop transaction
         '''
-        self.cursor.close()
-        self.cursor = None
-        self.user = None
-        self.context = None
-        self.create_records = None
-        self.delete_records = None
-        self.delete = None
-        self.timestamp = None
+        try:
+            self.cursor.close()
+        finally:
+            self.cursor = None
+            self.user = None
+            self.context = None
+            self.create_records = None
+            self.delete_records = None
+            self.delete = None
+            self.timestamp = None
 
     def set_context(self, context=None, **kwargs):
         if context is None: