Merge pull request #63 from zopefoundation/issue62

Fix unregistering utilities on old persistent adapter registries.

3 files changed, 47 insertions, 2 deletions

diff --git a/CHANGES.rst b/CHANGES.rst
index 6df82ea..061ace3 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -5,7 +5,9 @@
 5.0.1 (unreleased)
 ==================
 
-- Nothing changed yet.
+- Fix unregistering utilities on old persistent adapter registries.
+  Previously this could raise ``TypeError``. See `issue 62
+  <https://github.com/zopefoundation/zope.component/issues/62>`_.
 
 
 5.0.0 (2021-03-19)
diff --git a/src/zope/component/persistentregistry.py b/src/zope/component/persistentregistry.py
index f01111c..4f61e3d 100644
--- a/src/zope/component/persistentregistry.py
+++ b/src/zope/component/persistentregistry.py
@@ -23,7 +23,7 @@ class PersistentAdapterRegistry(VerifyingAdapterRegistry, Persistent):
     """
     An adapter registry that is also a persistent object.
 
-    .. versionchanged:: 4.7.0
+    .. versionchanged:: 5.0.0
         Internal data structures are now composed of
         :class:`persistent.mapping.PersistentMapping` and
         :class:`persistent.list.PersistentList`. This helps scale to
@@ -68,6 +68,10 @@ class PersistentAdapterRegistry(VerifyingAdapterRegistry, Persistent):
         return existing_leaf_sequence
 
     def _removeValueFromLeaf(self, existing_leaf_sequence, to_remove):
+        if isinstance(existing_leaf_sequence, tuple):
+            # Converting from old state
+            existing_leaf_sequence = self._leafSequenceType(existing_leaf_sequence)
+
         without_removed = VerifyingAdapterRegistry._removeValueFromLeaf(
             self,
             existing_leaf_sequence,
diff --git a/src/zope/component/tests/test_persistentregistry.py b/src/zope/component/tests/test_persistentregistry.py
index 606ce10..46224cc 100644
--- a/src/zope/component/tests/test_persistentregistry.py
+++ b/src/zope/component/tests/test_persistentregistry.py
@@ -136,9 +136,48 @@ class PersistentAdapterRegistryTests(unittest.TestCase):
     def test__addValueToLeaf_existing_is_tuple_converts(self):
         from persistent.list import PersistentList
         registry = self._makeOne()
+        # It converts when the tuple is not empty...
         result = registry._addValueToLeaf(('a',), 'b')
         self.assertIsInstance(result, PersistentList)
         self.assertEqual(result, ['a', 'b'])
+        # ...and when it is empty...
+        result = registry._addValueToLeaf((), 'b')
+        self.assertIsInstance(result, PersistentList)
+        self.assertEqual(result, ['b'])
+        # ...and in fact when it is even missing
+        result = registry._addValueToLeaf(None, 'b')
+        self.assertIsInstance(result, PersistentList)
+        self.assertEqual(result, ['b'])
+
+    def test__removeValueFromLeaf_existing_is_tuple_converts(self):
+        from persistent.list import PersistentList
+        registry = self._makeOne()
+        # It converts when the item is found...
+        result = registry._removeValueFromLeaf(('a', 'b'), 'b')
+        self.assertIsInstance(result, PersistentList)
+        self.assertEqual(result, ['a'])
+        # ...and when it is not found
+        result = registry._removeValueFromLeaf(('a',), 'b')
+        self.assertIsInstance(result, PersistentList)
+        self.assertEqual(result, ['a'])
+
+    def test__addValueFromLeaf_preserves_identity(self):
+        registry = self._makeOne()
+        first = registry._addValueToLeaf(None, 'a')
+        second = registry._addValueToLeaf(first, 'b')
+        self.assertIs(first, second)
+        self.assertEqual(second, ['a', 'b'])
+
+    def test__removeValueFromLeaf_preserves_identity(self):
+        registry = self._makeOne()
+        first = registry._addValueToLeaf(None, 'a')
+        second = registry._addValueToLeaf(first, 'b')
+        third = registry._addValueToLeaf(second, 'c')
+        fourth = registry._removeValueFromLeaf(third, 'c')
+        self.assertIs(first, second)
+        self.assertIs(third, fourth)
+        self.assertIs(first, fourth)
+        self.assertEqual(fourth, ['a', 'b'])
 
 
 @skipIfNoPersistent