OpenStack Compute (nova)

Merge lp:~rackspace-titan/nova/lp795045 into lp:~hudson-openstack/nova/trunk

  1. lp795045
  2. Merge into trunk
Proposed by William Wolf
Status: Merged
Approved by: Brian Lamar
Approved revision: 1172
Merged at revision: 1173
Proposed branch: lp:~rackspace-titan/nova/lp795045
Merge into: lp:~hudson-openstack/nova/trunk
Diff against target: 112 lines (+34/-2)
2 files modified
nova/api/openstack/server_metadata.py (+9/-2)
nova/tests/api/openstack/test_server_metadata.py (+25/-0)
To merge this branch: bzr merge lp:~rackspace-titan/nova/lp795045
Reviewer Review Type Date Requested Status
Brian Lamar (community) Approve
Devin Carlen (community) Approve
Review via email: mp+64253@code.launchpad.net

Description of the change

This fixes the server_metadata create and update functions that were returning req.body (as a string) instead of body (deserialized body dictionary object). It also adds checks where appropriate to make sure that body is not empty (and return 400 if it is). Tests updated/added where appropriate.

To post a comment you must log in.
Revision history for this message
Devin Carlen (devcamcar) wrote :

lgtm

review: Approve
lp:~rackspace-titan/nova/lp795045 updated
1171. By William Wolf

check for none and empty string, this way empty dicts/lists will be ok

1172. By William Wolf

merge trunk

Revision history for this message
Brian Lamar (blamar) wrote :

Looks good to me as well.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'nova/api/openstack/server_metadata.py'
2--- nova/api/openstack/server_metadata.py 2011-05-20 18:42:19 +0000
3+++ nova/api/openstack/server_metadata.py 2011-06-13 12:58:30 +0000
4@@ -37,12 +37,18 @@
5 meta_dict[key] = value
6 return dict(metadata=meta_dict)
7
8+ def _check_body(self, body):
9+ if body == None or body == "":
10+ expl = _('No Request Body')
11+ raise exc.HTTPBadRequest(explanation=expl)
12+
13 def index(self, req, server_id):
14 """ Returns the list of metadata for a given instance """
15 context = req.environ['nova.context']
16 return self._get_metadata(context, server_id)
17
18 def create(self, req, server_id, body):
19+ self._check_body(body)
20 context = req.environ['nova.context']
21 metadata = body.get('metadata')
22 try:
23@@ -51,9 +57,10 @@
24 metadata)
25 except quota.QuotaError as error:
26 self._handle_quota_error(error)
27- return req.body
28+ return body
29
30 def update(self, req, server_id, id, body):
31+ self._check_body(body)
32 context = req.environ['nova.context']
33 if not id in body:
34 expl = _('Request body and URI mismatch')
35@@ -68,7 +75,7 @@
36 except quota.QuotaError as error:
37 self._handle_quota_error(error)
38
39- return req.body
40+ return body
41
42 def show(self, req, server_id, id):
43 """ Return a single metadata item """
44
45=== modified file 'nova/tests/api/openstack/test_server_metadata.py'
46--- nova/tests/api/openstack/test_server_metadata.py 2011-04-19 17:46:43 +0000
47+++ nova/tests/api/openstack/test_server_metadata.py 2011-06-13 12:58:30 +0000
48@@ -89,6 +89,7 @@
49 res = req.get_response(fakes.wsgi_app())
50 res_dict = json.loads(res.body)
51 self.assertEqual(200, res.status_int)
52+ self.assertEqual('application/json', res.headers['Content-Type'])
53 self.assertEqual('value1', res_dict['metadata']['key1'])
54
55 def test_index_no_data(self):
56@@ -99,6 +100,7 @@
57 res = req.get_response(fakes.wsgi_app())
58 res_dict = json.loads(res.body)
59 self.assertEqual(200, res.status_int)
60+ self.assertEqual('application/json', res.headers['Content-Type'])
61 self.assertEqual(0, len(res_dict['metadata']))
62
63 def test_show(self):
64@@ -109,6 +111,7 @@
65 res = req.get_response(fakes.wsgi_app())
66 res_dict = json.loads(res.body)
67 self.assertEqual(200, res.status_int)
68+ self.assertEqual('application/json', res.headers['Content-Type'])
69 self.assertEqual('value5', res_dict['key5'])
70
71 def test_show_meta_not_found(self):
72@@ -140,8 +143,19 @@
73 res = req.get_response(fakes.wsgi_app())
74 res_dict = json.loads(res.body)
75 self.assertEqual(200, res.status_int)
76+ self.assertEqual('application/json', res.headers['Content-Type'])
77 self.assertEqual('value1', res_dict['metadata']['key1'])
78
79+ def test_create_empty_body(self):
80+ self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
81+ return_create_instance_metadata)
82+ req = webob.Request.blank('/v1.1/servers/1/meta')
83+ req.environ['api.version'] = '1.1'
84+ req.method = 'POST'
85+ req.headers["content-type"] = "application/json"
86+ res = req.get_response(fakes.wsgi_app())
87+ self.assertEqual(400, res.status_int)
88+
89 def test_update_item(self):
90 self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
91 return_create_instance_metadata)
92@@ -152,9 +166,20 @@
93 req.headers["content-type"] = "application/json"
94 res = req.get_response(fakes.wsgi_app())
95 self.assertEqual(200, res.status_int)
96+ self.assertEqual('application/json', res.headers['Content-Type'])
97 res_dict = json.loads(res.body)
98 self.assertEqual('value1', res_dict['key1'])
99
100+ def test_update_item_empty_body(self):
101+ self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
102+ return_create_instance_metadata)
103+ req = webob.Request.blank('/v1.1/servers/1/meta/key1')
104+ req.environ['api.version'] = '1.1'
105+ req.method = 'PUT'
106+ req.headers["content-type"] = "application/json"
107+ res = req.get_response(fakes.wsgi_app())
108+ self.assertEqual(400, res.status_int)
109+
110 def test_update_item_too_many_keys(self):
111 self.stubs.Set(nova.db.api, 'instance_metadata_update_or_create',
112 return_create_instance_metadata)