Skip to content

Commit

Permalink
Stop catching POSKeyError.
Browse files Browse the repository at this point in the history
Fixes #60.
  • Loading branch information
jamadden committed Jul 5, 2018
1 parent 48ba4b4 commit 0100856
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 28 deletions.
4 changes: 4 additions & 0 deletions CHANGES.rst
Expand Up @@ -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.
Expand Down
41 changes: 17 additions & 24 deletions src/nti/externalization/externalization/fields.py
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down
8 changes: 4 additions & 4 deletions src/nti/externalization/tests/test_externalization.py
Expand Up @@ -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
Expand Down

0 comments on commit 0100856

Please sign in to comment.