Merge pull request #94 from zopefoundation/issue93

Fix the cache getting out of sync with _utility_registrations.

CHANGES.rst

@@ -8,6 +8,10 @@ Changes
   ``zope.component.persistentregistry.PersistentRegistry`` instances.
   See `issue 85 <https://github.com/zopefoundation/zope.interface/issues/85>`_.
 
+- Fix a regression that could lead to the utility registration cache
+  of ``Components`` getting out of sync. See `issue 93
+  <https://github.com/zopefoundation/zope.interface/issues/93>`_.
+
 4.4.1 (2017-05-13)
 ------------------
 

src/zope/interface/registry.py

@@ -67,12 +67,14 @@ def __delitem__(self, component):
                 return
         raise KeyError(component) # pragma: no cover
 
+def _defaultdict_int():
+    return defaultdict(int)
 
 class _UtilityRegistrations(object):
 
     def __init__(self, utilities, utility_registrations):
         # {provided -> {component: count}}
-        self._cache = defaultdict(lambda: defaultdict(int))
+        self._cache = defaultdict(_defaultdict_int)
         self._utilities = utilities
         self._utility_registrations = utility_registrations
 
@@ -138,18 +140,17 @@ def unregisterUtility(self, provided, name, component):
 @implementer(IComponents)
 class Components(object):
 
+    _v_utility_registrations_cache = None
+
     def __init__(self, name='', bases=()):
+        # __init__ is used for test cleanup as well as initialization.
+        # XXX add a separate API for test cleanup.
         assert isinstance(name, STRING_TYPES)
         self.__name__ = name
         self._init_registries()
         self._init_registrations()
         self.__bases__ = tuple(bases)
-
-        # __init__ is used for test cleanup as well as initialization.
-        # XXX add a separate API for test cleanup.
-        # See _utility_registrations below.
-        if hasattr(self, '_v_utility_registrations_cache'):
-            del self._v_utility_registrations_cache
+        self._v_utility_registrations_cache = None
 
     def __repr__(self):
         return "<%s %s>" % (self.__class__.__name__, self.__name__)
@@ -185,12 +186,14 @@ def _init_registrations(self):
     def _utility_registrations_cache(self):
         # 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:
-            self._v_utility_registrations_cache = _UtilityRegistrations(
-                self.utilities, self._utility_registrations)
-            return self._v_utility_registrations_cache
+        cache = self._v_utility_registrations_cache
+        if (cache is None
+            or cache._utilities is not self.utilities
+            or cache._utility_registrations is not self._utility_registrations):
+            cache = self._v_utility_registrations_cache = _UtilityRegistrations(
+                self.utilities,
+                self._utility_registrations)
+        return cache
 
     def _getBases(self):
         # Subclasses might override

src/zope/interface/tests/test_registry.py

@@ -342,6 +342,59 @@ class IFoo(InterfaceClass):
             comp.registerUtility(_to_reg, ifoo, _name, _info, False)
         self.assertEqual(len(_events), 0)
 
+    def test_registerUtility_changes_object_identity_after(self):
+        # If a subclass changes the identity of the _utility_registrations,
+        # the cache is updated and the right thing still happens.
+        class CompThatChangesAfter1Reg(self._getTargetClass()):
+            reg_count = 0
+            def registerUtility(self, *args):
+                self.reg_count += 1
+                super(CompThatChangesAfter1Reg, self).registerUtility(*args)
+                if self.reg_count == 1:
+                    self._utility_registrations = dict(self._utility_registrations)
+
+        comp = CompThatChangesAfter1Reg()
+        comp.registerUtility(object(), Interface)
+
+        self.assertEqual(len(list(comp.registeredUtilities())), 1)
+
+        class IFoo(Interface):
+            pass
+
+        comp.registerUtility(object(), IFoo)
+        self.assertEqual(len(list(comp.registeredUtilities())), 2)
+
+    def test_registerUtility_changes_object_identity_before(self):
+        # If a subclass changes the identity of the _utility_registrations,
+        # the cache is updated and the right thing still happens.
+        class CompThatChangesAfter2Reg(self._getTargetClass()):
+            reg_count = 0
+            def registerUtility(self, *args):
+                self.reg_count += 1
+                if self.reg_count == 2:
+                    self._utility_registrations = dict(self._utility_registrations)
+
+                super(CompThatChangesAfter2Reg, self).registerUtility(*args)
+
+        comp = CompThatChangesAfter2Reg()
+        comp.registerUtility(object(), Interface)
+
+        self.assertEqual(len(list(comp.registeredUtilities())), 1)
+
+        class IFoo(Interface):
+            pass
+
+        comp.registerUtility(object(), IFoo)
+        self.assertEqual(len(list(comp.registeredUtilities())), 2)
+
+
+        class IBar(Interface):
+            pass
+
+        comp.registerUtility(object(), IBar)
+        self.assertEqual(len(list(comp.registeredUtilities())), 3)
+
+
     def test_unregisterUtility_neither_factory_nor_component_nor_provided(self):
         comp = self._makeOne()
         self.assertRaises(TypeError, comp.unregisterUtility,