From cc832e5d5865cc94f226d1d88f10b431f221f1f1 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Tue, 17 Jan 2012 01:23:00 +0000 Subject: [PATCH] bp/api-error-codes Restructured API error codes for Quantum API v1.1 This changeset provides the following changes: - Only standard HTTP errors for Quantum API v1.1 - Customized fault response body formatting according to API version - Changes to unit tests to deal with version specific status codes NOTE: Client side changes are not in this branch. They should be implemented within bp/quantum-client-1.1. NOTE-2: Fixing references to QuantumHTTPErrors in extensions framework Change-Id: I56f50b5c59d73fd6f02905106f0c2a3c8341a741 --- quantum/api/api_common.py | 20 ++++- quantum/api/faults.py | 106 +++++++++++++++++++++++++- quantum/api/networks.py | 11 +-- quantum/tests/unit/_test_api.py | 56 +++++++++----- quantum/tests/unit/test_api.py | 14 ++++ quantum/tests/unit/test_extensions.py | 2 +- quantum/wsgi.py | 36 +++++---- 7 files changed, 202 insertions(+), 43 deletions(-) diff --git a/quantum/api/api_common.py b/quantum/api/api_common.py index 7c9235116..82adc09cb 100644 --- a/quantum/api/api_common.py +++ b/quantum/api/api_common.py @@ -59,6 +59,8 @@ def create_resource(version, controller_dict): metadata = controller_dict[version][1] # and the third element the xml namespace xmlns = controller_dict[version][2] + # and also the function for building the fault body + fault_body_function = faults.fault_body_function(version) headers_serializer = HeaderSerializer() xml_serializer = wsgi.XMLDictSerializer(metadata, xmlns) @@ -79,21 +81,35 @@ def create_resource(version, controller_dict): serializer = wsgi.ResponseSerializer(body_serializers, headers_serializer) deserializer = wsgi.RequestDeserializer(body_deserializers) - return wsgi.Resource(controller, deserializer, serializer) + return wsgi.Resource(controller, + fault_body_function, + deserializer, + serializer) def APIFaultWrapper(errors=None): + quantum_error_dict = { + '1.0': faults.Quantum10HTTPError, + '1.1': faults.Quantum11HTTPError + } + def wrapper(func, **kwargs): def the_func(*args, **kwargs): try: + # Grab API version from type of controller + controller = args[0] + version = controller.version return func(*args, **kwargs) except Exception as e: if errors is not None and type(e) in errors: - raise faults.QuantumHTTPError(e) + # Version-specific behaviour + quantum_error_class = quantum_error_dict[version] + raise quantum_error_class(e) # otherwise just re-raise raise + the_func.__name__ = func.__name__ return the_func diff --git a/quantum/api/faults.py b/quantum/api/faults.py index 8ca15f300..c759a69da 100644 --- a/quantum/api/faults.py +++ b/quantum/api/faults.py @@ -29,7 +29,60 @@ _ALREADYATTACHED_EXPL = 'The resource is already attached to another port' _NOTIMPLEMENTED_EXPL = 'Not implemented' -class QuantumHTTPError(webob.exc.HTTPClientError): +def fault_body_function_v10(wrapped_exc): + """ This function creates the contents of the body for a fault + response for Quantum API v1.0. + + :param wrapped_exc: Exception thrown by the Quantum service + :type wrapped_exc: quantum.common.exceptions.QuantumException + :returns: response body contents and serialization metadata + :rtype: tuple + """ + code = wrapped_exc.status_int + fault_name = hasattr(wrapped_exc, 'title') and \ + wrapped_exc.title or "quantumServiceFault" + fault_data = { + fault_name: { + 'code': code, + 'message': wrapped_exc.explanation, + 'detail': str(wrapped_exc.detail)}} + metadata = {'attributes': {fault_name: ['code']}} + return fault_data, metadata + + +def fault_body_function_v11(wrapped_exc): + """ This function creates the contents of the body for a fault + response for Quantum API v1.0. + + :param wrapped_exc: Exception thrown by the Quantum service + :type wrapped_exc: quantum.common.exceptions.QuantumException + :returns: response body contents and serialization metadata + :rtype: tuple + """ + fault_name = hasattr(wrapped_exc, 'type') and \ + wrapped_exc.type or "QuantumServiceFault" + # Ensure first letter is capital + fault_name = fault_name[0].upper() + fault_name[1:] + fault_data = { + 'QuantumError': { + 'type': fault_name, + 'message': wrapped_exc.explanation, + 'detail': str(wrapped_exc.detail)}} + # Metadata not required for v11 + return fault_data, None + + +def fault_body_function(version): + # dict mapping API version to functions for building the + # fault response body + fault_body_function_dict = { + '1.0': fault_body_function_v10, + '1.1': fault_body_function_v11 + } + return fault_body_function_dict.get(version, None) + + +class Quantum10HTTPError(webob.exc.HTTPClientError): _fault_dict = { exceptions.NetworkNotFound: { @@ -76,3 +129,54 @@ class QuantumHTTPError(webob.exc.HTTPClientError): self.title = _fault_data['title'] self.explanation = _fault_data['explanation'] super(webob.exc.HTTPClientError, self).__init__(inner_exc) + + +class Quantum11HTTPError(webob.exc.HTTPClientError): + + _fault_dict = { + exceptions.NetworkNotFound: { + 'code': webob.exc.HTTPNotFound.code, + 'title': webob.exc.HTTPNotFound.title, + 'type': 'NetworkNotFound', + 'explanation': _NETNOTFOUND_EXPL + }, + exceptions.NetworkInUse: { + 'code': webob.exc.HTTPConflict.code, + 'title': webob.exc.HTTPConflict.title, + 'type': 'NetworkInUse', + 'explanation': _NETINUSE_EXPL + }, + exceptions.PortNotFound: { + 'code': webob.exc.HTTPNotFound.code, + 'title': webob.exc.HTTPNotFound.title, + 'type': 'PortNotFound', + 'explanation': _PORTNOTFOUND_EXPL + }, + exceptions.StateInvalid: { + 'code': webob.exc.HTTPBadRequest.code, + 'title': webob.exc.HTTPBadRequest.title, + 'type': 'RequestedStateInvalid', + 'explanation': _STATEINVALID_EXPL + }, + exceptions.PortInUse: { + 'code': webob.exc.HTTPConflict.code, + 'title': webob.exc.HTTPConflict.title, + 'type': 'PortInUse', + 'explanation': _PORTINUSE_EXPL + }, + exceptions.AlreadyAttached: { + 'code': webob.exc.HTTPConflict.code, + 'title': webob.exc.HTTPConflict.title, + 'type': 'AlreadyAttached', + 'explanation': _ALREADYATTACHED_EXPL + } + } + + def __init__(self, inner_exc): + _fault_data = self._fault_dict.get(type(inner_exc), None) + if _fault_data: + self.code = _fault_data['code'] + self.title = _fault_data['title'] + self.explanation = _fault_data['explanation'] + self.type = _fault_data['type'] + super(webob.exc.HTTPClientError, self).__init__(inner_exc) diff --git a/quantum/api/networks.py b/quantum/api/networks.py index 50b1e0aa7..4229b6020 100644 --- a/quantum/api/networks.py +++ b/quantum/api/networks.py @@ -76,16 +76,13 @@ class Controller(common.QuantumController): """ Returns a list of network ids """ return self._items(request, tenant_id) - @common.APIFaultWrapper() + @common.APIFaultWrapper([exception.NetworkNotFound]) def show(self, request, tenant_id, id): """ Returns network details for the given network id """ - try: - return self._item(request, tenant_id, id, - net_details=True, port_details=False) - except exception.NetworkNotFound as e: - raise faults.QuantumHTTPError(e) + return self._item(request, tenant_id, id, + net_details=True, port_details=False) - @common.APIFaultWrapper() + @common.APIFaultWrapper([exception.NetworkNotFound]) def detail(self, request, **kwargs): tenant_id = kwargs.get('tenant_id') network_id = kwargs.get('id') diff --git a/quantum/tests/unit/_test_api.py b/quantum/tests/unit/_test_api.py index 629c246b4..fa01fa137 100644 --- a/quantum/tests/unit/_test_api.py +++ b/quantum/tests/unit/_test_api.py @@ -184,7 +184,8 @@ class AbstractAPITest(unittest.TestCase): "A_BAD_ID", fmt) show_network_res = show_network_req.get_response(self.api) - self.assertEqual(show_network_res.status_int, 420) + self.assertEqual(show_network_res.status_int, + self._network_not_found_code) LOG.debug("_test_show_network_not_found - fmt:%s - END", fmt) def _test_rename_network(self, fmt): @@ -232,7 +233,8 @@ class AbstractAPITest(unittest.TestCase): new_name, fmt) update_network_res = update_network_req.get_response(self.api) - self.assertEqual(update_network_res.status_int, 420) + self.assertEqual(update_network_res.status_int, + self._network_not_found_code) LOG.debug("_test_rename_network_not_found - fmt:%s - END", fmt) @@ -279,7 +281,8 @@ class AbstractAPITest(unittest.TestCase): network_id, fmt) delete_network_res = delete_network_req.get_response(self.api) - self.assertEqual(delete_network_res.status_int, 421) + self.assertEqual(delete_network_res.status_int, + self._network_in_use_code) LOG.debug("_test_delete_network_in_use - fmt:%s - END", fmt) def _test_delete_network_with_unattached_port(self, fmt): @@ -324,7 +327,8 @@ class AbstractAPITest(unittest.TestCase): list_port_req = testlib.port_list_request(self.tenant_id, "A_BAD_ID", fmt) list_port_res = list_port_req.get_response(self.api) - self.assertEqual(list_port_res.status_int, 420) + self.assertEqual(list_port_res.status_int, + self._network_not_found_code) LOG.debug("_test_list_ports_networknotfound - fmt:%s - END", fmt) def _test_list_ports_detail(self, fmt): @@ -413,7 +417,8 @@ class AbstractAPITest(unittest.TestCase): "A_BAD_ID", port_id, fmt) show_port_res = show_port_req.get_response(self.api) - self.assertEqual(show_port_res.status_int, 420) + self.assertEqual(show_port_res.status_int, + self._network_not_found_code) LOG.debug("_test_show_port_networknotfound - fmt:%s - END", fmt) @@ -425,7 +430,8 @@ class AbstractAPITest(unittest.TestCase): "A_BAD_ID", fmt) show_port_res = show_port_req.get_response(self.api) - self.assertEqual(show_port_res.status_int, 430) + self.assertEqual(show_port_res.status_int, + self._port_not_found_code) LOG.debug("_test_show_port_portnotfound - fmt:%s - END", fmt) def _test_create_port_noreqbody(self, fmt): @@ -463,7 +469,7 @@ class AbstractAPITest(unittest.TestCase): fmt) port_state = "ACTIVE" self._create_port("A_BAD_ID", port_state, fmt, - expected_res_status=420) + expected_res_status=self._network_not_found_code) LOG.debug("_test_create_port_networknotfound - fmt:%s - END", fmt) @@ -520,7 +526,8 @@ class AbstractAPITest(unittest.TestCase): network_id, port_id, fmt) delete_port_res = delete_port_req.get_response(self.api) - self.assertEqual(delete_port_res.status_int, 432) + self.assertEqual(delete_port_res.status_int, + self._port_in_use_code) LOG.debug("_test_delete_port_in_use - fmt:%s - END", fmt) def _test_delete_port_with_bad_id(self, fmt): @@ -534,7 +541,8 @@ class AbstractAPITest(unittest.TestCase): network_id, "A_BAD_ID", fmt) delete_port_res = delete_port_req.get_response(self.api) - self.assertEqual(delete_port_res.status_int, 430) + self.assertEqual(delete_port_res.status_int, + self._port_not_found_code) LOG.debug("_test_delete_port_with_bad_id - fmt:%s - END", fmt) def _test_delete_port_networknotfound(self, fmt): @@ -547,7 +555,8 @@ class AbstractAPITest(unittest.TestCase): "A_BAD_ID", port_id, fmt) delete_port_res = delete_port_req.get_response(self.api) - self.assertEqual(delete_port_res.status_int, 420) + self.assertEqual(delete_port_res.status_int, + self._network_not_found_code) LOG.debug("_test_delete_port_networknotfound - fmt:%s - END", fmt) @@ -603,7 +612,8 @@ class AbstractAPITest(unittest.TestCase): new_port_state, fmt) update_port_res = update_port_req.get_response(self.api) - self.assertEqual(update_port_res.status_int, 420) + self.assertEqual(update_port_res.status_int, + self._network_not_found_code) LOG.debug("_test_set_port_state_networknotfound - fmt:%s - END", fmt) @@ -620,7 +630,8 @@ class AbstractAPITest(unittest.TestCase): new_port_state, fmt) update_port_res = update_port_req.get_response(self.api) - self.assertEqual(update_port_res.status_int, 430) + self.assertEqual(update_port_res.status_int, + self._port_not_found_code) LOG.debug("_test_set_port_state_portnotfound - fmt:%s - END", fmt) @@ -636,7 +647,8 @@ class AbstractAPITest(unittest.TestCase): new_port_state, fmt) update_port_res = update_port_req.get_response(self.api) - self.assertEqual(update_port_res.status_int, 431) + self.assertEqual(update_port_res.status_int, + self._port_state_invalid_code) LOG.debug("_test_set_port_state_stateinvalid - fmt:%s - END", fmt) @@ -691,7 +703,8 @@ class AbstractAPITest(unittest.TestCase): port_id, fmt) get_attachment_res = get_attachment_req.get_response(self.api) - self.assertEqual(get_attachment_res.status_int, 420) + self.assertEqual(get_attachment_res.status_int, + self._network_not_found_code) LOG.debug("_test_show_attachment_networknotfound - fmt:%s - END", fmt) @@ -706,7 +719,8 @@ class AbstractAPITest(unittest.TestCase): "A_BAD_ID", fmt) get_attachment_res = get_attachment_req.get_response(self.api) - self.assertEqual(get_attachment_res.status_int, 430) + self.assertEqual(get_attachment_res.status_int, + self._port_not_found_code) LOG.debug("_test_show_attachment_portnotfound - fmt:%s - END", fmt) @@ -738,7 +752,8 @@ class AbstractAPITest(unittest.TestCase): interface_id, fmt) put_attachment_res = put_attachment_req.get_response(self.api) - self.assertEqual(put_attachment_res.status_int, 420) + self.assertEqual(put_attachment_res.status_int, + self._network_not_found_code) LOG.debug("_test_put_attachment_networknotfound - fmt:%s - END", fmt) @@ -755,7 +770,8 @@ class AbstractAPITest(unittest.TestCase): interface_id, fmt) put_attachment_res = put_attachment_req.get_response(self.api) - self.assertEqual(put_attachment_res.status_int, 430) + self.assertEqual(put_attachment_res.status_int, + self._port_not_found_code) LOG.debug("_test_put_attachment_portnotfound - fmt:%s - END", fmt) @@ -791,7 +807,8 @@ class AbstractAPITest(unittest.TestCase): port_id, fmt) del_attachment_res = del_attachment_req.get_response(self.api) - self.assertEqual(del_attachment_res.status_int, 420) + self.assertEqual(del_attachment_res.status_int, + self._network_not_found_code) LOG.debug("_test_delete_attachment_networknotfound -" \ " fmt:%s - END", fmt) @@ -806,7 +823,8 @@ class AbstractAPITest(unittest.TestCase): "A_BAD_ID", fmt) del_attachment_res = del_attachment_req.get_response(self.api) - self.assertEqual(del_attachment_res.status_int, 430) + self.assertEqual(del_attachment_res.status_int, + self._port_not_found_code) LOG.debug("_test_delete_attachment_portnotfound - " \ "fmt:%s - END", fmt) diff --git a/quantum/tests/unit/test_api.py b/quantum/tests/unit/test_api.py index 57c5b4cb4..57ed6503f 100644 --- a/quantum/tests/unit/test_api.py +++ b/quantum/tests/unit/test_api.py @@ -16,6 +16,8 @@ # under the License. # @author: Salvatore Orlando, Citrix Systems +from webob import exc + import quantum.api.attachments as atts import quantum.api.networks as nets import quantum.api.ports as ports @@ -53,6 +55,12 @@ class APITestV10(test_api.AbstractAPITest): {test_api.NETS: nets.ControllerV10._serialization_metadata, test_api.PORTS: ports.ControllerV10._serialization_metadata, test_api.ATTS: atts.ControllerV10._serialization_metadata}) + self._network_not_found_code = 420 + self._network_in_use_code = 421 + self._port_not_found_code = 430 + self._port_state_invalid_code = 431 + self._port_in_use_code = 432 + self._already_attached_code = 440 class APITestV11(test_api.AbstractAPITest): @@ -93,3 +101,9 @@ class APITestV11(test_api.AbstractAPITest): {test_api.NETS: nets.ControllerV11._serialization_metadata, test_api.PORTS: ports.ControllerV11._serialization_metadata, test_api.ATTS: atts.ControllerV11._serialization_metadata}) + self._network_not_found_code = exc.HTTPNotFound.code + self._network_in_use_code = exc.HTTPConflict.code + self._port_not_found_code = exc.HTTPNotFound.code + self._port_state_invalid_code = exc.HTTPBadRequest.code + self._port_in_use_code = exc.HTTPConflict.code + self._already_attached_code = exc.HTTPConflict.code diff --git a/quantum/tests/unit/test_extensions.py b/quantum/tests/unit/test_extensions.py index 2eaaeb938..48730b184 100644 --- a/quantum/tests/unit/test_extensions.py +++ b/quantum/tests/unit/test_extensions.py @@ -67,7 +67,7 @@ class ResourceExtensionTest(unittest.TestCase): return {'data': {'id': id}} def notimplemented_function(self, request, id): - return faults.QuantumHTTPError( + return faults.Quantum10HTTPError( exceptions.NotImplementedError("notimplemented_function")) def custom_member_action(self, request, id): diff --git a/quantum/wsgi.py b/quantum/wsgi.py index 2efbf56c2..dfd1bd740 100644 --- a/quantum/wsgi.py +++ b/quantum/wsgi.py @@ -701,18 +701,23 @@ class Resource(Application): """ - def __init__(self, controller, deserializer=None, serializer=None): + def __init__(self, controller, fault_body_function, + deserializer=None, serializer=None): """ :param controller: object that implement methods created by routes lib :param deserializer: object that can serialize the output of a controller into a webob response :param serializer: object that can deserialize a webob request into necessary pieces + :param fault_body_function: a function that will build the response + body for HTTP errors raised by operations + on this resource object """ self.controller = controller self.deserializer = deserializer or RequestDeserializer() self.serializer = serializer or ResponseSerializer() + self._fault_body_function = fault_body_function # use serializer's xmlns for populating Fault generator xmlns xml_serializer = self.serializer.body_serializers['application/xml'] if hasattr(xml_serializer, 'xmlns'): @@ -742,7 +747,9 @@ class Resource(Application): action_result = self.dispatch(request, action, args) except webob.exc.HTTPException as ex: LOG.info(_("HTTP exception thrown: %s"), unicode(ex)) - action_result = Fault(ex, self._xmlns) + action_result = Fault(ex, + self._xmlns, + self._fault_body_function) if isinstance(action_result, dict) or action_result is None: response = self.serializer.serialize(action_result, @@ -776,29 +783,32 @@ class Resource(Application): self._xmlns) +def _default_body_function(wrapped_exc): + code = wrapped_exc.status_int + fault_data = { + 'Error': { + 'code': code, + 'message': wrapped_exc.explanation}} + # 'code' is an attribute on the fault tag itself + metadata = {'attributes': {'Error': 'code'}} + return fault_data, metadata + + class Fault(webob.exc.HTTPException): """ Generates an HTTP response from a webob HTTP exception""" - def __init__(self, exception, xmlns=None): + def __init__(self, exception, xmlns=None, body_function=None): """Creates a Fault for the given webob.exc.exception.""" self.wrapped_exc = exception self.status_int = self.wrapped_exc.status_int self._xmlns = xmlns + self._body_function = body_function or _default_body_function @webob.dec.wsgify(RequestClass=Request) def __call__(self, req): """Generate a WSGI response based on the exception passed to ctor.""" # Replace the body with fault details. - code = self.wrapped_exc.status_int - fault_name = hasattr(self.wrapped_exc, 'title') and \ - self.wrapped_exc.title or "quantumServiceFault" - fault_data = { - fault_name: { - 'code': code, - 'message': self.wrapped_exc.explanation, - 'detail': str(self.wrapped_exc.detail)}} - # 'code' is an attribute on the fault tag itself - metadata = {'application/xml': {'attributes': {fault_name: 'code'}}} + fault_data, metadata = self._body_function(self.wrapped_exc) xml_serializer = XMLDictSerializer(metadata, self._xmlns) content_type = req.best_match_content_type() serializer = { -- 2.45.2