Fixes case insensitive for resp body

Create metadata for a volume with key-value set, which key in uppercase
and lowercase(e.g.{"key": "v1", "KEY": "V1"), the server accept the
request and return the key-value set {"key": "v1", "KEY": "V1"}. But the
server just add one metadata because the server is not case sensitive.

The patch will modify the resp body with the one which the server added.

update_all has the same ploblem.

Fixes errors on v2 unittest without difficulty.

DocImpact
Closes-Bug: #1258004

Change-Id: Ic337c0a351ac234493e1d73b86ba87520f32289a

diff --git a/cinder/db/api.py b/cinder/db/api.py
index 05298c29ca2b06916918c26e8a41c12203420c0c..8af6c557a9f07af079e9e0cea749abfb24d8f89f 100644 (file)
--- a/cinder/db/api.py
+++ b/cinder/db/api.py
@@ -340,7 +340,7 @@ def volume_metadata_delete(context, volume_id, key):
 
 def volume_metadata_update(context, volume_id, metadata, delete):
     """Update metadata if it exists, otherwise create it."""
-    IMPL.volume_metadata_update(context, volume_id, metadata, delete)
+    return IMPL.volume_metadata_update(context, volume_id, metadata, delete)
 
 
 ##################

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index ca7186bba142133988bf09625edc89c9984ecb87..54a11e0a7b632c977e0afa4bf2a12febd44dc12d 100644 (file)
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -1316,7 +1316,7 @@ def _volume_x_metadata_update(context, volume_id, metadata, delete,
             meta_ref.update(item)
             meta_ref.save(session=session)
 
-        return metadata
+    return _volume_x_metadata_get(context, volume_id, model)
 
 
 def _volume_user_metadata_get_query(context, volume_id, session=None):

diff --git a/cinder/tests/api/v1/test_volume_metadata.py b/cinder/tests/api/v1/test_volume_metadata.py
index 1f1b76ec9b3cff58f661d7f8623de3d6ffc07410..c6a6c1c6f2e0a775007fcd0ad6e22ee378682100 100644 (file)
--- a/cinder/tests/api/v1/test_volume_metadata.py
+++ b/cinder/tests/api/v1/test_volume_metadata.py
@@ -42,6 +42,15 @@ def return_create_volume_metadata(context, volume_id, metadata, delete):
     return stub_volume_metadata()
 
 
+def return_new_volume_metadata(context, volume_id, metadata, delete):
+    return stub_new_volume_metadata()
+
+
+def return_create_volume_metadata_insensitive(context, snapshot_id,
+                                              metadata, delete):
+    return stub_volume_metadata_insensitive()
+
+
 def return_volume_metadata(context, volume_id):
     if not isinstance(volume_id, str) or not len(volume_id) == 36:
         msg = 'id %s must be a uuid in return volume metadata' % volume_id
@@ -53,6 +62,10 @@ def return_empty_volume_metadata(context, volume_id):
     return {}
 
 
+def return_empty_container_metadata(context, volume_id, metadata, delete):
+    return {}
+
+
 def delete_volume_metadata(context, volume_id, key):
     pass
 
@@ -66,6 +79,25 @@ def stub_volume_metadata():
     return metadata
 
 
+def stub_new_volume_metadata():
+    metadata = {
+        'key10': 'value10',
+        'key99': 'value99',
+        'KEY20': 'value20',
+    }
+    return metadata
+
+
+def stub_volume_metadata_insensitive():
+    metadata = {
+        "key1": "value1",
+        "key2": "value2",
+        "key3": "value3",
+        "KEY4": "value4",
+    }
+    return metadata
+
+
 def stub_max_volume_metadata():
     metadata = {"metadata": {}}
     for num in range(CONF.quota_metadata_items):
@@ -202,11 +234,38 @@ class volumeMetaDataTest(test.TestCase):
         req = fakes.HTTPRequest.blank('/v1/volume_metadata')
         req.method = 'POST'
         req.content_type = "application/json"
-        body = {"metadata": {"key9": "value9"}}
+        body = {"metadata": {"key1": "value1",
+                             "key2": "value2",
+                             "key3": "value3", }}
         req.body = jsonutils.dumps(body)
         res_dict = self.controller.create(req, self.req_id, body)
         self.assertEqual(body, res_dict)
 
+    def test_create_with_keys_in_uppercase_and_lowercase(self):
+        # if the keys in uppercase_and_lowercase, should return the one
+        # which server added
+        self.stubs.Set(cinder.db, 'volume_metadata_get',
+                       return_empty_volume_metadata)
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_create_volume_metadata_insensitive)
+
+        req = fakes.HTTPRequest.blank('/v1/volume_metadata')
+        req.method = 'POST'
+        req.content_type = "application/json"
+        body = {"metadata": {"key1": "value1",
+                             "KEY1": "value1",
+                             "key2": "value2",
+                             "KEY2": "value2",
+                             "key3": "value3",
+                             "KEY4": "value4"}}
+        expected = {"metadata": {"key1": "value1",
+                                 "key2": "value2",
+                                 "key3": "value3",
+                                 "KEY4": "value4"}}
+        req.body = jsonutils.dumps(body)
+        res_dict = self.controller.create(req, self.req_id, body)
+        self.assertEqual(expected, res_dict)
+
     def test_create_empty_body(self):
         self.stubs.Set(cinder.db, 'volume_metadata_update',
                        return_create_volume_metadata)
@@ -260,7 +319,7 @@ class volumeMetaDataTest(test.TestCase):
 
     def test_update_all(self):
         self.stubs.Set(cinder.db, 'volume_metadata_update',
-                       return_create_volume_metadata)
+                       return_new_volume_metadata)
         req = fakes.HTTPRequest.blank(self.url)
         req.method = 'PUT'
         req.content_type = "application/json"
@@ -268,6 +327,7 @@ class volumeMetaDataTest(test.TestCase):
             'metadata': {
                 'key10': 'value10',
                 'key99': 'value99',
+                'KEY20': 'value20',
             },
         }
         req.body = jsonutils.dumps(expected)
@@ -275,9 +335,37 @@ class volumeMetaDataTest(test.TestCase):
 
         self.assertEqual(expected, res_dict)
 
+    def test_update_all_with_keys_in_uppercase_and_lowercase(self):
+        self.stubs.Set(cinder.db, 'volume_metadata_get',
+                       return_create_volume_metadata)
+        self.stubs.Set(cinder.db, 'volume_metadata_update',
+                       return_new_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url)
+        req.method = 'PUT'
+        req.content_type = "application/json"
+        body = {
+            'metadata': {
+                'key10': 'value10',
+                'KEY10': 'value10',
+                'key99': 'value99',
+                'KEY20': 'value20',
+            },
+        }
+        expected = {
+            'metadata': {
+                'key10': 'value10',
+                'key99': 'value99',
+                'KEY20': 'value20',
+            },
+        }
+        req.body = jsonutils.dumps(expected)
+        res_dict = self.controller.update_all(req, self.req_id, body)
+
+        self.assertEqual(expected, res_dict)
+
     def test_update_all_empty_container(self):
         self.stubs.Set(cinder.db, 'volume_metadata_update',
-                       return_create_volume_metadata)
+                       return_empty_container_metadata)
         req = fakes.HTTPRequest.blank(self.url)
         req.method = 'PUT'
         req.content_type = "application/json"

diff --git a/cinder/tests/api/v2/test_snapshot_metadata.py b/cinder/tests/api/v2/test_snapshot_metadata.py
index ba250484778a54ee95a27aeed97e7c8c9d115844..bcbbaa49b1a60ab30bf55c0e69ec5a3120e6ea21 100644 (file)
--- a/cinder/tests/api/v2/test_snapshot_metadata.py
+++ b/cinder/tests/api/v2/test_snapshot_metadata.py
@@ -265,7 +265,7 @@ class SnapshotMetaDataTest(test.TestCase):
         self.stubs.Set(cinder.db, 'snapshot_metadata_update',
                        return_create_snapshot_metadata_insensitive)
 
-        req = fakes.HTTPRequest.blank('/v1/snapshot_metadata')
+        req = fakes.HTTPRequest.blank('/v2/snapshot_metadata')
         req.method = 'POST'
         req.content_type = "application/json"
         body = {"metadata": {"key1": "value1",

diff --git a/cinder/tests/api/v2/test_volume_metadata.py b/cinder/tests/api/v2/test_volume_metadata.py
index 16ea3b7ce8212d1540bc59a2a323bf64b744b030..1ef807ba2be1df3b2f5ff4b0c2a0028410dc5590 100644 (file)
--- a/cinder/tests/api/v2/test_volume_metadata.py
+++ b/cinder/tests/api/v2/test_volume_metadata.py
@@ -43,6 +43,15 @@ def return_create_volume_metadata(context, volume_id, metadata, delete):
     return stub_volume_metadata()
 
 
+def return_new_volume_metadata(context, volume_id, metadata, delete):
+    return stub_new_volume_metadata()
+
+
+def return_create_volume_metadata_insensitive(context, snapshot_id,
+                                              metadata, delete):
+    return stub_volume_metadata_insensitive()
+
+
 def return_volume_metadata(context, volume_id):
     if not isinstance(volume_id, str) or not len(volume_id) == 36:
         msg = 'id %s must be a uuid in return volume metadata' % volume_id
@@ -54,6 +63,10 @@ def return_empty_volume_metadata(context, volume_id):
     return {}
 
 
+def return_empty_container_metadata(context, volume_id, metadata, delete):
+    return {}
+
+
 def delete_volume_metadata(context, volume_id, key):
     pass
 
@@ -67,6 +80,25 @@ def stub_volume_metadata():
     return metadata
 
 
+def stub_new_volume_metadata():
+    metadata = {
+        'key10': 'value10',
+        'key99': 'value99',
+        'KEY20': 'value20',
+    }
+    return metadata
+
+
+def stub_volume_metadata_insensitive():
+    metadata = {
+        "key1": "value1",
+        "key2": "value2",
+        "key3": "value3",
+        "KEY4": "value4",
+    }
+    return metadata
+
+
 def stub_max_volume_metadata():
     metadata = {"metadata": {}}
     for num in range(CONF.quota_metadata_items):
@@ -203,11 +235,38 @@ class volumeMetaDataTest(test.TestCase):
         req = fakes.HTTPRequest.blank('/v2/volume_metadata')
         req.method = 'POST'
         req.content_type = "application/json"
-        body = {"metadata": {"key9": "value9"}}
+        body = {"metadata": {"key1": "value1",
+                             "key2": "value2",
+                             "key3": "value3", }}
         req.body = jsonutils.dumps(body)
         res_dict = self.controller.create(req, self.req_id, body)
         self.assertEqual(body, res_dict)
 
+    def test_create_with_keys_in_uppercase_and_lowercase(self):
+        # if the keys in uppercase_and_lowercase, should return the one
+        # which server added
+        self.stubs.Set(db, 'volume_metadata_get',
+                       return_empty_volume_metadata)
+        self.stubs.Set(db, 'volume_metadata_update',
+                       return_create_volume_metadata_insensitive)
+
+        req = fakes.HTTPRequest.blank('/v2/volume_metadata')
+        req.method = 'POST'
+        req.content_type = "application/json"
+        body = {"metadata": {"key1": "value1",
+                             "KEY1": "value1",
+                             "key2": "value2",
+                             "KEY2": "value2",
+                             "key3": "value3",
+                             "KEY4": "value4"}}
+        expected = {"metadata": {"key1": "value1",
+                                 "key2": "value2",
+                                 "key3": "value3",
+                                 "KEY4": "value4"}}
+        req.body = jsonutils.dumps(body)
+        res_dict = self.controller.create(req, self.req_id, body)
+        self.assertEqual(expected, res_dict)
+
     def test_create_empty_body(self):
         self.stubs.Set(db, 'volume_metadata_update',
                        return_create_volume_metadata)
@@ -261,7 +320,7 @@ class volumeMetaDataTest(test.TestCase):
 
     def test_update_all(self):
         self.stubs.Set(db, 'volume_metadata_update',
-                       return_create_volume_metadata)
+                       return_new_volume_metadata)
         req = fakes.HTTPRequest.blank(self.url)
         req.method = 'PUT'
         req.content_type = "application/json"
@@ -269,6 +328,7 @@ class volumeMetaDataTest(test.TestCase):
             'metadata': {
                 'key10': 'value10',
                 'key99': 'value99',
+                'KEY20': 'value20',
             },
         }
         req.body = jsonutils.dumps(expected)
@@ -276,9 +336,37 @@ class volumeMetaDataTest(test.TestCase):
 
         self.assertEqual(expected, res_dict)
 
+    def test_update_all_with_keys_in_uppercase_and_lowercase(self):
+        self.stubs.Set(db, 'volume_metadata_get',
+                       return_create_volume_metadata)
+        self.stubs.Set(db, 'volume_metadata_update',
+                       return_new_volume_metadata)
+        req = fakes.HTTPRequest.blank(self.url)
+        req.method = 'PUT'
+        req.content_type = "application/json"
+        body = {
+            'metadata': {
+                'key10': 'value10',
+                'KEY10': 'value10',
+                'key99': 'value99',
+                'KEY20': 'value20',
+            },
+        }
+        expected = {
+            'metadata': {
+                'key10': 'value10',
+                'key99': 'value99',
+                'KEY20': 'value20',
+            },
+        }
+        req.body = jsonutils.dumps(expected)
+        res_dict = self.controller.update_all(req, self.req_id, body)
+
+        self.assertEqual(expected, res_dict)
+
     def test_update_all_empty_container(self):
         self.stubs.Set(db, 'volume_metadata_update',
-                       return_create_volume_metadata)
+                       return_empty_container_metadata)
         req = fakes.HTTPRequest.blank(self.url)
         req.method = 'PUT'
         req.content_type = "application/json"

diff --git a/cinder/tests/api/v2/test_volumes.py b/cinder/tests/api/v2/test_volumes.py
index 8d2ba7b2f605209cf3145df3604e699b8b8c31a7..b2def7a81afcfe69c63dffdf842d6c12ce15b305 100644 (file)
--- a/cinder/tests/api/v2/test_volumes.py
+++ b/cinder/tests/api/v2/test_volumes.py
@@ -343,7 +343,7 @@ class VolumeApiTest(test.TestCase):
             "display_name": "Updated Test Name",
         }
         body = {"volume": updates}
-        req = fakes.HTTPRequest.blank('/v1/volumes/1')
+        req = fakes.HTTPRequest.blank('/v2/volumes/1')
         admin_ctx = context.RequestContext('admin', 'fake', True)
         req.environ['cinder.context'] = admin_ctx
         res_dict = self.controller.update(req, '1', body)

diff --git a/cinder/tests/test_db_api.py b/cinder/tests/test_db_api.py
index c5bcf74d65d923d0fc0d73cf7e0ba2b222143e9c..7ba3a5ff571973f74589082db5973b8d42d25b28 100644 (file)
--- a/cinder/tests/test_db_api.py
+++ b/cinder/tests/test_db_api.py
@@ -442,9 +442,9 @@ class DBAPIVolumeTestCase(BaseTest):
         should_be = {'a': '3', 'c': '2', 'd': '5'}
 
         db.volume_create(self.ctxt, {'id': 1, 'metadata': metadata1})
-        db.volume_metadata_update(self.ctxt, 1, metadata2, False)
+        db_meta = db.volume_metadata_update(self.ctxt, 1, metadata2, False)
 
-        self.assertEqual(should_be, db.volume_metadata_get(self.ctxt, 1))
+        self.assertEqual(should_be, db_meta)
 
     def test_volume_metadata_update_delete(self):
         metadata1 = {'a': '1', 'c': '2'}
@@ -452,9 +452,9 @@ class DBAPIVolumeTestCase(BaseTest):
         should_be = metadata2
 
         db.volume_create(self.ctxt, {'id': 1, 'metadata': metadata1})
-        db.volume_metadata_update(self.ctxt, 1, metadata2, True)
+        db_meta = db.volume_metadata_update(self.ctxt, 1, metadata2, True)
 
-        self.assertEqual(should_be, db.volume_metadata_get(self.ctxt, 1))
+        self.assertEqual(should_be, db_meta)
 
     def test_volume_metadata_delete(self):
         metadata = {'a': 'b', 'c': 'd'}

diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index cdcefedf870c1e0494e8a9ab1318099ab4738336..f6ca4724124d618f6638d2df7d22e2462a1344bf 100644 (file)
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -610,12 +610,12 @@ class API(base.Base):
 
         self._check_metadata_properties(context, _metadata)
 
-        self.db.volume_metadata_update(context, volume['id'],
-                                       _metadata, delete)
+        db_meta = self.db.volume_metadata_update(context, volume['id'],
+                                                 _metadata, delete)
 
         # TODO(jdg): Implement an RPC call for drivers that may use this info
 
-        return _metadata
+        return db_meta
 
     def get_volume_metadata_value(self, volume, key):
         """Get value of particular metadata key."""