Fixes case insensitive for resp body

Create metadata for a snapshot 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.

DocImpact
Change-Id: I684049412a4aa84f593e970c87157c74fffdfffe
Closes-Bug: #1255917

diff --git a/cinder/db/api.py b/cinder/db/api.py
index 966dbdae3f9eca13c8f7b8113ccf56329cb6f774..05298c29ca2b06916918c26e8a41c12203420c0c 100644 (file)
--- a/cinder/db/api.py
+++ b/cinder/db/api.py
@@ -321,7 +321,8 @@ def snapshot_metadata_delete(context, snapshot_id, key):
 
 def snapshot_metadata_update(context, snapshot_id, metadata, delete):
     """Update metadata if it exists, otherwise create it."""
-    IMPL.snapshot_metadata_update(context, snapshot_id, metadata, delete)
+    return IMPL.snapshot_metadata_update(context, snapshot_id,
+                                         metadata, delete)
 
 
 ####################

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index 0217b1d79f934bb8e524a97984c4b441354b67e7..ca7186bba142133988bf09625edc89c9984ecb87 100644 (file)
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -1641,7 +1641,7 @@ def snapshot_metadata_update(context, snapshot_id, metadata, delete):
             meta_ref.update(item)
             meta_ref.save(session=session)
 
-        return metadata
+    return snapshot_metadata_get(context, snapshot_id)
 
 ###################
 

diff --git a/cinder/tests/api/v1/test_snapshot_metadata.py b/cinder/tests/api/v1/test_snapshot_metadata.py
index 78c14d299916ebbd4eaa6881b1c27083a13605b0..65c4e0d69fcbea5a635bb9662f6cb9d12c947916 100644 (file)
--- a/cinder/tests/api/v1/test_snapshot_metadata.py
+++ b/cinder/tests/api/v1/test_snapshot_metadata.py
@@ -44,6 +44,15 @@ def return_create_snapshot_metadata(context, snapshot_id, metadata, delete):
     return stub_snapshot_metadata()
 
 
+def return_create_snapshot_metadata_insensitive(context, snapshot_id,
+                                                metadata, delete):
+    return stub_snapshot_metadata_insensitive()
+
+
+def return_new_snapshot_metadata(context, snapshot_id, metadata, delete):
+    return stub_new_snapshot_metadata()
+
+
 def return_snapshot_metadata(context, snapshot_id):
     if not isinstance(snapshot_id, str) or not len(snapshot_id) == 36:
         msg = 'id %s must be a uuid in return snapshot metadata' % snapshot_id
@@ -55,6 +64,10 @@ def return_empty_snapshot_metadata(context, snapshot_id):
     return {}
 
 
+def return_empty_container_metadata(context, snapshot_id, metadata, delete):
+    return {}
+
+
 def delete_snapshot_metadata(context, snapshot_id, key):
     pass
 
@@ -68,6 +81,25 @@ def stub_snapshot_metadata():
     return metadata
 
 
+def stub_snapshot_metadata_insensitive():
+    metadata = {
+        "key1": "value1",
+        "key2": "value2",
+        "key3": "value3",
+        "KEY4": "value4",
+    }
+    return metadata
+
+
+def stub_new_snapshot_metadata():
+    metadata = {
+        'key10': 'value10',
+        'key99': 'value99',
+        'KEY20': 'value20',
+    }
+    return metadata
+
+
 def stub_max_snapshot_metadata():
     metadata = {"metadata": {}}
     for num in range(CONF.quota_metadata_items):
@@ -218,11 +250,38 @@ class SnapshotMetaDataTest(test.TestCase):
         req = fakes.HTTPRequest.blank('/v1/snapshot_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, 'snapshot_metadata_get',
+                       return_empty_snapshot_metadata)
+        self.stubs.Set(cinder.db, 'snapshot_metadata_update',
+                       return_create_snapshot_metadata_insensitive)
+
+        req = fakes.HTTPRequest.blank('/v1/snapshot_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, 'snapshot_metadata_update',
                        return_create_snapshot_metadata)
@@ -275,8 +334,10 @@ class SnapshotMetaDataTest(test.TestCase):
                           self.controller.create, req, self.req_id, body)
 
     def test_update_all(self):
-        self.stubs.Set(cinder.db, 'snapshot_metadata_update',
+        self.stubs.Set(cinder.db, 'snapshot_metadata_get',
                        return_create_snapshot_metadata)
+        self.stubs.Set(cinder.db, 'snapshot_metadata_update',
+                       return_new_snapshot_metadata)
         req = fakes.HTTPRequest.blank(self.url)
         req.method = 'PUT'
         req.content_type = "application/json"
@@ -284,6 +345,7 @@ class SnapshotMetaDataTest(test.TestCase):
             'metadata': {
                 'key10': 'value10',
                 'key99': 'value99',
+                'KEY20': 'value20',
             },
         }
         req.body = jsonutils.dumps(expected)
@@ -291,9 +353,37 @@ class SnapshotMetaDataTest(test.TestCase):
 
         self.assertEqual(expected, res_dict)
 
+    def test_update_all_with_keys_in_uppercase_and_lowercase(self):
+        self.stubs.Set(cinder.db, 'snapshot_metadata_get',
+                       return_create_snapshot_metadata)
+        self.stubs.Set(cinder.db, 'snapshot_metadata_update',
+                       return_new_snapshot_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, 'snapshot_metadata_update',
-                       return_create_snapshot_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 83d0f829ef1dc6fe062eac97bf2b7633f18d0732..ba250484778a54ee95a27aeed97e7c8c9d115844 100644 (file)
--- a/cinder/tests/api/v2/test_snapshot_metadata.py
+++ b/cinder/tests/api/v2/test_snapshot_metadata.py
@@ -44,6 +44,15 @@ def return_create_snapshot_metadata(context, snapshot_id, metadata, delete):
     return stub_snapshot_metadata()
 
 
+def return_create_snapshot_metadata_insensitive(context, snapshot_id,
+                                                metadata, delete):
+    return stub_snapshot_metadata_insensitive()
+
+
+def return_new_snapshot_metadata(context, snapshot_id, metadata, delete):
+    return stub_new_snapshot_metadata()
+
+
 def return_snapshot_metadata(context, snapshot_id):
     if not isinstance(snapshot_id, str) or not len(snapshot_id) == 36:
         msg = 'id %s must be a uuid in return snapshot metadata' % snapshot_id
@@ -55,6 +64,10 @@ def return_empty_snapshot_metadata(context, snapshot_id):
     return {}
 
 
+def return_empty_container_metadata(context, snapshot_id, metadata, delete):
+    return {}
+
+
 def delete_snapshot_metadata(context, snapshot_id, key):
     pass
 
@@ -68,6 +81,25 @@ def stub_snapshot_metadata():
     return metadata
 
 
+def stub_snapshot_metadata_insensitive():
+    metadata = {
+        "key1": "value1",
+        "key2": "value2",
+        "key3": "value3",
+        "KEY4": "value4",
+    }
+    return metadata
+
+
+def stub_new_snapshot_metadata():
+    metadata = {
+        'key10': 'value10',
+        'key99': 'value99',
+        'KEY20': 'value20',
+    }
+    return metadata
+
+
 def stub_max_snapshot_metadata():
     metadata = {"metadata": {}}
     for num in range(CONF.quota_metadata_items):
@@ -218,11 +250,38 @@ class SnapshotMetaDataTest(test.TestCase):
         req = fakes.HTTPRequest.blank('/v2/snapshot_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, 'snapshot_metadata_get',
+                       return_empty_snapshot_metadata)
+        self.stubs.Set(cinder.db, 'snapshot_metadata_update',
+                       return_create_snapshot_metadata_insensitive)
+
+        req = fakes.HTTPRequest.blank('/v1/snapshot_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, 'snapshot_metadata_update',
                        return_create_snapshot_metadata)
@@ -276,7 +335,7 @@ class SnapshotMetaDataTest(test.TestCase):
 
     def test_update_all(self):
         self.stubs.Set(cinder.db, 'snapshot_metadata_update',
-                       return_create_snapshot_metadata)
+                       return_new_snapshot_metadata)
         req = fakes.HTTPRequest.blank(self.url)
         req.method = 'PUT'
         req.content_type = "application/json"
@@ -284,6 +343,7 @@ class SnapshotMetaDataTest(test.TestCase):
             'metadata': {
                 'key10': 'value10',
                 'key99': 'value99',
+                'KEY20': 'value20',
             },
         }
         req.body = jsonutils.dumps(expected)
@@ -291,9 +351,37 @@ class SnapshotMetaDataTest(test.TestCase):
 
         self.assertEqual(expected, res_dict)
 
+    def test_update_all_with_keys_in_uppercase_and_lowercase(self):
+        self.stubs.Set(cinder.db, 'snapshot_metadata_get',
+                       return_create_snapshot_metadata)
+        self.stubs.Set(cinder.db, 'snapshot_metadata_update',
+                       return_new_snapshot_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, 'snapshot_metadata_update',
-                       return_create_snapshot_metadata)
+                       return_empty_container_metadata)
         req = fakes.HTTPRequest.blank(self.url)
         req.method = 'PUT'
         req.content_type = "application/json"

diff --git a/cinder/tests/test_db_api.py b/cinder/tests/test_db_api.py
index 5b96d966b67002c72700de735ea3640c6e9990ea..c5bcf74d65d923d0fc0d73cf7e0ba2b222143e9c 100644 (file)
--- a/cinder/tests/test_db_api.py
+++ b/cinder/tests/test_db_api.py
@@ -503,9 +503,9 @@ class DBAPISnapshotTestCase(BaseTest):
         db.volume_create(self.ctxt, {'id': 1})
         db.snapshot_create(self.ctxt,
                            {'id': 1, 'volume_id': 1, 'metadata': metadata1})
-        db.snapshot_metadata_update(self.ctxt, 1, metadata2, False)
+        db_meta = db.snapshot_metadata_update(self.ctxt, 1, metadata2, False)
 
-        self.assertEqual(should_be, db.snapshot_metadata_get(self.ctxt, 1))
+        self.assertEqual(should_be, db_meta)
 
     def test_snapshot_metadata_update_delete(self):
         metadata1 = {'a': '1', 'c': '2'}
@@ -515,9 +515,9 @@ class DBAPISnapshotTestCase(BaseTest):
         db.volume_create(self.ctxt, {'id': 1})
         db.snapshot_create(self.ctxt,
                            {'id': 1, 'volume_id': 1, 'metadata': metadata1})
-        db.snapshot_metadata_update(self.ctxt, 1, metadata2, True)
+        db_meta = db.snapshot_metadata_update(self.ctxt, 1, metadata2, True)
 
-        self.assertEqual(should_be, db.snapshot_metadata_get(self.ctxt, 1))
+        self.assertEqual(should_be, db_meta)
 
     def test_snapshot_metadata_delete(self):
         metadata = {'a': '1', 'c': '2'}

diff --git a/cinder/volume/api.py b/cinder/volume/api.py
index 7b4dd70dadd7554d0923b38f1cb0ce0596660966..cdcefedf870c1e0494e8a9ab1318099ab4738336 100644 (file)
--- a/cinder/volume/api.py
+++ b/cinder/volume/api.py
@@ -689,14 +689,14 @@ class API(base.Base):
 
         self._check_metadata_properties(context, _metadata)
 
-        self.db.snapshot_metadata_update(context,
-                                         snapshot['id'],
-                                         _metadata,
-                                         True)
+        db_meta = self.db.snapshot_metadata_update(context,
+                                                   snapshot['id'],
+                                                   _metadata,
+                                                   True)
 
         # TODO(jdg): Implement an RPC call for drivers that may use this info
 
-        return _metadata
+        return db_meta
 
     def get_snapshot_metadata_value(self, snapshot, key):
         pass