From 576dead9ffeab35f7f5ee9b7b5d0145ca281a04d Mon Sep 17 00:00:00 2001 From: Ruby Loo Date: Thu, 30 Jul 2015 13:51:08 +0000 Subject: [PATCH] Save and re-raise exception When an exception is raised, we do "some stuff" and re-raise that original exception. This uses oslo_util's excutils.save_and_reraise_exception to do this, to handle the case where "some stuff" might cause the exception context to be cleared (which would have resulted in None being attempted to be re-raised). Change-Id: I91234797ab71399f6f9bac3a3ab35a6eeb2a87f3 --- oslo_versionedobjects/base.py | 41 ++++++++++++--------- oslo_versionedobjects/tests/test_fields.py | 31 ++++++++++++++++ oslo_versionedobjects/tests/test_objects.py | 8 ++++ 3 files changed, 62 insertions(+), 18 deletions(-) diff --git a/oslo_versionedobjects/base.py b/oslo_versionedobjects/base.py index e65a99ac..b103a682 100644 --- a/oslo_versionedobjects/base.py +++ b/oslo_versionedobjects/base.py @@ -20,6 +20,7 @@ import copy import logging import oslo_messaging as messaging +from oslo_utils import excutils import six from oslo_versionedobjects._i18n import _, _LE @@ -80,9 +81,10 @@ def _make_class_properties(cls): try: return setattr(self, attrname, field_value) except Exception: - attr = "%s.%s" % (self.obj_name(), name) - LOG.exception(_LE('Error setting %(attr)s'), {'attr': attr}) - raise + with excutils.save_and_reraise_exception(): + attr = "%s.%s" % (self.obj_name(), name) + LOG.exception(_LE('Error setting %(attr)s'), + {'attr': attr}) def deleter(self, name=name): attrname = _get_attrname(name) @@ -839,21 +841,24 @@ class VersionedObjectSerializer(messaging.NoOpSerializer): return self.OBJ_BASE_CLASS.obj_from_primitive( objprim, context=context) except exception.IncompatibleObjectVersion: - verkey = '%s.version' % self.OBJ_BASE_CLASS.OBJ_SERIAL_NAMESPACE - objver = objprim[verkey] - if objver.count('.') == 2: - # NOTE(danms): For our purposes, the .z part of the version - # should be safe to accept without requiring a backport - objprim[verkey] = \ - '.'.join(objver.split('.')[:2]) - return self._process_object(context, objprim) - namekey = '%s.name' % self.OBJ_BASE_CLASS.OBJ_SERIAL_NAMESPACE - objname = objprim[namekey] - supported = VersionedObjectRegistry.obj_classes().get(objname, []) - if self.OBJ_BASE_CLASS.indirection_api and supported: - return self._do_backport(context, objprim, supported[0]) - else: - raise + with excutils.save_and_reraise_exception(reraise=False) as ctxt: + verkey = \ + '%s.version' % self.OBJ_BASE_CLASS.OBJ_SERIAL_NAMESPACE + objver = objprim[verkey] + if objver.count('.') == 2: + # NOTE(danms): For our purposes, the .z part of the version + # should be safe to accept without requiring a backport + objprim[verkey] = \ + '.'.join(objver.split('.')[:2]) + return self._process_object(context, objprim) + namekey = '%s.name' % self.OBJ_BASE_CLASS.OBJ_SERIAL_NAMESPACE + objname = objprim[namekey] + supported = VersionedObjectRegistry.obj_classes().get(objname, + []) + if self.OBJ_BASE_CLASS.indirection_api and supported: + return self._do_backport(context, objprim, supported[0]) + else: + ctxt.reraise = True def _process_iterable(self, context, action_fn, values): """Process an iterable, taking an action on each value. diff --git a/oslo_versionedobjects/tests/test_fields.py b/oslo_versionedobjects/tests/test_fields.py index f795a46e..7de48398 100755 --- a/oslo_versionedobjects/tests/test_fields.py +++ b/oslo_versionedobjects/tests/test_fields.py @@ -15,6 +15,7 @@ import datetime import iso8601 +import mock import six from oslo_versionedobjects import base as obj_base @@ -470,6 +471,36 @@ class TestListOfSetsOfIntegers(TestField): self.assertEqual('[set([1,2])]', self.field.stringify([set([1, 2])])) +class TestLocalMethods(test.TestCase): + @mock.patch.object(obj_base.LOG, 'exception') + def test__make_class_properties_setter_value_error(self, mock_log): + @obj_base.VersionedObjectRegistry.register + class AnObject(obj_base.VersionedObject): + fields = { + 'intfield': fields.IntegerField(), + } + + self.assertRaises(ValueError, AnObject, intfield='badvalue') + self.assertFalse(mock_log.called) + + @mock.patch.object(obj_base.LOG, 'exception') + def test__make_class_properties_setter_setattr_fails(self, mock_log): + @obj_base.VersionedObjectRegistry.register + class AnObject(obj_base.VersionedObject): + fields = { + 'intfield': fields.IntegerField(), + } + + # We want the setattr() call in _make_class_properties.setter() to + # raise an exception + with mock.patch.object(obj_base, '_get_attrname') as mock_attr: + mock_attr.return_value = '__class__' + self.assertRaises(TypeError, AnObject, intfield=2) + mock_attr.assert_called_once_with('intfield') + mock_log.assert_called_once_with(mock.ANY, + {'attr': 'AnObject.intfield'}) + + class TestObject(TestField): def setUp(self): super(TestObject, self).setUp() diff --git a/oslo_versionedobjects/tests/test_objects.py b/oslo_versionedobjects/tests/test_objects.py index 069d3780..37d99aac 100755 --- a/oslo_versionedobjects/tests/test_objects.py +++ b/oslo_versionedobjects/tests/test_objects.py @@ -1590,6 +1590,14 @@ class TestObjectSerializer(_BaseTestCase): # .0 of the object. self.assertEqual('1.6', obj.VERSION) + def test_deserialize_entity_newer_version_no_indirection(self): + ser = base.VersionedObjectSerializer() + obj = MyObj() + obj.VERSION = '1.25' + primitive = obj.obj_to_primitive() + self.assertRaises(exception.IncompatibleObjectVersion, + ser.deserialize_entity, self.context, primitive) + def _test_nested_backport(self, old): @base.VersionedObjectRegistry.register class Parent(base.VersionedObject):