Skip to content

Commit

Permalink
Fixes case insensitive for resp body
Browse files Browse the repository at this point in the history
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
  • Loading branch information
huangtianhua committed Dec 5, 2013
1 parent 05cb719 commit b91e6e3
Show file tree
Hide file tree
Showing 8 changed files with 193 additions and 17 deletions.
2 changes: 1 addition & 1 deletion cinder/db/api.py
Expand Up @@ -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)


##################
Expand Down
2 changes: 1 addition & 1 deletion cinder/db/sqlalchemy/api.py
Expand Up @@ -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):
Expand Down
94 changes: 91 additions & 3 deletions cinder/tests/api/v1/test_volume_metadata.py
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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):
Expand Down Expand Up @@ -202,11 +234,38 @@ def test_create(self):
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)
Expand Down Expand Up @@ -260,24 +319,53 @@ def test_create_nonexistent_volume(self):

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"
expected = {
'metadata': {
'key10': 'value10',
'key99': 'value99',
'KEY20': 'value20',
},
}
req.body = jsonutils.dumps(expected)
res_dict = self.controller.update_all(req, self.req_id, expected)

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"
Expand Down
2 changes: 1 addition & 1 deletion cinder/tests/api/v2/test_snapshot_metadata.py
Expand Up @@ -265,7 +265,7 @@ def test_create_with_keys_in_uppercase_and_lowercase(self):
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",
Expand Down
94 changes: 91 additions & 3 deletions cinder/tests/api/v2/test_volume_metadata.py
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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):
Expand Down Expand Up @@ -203,11 +235,38 @@ def test_create(self):
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)
Expand Down Expand Up @@ -261,24 +320,53 @@ def test_create_nonexistent_volume(self):

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"
expected = {
'metadata': {
'key10': 'value10',
'key99': 'value99',
'KEY20': 'value20',
},
}
req.body = jsonutils.dumps(expected)
res_dict = self.controller.update_all(req, self.req_id, expected)

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"
Expand Down
2 changes: 1 addition & 1 deletion cinder/tests/api/v2/test_volumes.py
Expand Up @@ -343,7 +343,7 @@ def test_volume_update_with_admin_metadata(self):
"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)
Expand Down
8 changes: 4 additions & 4 deletions cinder/tests/test_db_api.py
Expand Up @@ -442,19 +442,19 @@ def test_volume_metadata_update(self):
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'}
metadata2 = {'a': '3', 'd': '4'}
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'}
Expand Down
6 changes: 3 additions & 3 deletions cinder/volume/api.py
Expand Up @@ -610,12 +610,12 @@ def update_volume_metadata(self, context, volume, metadata, delete=False):

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."""
Expand Down

0 comments on commit b91e6e3

Please sign in to comment.