From 4eabf17683da9c6d72f99b773b5b8afc4389ba00 Mon Sep 17 00:00:00 2001
From: Terry Wilson <twilson@redhat.com>
Date: Wed, 13 Mar 2019 16:32:18 -0500
Subject: [PATCH] Make it possible to opt out of nested transactions

Nested transactions work for simple things, but intermediate
results from transaction commands like create/add that are used
in subsequent nested transactions won't exist until the the end of
the whole nested transaction, causing failures. Since this can be
surprising, make it possible to easily opt out on a per-transaction
or per-API basis.

Change-Id: I4f7997d5dc11a190e188c16e8eac9fe5df8b0d0d
---
 ovsdbapp/api.py                               | 13 +++++++++---
 ovsdbapp/backend/ovs_idl/__init__.py          |  4 ++--
 .../unit/schema/open_vswitch/test_impl_idl.py |  7 +++++++
 ovsdbapp/tests/unit/test_api.py               | 20 +++++++++++++++++--
 4 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/ovsdbapp/api.py b/ovsdbapp/api.py
index dd0b9243..61925e81 100644
--- a/ovsdbapp/api.py
+++ b/ovsdbapp/api.py
@@ -71,8 +71,9 @@ class Transaction(object):
 
 @six.add_metaclass(abc.ABCMeta)
 class API(object):
-    def __init__(self):
+    def __init__(self, nested_transactions=True):
         # Mapping between a (green)thread and its transaction.
+        self._nested_txns = nested_transactions
         self._nested_txns_map = {}
 
     @abc.abstractmethod
@@ -88,17 +89,23 @@ class API(object):
         """
 
     @contextlib.contextmanager
-    def transaction(self, check_error=False, log_errors=True, **kwargs):
+    def transaction(self, check_error=False, log_errors=True, nested=True,
+                    **kwargs):
         """Create a transaction context.
 
         :param check_error: Allow the transaction to raise an exception?
         :type check_error:  bool
         :param log_errors:  Log an error if the transaction fails?
         :type log_errors:   bool
+        :param nested:      Allow nested transactions be merged into one txn
+        :type nested:       bool
         :returns: Either a new transaction or an existing one.
         :rtype: :class:`Transaction`
         """
-        cur_thread_id = thread.get_ident()
+        # ojbect() is unique, so if we are not nested, this will always result
+        # in a KeyError on lookup and so a unique Transaction
+        nested = nested and self._nested_txns
+        cur_thread_id = thread.get_ident() if nested else object()
 
         try:
             yield self._nested_txns_map[cur_thread_id]
diff --git a/ovsdbapp/backend/ovs_idl/__init__.py b/ovsdbapp/backend/ovs_idl/__init__.py
index 9413c3d3..5a570c6f 100644
--- a/ovsdbapp/backend/ovs_idl/__init__.py
+++ b/ovsdbapp/backend/ovs_idl/__init__.py
@@ -26,8 +26,8 @@ class Backend(object):
     lookup_table = {}
     ovsdb_connection = None
 
-    def __init__(self, connection):
-        super(Backend, self).__init__()
+    def __init__(self, connection, **kwargs):
+        super(Backend, self).__init__(**kwargs)
         self.start_connection(connection)
 
     @classmethod
diff --git a/ovsdbapp/tests/unit/schema/open_vswitch/test_impl_idl.py b/ovsdbapp/tests/unit/schema/open_vswitch/test_impl_idl.py
index 54ab19d5..14a41c79 100644
--- a/ovsdbapp/tests/unit/schema/open_vswitch/test_impl_idl.py
+++ b/ovsdbapp/tests/unit/schema/open_vswitch/test_impl_idl.py
@@ -33,3 +33,10 @@ class TransactionTestCase(base.TestCase):
             transaction = impl_idl.OvsVsctlTransaction(mock.sentinel,
                                                        mock.Mock(), 0)
             transaction.post_commit(mock.Mock())
+
+
+class TestOvsdbIdl(base.TestCase):
+    def test_nested_txns(self):
+        conn = mock.MagicMock()
+        api = impl_idl.OvsdbIdl(conn, nested_transactions=False)
+        self.assertFalse(api._nested_txns)
diff --git a/ovsdbapp/tests/unit/test_api.py b/ovsdbapp/tests/unit/test_api.py
index 5dd8c670..9fcc11de 100644
--- a/ovsdbapp/tests/unit/test_api.py
+++ b/ovsdbapp/tests/unit/test_api.py
@@ -65,7 +65,9 @@ class FakeTransaction(object):
 
 class TestingAPI(api.API):
     def create_transaction(self, check_error=False, log_errors=True, **kwargs):
-        return FakeTransaction()
+        txn = FakeTransaction()
+        mock.patch.object(txn, 'commit').start()
+        return txn
 
 
 TestingAPI.__abstractmethods__ = set()
@@ -75,7 +77,6 @@ class TransactionTestCase(base.TestCase):
     def setUp(self):
         super(TransactionTestCase, self).setUp()
         self.api = TestingAPI()
-        mock.patch.object(FakeTransaction, 'commit').start()
         self.useFixture(GreenThreadingFixture())
 
     def test_transaction_nested(self):
@@ -84,6 +85,21 @@ class TransactionTestCase(base.TestCase):
                 self.assertIs(txn1, txn2)
         txn1.commit.assert_called_once_with()
 
+    def test_transaction_nested_false(self):
+        with self.api.transaction(nested=False) as txn1:
+            with self.api.transaction() as txn2:
+                self.assertIsNot(txn1, txn2)
+        txn1.commit.assert_called_once_with()
+        txn2.commit.assert_called_once_with()
+
+    def test_api_level_transaction_nested_fales(self):
+        api = TestingAPI(nested_transactions=False)
+        with api.transaction() as txn1:
+            with api.transaction() as txn2:
+                self.assertIsNot(txn1, txn2)
+        txn1.commit.assert_called_once_with()
+        txn2.commit.assert_called_once_with()
+
     def test_transaction_no_nested_transaction_after_error(self):
         class TestException(Exception):
             pass