Remove special handling of creator from choose_field. Instead it is d…

…one by get_creator.

get_creator no longer calls choose_field, it doesn't use any of the special features that were possible (like adaptation). This lets cython directly call system_user_converter.

Fixes #59.

CHANGES.rst

@@ -38,6 +38,9 @@
   ``decorate`` is false. This argument is also confusing and should be
   considered deprecated.
 
+- ``choose_field`` no longer has the undocumented conversion behaviour for the
+  CREATOR external field name.
+
 1.0.0a1 (2017-09-29)
 ====================
 

src/nti/externalization/externalization/__init__.py

@@ -33,8 +33,8 @@
 from .replacers import NonExternalizableObjectError
 
 from .fields import choose_field
-from .fields import SYSTEM_USER_NAME
 
+from .standard_fields import SYSTEM_USER_NAME
 from .standard_fields import get_last_modified_time
 from .standard_fields import get_created_time
 

src/nti/externalization/externalization/_fields.pxd

@@ -7,21 +7,12 @@ from nti.externalization.__base_interfaces cimport StandardExternalFields as SEF
 
 cdef SEF StandardExternalFields
 
-
 # Imports
-cdef type text_type
 
 
 # Constants
-cdef basestring _SYSTEM_USER_NAME
-cdef basestring _SYSTEM_USER_ID
-cdef identity
-cdef IPrincipal_providedBy
 cdef logger
 
-
-cdef bint is_system_user(obj) except *
-
 cpdef choose_field(result, self,
                    unicode ext_name,
                    converter=*,

src/nti/externalization/externalization/_standard_fields.pxd

@@ -9,6 +9,7 @@ from nti.externalization.externalization._fields cimport choose_field
 
 # Imports
 cdef IDCTimes
+cdef type text_type
 
 
 # Constants
@@ -23,12 +24,20 @@ cdef tuple _CREATOR_FIELDS
 cdef tuple _CONTAINER_FIELDS
 cdef _EXT_CLASS_IGNORED_MODULES
 
+cdef basestring _SYSTEM_USER_NAME
+cdef basestring _SYSTEM_USER_ID
+cdef IPrincipal_providedBy
+
+
 cpdef datetime_to_unix_time(dt)
 
 cpdef get_last_modified_time(context, default=*, _write_into=*)
 cpdef get_created_time(context, default=*, _write_into=*)
 
+cdef _system_user_converter(obj)
 cpdef get_creator(context, default=*, _write_into=*)
+
+
 cpdef get_container_id(context, default=*, _write_into=*)
 
 cpdef get_class(context, _write_into=*)

src/nti/externalization/externalization/fields.py

@@ -8,31 +8,14 @@
 from __future__ import division
 from __future__ import print_function
 
-from six import text_type
 
-from zope.security.management import system_user
-from zope.security.interfaces import IPrincipal
 
 
 from nti.externalization._base_interfaces import get_standard_external_fields
 
 logger = __import__('logging').getLogger(__name__)
 
 StandardExternalFields = get_standard_external_fields()
-identity = lambda obj: obj
-
-
-_SYSTEM_USER_NAME = getattr(system_user, 'title').lower()
-SYSTEM_USER_NAME = _SYSTEM_USER_NAME # Export from cython to python
-_SYSTEM_USER_ID = system_user.id
-del system_user
-
-IPrincipal_providedBy = IPrincipal.providedBy
-del IPrincipal
-
-def is_system_user(obj):
-    return IPrincipal_providedBy(obj) and obj.id == _SYSTEM_USER_ID
-
 
 def choose_field(result, self, ext_name,
                  converter=None,
@@ -46,23 +29,13 @@ def choose_field(result, self, ext_name,
         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:
             result[ext_name] = value
             return value
 
+
     # Nothing. Can we adapt it?
     if sup_iface is not None and sup_fields:
         self = sup_iface(self, None)

src/nti/externalization/externalization/standard_fields.py

@@ -12,7 +12,11 @@
 
 from calendar import timegm as dt_tuple_to_unix
 
+from six import text_type
+
 from zope.dublincore.interfaces import IDCTimes
+from zope.security.management import system_user
+from zope.security.interfaces import IPrincipal
 
 from nti.externalization._base_interfaces import get_standard_external_fields
 from nti.externalization._base_interfaces import get_standard_internal_fields
@@ -22,6 +26,14 @@
 StandardExternalFields = get_standard_external_fields()
 StandardInternalFields = get_standard_internal_fields()
 
+_SYSTEM_USER_NAME = getattr(system_user, 'title').lower()
+SYSTEM_USER_NAME = _SYSTEM_USER_NAME # Export from cython to python
+_SYSTEM_USER_ID = system_user.id
+del system_user
+
+IPrincipal_providedBy = IPrincipal.providedBy
+del IPrincipal
+
 
 def datetime_to_unix_time(dt):
     if dt is not None:
@@ -95,14 +107,28 @@ def get_created_time(context, default=None, _write_into=None):
     StandardExternalFields.CREATOR,
 )
 
-def get_creator(context, default=None, _write_into=None):
-    holder = _write_into if _write_into is not None else {}
 
-    choose_field(holder, context, StandardExternalFields.CREATOR,
-                 None,
-                 _CREATOR_FIELDS)
+def _system_user_converter(value):
+    if IPrincipal_providedBy(value) and value.id == _SYSTEM_USER_ID:
+        # Catch the system user
+        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)
+    return value
 
-    return holder.get(StandardExternalFields.CREATOR, default)
+
+def get_creator(context, default=None, _write_into=None):
+    for field_name in _CREATOR_FIELDS:
+        result = getattr(context, field_name, None)
+        if result is not None:
+            result = _system_user_converter(result)
+            if _write_into is not None:
+                _write_into[StandardExternalFields.CREATOR] = result
+            return result
+
+    return default
 
 
 _CONTAINER_FIELDS = (

src/nti/externalization/tests/test_externalization.py

@@ -38,6 +38,7 @@
 from ..extension_points import set_external_identifiers
 from ..externalization import to_standard_external_dictionary
 from ..externalization import toExternalObject
+from ..externalization.standard_fields import get_creator
 from ..interfaces import EXT_REPR_JSON
 from ..interfaces import EXT_REPR_YAML
 from ..interfaces import IExternalObject
@@ -263,7 +264,7 @@ def __getattr__(self, name):
             choose_field({}, Raises(), u'ext_name',
                          fields=('a', 'b'))
 
-    def test_choose_field_system_user(self):
+    def test_choose_field_system_user_not_special(self):
         from nti.externalization.externalization import SYSTEM_USER_NAME
         from zope.security.interfaces import IPrincipal
         from zope.security.management import system_user
@@ -279,9 +280,29 @@ class WithSystemUser(object):
         result = {}
         choose_field(result, WithSystemUser,
                      StandardExternalFields.CREATOR, fields=('user',))
+        assert_that(result, is_({StandardExternalFields.CREATOR: WithSystemUser.user}))
+
+    def test_get_creator_system_user(self):
+        from nti.externalization.externalization import SYSTEM_USER_NAME
+        from zope.security.interfaces import IPrincipal
+        from zope.security.management import system_user
+
+        @interface.implementer(IPrincipal)
+        class MySystemUser(object):
+            id = system_user.id
+
+
+        class WithSystemUser(object):
+            creator = MySystemUser()
+
+        result = {}
+
+        value = get_creator(WithSystemUser, None, result)
+        assert_that(value, is_(SYSTEM_USER_NAME))
         assert_that(result, is_({StandardExternalFields.CREATOR: SYSTEM_USER_NAME}))
 
 
+
 class TestDecorators(CleanUp,
                      unittest.TestCase):