Ensure Interface is the last item in the __sro__.

None of the elegant solutions mentioned in the issue worked out, so I had to brute force it.

Fixes #8
This commit is contained in:
Jason Madden 2020-03-16 17:24:29 -05:00
parent bd5a749b5b
commit 4c4e1c985f
No known key found for this signature in database
GPG Key ID: 349F84431A08B99E
5 changed files with 154 additions and 25 deletions

View File

@ -160,7 +160,7 @@
- Adopt Python's standard `C3 resolution order
<https://www.python.org/download/releases/2.3/mro/>`_ to compute the
``__iro___`` and ``__sro___`` of interfaces, with tweaks to support
``__iro__`` and ``__sro__`` of interfaces, with tweaks to support
additional cases that are common in interfaces but disallowed for
Python classes. Previously, an ad-hoc ordering that made no
particular guarantees was used.
@ -204,6 +204,12 @@
`issue 190
<https://github.com/zopefoundation/zope.interface/issues/190>`_.
- Ensure that ``Interface`` is always the last item in the ``__iro__``
and ``__sro__``. This is usually the case, but if classes that do
not implement any interfaces are part of a class inheritance
hierarchy, ``Interface`` could be assigned too high a priority.
See `issue 8 <https://github.com/zopefoundation/zope.interface/issues/8>`_.
4.7.2 (2020-03-10)
==================

View File

@ -689,9 +689,22 @@ that lists the specification and all of it's ancestors:
<InterfaceClass builtins.IBaz>,
<InterfaceClass builtins.IFoo>,
<InterfaceClass builtins.IBlat>,
<InterfaceClass zope.interface.Interface>,
<implementedBy ...object>)
<implementedBy ...object>,
<InterfaceClass zope.interface.Interface>)
>>> class IBiz(zope.interface.Interface):
... pass
>>> @zope.interface.implementer(IBiz)
... class Biz(Baz):
... pass
>>> pprint(zope.interface.implementedBy(Biz).__sro__)
(<implementedBy builtins.Biz>,
<InterfaceClass builtins.IBiz>,
<implementedBy builtins.Baz>,
<InterfaceClass builtins.IBaz>,
<InterfaceClass builtins.IFoo>,
<InterfaceClass builtins.IBlat>,
<implementedBy ...object>,
<InterfaceClass zope.interface.Interface>)
Tagged Values
=============

View File

@ -73,14 +73,24 @@ def add_verify_tests(cls, iface_classes_iter):
def test_ro(self, stdlib_class=stdlib_class, iface=iface):
from zope.interface import ro
from zope.interface import implementedBy
from zope.interface import Interface
self.assertEqual(
tuple(ro.ro(iface, strict=True)),
iface.__sro__)
implements = implementedBy(stdlib_class)
sro = implements.__sro__
self.assertIs(sro[-1], Interface)
# Check that we got the strict C3 resolution order, unless we
# know we cannot. Note that 'Interface' is virtual base that doesn't
# necessarily appear at the same place in the calculated SRO as in the
# final SRO.
strict = stdlib_class not in self.NON_STRICT_RO
self.assertEqual(
tuple(ro.ro(implements, strict=strict)),
implements.__sro__)
isro = ro.ro(implements, strict=strict)
isro.remove(Interface)
isro.append(Interface)
self.assertEqual(tuple(isro), sro)
name = 'test_auto_ro_' + suffix
test_ro.__name__ = name

View File

@ -22,6 +22,7 @@ import weakref
from zope.interface._compat import _use_c_impl
from zope.interface.exceptions import Invalid
from zope.interface.ro import ro
from zope.interface.ro import C3
__all__ = [
# Most of the public API from this module is directly exported
@ -226,6 +227,10 @@ class Specification(SpecificationBase):
"""
__slots__ = ()
# The root of all Specifications. This will be assigned `Interface`,
# once it is defined.
_ROOT = None
# Copy some base class methods for speed
isOrExtends = SpecificationBase.isOrExtends
providedBy = SpecificationBase.providedBy
@ -274,7 +279,7 @@ class Specification(SpecificationBase):
for b in self.__bases__:
b.unsubscribe(self)
# Register ourselves as a dependent of our bases
# Register ourselves as a dependent of our new bases
self._bases = bases
for b in bases:
b.subscribe(self)
@ -286,29 +291,76 @@ class Specification(SpecificationBase):
__setBases,
)
def _calculate_sro(self):
"""
Calculate and return the resolution order for this object, using its ``__bases__``.
Ensures that ``Interface`` is always the last (lowest priority) element.
"""
# We'd like to make Interface the lowest priority as a
# property of the resolution order algorithm. That almost
# works out naturally, but it fails when class inheritance has
# some bases that DO implement an interface, and some that DO
# NOT. In such a mixed scenario, you wind up with a set of
# bases to consider that look like this: [[..., Interface],
# [..., object], ...]. Depending on the order if inheritance,
# Interface can wind up before or after object, and that can
# happen at any point in the tree, meaning Interface can wind
# up somewhere in the middle of the order. Since Interface is
# treated as something that everything winds up implementing
# anyway (a catch-all for things like adapters), having it high up
# the order is bad. It's also bad to have it at the end, just before
# some concrete class: concrete classes should be HIGHER priority than
# interfaces (because there's only one class, but many implementations).
#
# One technically nice way to fix this would be to have
# ``implementedBy(object).__bases__ = (Interface,)``
#
# But: (1) That fails for old-style classes and (2) that causes
# everything to appear to *explicitly* implement Interface, when up
# to this point it's been an implicit virtual sort of relationship.
#
# So we force the issue by mutating the resolution order.
# TODO: Caching. Perhaps make ro.C3 able to re-use the computed ``__sro__``
# instead of re-doing it for the entire tree.
base_count = len(self._bases)
if base_count == 1:
# Fast path: One base makes it trivial to calculate
# the MRO.
sro = [self]
sro.extend(self.__bases__[0].__sro__)
else:
sro = ro(self)
if self._ROOT is not None:
# Once we don't have to deal with old-style classes,
# we can add a check and only do this if base_count > 1,
# if we tweak the bootstrapping for ``<implementedBy object>``
root = self._ROOT
sro = [
x
for x in sro
if x is not root
]
sro.append(root)
assert sro[-1] is root, sro
return sro
def changed(self, originally_changed):
"""We, or something we depend on, have changed
"""
We, or something we depend on, have changed.
By the time this is called, the things we depend on,
such as our bases, should themselves be stable.
"""
self._v_attrs = None
implied = self._implied
implied.clear()
if len(self.__bases__) == 1:
# Fast path: One base makes it trivial to calculate
# the MRO.
sro = self.__bases__[0].__sro__
ancestors = [self]
ancestors.extend(sro)
else:
ancestors = ro(self)
try:
if Interface not in ancestors:
ancestors.append(Interface)
except NameError:
pass # defining Interface itself
ancestors = self._calculate_sro()
self.__sro__ = tuple(ancestors)
self.__iro__ = tuple([ancestor for ancestor in ancestors
if isinstance(ancestor, InterfaceClass)
@ -318,7 +370,8 @@ class Specification(SpecificationBase):
# We directly imply our ancestors:
implied[ancestor] = ()
# Now, advise our dependents of change:
# Now, advise our dependents of change
# (being careful not to create the WeakKeyDictionary if not needed):
for dependent in tuple(self._dependents.keys() if self._dependents else ()):
dependent.changed(originally_changed)
@ -647,6 +700,11 @@ class InterfaceClass(Element, InterfaceBase, Specification):
Interface = InterfaceClass("Interface", __module__='zope.interface')
# Interface is the only member of its own SRO.
Interface._calculate_sro = lambda: (Interface,)
Interface.changed(Interface)
assert Interface.__sro__ == (Interface,)
Specification._ROOT = Interface
class Attribute(Element):

View File

@ -438,6 +438,48 @@ class SpecificationTests(unittest.TestCase):
self.assertTrue(spec.get('foo') is IFoo.get('foo'))
self.assertTrue(spec.get('bar') is IBar.get('bar'))
def test_multiple_inheritance_no_interfaces(self):
# If we extend an object that implements interfaces,
# plus ane that doesn't, we do not interject `Interface`
# early in the resolution order. It stays at the end,
# like it should.
# See https://github.com/zopefoundation/zope.interface/issues/8
from zope.interface.interface import Interface
from zope.interface.declarations import implementer
from zope.interface.declarations import implementedBy
class IDefaultViewName(Interface):
pass
class Context(object):
pass
class RDBModel(Context):
pass
class IOther(Interface):
pass
@implementer(IOther)
class OtherBase(object):
pass
class Model(OtherBase, Context):
pass
self.assertEqual(
implementedBy(Model).__sro__,
(
implementedBy(Model),
implementedBy(OtherBase),
IOther,
implementedBy(Context),
implementedBy(object),
Interface, # This used to be wrong, it used to be 2 too high.
)
)
class InterfaceClassTests(unittest.TestCase):
def _getTargetClass(self):