From 9de7239b28a9817ad43c04e438743a22d964545e Mon Sep 17 00:00:00 2001 From: Jason Madden Date: Wed, 14 Jun 2017 06:51:22 -0500 Subject: [PATCH] Fix #85 by implementing __reduce__ to omit _v_ attributes. This is an alternative to #86 and was suggested by @mgedmin in #85. There are tests to be sure this works for subclasses like Pyramid's Registry that extend dict too. This doesn't change the pickle format, we were storing the output of object.__reduce__ previously. --- CHANGES.rst | 8 +- src/zope/interface/registry.py | 23 ++++- src/zope/interface/tests/test_registry.py | 103 +++++++++++++++++++++- 3 files changed, 126 insertions(+), 8 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 92f3762c..f94f3c2c 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -4,15 +4,17 @@ Changes 4.4.2 (unreleased) ------------------ -- Nothing changed yet. - +- Fix a regression storing + ``zope.component.persistentregistry.PersistentRegistry`` instances. + See `issue 85 `_. 4.4.1 (2017-05-13) ------------------ - Simplify the caching of utility-registration data. In addition to simplification, avoids spurious test failures when checking for - leaks in tests with persistent registries. + leaks in tests with persistent registries. See `pull 84 + `_. - Raise ``ValueError`` when non-text names are passed to adapter registry methods: prevents corruption of lookup caches. diff --git a/src/zope/interface/registry.py b/src/zope/interface/registry.py index 8539d278..26855ca9 100644 --- a/src/zope/interface/registry.py +++ b/src/zope/interface/registry.py @@ -154,7 +154,24 @@ def __init__(self, name='', bases=()): def __repr__(self): return "<%s %s>" % (self.__class__.__name__, self.__name__) + def __reduce__(self): + # Mimic what a persistent.Persistent object does and elide + # _v_ attributes so that they don't get saved in ZODB. + # This allows us to store things that cannot be pickled in such + # attributes. + reduction = super(Components, self).__reduce__() + # (callable, args, state, listiter, dictiter) + # We assume the state is always a dict; the last three items + # are technically optional and can be missing or None. + filtered_state = {k: v for k, v in reduction[2].items() + if not k.startswith('_v_')} + reduction = list(reduction) + reduction[2] = filtered_state + return tuple(reduction) + def _init_registries(self): + # Subclasses have never been required to call this method + # if they override it, merely to fill in these two attributes. self.adapters = AdapterRegistry() self.utilities = AdapterRegistry() @@ -166,10 +183,8 @@ def _init_registrations(self): @property def _utility_registrations_cache(self): - # We use a _v_ attribute internally so that data aren't saved in ZODB. - # If data are pickled in other contexts, the data will be carried along. - # There's no harm in pickling the extra data othr than that it would - # be somewhat wasteful. It's doubtful that that's an issue anyway. + # We use a _v_ attribute internally so that data aren't saved in ZODB, + # because this object cannot be pickled. try: return self._v_utility_registrations_cache except AttributeError: diff --git a/src/zope/interface/tests/test_registry.py b/src/zope/interface/tests/test_registry.py index cdbbe988..dd3cbbea 100644 --- a/src/zope/interface/tests/test_registry.py +++ b/src/zope/interface/tests/test_registry.py @@ -15,11 +15,14 @@ # pylint:disable=protected-access import unittest +from zope.interface import Interface +from zope.interface.adapter import VerifyingAdapterRegistry + +from zope.interface.registry import Components class ComponentsTests(unittest.TestCase): def _getTargetClass(self): - from zope.interface.registry import Components return Components def _makeOne(self, name='test', *args, **kw): @@ -574,6 +577,7 @@ class IFoo(InterfaceClass): # zope.component.testing does this comp.__init__('base') + comp.registerUtility(_to_reg, ifoo, _name2, _info) _monkey, _events = self._wrapEvents() @@ -2617,6 +2621,103 @@ def __repr__(self): ("HandlerRegistration(_REGISTRY, [IFoo], %r, TEST, " + "'DOCSTRING')") % (_name)) +class PersistentAdapterRegistry(VerifyingAdapterRegistry): + + def __getstate__(self): + state = self.__dict__.copy() + for k in list(state): + if k in self._delegated or k.startswith('_v'): + state.pop(k) + state.pop('ro', None) + return state + + def __setstate__(self, state): + bases = state.pop('__bases__', ()) + self.__dict__.update(state) + self._createLookup() + self.__bases__ = bases + self._v_lookup.changed(self) + +class PersistentComponents(Components): + # Mimic zope.component.persistentregistry.PersistentComponents: + # we should be picklalable, but not persistent.Persistent ourself. + + def _init_registries(self): + self.adapters = PersistentAdapterRegistry() + self.utilities = PersistentAdapterRegistry() + +class PersistentDictComponents(PersistentComponents, dict): + # Like Pyramid's Registry, we subclass Components and dict + pass + + +class PersistentComponentsDict(dict, PersistentComponents): + # Like the above, but inheritance is flipped + def __init__(self, name): + dict.__init__(self) + PersistentComponents.__init__(self, name) + +class TestPersistentComponents(unittest.TestCase): + + def _makeOne(self): + return PersistentComponents('test') + + def _check_equality_after_pickle(self, made): + pass + + def test_pickles_empty(self): + import pickle + comp = self._makeOne() + pickle.dumps(comp) + comp2 = pickle.loads(pickle.dumps(comp)) + + self.assertEqual(comp2.__name__, 'test') + + def test_pickles_with_utility_registration(self): + import pickle + comp = self._makeOne() + utility = object() + comp.registerUtility( + utility, + Interface) + + self.assertIs(utility, + comp.getUtility(Interface)) + + comp2 = pickle.loads(pickle.dumps(comp)) + self.assertEqual(comp2.__name__, 'test') + + # The utility is still registered + self.assertIsNotNone(comp2.getUtility(Interface)) + + # We can register another one + comp2.registerUtility( + utility, + Interface) + self.assertIs(utility, + comp2.getUtility(Interface)) + + self._check_equality_after_pickle(comp2) + + +class TestPersistentDictComponents(TestPersistentComponents): + + def _getTargetClass(self): + return PersistentDictComponents + + def _makeOne(self): + comp = self._getTargetClass()(name='test') + comp['key'] = 42 + return comp + + def _check_equality_after_pickle(self, made): + self.assertIn('key', made) + self.assertEqual(made['key'], 42) + +class TestPersistentComponentsDict(TestPersistentDictComponents): + + def _getTargetClass(self): + return PersistentComponentsDict class _Monkey(object): # context-manager for replacing module names in the scope of a test.