From b625f558862465184dd28da7215f34c77ec1ece6 Mon Sep 17 00:00:00 2001 From: huangtianhua Date: Fri, 29 Nov 2013 16:56:35 +0800 Subject: [PATCH] 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 --- cinder/db/api.py | 3 +- cinder/db/sqlalchemy/api.py | 2 +- cinder/tests/api/v1/test_snapshot_metadata.py | 96 ++++++++++++++++++- cinder/tests/api/v2/test_snapshot_metadata.py | 94 +++++++++++++++++- cinder/tests/test_db_api.py | 8 +- cinder/volume/api.py | 10 +- 6 files changed, 196 insertions(+), 17 deletions(-) diff --git a/cinder/db/api.py b/cinder/db/api.py index 966dbdae3..05298c29c 100644 --- 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 0217b1d79..ca7186bba 100644 --- 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 78c14d299..65c4e0d69 100644 --- 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 83d0f829e..ba2504847 100644 --- 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 5b96d966b..c5bcf74d6 100644 --- 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 7b4dd70da..cdcefedf8 100644 --- 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 -- 2.45.2