From 955047edbd05a5daa5bfbb443dfc9e31af27f3a0 Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Tue, 23 Jun 2020 17:07:40 -0500 Subject: [PATCH 1/2] Correctly notify IObjectWillUpdateFromExternalEvent before updating objects. --- CHANGES.rst | 3 +- src/nti/externalization/interfaces.py | 21 +++- .../internalization/_updater.pxd | 4 +- .../internalization/tests/test_updater.py | 117 ++++++++++++++---- .../internalization/updater.py | 17 ++- 5 files changed, 133 insertions(+), 29 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 4203d62..4597c48 100644 --- a/CHANGES.rst +++ b/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) diff --git a/src/nti/externalization/interfaces.py b/src/nti/externalization/interfaces.py index 59a5c4c..72e8c47 100644 --- a/src/nti/externalization/interfaces.py +++ b/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): diff --git a/src/nti/externalization/internalization/_updater.pxd b/src/nti/externalization/internalization/_updater.pxd index 99457d8..1e0db0b 100644 --- a/src/nti/externalization/internalization/_updater.pxd +++ b/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 diff --git a/src/nti/externalization/internalization/tests/test_updater.py b/src/nti/externalization/internalization/tests/test_updater.py index d01fcfe..d3a8193 100644 --- a/src/nti/externalization/internalization/tests/test_updater.py +++ b/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 = ext.copy() + 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)) diff --git a/src/nti/externalization/internalization/updater.py b/src/nti/externalization/internalization/updater.py index 819c142..82fd1e3 100644 --- a/src/nti/externalization/internalization/updater.py +++ b/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 From 072cfaa73432de05678b551dd37b449e3911932f Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 24 Jun 2020 14:43:00 -0500 Subject: [PATCH 2/2] list.copy is new in Python 3; use the constructor instead. --- src/nti/externalization/internalization/tests/test_updater.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nti/externalization/internalization/tests/test_updater.py b/src/nti/externalization/internalization/tests/test_updater.py index d3a8193..20c2bac 100644 --- a/src/nti/externalization/internalization/tests/test_updater.py +++ b/src/nti/externalization/internalization/tests/test_updater.py @@ -124,7 +124,7 @@ def test_will_update_event_sequence(self): events = self.events ext = [{'MimeType': 'abc'}, {'MimeType': 'abc'}] - orig_ext = ext.copy() + orig_ext = list(ext) domain = [DomainObject(), DomainObject()] updater.update_from_external_object(domain, ext, require_updater=True)