From 8b98ecd05699633819d3b9e50a410e1b01e60968 Mon Sep 17 00:00:00 2001 From: chenying Date: Mon, 19 Jun 2017 19:31:06 +0800 Subject: [PATCH] Fix the error about the translation of KarborException message 1, Introduce FaultWrapper from cinder to make exceptions into karbor's wsgi faults. So that the messages can be translated, it can be shown in a WSGI response. 2, Avoid Forcing the Translation of Translatable Variables[1] [1]http://docs.openstack.org/developer/oslo.i18n/guidelines.html#avoid-forcing-the-translation-of-translatable-variables Closes-Bug: 1698757 Change-Id: Id85871b3a4c9ec7049f42e15141a678dba7316c2 --- etc/api-paste.ini | 7 +++- karbor/api/middleware/fault.py | 65 +++++++++++++++++++++++++++++++++ karbor/exception.py | 4 +- karbor/tests/unit/test_utils.py | 42 +++++++++++++++++++++ karbor/utils.py | 13 +++++++ 5 files changed, 127 insertions(+), 4 deletions(-) create mode 100644 karbor/api/middleware/fault.py create mode 100644 karbor/tests/unit/test_utils.py diff --git a/etc/api-paste.ini b/etc/api-paste.ini index fedc7a0e..d5d54068 100644 --- a/etc/api-paste.ini +++ b/etc/api-paste.ini @@ -9,12 +9,15 @@ use = egg:Paste#urlmap [composite:openstack_karbor_api_v1] use = call:karbor.api.middleware.auth:pipeline_factory -noauth = request_id catch_errors noauth apiv1 -keystone = request_id catch_errors authtoken keystonecontext apiv1 +noauth = request_id faultwrap noauth apiv1 +keystone = request_id faultwrap authtoken keystonecontext apiv1 [filter:request_id] paste.filter_factory = oslo_middleware:RequestId.factory +[filter:faultwrap] +paste.filter_factory = karbor.api.middleware.fault:FaultWrapper.factory + [filter:catch_errors] paste.filter_factory = oslo_middleware:CatchErrors.factory diff --git a/karbor/api/middleware/fault.py b/karbor/api/middleware/fault.py new file mode 100644 index 00000000..90568a44 --- /dev/null +++ b/karbor/api/middleware/fault.py @@ -0,0 +1,65 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from oslo_log import log as logging +import six +from six.moves import http_client +import webob.dec +import webob.exc + +from karbor.api.openstack import wsgi +from karbor import exception +from karbor import utils +from karbor.wsgi import common as base_wsgi + + +LOG = logging.getLogger(__name__) + + +class FaultWrapper(base_wsgi.Middleware): + """Calls down the middleware stack, making exceptions into faults.""" + + _status_to_type = {} + + @staticmethod + def status_to_type(status): + if not FaultWrapper._status_to_type: + for clazz in utils.walk_class_hierarchy(webob.exc.HTTPError): + FaultWrapper._status_to_type[clazz.code] = clazz + return FaultWrapper._status_to_type.get( + status, webob.exc.HTTPInternalServerError)() + + def _error(self, inner, req): + safe = getattr(inner, 'safe', False) + headers = getattr(inner, 'headers', None) + status = getattr(inner, 'code', http_client.INTERNAL_SERVER_ERROR) + if status is None: + status = http_client.INTERNAL_SERVER_ERROR + + msg_dict = dict(url=req.url, status=status) + LOG.info("%(url)s returned with HTTP %(status)d", msg_dict) + outer = self.status_to_type(status) + if headers: + outer.headers = headers + + if safe: + msg = (inner.msg if isinstance(inner, exception.KarborException) + else six.text_type(inner)) + outer.explanation = msg + return wsgi.Fault(outer) + + @webob.dec.wsgify(RequestClass=wsgi.Request) + def __call__(self, req): + try: + return req.get_response(self.application) + except Exception as ex: + return self._error(ex, req) diff --git a/karbor/exception.py b/karbor/exception.py index 3695cd00..15a03dad 100644 --- a/karbor/exception.py +++ b/karbor/exception.py @@ -83,7 +83,7 @@ class KarborException(Exception): message = _("An unknown exception occurred.") code = http_client.INTERNAL_SERVER_ERROR headers = {} - safe = False + safe = True def __init__(self, message=None, **kwargs): """Initiate the instance of KarborException @@ -132,7 +132,7 @@ class KarborException(Exception): super(KarborException, self).__init__(message) def __unicode__(self): - return six.text_type(self.msg) + return self.msg class NotAuthorized(KarborException): diff --git a/karbor/tests/unit/test_utils.py b/karbor/tests/unit/test_utils.py new file mode 100644 index 00000000..d0fc2b5e --- /dev/null +++ b/karbor/tests/unit/test_utils.py @@ -0,0 +1,42 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from karbor.tests import base + +from karbor import utils + + +class WalkClassHierarchyTestCase(base.TestCase): + def test_walk_class_hierarchy(self): + class A(object): + pass + + class B(A): + pass + + class C(A): + pass + + class D(B): + pass + + class E(A): + pass + + class_pairs = zip((D, B, E), + utils.walk_class_hierarchy(A, encountered=[C])) + for actual, expected in class_pairs: + self.assertEqual(expected, actual) + + class_pairs = zip((D, B, C, E), utils.walk_class_hierarchy(A)) + for actual, expected in class_pairs: + self.assertEqual(expected, actual) diff --git a/karbor/utils.py b/karbor/utils.py index 81c71816..3d5ac871 100644 --- a/karbor/utils.py +++ b/karbor/utils.py @@ -162,3 +162,16 @@ def validate_integer(value, name, min_value=None, max_value=None): {'value_name': name, 'max_value': max_value})) return value + + +def walk_class_hierarchy(clazz, encountered=None): + """Walk class hierarchy, yielding most derived classes first.""" + if not encountered: + encountered = [] + for subclass in clazz.__subclasses__(): + if subclass not in encountered: + encountered.append(subclass) + # drill down to leaves first + for subsubclass in walk_class_hierarchy(subclass, encountered): + yield subsubclass + yield subclass