Add explicit tests for recursion. Optimize the memo case slightly.

src/nti/externalization/externalization.py

@@ -147,16 +147,16 @@ class _ExternalizationState(object):
     name = u''
     request = None
     registry = None
-
-    def __init__(self, kwargs):
-        # We take a similar approach to pickle.Pickler
-        # for memoizing objects we've seen:
-        # we map the id of an object to a two tuple: (obj, external-value)
-        # the original object is kept in the tuple to keep transient objects alive
-        # and thus ensure no overlapping ids
-        self.memo = {}
-        # Allow it to be passed as a kwarg to override
+    # We take a similar approach to pickle.Pickler
+    # for memoizing objects we've seen:
+    # we map the id of an object to a two tuple: (obj, external-value)
+    # the original object is kept in the tuple to keep transient objects alive
+    # and thus ensure no overlapping ids
+    memo = None
+
+    def __init__(self, memos, kwargs):
         self.__dict__.update(kwargs)
+        self.memo = memos[self.name]
 
 class _RecursiveCallState(dict):
     pass
@@ -170,11 +170,11 @@ def _to_external_object_state(obj, state, top_level=False, decorate=True,
     __traceback_info__ = obj
 
     orig_obj = obj
-    orig_obj_id = id(obj)
+    orig_obj_id = id(obj) # XXX: Relatively expensive on PyPy
     if useCache:
         value = state.memo.get(orig_obj_id, None)
         result = value[1] if value is not None else None
-        if result is None:  # mark
+        if result is None:  # mark as in progress
             state.memo[orig_obj_id] = (orig_obj, _marker)
         elif result is not _marker:
             return result
@@ -337,27 +337,23 @@ def toExternalObject(obj,
     if isinstance(obj, _primitives):
         return obj
 
-    v = dict(locals())
-    v.pop('obj')
-    state = _ExternalizationState(v)
-
     if name is _NotGiven:
         name = _manager.get()['name']
     if name is _NotGiven:
         name = ''
+    if request is _NotGiven:
+        request = get_current_request()
 
     memos = _manager.get()['memos']
     if memos is None:
         memos = defaultdict(dict)
 
-    _manager.push({'name': name, 'memos': memos})
-
-    state.name = name
-    state.memo = memos[name]
+    v = dict(locals())
+    v.pop('obj')
+    v.pop("memos")
+    state = _ExternalizationState(memos, v)
 
-    if request is _NotGiven:
-        request = get_current_request()
-    state.request = request
+    _manager.push({'name': name, 'memos': memos})
 
     try:
         return _to_external_object_state(obj, state, top_level=True,

src/nti/externalization/tests/test_externalization.py

@@ -255,6 +255,57 @@ class WithSystemUser(object):
         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'}}}))
+
+
 class TestDecorators(CleanUp,
                      unittest.TestCase):