From: Sheel Rana <ranasheel2000@gmail.com>
Date: Wed, 3 Feb 2016 18:53:16 +0000 (+0530)
Subject: Use of metadata id instead of metadata name
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=7d8369099180bb65449685755b0f69f1378bc8ae;p=openstack-build%2Fcinder-build.git

Use of metadata id instead of metadata name

If multiple image-metadata entries are present with same name
but different cases, 'image-metadata unset' command deletes all
of them irrespective of case.

For ex, "image-metadata unset abc=123" will delete all image-metadata
entries with name abc, ABC, Abc etc.

This is fixed by using id instead of name to fetch image-metadata
during 'image-metadata unset' command execution flow.

Closes-Bug: #1538418

Change-Id: I9fb29667964a4180af822b811345fa1148dbcd66
---

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index e82edd890..445378153 100644
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -1881,6 +1881,16 @@ def _volume_image_metadata_update(context, volume_id, metadata, delete,
                                      session=session)
 
 
+@require_context
+def _volume_glance_metadata_key_to_id(context, volume_id, key):
+    db_data = volume_glance_metadata_get(context, volume_id)
+    metadata = {meta_entry.key: meta_entry.id
+                for meta_entry in db_data
+                if meta_entry.key == key}
+    metadata_id = metadata[key]
+    return metadata_id
+
+
 @require_context
 @require_volume_exists
 def volume_metadata_get(context, volume_id):
@@ -1898,8 +1908,10 @@ def volume_metadata_delete(context, volume_id, key, meta_type):
                     'deleted_at': timeutils.utcnow(),
                     'updated_at': literal_column('updated_at')}))
     elif meta_type == common.METADATA_TYPES.image:
+        metadata_id = _volume_glance_metadata_key_to_id(context,
+                                                        volume_id, key)
         (_volume_image_metadata_get_query(context, volume_id).
-            filter_by(key=key).
+            filter_by(id=metadata_id).
             update({'deleted': True,
                     'deleted_at': timeutils.utcnow(),
                     'updated_at': literal_column('updated_at')}))
diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py
index ed3215378..b2dd9fade 100644
--- a/cinder/tests/unit/test_db_api.py
+++ b/cinder/tests/unit/test_db_api.py
@@ -967,6 +967,66 @@ class DBAPIVolumeTestCase(BaseTest):
 
         self.assertEqual(should_be, db_meta)
 
+    @mock.patch.object(db.sqlalchemy.api,
+                       '_volume_glance_metadata_key_to_id',
+                       return_value = '1')
+    def test_volume_glance_metadata_key_to_id_called(self,
+                                                     metadata_key_to_id_mock):
+        image_metadata = {'abc': '123'}
+
+        # create volume with metadata.
+        db.volume_create(self.ctxt, {'id': 1,
+                                     'metadata': image_metadata})
+
+        # delete metadata associated with the volume.
+        db.volume_metadata_delete(self.ctxt,
+                                  1,
+                                  'abc',
+                                  meta_type=common.METADATA_TYPES.image)
+
+        # assert _volume_glance_metadata_key_to_id() was called exactly once
+        metadata_key_to_id_mock.assert_called_once_with(self.ctxt, 1, 'abc')
+
+    def test_case_sensitive_glance_metadata_delete(self):
+        user_metadata = {'a': '1', 'c': '2'}
+        image_metadata = {'abc': '123', 'ABC': '123'}
+
+        # create volume with metadata.
+        db.volume_create(self.ctxt, {'id': 1,
+                                     'metadata': user_metadata})
+
+        # delete user metadata associated with the volume.
+        db.volume_metadata_delete(self.ctxt, 1, 'c',
+                                  meta_type=common.METADATA_TYPES.user)
+        user_metadata.pop('c')
+
+        self.assertEqual(user_metadata,
+                         db.volume_metadata_get(self.ctxt, 1))
+
+        # create image metadata associated with the volume.
+        db.volume_metadata_update(
+            self.ctxt,
+            1,
+            image_metadata,
+            False,
+            meta_type=common.METADATA_TYPES.image)
+
+        # delete image metadata associated with the volume.
+        db.volume_metadata_delete(
+            self.ctxt,
+            1,
+            'abc',
+            meta_type=common.METADATA_TYPES.image)
+
+        image_metadata.pop('abc')
+
+        # parse the result to build the dict.
+        rows = db.volume_glance_metadata_get(self.ctxt, 1)
+        result = {}
+        for row in rows:
+            result[row['key']] = row['value']
+        self.assertEqual(image_metadata, result)
+
     def test_volume_metadata_update_with_metatype(self):
         user_metadata1 = {'a': '1', 'c': '2'}
         user_metadata2 = {'a': '3', 'd': '5'}