diff options
| -rw-r--r-- | horizon/tables/actions.py | 95 | ||||
| -rw-r--r-- | horizon/test/unit/tables/test_tables.py | 84 |
2 files changed, 175 insertions, 4 deletions
diff --git a/horizon/tables/actions.py b/horizon/tables/actions.py index cf6d3eca3..906560acc 100644 --- a/horizon/tables/actions.py +++ b/horizon/tables/actions.py @@ -15,6 +15,7 @@ from collections import defaultdict from collections import OrderedDict import copy +import functools import logging import types @@ -28,6 +29,7 @@ from django.utils.translation import ugettext_lazy as _ from django.utils.translation import ungettext_lazy import six +from horizon import exceptions from horizon import messages from horizon.utils import functions from horizon.utils import html @@ -775,10 +777,22 @@ class BatchAction(Action): {'action': self._get_action_name(past=True), 'datum_display': datum_display}) except Exception as ex: - # Handle the exception but silence it since we'll display - # an aggregate error message later. Otherwise we'd get - # multiple error messages displayed to the user. - action_failure.append(datum_display) + handled_exc = isinstance(ex, exceptions.HandledException) + if handled_exc: + # In case of HandledException, an error message should be + # handled in exceptions.handle() or other logic, + # so we don't need to handle the error message here. + # NOTE(amotoki): To raise HandledException from the logic, + # pass escalate=True and do not pass redirect argument + # to exceptions.handle(). + # If an exception is handled, the original exception object + # is stored in ex.wrapped[1]. + ex = ex.wrapped[1] + else: + # Handle the exception but silence it since we'll display + # an aggregate error message later. Otherwise we'd get + # multiple error messages displayed to the user. + action_failure.append(datum_display) action_description = ( self._get_action_name(past=True).lower(), datum_display) LOG.warning( @@ -879,6 +893,79 @@ class DeleteAction(BatchAction): """ +class handle_exception_with_detail_message(object): + """Decorator to allow special exception handling in BatchAction.action(). + + An exception from BatchAction.action() or DeleteAction.delete() is + normally caught by BatchAction.handle() and BatchAction.handle() displays + an aggregated error message. However, there are cases where we would like + to provide an error message which explains a failure reason if some + exception occurs so that users can understand situation better. + + This decorator allows us to do this kind of special handling easily. + This can be applied to BatchAction.action() and DeleteAction.delete() + methods. + + :param normal_log_message: Log message template when an exception other + than ``target_exception`` is detected. Keyword substituion "%(id)s" + and "%(exc)s" can be used. + + :param target_exception: Exception class should be handled specially. + If this exception is caught, a log message will be logged using + ``target_log_message`` and a user visible will be shown using + ``target_user_message``. In this case, an aggregated error message + generated by BatchAction.handle() does not include an object which + causes this exception. + + :param target_log_message: Log message template when an exception specified + in ``target_exception`` is detected. Keyword substituion "%(id)s" + and "%(exc)s" can be used. + + :param target_user_message: User visible message template when an exception + specified in ``target_exception`` is detected. It is recommended to + use an internationalized string. Keyword substituion "%(name)s" + and "%(exc)s" can be used. + + :param logger_name: (optional) Logger name to be used. + The usual pattern is to pass __name__ of a caller. + This allows us to show a module name of a caller in a logged message. + """ + + def __init__(self, normal_log_message, target_exception, + target_log_message, target_user_message, logger_name=None): + self.logger = logging.getLogger(logger_name or __name__) + self.normal_log_message = normal_log_message + self.target_exception = target_exception + self.target_log_message = target_log_message + self.target_user_message = target_user_message + + def __call__(self, fn): + @functools.wraps(fn) + def decorated(instance, request, obj_id): + try: + fn(instance, request, obj_id) + except self.target_exception as e: + self.logger.info(self.target_log_message, + {'id': obj_id, 'exc': e}) + obj = instance.table.get_object_by_id(obj_id) + name = instance.table.get_object_display(obj) + msg = self.target_user_message % {'name': name, 'exc': e} + # 'escalate=True' is required to notify the caller + # (DeleteAction) of the failure. exceptions.handle() will + # raise a wrapped exception of HandledException and BatchAction + # will handle it. 'redirect' should not be passed here as + # 'redirect' has a priority over 'escalate' argument. + exceptions.handle(request, msg, escalate=True) + except Exception as e: + self.logger.info(self.normal_log_message, + {'id': obj_id, 'exc': e}) + # NOTE: No exception handling is required here because + # BatchAction.handle() does it. What we need to do is + # just to re-raise the exception. + raise + return decorated + + class Deprecated(type): # TODO(lcastell) Replace class with similar functionality from # oslo_log.versionutils when it's finally added in 11.0 diff --git a/horizon/test/unit/tables/test_tables.py b/horizon/test/unit/tables/test_tables.py index eaac02069..77ed4a454 100644 --- a/horizon/test/unit/tables/test_tables.py +++ b/horizon/test/unit/tables/test_tables.py @@ -15,6 +15,9 @@ # License for the specific language governing permissions and limitations # under the License. +import unittest +import uuid + from django.core.urlresolvers import reverse from django import forms from django import http @@ -23,10 +26,13 @@ from django.template import defaultfilters from django.test.utils import override_settings from django.utils.translation import ungettext_lazy +import mock from mox3.mox import IsA import six +from horizon import exceptions from horizon import tables +from horizon.tables import actions from horizon.tables import formset as table_formset from horizon.tables import views as table_views from horizon.test import helpers as test @@ -1603,3 +1609,81 @@ class FormsetTableTests(test.TestCase): form_data = form.initial self.assertEqual('object_1', form_data['name']) self.assertEqual(2, form_data['value']) + + +class MyException(Exception): + pass + + +class OtherException(Exception): + pass + + +class BatchActionDecoratorTests(unittest.TestCase): + + def setUp(self): + obj = uuid.uuid4() + self.obj_id = 'id-%s' % str(obj) + self.obj_name = 'name-%s' % str(obj) + table = mock.Mock() + table.get_object_by_id.return_value = obj + table.get_object_display.return_value = self.obj_name + + # getLogger is called insdie handle_exception_with_detail_message + # decorator, so this needs to be mocked before using the decorator. + self.logger = mock.Mock() + with mock.patch('logging.getLogger', + return_value=self.logger) as mock_getlogger: + class MyAction(object): + def __init__(self, table): + self.table = table + + @actions.handle_exception_with_detail_message( + 'normal log message %(id)s %(exc)s', + MyException, + 'target log message %(id)s %(exc)s', + 'target user message %(name)s %(exc)s', + 'mylogger') + def action(self, request, obj_id): + self._to_be_mocked() + + # This is required because if mock.patch replaces + # a decorated method. We are testing a decorated method + # so we need a separate method to mock, + def _to_be_mocked(self): + pass + + self.action = MyAction(table) + self.mock_getlogger = mock_getlogger + + def test_normal_exception(self): + myexc = OtherException() + with mock.patch.object(self.action, '_to_be_mocked', + side_effect=myexc): + self.assertRaises(OtherException, + self.action.action, + mock.sentinel.request, self.obj_id) + self.mock_getlogger.assert_called_once_with('mylogger') + self.logger.info.assert_called_once_with( + 'normal log message %(id)s %(exc)s', + {'id': self.obj_id, 'exc': myexc}) + + def test_target_exception(self): + myexc = MyException() + handled = exceptions.HandledException(myexc) + with mock.patch.object(self.action, '_to_be_mocked', + side_effect=myexc), \ + mock.patch.object(exceptions, 'handle', + side_effect=handled) as mocked_handle: + self.assertRaises(exceptions.HandledException, + self.action.action, + mock.sentinel.request, self.obj_id) + self.mock_getlogger.assert_called_once_with('mylogger') + self.logger.info.assert_called_once_with( + 'target log message %(id)s %(exc)s', + {'id': self.obj_id, 'exc': myexc}) + mocked_handle.assert_called_once_with( + mock.sentinel.request, + 'target user message %(name)s %(exc)s' % + {'name': self.obj_name, 'exc': myexc}, + escalate=True) |