objects: automatically detect whether engine facade is used

While we added new_facade object attribute to framework and it has its
niche and used in some stadium subprojects, it's not ideal because it's
global to an object. Meaning that if you mark an object for new_facade =
True, *all* business logic using the object must also switch to new
facade in the same step, which is a pain and sometimes close to
impossible to do without changing thousands loosely related lines of
code in multiple modules. It would be nice to instead use objects as
usual in different contexts - some using engine facade and some still
using session.begin(...) - and allow the OVO framework to pick the right
way to nest subtransactions.

This patch does exactly that. We call an internal function from oslo.db
and check whether it raises an exception. If it does, it means that the
engine facade is not used; otherwise, we use the new style of nested
transactions management.

By default, if session is not active when OVO action is called, we stick
to the old facade. Once we are done with switching the rest of the
plugin code / OVO objects to the new facade, we will rip off the
transitionary logic.

Change-Id: Ia05df925eccf3c9d397748f282e995203a058de9
Partially-Implements: blueprint enginefacade-switch
Partially-Implements: blueprint adopt-oslo-versioned-objects-for-db

doc/source/contributor/internals/objects_usage.rst

@@ -360,6 +360,10 @@ following database session decorators:
 ``db_context_reader`` and ``db_context_writer`` decorators abstract the choice
 of engine facade used for particular object from action implementation.
 
+Alternatively, you can call all OVO actions under an active ``reader.using`` /
+``writer.using`` context manager (or ``session.begin``). In this case, OVO will
+pick the appropriate method to open a subtransaction.
+
 Synthetic fields
 ----------------
 :code:`synthetic_fields` is a list of fields, that are not directly backed by

neutron/objects/base.py

@@ -19,6 +19,7 @@ import itertools
 from neutron_lib import exceptions as n_exc
 from neutron_lib.objects import exceptions as o_exc
 from oslo_db import exception as obj_exc
+from oslo_db.sqlalchemy import enginefacade
 from oslo_db.sqlalchemy import utils as db_utils
 from oslo_log import log as logging
 from oslo_serialization import jsonutils
@@ -505,17 +506,26 @@ class NeutronDbObject(NeutronObject):
         if is_attr_nullable:
             self[attrname] = None
 
+    # TODO(ihrachys) remove once we switch plugin code to enginefacade
+    @staticmethod
+    def _use_db_facade(context):
+        try:
+            enginefacade._transaction_ctx_for_context(context)
+        except obj_exc.NoEngineContextEstablished:
+            return False
+        return True
+
     @classmethod
     def db_context_writer(cls, context):
         """Return read-write session activation decorator."""
-        if cls.new_facade:
+        if cls.new_facade or cls._use_db_facade(context):
             return db_api.context_manager.writer.using(context)
         return db_api.autonested_transaction(context.session)
 
     @classmethod
     def db_context_reader(cls, context):
         """Return read-only session activation decorator."""
-        if cls.new_facade:
+        if cls.new_facade or cls._use_db_facade(context):
             return db_api.context_manager.reader.using(context)
         return db_api.autonested_transaction(context.session)
 

neutron/tests/unit/objects/test_base.py

@@ -32,6 +32,7 @@ from oslo_versionedobjects import fields as obj_fields
 import testtools
 
 from neutron.db import _model_query as model_query
+from neutron.db import api as db_api
 from neutron import objects
 from neutron.objects import agent
 from neutron.objects import base
@@ -1699,16 +1700,23 @@ class BaseDbObjectTestCase(_BaseObjectTestCase,
         self.assertEqual(1, mock_commit.call_count)
 
     def _get_ro_txn_exit_func_name(self):
-        # for old engine facade, we didn't have distinction between r/o and r/w
-        # transactions and so we always call commit even for getters when the
-        # old facade is used
+        # with no engine facade, we didn't have distinction between r/o and
+        # r/w transactions and so we always call commit even for getters when
+        # no facade is used
         return (
             SQLALCHEMY_CLOSE
-            if self._test_class.new_facade else SQLALCHEMY_COMMIT)
+            if self._test_class._use_db_facade else SQLALCHEMY_COMMIT)
 
     def test_get_objects_single_transaction(self):
         with mock.patch(self._get_ro_txn_exit_func_name()) as mock_exit:
-            self._test_class.get_objects(self.context)
+            with db_api.autonested_transaction(self.context.session):
+                self._test_class.get_objects(self.context)
+        self.assertEqual(1, mock_exit.call_count)
+
+    def test_get_objects_single_transaction_enginefacade(self):
+        with mock.patch(self._get_ro_txn_exit_func_name()) as mock_exit:
+            with db_api.context_manager.reader.using(self.context):
+                self._test_class.get_objects(self.context)
         self.assertEqual(1, mock_exit.call_count)
 
     def test_get_object_single_transaction(self):
@@ -1716,8 +1724,19 @@ class BaseDbObjectTestCase(_BaseObjectTestCase,
         obj.create()
 
         with mock.patch(self._get_ro_txn_exit_func_name()) as mock_exit:
-            obj = self._test_class.get_object(self.context,
-                                              **obj._get_composite_keys())
+            with db_api.autonested_transaction(self.context.session):
+                obj = self._test_class.get_object(self.context,
+                                                  **obj._get_composite_keys())
+        self.assertEqual(1, mock_exit.call_count)
+
+    def test_get_object_single_transaction_enginefacade(self):
+        obj = self._make_object(self.obj_fields[0])
+        obj.create()
+
+        with mock.patch(self._get_ro_txn_exit_func_name()) as mock_exit:
+            with db_api.context_manager.reader.using(self.context):
+                obj = self._test_class.get_object(self.context,
+                                                  **obj._get_composite_keys())
         self.assertEqual(1, mock_exit.call_count)
 
     def test_get_objects_supports_extra_filtername(self):

neutron/tests/unit/plugins/ml2/test_plugin.py

@@ -1685,7 +1685,7 @@ class TestMl2DvrPortsV2(TestMl2PortsV2):
                                             {'subnet_id': s['subnet']['id']})
 
         # lie to turn the port into an SNAT interface
-        with db_api.context_manager.reader.using(self.context):
+        with db_api.context_manager.writer.using(self.context):
             pager = base_obj.Pager(limit=1)
             rp = l3_obj.RouterPort.get_objects(
                 self.context, _pager=pager, port_id=p['port_id'])