Merge "Fix API exceptions handling for message and error code"

climate/api/utils.py

@@ -211,21 +211,23 @@ def internal_error(status_code, descr, exc=None):
 
 def bad_request(error):
     """Called if Climate exception occurred."""
-    error_code = 400
+    if not error.code:
+        error.code = 400
 
     LOG.debug("Validation Error occurred: "
               "error_code=%s, error_message=%s, error_name=%s",
-              error_code, error.message, error.code)
+              error.code, error.message, error.code)
 
-    return render_error_message(error_code, error.message, error.code)
+    return render_error_message(error.code, error.message, error.code)
 
 
 def not_found(error):
     """Called if object was not found."""
-    error_code = 404
+    if not error.code:
+        error.code = 404
 
     LOG.debug("Not Found exception occurred: "
               "error_code=%s, error_message=%s, error_name=%s",
-              error_code, error.message, error.code)
+              error.code, error.message, error.code)
 
-    return render_error_message(error_code, error.message, error.code)
+    return render_error_message(error.code, error.message, error.code)

climate/api/validation.py

@@ -49,10 +49,7 @@ def check_exists(get_function, object_id=None, **get_args):
             except exceptions.NotFound:
                 obj = None
             if obj is None:
-                e = exceptions.NotFound(
-                    get_kwargs,
-                    template='Object with %s not found',
-                )
+                e = exceptions.NotFound(object=get_kwargs)
                 return api_utils.not_found(e)
 
             return func(*args, **kwargs)

climate/exceptions.py

@@ -1,4 +1,5 @@
 # Copyright (c) 2013 Mirantis Inc.
+# Copyright (c) 2013 Bull.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -13,38 +14,57 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from climate.openstack.common.gettextutils import _  # noqa
+from climate.openstack.common import log as logging
+
+
+LOG = logging.getLogger(__name__)
+
 
 class ClimateException(Exception):
-    """Base Exception for the Climate.
+    """Base Climate Exception.
 
     To correctly use this class, inherit from it and define
-    a 'message' and 'code' properties.
+    a 'msg_fmt' and 'code' properties.
     """
-    template = "An unknown exception occurred"
-    code = "UNKNOWN_EXCEPTION"
+    msg_fmt = _("An unknown exception occurred")
+    code = 500
 
-    def __init__(self, *args, **kwargs):
-        super(ClimateException, self).__init__(*args)
-        template = kwargs.pop('template', None)
-        if template:
-            self.template = template
+    def __init__(self, message=None, **kwargs):
+        self.kwargs = kwargs
 
-    def __str__(self):
-        return self.template % self.args
+        if 'code' not in self.kwargs:
+            self.kwargs['code'] = self.code
 
-    def __repr__(self):
-        if self.template != type(self).template:
-            tmpl = ", template=%r" % (self.template,)
-        else:
-            tmpl = ""
-        args = ", ".join(map(repr, self.args))
-        return "%s(%s%s)" % (type(self).__name__, args, tmpl)
+        if not message:
+            try:
+                message = self.msg_fmt % kwargs
+            except KeyError:
+                # kwargs doesn't match a variable in the message
+                # log the issue and the kwargs
+                LOG.exception(_('Exception in string format operation'))
+                for name, value in kwargs.iteritems():
+                    LOG.error("%s: %s" % (name, value))
+
+                message = self.msg_fmt
+
+        super(ClimateException, self).__init__(message)
 
 
 class NotFound(ClimateException):
     """Object not found exception."""
-    template = "Object not found"
-    code = "NOT_FOUND"
+    msg_fmt = _("Object with %(object)s not found")
+    code = 404
 
-    def __init__(self, *args, **kwargs):
-        super(NotFound, self).__init__(*args, **kwargs)
+
+class NotAuthorized(ClimateException):
+    msg_fmt = _("Not authorized")
+    code = 403
+
+
+class PolicyNotAuthorized(NotAuthorized):
+    msg_fmt = _("Policy doesn't allow %(action)s to be performed")
+
+
+class ConfigNotFound(ClimateException):
+    msg_fmt = _("Could not find config at %(path)s")

climate/tests/api/test_utils.py

@@ -122,11 +122,21 @@ class UtilsTestCase(tests.TestCase):
     def test_bad_request(self):
         error_message = self.patch(self.utils, 'render_error_message')
         self.utils.bad_request(self.error)
-        error_message.assert_called_once_with(
-            400, 'message', 'code')
+        error_message.assert_called_once_with('code', 'message', 'code')
+
+    def test_bad_request_with_default_errorcode(self):
+        error_message = self.patch(self.utils, 'render_error_message')
+        error = Error("message")
+        self.utils.bad_request(error)
+        error_message.assert_called_once_with(400, 'message', 400)
 
     def test_not_found(self):
         error_message = self.patch(self.utils, 'render_error_message')
         self.utils.not_found(self.error)
-        error_message.assert_called_once_with(
-            404, 'message', 'code')
+        error_message.assert_called_once_with('code', 'message', 'code')
+
+    def test_not_found_with_default_errorcode(self):
+        error_message = self.patch(self.utils, 'render_error_message')
+        error = Error("message")
+        self.utils.not_found(error)
+        error_message.assert_called_once_with(404, 'message', 404)

climate/tests/test_exceptions.py

@@ -0,0 +1,66 @@
+# Copyright (c) 2013 Bull.
+#
+# 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 climate import exceptions
+from climate import tests
+
+
+class ClimateExceptionTestCase(tests.TestCase):
+    def test_default_error_msg(self):
+        class FakeClimateException(exceptions.ClimateException):
+            msg_fmt = "default message"
+
+        exc = FakeClimateException()
+        self.assertEqual(unicode(exc), 'default message')
+
+    def test_error_msg(self):
+        self.assertEqual(unicode(exceptions.ClimateException('test')),
+                         'test')
+
+    def test_default_error_msg_with_kwargs(self):
+        class FakeClimateException(exceptions.ClimateException):
+            msg_fmt = "default message: %(code)s"
+
+        exc = FakeClimateException(code=500)
+        self.assertEqual(unicode(exc), 'default message: 500')
+        self.assertEqual(exc.message, 'default message: 500')
+
+    def test_error_msg_exception_with_kwargs(self):
+        class FakeClimateException(exceptions.ClimateException):
+            msg_fmt = "default message: %(mispelled_code)s"
+
+        exc = FakeClimateException(code=500, mispelled_code='blah')
+        self.assertEqual(unicode(exc), 'default message: blah')
+        self.assertEqual(exc.message, 'default message: blah')
+
+    def test_default_error_code(self):
+        class FakeClimateException(exceptions.ClimateException):
+            code = 404
+
+        exc = FakeClimateException()
+        self.assertEqual(exc.kwargs['code'], 404)
+
+    def test_error_code_from_kwarg(self):
+        class FakeClimateException(exceptions.ClimateException):
+            code = 500
+
+        exc = FakeClimateException(code=404)
+        self.assertEqual(exc.kwargs['code'], 404)
+
+    def test_policynotauthorized_exception(self):
+        exc = exceptions.PolicyNotAuthorized(action='foo')
+        self.assertEqual(unicode(exc.message),
+                         "Policy doesn't allow foo to be performed")
+        self.assertEqual(exc.kwargs['code'], 403)