db: Remove unnecessary engine facade decorator

We're using a context manager in one function so there's no need to wrap
that function in a decorator.

While we're here, we add tests to prove that the decorator is sufficient
elsewhere.

Change-Id: I3be92fcd4ac1f86230adf7c2998e2f0100a2ec33
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>

cinder/db/sqlalchemy/api.py

@@ -8287,8 +8287,9 @@ def cleanup_expired_messages(context):
 ###############################
 
 
+# NOTE: We don't need a transaction context manager decorator since we're using
+# the context manager directly inside
 @require_context
-@main_context_manager.writer
 def driver_initiator_data_insert_by_key(
     context,
     initiator,
@@ -8302,9 +8303,12 @@ def driver_initiator_data_insert_by_key(
     data.key = key
     data.value = value
     try:
+        # NOTE: We use a context manager since the decorator pattern requires
+        # we raise an exception or succeed, while we want to return a boolean.
+        # The context manager allows us to do manual cleanup here.
         with main_context_manager.writer.savepoint.using(context):
             data.save(context.session)
-            return True
+        return True
     except db_exc.DBDuplicateEntry:
         return False
 

cinder/tests/unit/db/test_volume_type.py

@@ -68,6 +68,25 @@ class VolumeTypeTestCase(test.TestCase):
         db.group_destroy(self.ctxt, group['id'])
         volume_types.destroy(self.ctxt, volume_type['id'])
 
+    def test_volume_type_mark_in_use_exists(self):
+        volume_type = db.volume_type_create(
+            self.ctxt,
+            {'name': 'fake volume type'},
+        )
+        group = db.group_create(self.ctxt, {})
+        db.group_volume_type_mapping_create(
+            self.ctxt,
+            group['id'],
+            volume_type['id'],
+        )
+        self.assertRaises(
+            exception.GroupVolumeTypeMappingExists,
+            db.group_volume_type_mapping_create,
+            self.ctxt,
+            group['id'],
+            volume_type['id'],
+        )
+
     def test_volume_type_delete_with_consistencygroups_in_use(self):
         volume_type = db.volume_type_create(self.ctxt, {'name':
                                                         'fake volume type'})

cinder/tests/unit/test_db_api.py

@@ -2373,20 +2373,18 @@ class DBAPICgsnapshotTestCase(BaseTest):
 
 
 class DBAPIVolumeTypeTestCase(BaseTest):
-
     """Tests for the db.api.volume_type_* methods."""
 
-    def setUp(self):
-        self.ctxt = context.get_admin_context()
-        super(DBAPIVolumeTypeTestCase, self).setUp()
+    def test_volume_type_create__exists(self):
+        vt = db.volume_type_create(self.ctxt, {'name': 'n2'})
+        self.assertRaises(
+            exception.VolumeTypeExists,
+            db.volume_type_create,
+            self.ctxt,
+            {'name': 'n2', 'id': vt['id']},
+        )
 
-    def test_volume_type_create_exists(self):
-        self.assertRaises(exception.VolumeTypeExists,
-                          db.volume_type_create,
-                          self.ctxt,
-                          {'name': 'n2', 'id': self.vt['id']})
-
-    def test_volume_type_access_remove(self):
+    def test_volume_type_access_add_remove(self):
         vt = db.volume_type_create(self.ctxt, {'name': 'n2'})
         db.volume_type_access_add(self.ctxt, vt['id'], 'fake_project')
         vtas = db.volume_type_access_get_all(self.ctxt, vt['id'])
@@ -2395,7 +2393,20 @@ class DBAPIVolumeTypeTestCase(BaseTest):
         vtas = db.volume_type_access_get_all(self.ctxt, vt['id'])
         self.assertEqual(0, len(vtas))
 
-    def test_volume_type_access_remove_high_id(self):
+    def test_volume_type_access_add__exists(self):
+        vt = db.volume_type_create(self.ctxt, {'name': 'n2'})
+        db.volume_type_access_add(self.ctxt, vt['id'], 'fake_project')
+        vtas = db.volume_type_access_get_all(self.ctxt, vt['id'])
+        self.assertEqual(1, len(vtas))
+        self.assertRaises(
+            exception.VolumeTypeAccessExists,
+            db.volume_type_access_add,
+            self.ctxt,
+            vt['id'],
+            'fake_project',
+        )
+
+    def test_volume_type_access_remove__high_id(self):
         vt = db.volume_type_create(self.ctxt, {'name': 'n2'})
         vta = db.volume_type_access_add(self.ctxt, vt['id'], 'fake_project')
         vtas = db.volume_type_access_get_all(self.ctxt, vt['id'])
@@ -3720,7 +3731,40 @@ class DBAPIBackendTestCase(BaseTest):
 
 
 @ddt.ddt
-class DBAPIGroupTestCase(BaseTest):
+class DBAPIGroupTypeTestCase(BaseTest):
+    """Tests for the db.api.group_type_* methods."""
+
+    def test_group_type_create__exists(self):
+        gt = db.group_type_create(self.ctxt, {'name': 'n2'})
+        self.assertRaises(
+            exception.GroupTypeExists,
+            db.group_type_create,
+            self.ctxt,
+            {'name': gt['name'], 'id': gt['id']},
+        )
+
+    def test_volume_type_access_add_remove(self):
+        gt = db.group_type_create(self.ctxt, {'name': 'n2'})
+        db.group_type_access_add(self.ctxt, gt['id'], 'fake_project')
+        gtas = db.group_type_access_get_all(self.ctxt, gt['id'])
+        self.assertEqual(1, len(gtas))
+        db.group_type_access_remove(self.ctxt, gt['id'], 'fake_project')
+        gtas = db.group_type_access_get_all(self.ctxt, gt['id'])
+        self.assertEqual(0, len(gtas))
+
+    def test_group_type_access_add__exists(self):
+        gt = db.group_type_create(self.ctxt, {'name': 'my_group_type'})
+        db.group_type_access_add(self.ctxt, gt['id'], 'fake_project')
+        gtas = db.group_type_access_get_all(self.ctxt, gt['id'])
+        self.assertEqual(1, len(gtas))
+        self.assertRaises(
+            exception.GroupTypeAccessExists,
+            db.group_type_access_add,
+            self.ctxt,
+            gt['id'],
+            'fake_project',
+        )
+
     def test_group_get_all_by_host(self):
         grp_type = db.group_type_create(self.ctxt, {'name': 'my_group_type'})
         groups = []