Merge pull request #107 from NextThought/will-update-events

Correctly notify IObjectWillUpdateFromExternalEvent before updating objects

CHANGES.rst

@@ -6,7 +6,8 @@
 1.1.3 (unreleased)
 ==================
 
-- Nothing changed yet.
+- Correctly fire ``IObjectWillUpdateFromExternalEvent`` events before
+  updating an object.
 
 
 1.1.2 (2020-04-07)

src/nti/externalization/interfaces.py

@@ -386,12 +386,31 @@ class IObjectWillUpdateFromExternalEvent(IObjectEvent):
     """
     An object will be updated from an external value.
     """
-    external_value = interface.Attribute("The external value")
+    external_value = interface.Attribute(
+        "The external value. "
+        "This is not necessarily a pristine object as decoded from, e.g., JSON. "
+        "It will be mutated as sub-objects get updated and parsed. For example, strings "
+        "may get replaced with datetimes, and so on. "
+        "The consequences of modifying this object in an event subscriber are undefined. "
+    )
+    root = interface.Attribute(
+        "The object initially passed to update_from_external_object. "
+        "For nested objects, this will be some ancestor of the object this event is for. "
+        "For updaters that manually update sub-objects, this isn't guaranteed to be the actual "
+        "true root object being updated."
+    )
+
 
 
 @interface.implementer(IObjectWillUpdateFromExternalEvent)
 class ObjectWillUpdateFromExternalEvent(ObjectEvent):
     external_value = None
+    root = None
+
+    def __init__(self, object, external_value=None, root=None):
+        ObjectEvent.__init__(self, object)
+        self.external_value = external_value
+        self.root = root
 
 
 class IObjectModifiedFromExternalEvent(IObjectModifiedEvent):

src/nti/externalization/internalization/_updater.pxd

@@ -8,7 +8,6 @@ from ._factories cimport find_factory_for
 
 # imports
 cdef NotGiven
-cdef component
 cdef MutableSequence
 cdef MutableMapping
 cdef inspect
@@ -21,6 +20,8 @@ cdef interface
 cdef IInternalObjectUpdater
 cdef IInternalObjectIO
 cdef INamedExternalizedObjectFactoryFinder
+cdef notify
+cdef ObjectWillUpdateFromExternalEvent
 
 # optimizations
 
@@ -38,6 +39,7 @@ cdef dict _EMPTY_DICT
 cdef class _RecallArgs(object):
     cdef context
     cdef pre_hook
+    cdef root
     cdef bint require_updater
     cdef bint notify
 

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

@@ -20,42 +20,73 @@
 
 from hamcrest import assert_that
 from hamcrest import has_property
+from hamcrest import has_length
+from hamcrest import is_
+from hamcrest import same_instance
 
 from nti.externalization import interfaces
 
 from nti.externalization.internalization import updater
 
+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
+
+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
+
+
+@interface.implementer(interfaces.IExternalizedObjectFactoryFinder)
+@component.adapter(object)
+class WorkingExternalizedObjectFactoryFinder(object):
+
+    def __init__(self, context):
+        pass
+
+    def find_factory(self, ext):
+        return DomainObject
+
 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
+    def setUp(self):
+        from zope import event
 
-        @interface.implementer(interfaces.IInternalObjectIOFinder)
-        @component.adapter(object)
-        class NEOFF(CreateCount):
-            created = 0
-            found = 0
+        self.events = events = []
+        event.subscribers.append(events.append)
 
-            def find_factory_for_named_value(self, *args):
-                NEOFF.found += 1
-                return None
 
-        class DomainObject(object):
-            pass
+    def tearDown(self):
+        from zope import event
+        CreateCount.created = 0
+        NEOFF.created = NEOFF.found = 0
+        CorrectUpdater.created = CorrectUpdater.updated = 0
+        event.subscribers.remove(self.events.append)
 
-        @interface.implementer(interfaces.IInternalObjectUpdater)
-        @component.adapter(DomainObject)
-        class CorrectUpdater(CreateCount):
-            created = 0
-            updated = 0
-            def updateFromExternalObject(self, *args):
-                CorrectUpdater.updated += 1
+        registry = component.getSiteManager()
+        registry.unregisterAdapter(CorrectUpdater)
+        registry.unregisterAdapter(NEOFF)
+        registry.unregisterAdapter(WorkingExternalizedObjectFactoryFinder)
 
+    def test_less_specific_named_externalizer_doesnt_trump_specific_updater(self):
         component.provideAdapter(CorrectUpdater)
         component.provideAdapter(NEOFF)
 
@@ -67,3 +98,43 @@ def updateFromExternalObject(self, *args):
         assert_that(NEOFF, has_property('found', 1))
         assert_that(CorrectUpdater, has_property('created', 1))
         assert_that(CorrectUpdater, has_property('updated', 1))
+
+    def test_will_update_event(self):
+        component.provideAdapter(CorrectUpdater)
+        events = self.events
+
+        ext = {'a': object()}
+        domain = DomainObject()
+
+        updater.update_from_external_object(domain, ext, require_updater=True)
+
+        assert_that(events, has_length(2))
+        assert_that(events[0], is_(interfaces.ObjectWillUpdateFromExternalEvent))
+        will = events[0]
+        assert_that(will.root, is_(same_instance(domain)))
+        assert_that(will.object, is_(same_instance(domain)))
+        assert_that(will.external_value, is_(ext))
+
+        assert_that(events[1], is_(interfaces.ObjectModifiedFromExternalEvent))
+
+    def test_will_update_event_sequence(self):
+        component.provideAdapter(CorrectUpdater)
+        component.provideAdapter(WorkingExternalizedObjectFactoryFinder)
+
+        events = self.events
+
+        ext = [{'MimeType': 'abc'}, {'MimeType': 'abc'}]
+        orig_ext = list(ext)
+        domain = [DomainObject(), DomainObject()]
+
+        updater.update_from_external_object(domain, ext, require_updater=True)
+
+        assert_that(events, has_length(4))
+        assert_that(events[0], is_(interfaces.ObjectWillUpdateFromExternalEvent))
+        assert_that(events[2], is_(interfaces.ObjectWillUpdateFromExternalEvent))
+        will = events[0]
+        assert_that(will.root, is_(domain))
+        assert_that(will.external_value, is_(orig_ext[0]))
+
+        assert_that(events[1], is_(interfaces.ObjectModifiedFromExternalEvent))
+        assert_that(events[3], is_(interfaces.ObjectModifiedFromExternalEvent))

src/nti/externalization/internalization/updater.py

@@ -24,14 +24,15 @@
 
 from persistent.interfaces import IPersistent
 from six import iteritems
-from zope import component
 from zope import interface
+from zope.event import notify
 
 from nti.externalization._base_interfaces import PRIMITIVES
 from nti.externalization._base_interfaces import NotGiven
 from nti.externalization.interfaces import IInternalObjectUpdater
 from nti.externalization.interfaces import IInternalObjectIO
 from nti.externalization.interfaces import INamedExternalizedObjectFactoryFinder
+from nti.externalization.interfaces import ObjectWillUpdateFromExternalEvent
 
 from .factories import find_factory_for
 from .events import _notifyModified
@@ -49,6 +50,7 @@ class _RecallArgs(object):
         'require_updater',
         'notify',
         'pre_hook',
+        'root',
     )
 
     # We don't have an __init__, we ask the caller
@@ -60,6 +62,7 @@ def __init__(self):
         self.require_updater = False
         self.notify = True
         self.pre_hook = None
+        self.root = None
 
 ##
 # Note on caching: We do not expect the updater objects to be proxied.
@@ -95,7 +98,7 @@ def _get_update_signature(updater):
                 argspec = inspect.getfullargspec(func) # pylint:disable=no-member
                 keywords = argspec.varkw
             else: # Python 2
-                argspec = inspect.getargspec(func)
+                argspec = inspect.getargspec(func) # pylint:disable=deprecated-method
                 keywords = argspec.keywords
             args = argspec.args
             defaults = argspec.defaults
@@ -218,10 +221,16 @@ def update_from_external_object(containedObject, externalObject,
         object. Deprecated.
     :return: *containedObject* after updates from *externalObject*
 
+    Notifies `~.IObjectModifiedFromExternalEvent` for each object that is modified,
+    and `~.IObjectWillUpdateFromExternalEvent` before doing so.
+
     .. seealso:: `~.INamedExternalizedObjectFactoryFinder`
 
     .. versionchanged:: 1.0.0a2
        Remove the ``object_hook`` parameter.
+    .. versionchanged:: 1.1.3
+       Correctly file `~.IObjectWillUpdateFromExternalEvent` before updating
+       each object.
     """
 
     if pre_hook is not None: # pragma: no cover
@@ -239,6 +248,7 @@ def update_from_external_object(containedObject, externalObject,
     kwargs.require_updater = require_updater
     kwargs.notify = notify
     kwargs.pre_hook = pre_hook
+    kwargs.root = containedObject
 
     return _update_from_external_object(containedObject, externalObject, kwargs)
 
@@ -335,7 +345,7 @@ def _update_from_external_object(containedObject, externalObject, args):
     if isinstance(externalObject, MutableSequence):
         return _update_sequence(externalObject, args)
 
-    assert isinstance(externalObject, MutableMapping)
+    assert isinstance(externalObject, MutableMapping), externalObject
 
     factory_finder = _find_INamedExternalizedObjectFactoryFinder(containedObject)
 
@@ -379,6 +389,7 @@ def _update_from_external_object(containedObject, externalObject, args):
 
 
     if updater is not None:
+        notify(ObjectWillUpdateFromExternalEvent(containedObject, externalObject, args.root))
         _invoke_updater(containedObject, externalObject, updater, external_keys, args)
 
     return containedObject