Merge pull request #65 from NextThought/issue30

Optimize and cache the signature computation for update.updateFromExternalObject

CHANGES.rst

@@ -6,7 +6,22 @@
 1.0.0a3 (unreleased)
 ====================
 
-- Nothing changed yet.
+- Proxies around objects that implement ``toExternalObject`` are
+  allowed again; the proxied object's ``toExternalObject`` will be called.
+
+- The signature for ``updateFromExternalObject()`` has been tightened.
+  It should be ``(self, external_object, context, **kwargs)``, where
+  ``**kwargs`` is optional, as is context. ``**kwargs`` currently
+  contains nothing useful. Uses of ``dataserver=None`` in the
+  signature will generate a warning. This may be tightened further in
+  the future. See `issue 30
+  <https://github.com/NextThought/nti.externalization/issues/30>`_.
+
+- ``__ext_ignore_updateFromExternalObject__`` is officially
+  deprecated and generates a warning.
+
+- ``update_from_external_object`` caches certain information about the
+  types of the updater objects, making it 8-25% faster.
 
 
 1.0.0a2 (2018-07-05)

src/nti/externalization/datastructures.py

@@ -432,8 +432,8 @@ def _ext_accept_external_id(self, ext_self, parsed):
                 cache.ext_accept_external_id = False
         return cache.ext_accept_external_id
 
-    def updateFromExternalObject(self, parsed, *args, **kwargs):
-        result = super(InterfaceObjectIO, self).updateFromExternalObject(parsed, *args, **kwargs)
+    def updateFromExternalObject(self, parsed, *unused_args, **unused_kwargs):
+        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/externalization/__init__.py

@@ -107,18 +107,20 @@ def toExternalDictionary(*args, **kwargs): # pragma: no cover
     return to_standard_external_dictionary(*args, **kwargs)
 
 
-def is_nonstr_iter(v):
-    warnings.warn("'is_nonstr_iter' will be deleted.", FutureWarning)
+def is_nonstr_iter(v): # pragma: no cover
+    warnings.warn("'is_nonstr_iter' will be deleted. It is broken on Python 3",
+                  FutureWarning, stacklevel=2)
     return hasattr(v, '__iter__')
 
 
 def removed_unserializable(ext):
     # pylint:disable=too-many-branches
     # XXX: Why is this here? We don't use it anymore.
     # Can it be removed?
-    warnings.warn("'removed_unserializable' will be deleted.", FutureWarning)
+    warnings.warn("'removed_unserializable' will be deleted.", FutureWarning, stacklevel=2)
     def _is_sequence(m):
-        return not isinstance(m, collections.Mapping) and is_nonstr_iter(m)
+        return (not isinstance(m, (str, collections.Mapping))
+                and hasattr(m, '__iter__'))
 
     def _clean(m):
         if isinstance(m, collections.Mapping):

src/nti/externalization/externalization/_externalizer.pxd

@@ -91,7 +91,7 @@ cdef _usable_externalObject_cache_get
 @cython.locals(
     has_ext_obj=bint,
 )
-cdef _obj_has_usable_externalObject(obj)
+cpdef _obj_has_usable_externalObject(obj)
 
 cdef _externalize_object(obj, _ExternalizationState state)
 

src/nti/externalization/externalization/externalizer.py

@@ -184,9 +184,11 @@ def _externalize_sequence(obj, state):
 
 
 def _obj_has_usable_externalObject(obj):
-    # This is for legacy code support, to allow existing methods to move to adapters
-    # and call us without infinite recursion
-    kind = type(obj)
+    # This is for legacy code support, to allow existing methods to
+    # move to adapters and call us without infinite recursion.
+    # We use __class__ instead of type() to allow for proxies;
+    # The proxy itself cannot implement toExternalObject
+    kind = obj.__class__
     answer = _usable_externalObject_cache_get(kind)
     if answer is None:
         answer = False
@@ -250,7 +252,7 @@ def _to_external_object_state(obj, state, top_level=False):
         elif result is not _marker:
             return result
         else:
-            logger.warn("Recursive call to object %s.", obj)
+            logger.warning("Recursive call to object %s.", obj)
             result = internal_to_standard_external_dictionary(obj,
                                                               decorate=False)
 

src/nti/externalization/externalization/tests/test_externalizer.py

@@ -0,0 +1,49 @@
+# -*- coding: utf-8 -*-
+
+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
+
+import unittest
+
+from zope.proxy import ProxyBase
+
+from nti.externalization.externalization import to_external_object
+from ..externalizer import _obj_has_usable_externalObject
+
+
+class WithToExternalObject(object):
+
+    def toExternalObject(self, **kwargs):
+        return {'a': 42}
+
+
+
+class TestFunctions(unittest.TestCase):
+
+    def test_non_proxied(self):
+        self.assertTrue(_obj_has_usable_externalObject(WithToExternalObject()))
+
+    def test_has_usable_external_object_proxied(self):
+
+        obj = WithToExternalObject()
+        proxied = ProxyBase(obj)
+
+        self.assertTrue(_obj_has_usable_externalObject(proxied))
+        self.assertEqual({'a': 42}, to_external_object(proxied))
+
+    def test_proxy_has_usable_external_object_not_allowed(self):
+
+        class Proxy(ProxyBase):
+
+            def toExternalObject(self, **kwargs):
+                raise NotImplementedError
+
+        self.assertFalse(_obj_has_usable_externalObject(Proxy(object())))
+
+
+if __name__ == '__main__':
+    unittest.main()

src/nti/externalization/interfaces.py

@@ -288,15 +288,19 @@ class IInternalObjectUpdater(interface.Interface):
 		It should return the new value. Note that the function here is at most
 		a class or static method, not an instance method.""")
 
-    def updateFromExternalObject(externalObject, *args, **kwargs):
+    def updateFromExternalObject(externalObject, context, **kwargs):
         """
         Update the object this is adapting from the external object.
-        Two alternate signatures are supported, one with ``dataserver`` instead of
-        context, and one with no keyword args.
+
+        Alternately, the signature can be ``updateFromExternalObject(externalObject)``
+        or simply ``updateFromExternalObject(externalObject, **kwargs)``. In this
+        last case, ``context`` will be passed as a value in ``**kwargs``.
+
 
         :return: If not ``None``, a value that can be interpreted as a boolean,
                 indicating whether or not the internal object actually
-                underwent updates. If ``None``, no meaning is assigned (to allow older
+                underwent updates. If ``None``, the caller should assume that the object
+                was updated (to allow older
                 code that doesn't return at all.)
         """
 

src/nti/externalization/internalization/_updater.pxd

@@ -31,9 +31,9 @@ cdef dict _EMPTY_DICT
 cdef class _RecallArgs(object):
     cdef registry
     cdef context
-    cdef require_updater
-    cdef notify
     cdef pre_hook
+    cdef bint require_updater
+    cdef bint notify
 
 
 cdef _recall(k, obj, ext_obj, _RecallArgs kwargs)
@@ -42,6 +42,15 @@ cpdef update_from_external_object(containedObject,
                                   externalObject,
                                   registry=*,
                                   context=*,
-                                  require_updater=*,
-                                  notify=*,
+                                  bint require_updater=*,
+                                  bint notify=*,
                                   pre_hook=*)
+
+cdef dict _argspec_cacheg
+cdef str _UPDATE_ARGS_TWO
+cdef str _UPDATE_ARGS_ONE
+cdef str _UPDATE_ARGS_CONTEXT_KW
+cdef _get_update_signature(updater)
+
+cdef dict _upsable_updateFromExternalObject_cache
+cdef _obj_has_usable_updateFromExternalObject(obj)