BaseAdapterRegistry.subscribe/unsubscribe non-symmetric updating _provided, leaves orphan entries

[jamadden]

While working on #224 I noticed that there were no tests for BaseAdapterRegistry._provided (which keeps a map of {interface: reference count} of interfaces it knows about in order to add them as "extendors" to the AdapterLookup classes).

The register method increments this count and stores it:

zope.interface/src/zope/interface/adapter.py

Lines 154 to 157 in bf0de08

	n = self._provided.get(provided, 0) + 1
	self._provided[provided] = n
	if n == 1:
	self._v_lookup.add_extendor(provided)

As does the subscribe method:

zope.interface/src/zope/interface/adapter.py

Lines 247 to 251 in bf0de08

	if provided is not None:
	n = self._provided.get(provided, 0) + 1
	self._provided[provided] = n
	if n == 1:
	self._v_lookup.add_extendor(provided)

The inverse of register, unregister, decrements this count and either stores it, or, if it hits 0, removes it:

zope.interface/src/zope/interface/adapter.py

Lines 219 to 224 in bf0de08

	n = self._provided[provided] - 1
	if n == 0:
	del self._provided[provided]
	self._v_lookup.remove_extendor(provided)
	else:
	self._provided[provided] = n

However, the inverse of subscribe, unsubscribe, appears to have a bug. It decrements the count, but only if it hits zero is it removed. It never stores it, which means it can never hit 0 just by unsubscribing.

zope.interface/src/zope/interface/adapter.py

Lines 305 to 309 in bf0de08

	if provided is not None:
	n = self._provided[provided] + len(new) - len(old)
	if n == 0:
	del self._provided[provided]
	self._v_lookup.remove_extendor(provided)

Making unsubscribe store the decremented value doesn't break any tests (and fixes the tests I was adding). Because the high-level class Components automatically both registers and subscribes utilities, it's fairly easy to end up with orphaned entries in _provided.

Here, I'll define an interface and class and register it both as the default (unnamed) and a named utility for the same interface.

>>> from zope.interface import Interface, implementer
>>> from zope.interface.registry import Components
>>> class IFoo(Interface):
...     pass
...
>>> @implementer(IFoo)
... class Foo:
...     pass
...
>>> foo = Foo()
>>> foo2 = Foo()
>>> comp = Components()
>>> comp.registerUtility(foo)
None
>>> comp.registerUtility(foo2, name='bar')
None

Now, because IFoo was both provided and subscribed twice, it has a refcount of 4, and the two objects show up once each in _adapters and _subscribers of the adapter registry:

>>> comp.utilities._provided
{<InterfaceClass __main__.IFoo>: 4}
>>> comp.utilities._adapters
[{<InterfaceClass __main__.IFoo>: {'': <__main__.Foo object at 0x10a09dfa0>,
                                   'bar': <__main__.Foo object at 0x10a08e140>}}]
>>> comp.utilities._subscribers
[{<InterfaceClass __main__.IFoo>: {'': (<__main__.Foo object at 0x10a09dfa0>,
                                        <__main__.Foo object at 0x10a08e140>)}}]

Now we can unregister both utilities, and their entries will be gone from _adapters and _subscribers:

>>> comp.unregisterUtility(foo)
True
>>> comp.unregisterUtility(foo2, name='bar')
True
>>> comp.utilities._adapters, comp.utilities._subscribers
([], [])

But IFoo is still in _provided because of this issue:

>>> comp.utilities._provided
{<InterfaceClass __main__.IFoo>: 2}

Because of the way unsubscribe calculates the delta (removing all equal objects) if I register the same identical object (which is if course equal to itself) as both the named and unnamed utility, this appears to work. That is, _provided has no remaining references to IFoo.

When using non-equal objects, this only works (empty _provided) if I make unsubscribe store the decremented value. The equal object case still works then.