Replace soft_delete in volume_type_access_remove

This commit replaces oslo.db's soft_delete used in
volume_type_access_remove in db.api with a simple update setting
deleted column to True.

As deleted column is boolean it is represented in the DB by TINYINT,
which max value is 127. This introduces problems when removing records
with id higher than that, because soft_delete sets deleted column to the
value of id. This commit fixes the issue and adds unit tests for such
case.

Change-Id: I484e66125b5a29f490c070696b336be4a2693b3e
Closes-Bug: 1496747

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index 622b7eb0d320f22109dd98131a0338f394c01c7d..a6aa6ffe48e8ea4d0897dd94c773b166e39a6a0a 100644 (file)
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -2770,10 +2770,12 @@ def volume_type_access_remove(context, type_id, project_id):
     """Remove given tenant from the volume type access list."""
     volume_type_id = _volume_type_get_id_from_volume_type(context, type_id)
 
-    count = _volume_type_access_query(context).\
-        filter_by(volume_type_id=volume_type_id).\
-        filter_by(project_id=project_id).\
-        soft_delete(synchronize_session=False)
+    count = (_volume_type_access_query(context).
+             filter_by(volume_type_id=volume_type_id).
+             filter_by(project_id=project_id).
+             update({'deleted': True,
+                     'deleted_at': timeutils.utcnow(),
+                     'updated_at': literal_column('updated_at')}))
     if count == 0:
         raise exception.VolumeTypeAccessNotFound(
             volume_type_id=type_id, project_id=project_id)

diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py
index d732f61ee5c180e0d5fd7a302f239c365b1accfa..ab944e339bf9f13c630df0bfb6c353cfb33771d6 100644 (file)
--- a/cinder/tests/unit/test_db_api.py
+++ b/cinder/tests/unit/test_db_api.py
@@ -1351,6 +1351,34 @@ class DBAPIVolumeTypeTestCase(BaseTest):
                           self.ctxt,
                           {'name': 'n2', 'id': vt['id']})
 
+    def test_volume_type_access_remove(self):
+        vt = db.volume_type_create(self.ctxt, {'name': 'n1'})
+        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))
+        db.volume_type_access_remove(self.ctxt, vt['id'], 'fake_project')
+        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):
+        vt = db.volume_type_create(self.ctxt, {'name': 'n1'})
+        vta = 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))
+
+        # NOTE(dulek): Bug 1496747 uncovered problems when deleting accesses
+        # with id column higher than 128. This is regression test for that
+        # case.
+
+        session = sqlalchemy_api.get_session()
+        vta.id = 150
+        vta.save(session=session)
+        session.close()
+
+        db.volume_type_access_remove(self.ctxt, vt['id'], 'fake_project')
+        vtas = db.volume_type_access_get_all(self.ctxt, vt['id'])
+        self.assertEqual(0, len(vtas))
+
     def test_get_volume_type_extra_specs(self):
         # Ensure that volume type extra specs can be accessed after
         # the DB session is closed.