diff --git a/src/nti/externalization/_compat.py b/src/nti/externalization/_compat.py index 91d13bb..f3ef53a 100644 --- a/src/nti/externalization/_compat.py +++ b/src/nti/externalization/_compat.py @@ -8,12 +8,15 @@ from __future__ import division from __future__ import print_function +from six import text_type def to_unicode(s, encoding='utf-8', err='strict'): """ Decode a byte sequence and unicode result """ - return s.decode(encoding, err) if isinstance(s, bytes) else s + if not isinstance(s, text_type) and s is not None: + s = s.decode(encoding, err) + return s text_ = to_unicode diff --git a/src/nti/externalization/externalization.py b/src/nti/externalization/externalization.py index 1ae9641..f4f1ae8 100644 --- a/src/nti/externalization/externalization.py +++ b/src/nti/externalization/externalization.py @@ -19,6 +19,7 @@ from ZODB.POSException import POSKeyError import persistent import six +from six import text_type from zope import component @@ -414,8 +415,14 @@ def choose_field(result, self, ext_name, if value is not None: # If the creator is the system user, catch it here - if ext_name == StandardExternalFields_CREATOR and is_system_user(value): - value = SYSTEM_USER_NAME + # 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 value = converter(value) @@ -500,6 +507,8 @@ def setExternalIdentifiers(self, result): return (oid, ntiid) set_external_identifiers = hookable(setExternalIdentifiers) +def _should_never_convert(x): + raise AssertionError("We should not be converting") def to_standard_external_dictionary(self, mergeFrom=None, registry=component, @@ -536,7 +545,7 @@ def to_standard_external_dictionary(self, mergeFrom=None, _choose_field(result, self, StandardExternalFields_CREATOR, fields=(StandardInternalFields_CREATOR, StandardExternalFields_CREATOR), - converter=to_unicode) + converter=_should_never_convert) to_standard_external_last_modified_time(self, _write_into=result) to_standard_external_created_time(self, _write_into=result) diff --git a/src/nti/externalization/tests/test_externalization.py b/src/nti/externalization/tests/test_externalization.py index 38c071a..70b32e3 100644 --- a/src/nti/externalization/tests/test_externalization.py +++ b/src/nti/externalization/tests/test_externalization.py @@ -254,57 +254,10 @@ class WithSystemUser(object): StandardExternalFields.CREATOR, fields=('user',)) assert_that(result, is_({StandardExternalFields.CREATOR: SYSTEM_USER_NAME})) - - def test_recursive_call_name(self): - - class Top(object): - - def toExternalObject(self, **kwargs): - assert_that(kwargs, has_entry('name', 'TopName')) - - middle = Middle() - - return toExternalObject(middle) # No name argument - - class Middle(object): - - def toExternalObject(self, **kwargs): - assert_that(kwargs, has_entry('name', 'TopName')) - - bottom = Bottom() - - return toExternalObject(bottom, name='BottomName') - - class Bottom(object): - - def toExternalObject(self, **kwargs): - assert_that(kwargs, has_entry('name', 'BottomName')) - - return "Bottom" - - assert_that(toExternalObject(Top(), name='TopName'), - is_("Bottom")) - - def test_recursive_call_minimizes_dict(self): - - class O(object): - ext_obj = None - - def toExternalObject(self, **kwargs): - return {"Hi": 42, - "kid": toExternalObject(self.ext_obj)} - - top = O() - child = O() - top.ext_obj = child - child.ext_obj = top - - result = toExternalObject(top) - assert_that(result, - is_({'Hi': 42, - 'kid': {'Hi': 42, - 'kid': {u'Class': 'O'}}})) - + def test_never_happens(self): + from nti.externalization.externalization import _should_never_convert + with self.assertRaises(AssertionError): + _should_never_convert(None) class TestDecorators(CleanUp, unittest.TestCase): @@ -608,6 +561,78 @@ def toExternalObject(self, *args, **kwargs): assert_that(calling(toExternalObject).with_args(Foo(), catch_components=MyException), raises(MyException)) + def test_recursive_call_name(self): + + class Top(object): + + def toExternalObject(self, **kwargs): + assert_that(kwargs, has_entry('name', 'TopName')) + + middle = Middle() + + return toExternalObject(middle) # No name argument + + class Middle(object): + + def toExternalObject(self, **kwargs): + assert_that(kwargs, has_entry('name', 'TopName')) + + bottom = Bottom() + + return toExternalObject(bottom, name='BottomName') + + class Bottom(object): + + def toExternalObject(self, **kwargs): + assert_that(kwargs, has_entry('name', 'BottomName')) + + return "Bottom" + + assert_that(toExternalObject(Top(), name='TopName'), + is_("Bottom")) + + def test_recursive_call_minimizes_dict(self): + + class O(object): + ext_obj = None + + def toExternalObject(self, **kwargs): + return {"Hi": 42, + "kid": toExternalObject(self.ext_obj)} + + top = O() + child = O() + top.ext_obj = child + child.ext_obj = top + + result = toExternalObject(top) + assert_that(result, + is_({'Hi': 42, + 'kid': {'Hi': 42, + 'kid': {u'Class': 'O'}}})) + + def test_recursive_call_on_creator(self): + # Make sure that we properly handle recursive calls on a + # field we want to pre-convert to a str, creator. + + class O(object): + def __init__(self): + self.creator = self + + def __str__(self): + return "creator" + + def toExternalObject(self, *args, **kwargs): + return to_standard_external_dictionary(self) + + result = toExternalObject(O()) + assert_that(result, has_entry('Creator', 'creator')) + + # Serialize to JSON too to make sure we get the right thing + from ..representation import to_json_representation_externalized + s = to_json_representation_externalized(result) + assert_that(s, is_('{"Class": "O", "Creator": "creator"}')) + @NoPickle class DoNotPickleMe(object):