diff --git a/CHANGES.rst b/CHANGES.rst index 08692d3..59a06a9 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -6,6 +6,10 @@ 1.0.0a2 (unreleased) ==================== +- The low levels of externalization no longer catch and hide + POSKeyError. This indicates a problem with the database. See + https://github.com/NextThought/nti.externalization/issues/60 + - Remove support for ``object_hook`` in ``update_from_external_object``. See https://github.com/NextThought/nti.externalization/issues/29. diff --git a/src/nti/externalization/externalization/fields.py b/src/nti/externalization/externalization/fields.py index ebffccd..7930017 100644 --- a/src/nti/externalization/externalization/fields.py +++ b/src/nti/externalization/externalization/fields.py @@ -13,7 +13,6 @@ from zope.security.management import system_user from zope.security.interfaces import IPrincipal -from ZODB.POSException import POSKeyError from nti.externalization._base_interfaces import get_standard_external_fields @@ -43,32 +42,26 @@ def choose_field(result, self, ext_name, # to document this and probably move it to a different module, or # provide a cleaner simpler replacement. for x in fields: - try: - value = getattr(self, x) - except AttributeError: - continue - except POSKeyError: - logger.exception("Could not get attribute %s for object %s", - x, self) + value = getattr(self, x, None) + if value is None: continue + # If the creator is the system user, catch it here + # XXX: Document this behaviour. + if ext_name == StandardExternalFields.CREATOR: + if is_system_user(value): + value = SYSTEM_USER_NAME + else: + # This is a likely recursion point, we want to be + # sure we don't do that. + value = text_type(value) + result[ext_name] = value + return value + if converter is not None: + value = converter(value) if value is not None: - # If the creator is the system user, catch it here - # XXX: Document this behaviour. - if ext_name == StandardExternalFields.CREATOR: - if is_system_user(value): - value = SYSTEM_USER_NAME - else: - # This is a likely recursion point, we want to be - # sure we don't do that. - value = text_type(value) - result[ext_name] = value - return value - if converter is not None: - value = converter(value) - if value is not None: - result[ext_name] = value - return value + result[ext_name] = value + return value # Nothing. Can we adapt it? if sup_iface is not None and sup_fields: diff --git a/src/nti/externalization/tests/test_externalization.py b/src/nti/externalization/tests/test_externalization.py index 63ae93b..a22073a 100644 --- a/src/nti/externalization/tests/test_externalization.py +++ b/src/nti/externalization/tests/test_externalization.py @@ -253,15 +253,15 @@ def test_isSyntheticKey(self): assert_that(isSyntheticKey('OID'), is_true()) assert_that(isSyntheticKey('key'), is_false()) - def test_choose_field_POSKeyError_ignored(self): + def test_choose_field_POSKeyError_not_ignored(self): from ZODB.POSException import POSKeyError class Raises(object): def __getattr__(self, name): raise POSKeyError(name) - assert_that(choose_field({}, Raises(), u'ext_name', - fields=('a', 'b')), - is_(none())) + with self.assertRaises(POSKeyError): + choose_field({}, Raises(), u'ext_name', + fields=('a', 'b')) def test_choose_field_system_user(self): from nti.externalization.externalization import SYSTEM_USER_NAME