Merge pull request #81 from NextThought/premature-optimization

Handle INamedExternalizedOFF being different from IInternalObjectUpdater

CHANGES.rst

@@ -12,6 +12,13 @@
 - ``StandardExternalFields`` and ``StandardInternalFields`` are
   deprecated aliases in ``nti.externalization.externalization``.
 
+- ``update_from_external_object`` properly handles the case where
+  ``INamedExternalizedObjectFactoryFinder`` and
+  ``IInternalObjectUpdater`` are registered with different levels of
+  specificity, and the finder also implements
+  ``IInternalObjectUpdater``. Before, the finder would, perhaps
+  incorrectly, be used as the updater.
+
 1.0.0a5 (2018-07-30)
 ====================
 

src/nti/externalization/internalization/_updater.pxd

@@ -22,7 +22,7 @@ cdef INamedExternalizedObjectFactoryFinder
 # optimizations
 
 cdef IPersistent_providedBy
-cdef IInternalObjectUpdater_providedBy
+
 
 # constants
 cdef tuple PRIMITIVES

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

@@ -0,0 +1,69 @@
+# -*- coding: utf-8 -*-
+"""
+Tests for updater.py
+
+"""
+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 import interface
+from zope import component
+
+from zope.testing.cleanup import CleanUp
+
+from hamcrest import assert_that
+from hamcrest import has_property
+
+from nti.externalization import interfaces
+
+from nti.externalization.internalization import updater
+
+class TestUpdater(CleanUp,
+                  unittest.TestCase):
+
+    def test_less_specific_named_externalizer_doesnt_trump_specific_updater(self):
+
+        class CreateCount(object):
+            created = 0
+            def __init__(self, context):
+                type(self).created += 1
+
+        @interface.implementer(interfaces.IInternalObjectIOFinder)
+        @component.adapter(object)
+        class NEOFF(CreateCount):
+            created = 0
+            found = 0
+
+            def find_factory_for_named_value(self, *args):
+                NEOFF.found += 1
+                return None
+
+        class DomainObject(object):
+            pass
+
+        @interface.implementer(interfaces.IInternalObjectUpdater)
+        @component.adapter(DomainObject)
+        class CorrectUpdater(CreateCount):
+            created = 0
+            updated = 0
+            def updateFromExternalObject(self, *args):
+                CorrectUpdater.updated += 1
+
+        component.provideAdapter(CorrectUpdater)
+        component.provideAdapter(NEOFF)
+
+        ext = {'a': object()}
+        domain = DomainObject()
+        updater.update_from_external_object(domain, ext, require_updater=True)
+
+        assert_that(NEOFF, has_property('created', 1))
+        assert_that(NEOFF, has_property('found', 1))
+        assert_that(CorrectUpdater, has_property('created', 1))
+        assert_that(CorrectUpdater, has_property('updated', 1))

src/nti/externalization/internalization/updater.py

@@ -40,7 +40,7 @@
 
 _EMPTY_DICT = {}
 IPersistent_providedBy = IPersistent.providedBy
-IInternalObjectUpdater_providedBy = IInternalObjectUpdater.providedBy
+
 
 class _RecallArgs(object):
     __slots__ = (
@@ -364,13 +364,16 @@ def _update_from_external_object(containedObject, externalObject, args):
         # existing callers and without triggering infinite recursion
         updater = containedObject
     else:
-        if not IInternalObjectUpdater_providedBy(updater):
-            if args.require_updater:
-                get = args.registry.getAdapter
-            else:
-                get = args.registry.queryAdapter
+        # It's possible for INamedExternalizedObjectFactoryFinder and
+        # IInternalObjectUpdater to be registered at two different levels
+        # of specificity, so we need to look up IInternalObjectUpdater,
+        # not test if it's provided by what we already have.
+        if args.require_updater:
+            get = args.registry.getAdapter
+        else:
+            get = args.registry.queryAdapter
 
-            updater = get(containedObject, IInternalObjectUpdater)
+        updater = get(containedObject, IInternalObjectUpdater)
 
     if updater is not None:
         _invoke_updater(containedObject, externalObject, updater, external_keys, args)