Skip to content

Commit

Permalink
Make provided/implementedBy and adapter registries respect super().
Browse files Browse the repository at this point in the history
The query functions now start by looking at the next class in the MRO (interfaces directly provided by the underlying object are not found).

Adapter registries automatically pick up providedBy change to start finding the correct implementations of adapters, but to make that really useful they needed to change to unpack super() arguments and pass __self__ to the factory.

Fixes #11

Unfortunately, this makes PyPy unable to build the C extensions.

Additional crash-safety for adapter lookup.

Make the C functions get the cache only after resolving the
``required`` into a tuple, in case of side-effects like...clearing the
cache. This could lead to the ``cache`` object being deallocated
before we used it.

Drop the ``tuplefy`` function in favor of a direct call to
``PySequence_Tuple``. It's what the ``tuple`` constructor would do
anyway and saves a few steps.

Make sure that getting ``providedBy(super())`` and
``implementedBy(super())`` have no side effects.
  • Loading branch information
jamadden committed Mar 9, 2020
1 parent c931999 commit 1e720c3
Show file tree
Hide file tree
Showing 10 changed files with 697 additions and 55 deletions.
4 changes: 0 additions & 4 deletions .travis.yml
Expand Up @@ -27,10 +27,6 @@ jobs:
- sphinx-build -b doctest -d docs/_build/doctrees docs docs/_build/doctest
after_success:

- name: PyPy C Extensions
env: PURE_PYTHON=0
python: pypy

- name: CPython No C Extension
env: PURE_PYTHON=1
python: 3.8
Expand Down
17 changes: 17 additions & 0 deletions CHANGES.rst
Expand Up @@ -134,6 +134,23 @@
`dolmen.builtins <https://pypi.org/project/dolmen.builtins/>`_.
See `issue 138 <https://github.com/zopefoundation/zope.interface/issues/138>`_.

- Make ``providedBy()`` and ``implementedBy()`` respect ``super``
objects. For instance, if class ``Derived`` implements ``IDerived``
and extends ``Base`` which in turn implements ``IBase``, then
``providedBy(super(Derived, derived))`` will return ``[IBase]``.
Previously it would have returned ``[IDerived]`` (in general, it
would previously have returned whatever would have been returned
without ``super``).

Along with this change, adapter registries will unpack ``super``
objects into their ``__self___`` before passing it to the factory.
Together, this means that ``component.getAdapter(super(Derived,
self), ITarget)`` is now meaningful.

See `issue 11 <https://github.com/zopefoundation/zope.interface/issues/11>`_.

- Fix a potential interpreter crash in the low-level adapter
registry lookup functions. See issue 11.

4.7.1 (2019-11-11)
==================
Expand Down
27 changes: 25 additions & 2 deletions docs/adapter.rst
Expand Up @@ -202,11 +202,11 @@ factories:
... pass

>>> @zope.interface.implementer(IR)
... class X:
... class X(object):
... pass

>>> @zope.interface.implementer(IProvide1)
... class Y:
... class Y(object):
... def __init__(self, context):
... self.context = context

Expand Down Expand Up @@ -238,6 +238,29 @@ We can register and lookup by name too:
>>> y.context is x
True

Passing ``super`` objects works as expected to find less specific adapters:

.. doctest::

>>> class IDerived(IR):
... pass
>>> @zope.interface.implementer(IDerived)
... class Derived(X):
... pass
>>> class DerivedAdapter(Y):
... def query_next(self):
... return registry.queryAdapter(
... super(type(self.context), self.context),
... IProvide1)
>>> registry.register([IDerived], IProvide1, '', DerivedAdapter)
>>> derived = Derived()
>>> adapter = registry.queryAdapter(derived, IProvide1)
>>> adapter.__class__.__name__
'DerivedAdapter'
>>> adapter = adapter.query_next()
>>> adapter.__class__.__name__
'Y'

When the adapter factory produces ``None``, then this is treated as if no
adapter has been found. This allows us to prevent adaptation (when desired)
and let the adapter factory determine whether adaptation is possible based on
Expand Down
4 changes: 3 additions & 1 deletion docs/conf.py
Expand Up @@ -17,7 +17,9 @@
import pkg_resources
sys.path.append(os.path.abspath('../src'))
rqmt = pkg_resources.require('zope.interface')[0]

# Import and document the pure-python versions of things; they tend to have better
# docstrings and signatures.
os.environ['PURE_PYTHON'] = '1'
# If extensions (or modules to document with autodoc) are in another directory,
# add these directories to sys.path here. If the directory is relative to the
# documentation root, use os.path.abspath to make it absolute, like shown here.
Expand Down
8 changes: 5 additions & 3 deletions setup.py
Expand Up @@ -69,10 +69,12 @@ def _unavailable(self, e):
]

is_jython = 'java' in sys.platform
is_pypy = hasattr(sys, 'pypy_version_info')

# Jython cannot build the C optimizations. Everywhere else,
# including PyPy, defer the decision to runtime.
if is_jython:
# Jython cannot build the C optimizations. Nor, as of 7.3, can PyPy (
# it doesn't have PySuper_Type) Everywhere else, defer the decision to
# runtime.
if is_jython or is_pypy:
ext_modules = []
else:
ext_modules = codeoptimization
Expand Down
91 changes: 61 additions & 30 deletions src/zope/interface/_zope_interface_coptimizations.c
Expand Up @@ -43,6 +43,11 @@ static PyObject *str__conform__, *str_call_conform, *adapter_hooks;
static PyObject *str_uncached_lookup, *str_uncached_lookupAll;
static PyObject *str_uncached_subscriptions;
static PyObject *str_registry, *strro, *str_generation, *strchanged;
static PyObject *str__get__;
static PyObject *str__self__;
static PyObject *str__thisclass__;
static PyObject *str__self_class__;
static PyObject *str__mro__;

static PyTypeObject *Implements;

Expand Down Expand Up @@ -91,6 +96,7 @@ import_declarations(void)
return 0;
}


static PyTypeObject SpecType; /* Forward */

static PyObject *
Expand All @@ -111,6 +117,12 @@ implementedBy(PyObject *ignored, PyObject *cls)

PyObject *dict = NULL, *spec;

if (PyObject_TypeCheck(cls, &PySuper_Type))
{
// Let merging be handled by Python.
return implementedByFallback(cls);
}

if (PyType_Check(cls))
{
dict = TYPE(cls)->tp_dict;
Expand Down Expand Up @@ -168,6 +180,7 @@ getObjectSpecification(PyObject *ignored, PyObject *ob)
if (result != NULL && PyObject_TypeCheck(result, &SpecType))
return result;


PyErr_Clear();

/* We do a getattr here so as not to be defeated by proxies */
Expand All @@ -176,11 +189,11 @@ getObjectSpecification(PyObject *ignored, PyObject *ob)
{
PyErr_Clear();
if (imported_declarations == 0 && import_declarations() < 0)
return NULL;
return NULL;

Py_INCREF(empty);
return empty;
}

result = implementedBy(NULL, cls);
Py_DECREF(cls);

Expand All @@ -192,7 +205,15 @@ providedBy(PyObject *ignored, PyObject *ob)
{
PyObject *result, *cls, *cp;

result = NULL;

if (PyObject_TypeCheck(ob, &PySuper_Type))
{
return implementedBy(NULL, ob);
}

result = PyObject_GetAttr(ob, str__providedBy__);

if (result == NULL)
{
PyErr_Clear();
Expand Down Expand Up @@ -947,27 +968,14 @@ _getcache(lookup *self, PyObject *provided, PyObject *name)
return result
*/
static PyObject *
tuplefy(PyObject *v)
{
if (! PyTuple_Check(v))
{
v = PyObject_CallFunctionObjArgs(OBJECT(&PyTuple_Type), v, NULL);
if (v == NULL)
return NULL;
}
else
Py_INCREF(v);

return v;
}
static PyObject *
_lookup(lookup *self,
PyObject *required, PyObject *provided, PyObject *name,
PyObject *default_)
{
PyObject *result, *key, *cache;

result = key = cache = NULL;
#ifdef PY3K
if ( name && !PyUnicode_Check(name) )
#else
Expand All @@ -978,14 +986,19 @@ _lookup(lookup *self,
"name is not a string or unicode");
return NULL;
}
cache = _getcache(self, provided, name);
if (cache == NULL)
return NULL;

required = tuplefy(required);
/* If `required` is a lazy sequence, it could have arbitrary side-effects,
such as clearing our caches. So we must not retreive the cache until
after resolving it. */
required = PySequence_Tuple(required);
if (required == NULL)
return NULL;


cache = _getcache(self, provided, name);
if (cache == NULL)
return NULL;

if (PyTuple_GET_SIZE(required) == 1)
key = PyTuple_GET_ITEM(required, 0);
else
Expand Down Expand Up @@ -1154,12 +1167,23 @@ _adapter_hook(lookup *self,
return NULL;

if (factory != Py_None)
{
{
if (PyObject_TypeCheck(object, &PySuper_Type)) {
PyObject* self = PyObject_GetAttr(object, str__self__);
if (self == NULL)
{
Py_DECREF(factory);
return NULL;
}
// Borrow the reference to self
Py_DECREF(self);
object = self;
}
result = PyObject_CallFunctionObjArgs(factory, object, NULL);
Py_DECREF(factory);
if (result == NULL || result != Py_None)
return result;
}
}
else
result = factory; /* None */

Expand Down Expand Up @@ -1217,15 +1241,16 @@ _lookupAll(lookup *self, PyObject *required, PyObject *provided)
{
PyObject *cache, *result;

/* resolve before getting cache. See note in _lookup. */
required = PySequence_Tuple(required);
if (required == NULL)
return NULL;

ASSURE_DICT(self->_mcache);
cache = _subcache(self->_mcache, provided);
if (cache == NULL)
return NULL;

required = tuplefy(required);
if (required == NULL)
return NULL;

result = PyDict_GetItem(cache, required);
if (result == NULL)
{
Expand Down Expand Up @@ -1287,15 +1312,16 @@ _subscriptions(lookup *self, PyObject *required, PyObject *provided)
{
PyObject *cache, *result;

/* resolve before getting cache. See note in _lookup. */
required = PySequence_Tuple(required);
if (required == NULL)
return NULL;

ASSURE_DICT(self->_scache);
cache = _subcache(self->_scache, provided);
if (cache == NULL)
return NULL;

required = tuplefy(required);
if (required == NULL)
return NULL;

result = PyDict_GetItem(cache, required);
if (result == NULL)
{
Expand Down Expand Up @@ -1736,6 +1762,11 @@ init(void)
DEFINE_STRING(_generation);
DEFINE_STRING(ro);
DEFINE_STRING(changed);
DEFINE_STRING(__self__);
DEFINE_STRING(__get__);
DEFINE_STRING(__thisclass__);
DEFINE_STRING(__self_class__);
DEFINE_STRING(__mro__);
#undef DEFINE_STRING
adapter_hooks = PyList_New(0);
if (adapter_hooks == NULL)
Expand Down

0 comments on commit 1e720c3

Please sign in to comment.