From: Michal Dulko <michal.dulko@intel.com>
Date: Mon, 21 Sep 2015 13:58:01 +0000 (+0200)
Subject: Replace soft_delete in volume_type_access_remove
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=0e66d3ea1ec83be3e42726063198183097d40cde;p=openstack-build%2Fcinder-build.git

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 622b7eb0d..a6aa6ffe4 100644
--- 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 d732f61ee..ab944e339 100644
--- 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.