Pre-emptively convert the creator to a string like we used to.

Test this and other recursion scenarios.

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
 

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)

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):