Merge lp:~salvatore-orlando/neutron/bug814518 into lp:neutron/diablo

[Salvatore Orlando]

Fixing the issue causing a 400 error to not be raised if an invalid request body was specified.
The issue occurred in port creation, as the request body is optional; an invalid request body was simply ignored.

Also adding unit tests for creating a port without a request body in order to further improve code coverage.

[Brad Hall (bgh)]

Looks good to me

review: Approve

[dan wendlandt (danwent)]

Change looks good.

This does remind me of something I was wondering when looking at the code earlier, though. Why do we call _deserialize() on the body for each iteration of the loop?

review: Approve

[Salvatore Orlando (salvatore-orlando)]

The _deserialize call is for parsing the request body from xml/json into a dictionary.
It is invoked only for operations which require a request body.

I think the Openstack API has a more sophisticated mechanism in which header/body deserialization is performed withing the wsgi middleware. We can do something similar in Quantum, but I don't think it is a high priority task at this stage.

=== modified file 'quantum/api/api_common.py'
--- quantum/api/api_common.py	2011-07-21 16:24:24 +0000
+++ quantum/api/api_common.py	2011-08-01 14:41:39 +0000
@@ -43,7 +43,13 @@
                 des_body = self._deserialize(req.body,
                                              req.best_match_content_type())
                 data = des_body and des_body.get(self._resource_name, None)
-                param_value = data and data.get(param_name, None)
+                if not data:
+                    msg = ("Failed to parse request. Resource: " +
+                           self._resource_name + " not found in request body")
+                    for line in msg.split('\n'):
+                        LOG.error(line)
+                    raise exc.HTTPBadRequest(msg)
+                param_value = data.get(param_name, None)
             if not param_value:
                 # 2- parse request headers
                 # prepend param name with a 'x-' prefix

=== modified file 'tests/unit/test_api.py'
--- tests/unit/test_api.py	2011-07-29 23:39:45 +0000
+++ tests/unit/test_api.py	2011-08-01 14:41:39 +0000
@@ -285,6 +285,21 @@
         self.assertEqual(show_port_res.status_int, 430)
         LOG.debug("_test_show_port_portnotfound - format:%s - END", format)
 
+    def _test_create_port_noreqbody(self, format):
+        LOG.debug("_test_create_port_noreqbody - format:%s - START", format)
+        content_type = "application/%s" % format
+        network_id = self._create_network(format)
+        port_id = self._create_port(network_id, None, format,
+                                    custom_req_body='')
+        show_port_req = testlib.show_port_request(self.tenant_id,
+                                                  network_id, port_id, format)
+        show_port_res = show_port_req.get_response(self.api)
+        self.assertEqual(show_port_res.status_int, 200)
+        port_data = self._port_serializer.deserialize(
+                        show_port_res.body, content_type)
+        self.assertEqual(port_id, port_data['port']['id'])
+        LOG.debug("_test_create_port_noreqbody - format:%s - END", format)
+
     def _test_create_port(self, format):
         LOG.debug("_test_create_port - format:%s - START", format)
         content_type = "application/%s" % format
@@ -741,6 +756,12 @@
     def test_create_port_xml(self):
         self._test_create_port('xml')
 
+    def test_create_port_noreqbody_json(self):
+        self._test_create_port_noreqbody('json')
+
+    def test_create_port_noreqbody_xml(self):
+        self._test_create_port_noreqbody('xml')
+
     def test_create_port_networknotfound_json(self):
         self._test_create_port_networknotfound('json')
 

=== modified file 'tests/unit/testlib_api.py'
--- tests/unit/testlib_api.py	2011-07-18 22:51:22 +0000
+++ tests/unit/testlib_api.py	2011-08-01 14:41:39 +0000
@@ -77,9 +77,10 @@
     method = 'POST'
     path = "/tenants/%(tenant_id)s/networks/" \
            "%(network_id)s/ports.%(format)s" % locals()
-    data = custom_req_body or {'port': {'port-state': '%s' % port_state}}
+    data = custom_req_body or port_state and \
+           {'port': {'port-state': '%s' % port_state}}
     content_type = "application/%s" % format
-    body = Serializer().serialize(data, content_type)
+    body = data and Serializer().serialize(data, content_type)
     return create_request(path, body, content_type, method)