Some explaratory programming for #51

Move the factory finding responsibility to a new object that is given the key to assign to. The normal implementation will check the matching schema field's tagged value if one can't be found from the data. Currently this must be a callable object but there are some options for using strings.

This could also allow us to get the actual object that's currently on the parent object, so that we really are updating in place intsead of constructing all new objects. This could have some nice database properties.

src/nti/externalization/_datastructures.pxd

@@ -13,10 +13,13 @@ from nti.externalization.__base_interfaces cimport get_standard_internal_fields
 from nti.externalization.__base_interfaces cimport StandardInternalFields as SIF
 
 from nti.externalization.internalization._fields cimport validate_named_field_value
+from nti.externalization.internalization._factories cimport find_factory_for
 
 from nti.externalization.__interface_cache cimport cache_for
 
-cdef IInternalObjectIO
+
+
+cdef IInternalObjectIOFinder
 cdef SEF StandardExternalFields
 cdef SIF StandardInternalFields
 cdef validate_named_field_value
@@ -46,6 +49,9 @@ cdef class AbstractDynamicObjectIO(ExternalizableDictionaryMixin):
     cpdef _ext_accept_update_key(self, k, ext_self, ext_keys)
     cpdef _ext_accept_external_id(self, ext_self, parsed)
 
+    cpdef get_object_to_update(self, key, value, registry)
+    cdef _updateFromExternalObject(self, parsed)
+
 
 cdef class ExternalizableInstanceDict(AbstractDynamicObjectIO):
     pass

src/nti/externalization/datastructures.py

@@ -25,7 +25,7 @@
 
 from nti.schema.interfaces import find_most_derived_interface
 
-from .interfaces import IInternalObjectIO
+from .interfaces import IInternalObjectIOFinder
 from .interfaces import StandardInternalFields
 
 # Things imported from cython with matching cimport
@@ -36,6 +36,7 @@
 from .externalization.externalizer import to_external_object as _toExternalObject
 
 from .internalization import validate_named_field_value
+from .internalization.factories import find_factory_for
 from .representation import make_repr
 
 from ._base_interfaces import get_standard_external_fields
@@ -56,9 +57,6 @@ class ExternalizableDictionaryMixin(object):
     #: produce the *minimal* dictionary. See :func:`~to_minimal_standard_external_dictionary`
     __external_use_minimal_base__ = False
 
-    def __init__(self, *args):
-        super(ExternalizableDictionaryMixin, self).__init__(*args)
-
     def _ext_replacement(self):
         return self
 
@@ -123,6 +121,8 @@ class AbstractDynamicObjectIO(ExternalizableDictionaryMixin):
     _ext_primitive_out_ivars_ = frozenset()
     _prefer_oid_ = False
 
+    def get_object_to_update(self, key, value, registry):
+        return find_factory_for(value, registry)
 
     def _ext_all_possible_keys(self):
         """
@@ -239,6 +239,9 @@ def _ext_accept_external_id(self, ext_self, parsed):
         return False  # false by default
 
     def updateFromExternalObject(self, parsed, *unused_args, **unused_kwargs):
+        return self._updateFromExternalObject(parsed)
+
+    def _updateFromExternalObject(self, parsed):
         updated = False
 
         ext_self = self._ext_replacement()
@@ -270,7 +273,7 @@ def updateFromExternalObject(self, parsed, *unused_args, **unused_kwargs):
 
         return updated
 
-interface.classImplements(AbstractDynamicObjectIO, IInternalObjectIO)
+interface.classImplements(AbstractDynamicObjectIO, IInternalObjectIOFinder)
 
 
 class ExternalizableInstanceDict(AbstractDynamicObjectIO):
@@ -346,7 +349,7 @@ def __init__(self, ext_self, iface_upper_bound=None, validate_after_update=True)
             schema will be validated after an object has been updated with
             :meth:`update_from_external_object`, not just the keys that were assigned.
         """
-        super(InterfaceObjectIO, self).__init__()
+        AbstractDynamicObjectIO.__init__(self)
         self._ext_self = ext_self
         # Cache all of this data that we use. It's required often and, if not quite a bottleneck,
         # does show up in the profiling data
@@ -360,7 +363,7 @@ def __init__(self, ext_self, iface_upper_bound=None, validate_after_update=True)
 
         if not cache.ext_primitive_out_ivars:
             keys = self._ext_find_primitive_keys()
-            cache.ext_primitive_out_ivars = self._ext_primitive_out_ivars_.union(keys)
+            cache.ext_primitive_out_ivars = self._ext_primitive_out_ivars_ | keys
         self._ext_primitive_out_ivars_ = cache.ext_primitive_out_ivars
 
         self.validate_after_update = validate_after_update
@@ -432,8 +435,20 @@ def _ext_accept_external_id(self, ext_self, parsed):
                 cache.ext_accept_external_id = False
         return cache.ext_accept_external_id
 
+    def get_object_to_update(self, key, value, registry):
+        factory = AbstractDynamicObjectIO.get_object_to_update(self, key, value, registry)
+        if factory is None:
+            # Is there a factory on the field?
+            try:
+                field = self._iface[key]
+                # XXX: Maybe this should be a string naming a factory we find in the registry?
+                # Or a string giving the dottedname to a class we resolve at runtime (and reify)
+                return field.getTaggedValue('__external_factory__')
+            except KeyError:
+                return None
+
     def updateFromExternalObject(self, parsed, *unused_args, **unused_kwargs):
-        result = AbstractDynamicObjectIO.updateFromExternalObject(self, parsed)
+        result = AbstractDynamicObjectIO._updateFromExternalObject(self, parsed)
         # If we make it this far, then validate the object.
 
         # TODO: Should probably just make sure that there are no /new/

src/nti/externalization/interfaces.py

@@ -268,6 +268,15 @@ def resolve(reference):
         Resolve the external reference and return it.
         """
 
+class IInternalObjectFactoryFinder(interface.Interface):
+    """
+    TODO: Document me.
+    """
+
+    def get_object_to_update(key, value, registry):
+        """
+        Document me.
+        """
 
 class IInternalObjectUpdater(interface.Interface):
     """
@@ -311,6 +320,9 @@ class IInternalObjectIO(IInternalObjectExternalizer, IInternalObjectUpdater):
     in external forms.
     """
 
+class IInternalObjectIOFinder(IInternalObjectFactoryFinder, IInternalObjectIO):
+    pass
+
 class IObjectWillUpdateFromExternalEvent(IObjectEvent):
     """
     An object will be updated from an external value.

src/nti/externalization/internalization/_updater.pxd

@@ -14,7 +14,9 @@ cdef warnings
 cdef string_types
 cdef iteritems
 cdef component
+cdef interface
 cdef IInternalObjectUpdater
+cdef IInternalObjectFactoryFinder
 
 # optimizations
 
@@ -35,6 +37,10 @@ cdef class _RecallArgs(object):
     cdef bint require_updater
     cdef bint notify
 
+@cython.internal
+@cython.final
+cdef class DefaultInternalObjectFactoryFinder(object):
+    pass
 
 cdef _recall(k, obj, ext_obj, _RecallArgs kwargs)
 

src/nti/externalization/internalization/tests/test_externals_wo_class_mimetype.py

@@ -0,0 +1,74 @@
+# -*- coding: utf-8 -*-
+"""
+Tests for reading data into objects from external data *not* following
+the conventions of this package, e.g., missing Class and MimeType values.
+
+"""
+from __future__ import absolute_import
+from __future__ import division
+from __future__ import print_function
+
+# disable: accessing protected members, too many methods
+# pylint: disable=W0212,R0904
+# pylint:disable=inherit-non-class
+
+import unittest
+
+from hamcrest import is_
+from hamcrest import assert_that
+from hamcrest import has_property as has_attr
+
+from zope import interface
+from zope import component
+
+from zope.schema import Object
+from zope.schema import Int
+
+from zope.testing.cleanup import CleanUp
+
+from nti.externalization.internalization import update_from_external_object
+from nti.externalization.datastructures import InterfaceObjectIO
+from nti.externalization.datastructures import ModuleScopedInterfaceObjectIO
+
+class TestExternals(CleanUp,
+                    unittest.TestCase):
+
+    def test_single_object_field_names_match_non_primitive(self):
+        class INestedThing(interface.Interface):
+            value = Int(title=u"An integer")
+
+        class IRoot(interface.Interface):
+            field = Object(INestedThing)
+
+
+        class IO(InterfaceObjectIO):
+            _ext_iface_upper_bound = IRoot
+        component.provideAdapter(IO, adapts=(IRoot,))
+
+        class IO(InterfaceObjectIO):
+            _ext_iface_upper_bound = INestedThing
+
+        component.provideAdapter(IO, adapts=(INestedThing,))
+
+        @interface.implementer(IRoot)
+        class Root(object):
+
+            def __init__(self):
+                self.field = None
+
+        @interface.implementer(INestedThing)
+        class NestedThing(object):
+
+            def __init__(self):
+                self.value = -1
+
+        IRoot['field'].setTaggedValue('__external_factory__', NestedThing)
+
+        external = {'field': {'value': 42}}
+
+        root = Root()
+
+        update_from_external_object(root, external, require_updater=True)
+
+        assert_that(root, has_attr('field', is_(NestedThing)))
+        assert_that(root.field, has_attr('value', 42))

src/nti/externalization/internalization/updater.py

@@ -23,9 +23,11 @@
 from persistent.interfaces import IPersistent
 from six import iteritems
 from zope import component
+from zope import interface
 
 from nti.externalization._base_interfaces import PRIMITIVES
 from nti.externalization.interfaces import IInternalObjectUpdater
+from nti.externalization.interfaces import IInternalObjectFactoryFinder
 
 from .factories import find_factory_for
 from .events import _notifyModified
@@ -178,6 +180,16 @@ def _obj_has_usable_updateFromExternalObject(obj):
     cleanup.addCleanUp(_usable_updateFromExternalObject_cache.clear)
 
 
+class DefaultInternalObjectFactoryFinder(object):
+
+    def get_object_to_update(self, key, value, registry):
+        return find_factory_for(value, registry)
+
+
+interface.classImplements(DefaultInternalObjectFactoryFinder, IInternalObjectFactoryFinder)
+
+_default_factory_finder = DefaultInternalObjectFactoryFinder()
+
 def update_from_external_object(containedObject, externalObject,
                                 registry=component, context=None,
                                 require_updater=False,
@@ -254,6 +266,10 @@ def update_from_external_object(containedObject, externalObject,
 
     assert isinstance(externalObject, MutableMapping)
 
+    updater = registry.queryAdapter(containedObject, IInternalObjectFactoryFinder,
+                                    u'', _default_factory_finder)
+    get_object_to_update = updater.get_object_to_update
+
     # We have to save the list of keys, it's common that they get popped during the update
     # process, and then we have no descriptions to send
     external_keys = list()
@@ -273,23 +289,24 @@ def update_from_external_object(containedObject, externalObject,
             v = _recall(k, (), v, kwargs)
             externalObject[k] = v
         else:
-            factory = find_factory_for(v, registry=registry)
+            factory = get_object_to_update(k, v, registry)
             if factory is not None:
                 externalObject[k] = _recall(k, factory(), v, kwargs)
 
-    updater = None
+
     if _obj_has_usable_updateFromExternalObject(containedObject):
         # legacy support. The __ext_ignore_updateFromExternalObject__
         # allows a transition to an adapter without changing
         # existing callers and without triggering infinite recursion
         updater = containedObject
     else:
-        if require_updater:
-            get = registry.getAdapter
-        else:
-            get = registry.queryAdapter
+        if not IInternalObjectUpdater.providedBy(updater):
+            if require_updater:
+                get = registry.getAdapter
+            else:
+                get = registry.queryAdapter
 
-        updater = get(containedObject, IInternalObjectUpdater)
+            updater = get(containedObject, IInternalObjectUpdater)
 
     if updater is not None:
         # Let the updater resolve externals too