Fix #85 by implementing __reduce__ to omit _v_ attributes.issue85-take2-reduce

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.

3 files changed, 126 insertions, 8 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 92f3762..f94f3c2 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 <https://github.com/zopefoundation/zope.interface/issues/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
+  <https://github.com/zopefoundation/zope.interface/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 8539d27..26855ca 100644
--- a/src/zope/interface/registry.py
+++ b/src/zope/interface/registry.py
@@ -154,7 +154,24 @@ class Components(object):
     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 @@ class Components(object):
 
     @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 cdbbe98..dd3cbbe 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 ComponentsTests(unittest.TestCase):
 
         # zope.component.testing does this
         comp.__init__('base')
+
         comp.registerUtility(_to_reg, ifoo, _name2, _info)
 
         _monkey, _events = self._wrapEvents()
@@ -2617,6 +2621,103 @@ class HandlerRegistrationTests(unittest.TestCase):
             ("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.