From 23e8cd65d54b0f5c79afa2c2402425ce33a663c1 Mon Sep 17 00:00:00 2001 From: Philipp von Weitershausen Date: Tue, 24 Jul 2007 19:40:28 +0000 Subject: [PATCH] Merged from old philikon-aq-and-__parent__ branch: Log message for revision 71221: Step 2: Make aq_acquire aware of __parent__ pointers, even if the object isn't acquisition wrapped. Log message for revision 71223: Add another test that tests acquisition wrappers with containers that have __parent__. Log message for revision 71225: Cosmetics: adjust a piece of code that I added earlier to the indentation style of the overall file Log message for revision 71226: Cleanup: * no need to introduce another variable where we check for a __parent__ attribute * clean up after failed getattr (it throws an AttributeError) * properly DECREF the __parent__ attribute when it's no longer needed and the wrapper that is temporarily created from the __parent__ attribute. Log message for revision 75578: Added a test to Acquisition that shows the current segmentation fault problem, when Acquisition goes in circles. Log message for revision 76140: First attempt to fix 'Acquisition problem' when encountering cyclic hierarchies via __parent__ pointers. [hannosch, nouri] In addition, Hanno and Nouri's fix was expanded to not only cover circular __parent__ pointers but also to cover a mixture of circular __parent__ and aq_parent pointers (which can occur when old Implicit acquisition meets new __parent__ pointer code). Also cleaned up much of the comments and added more comments. --- _Acquisition.c | 133 ++++++++++++++++---- tests.py | 320 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 433 insertions(+), 20 deletions(-) diff --git a/_Acquisition.c b/_Acquisition.c index 9736e7e..1e120c3 100644 --- a/_Acquisition.c +++ b/_Acquisition.c @@ -38,7 +38,8 @@ static PyObject *py__add__, *py__sub__, *py__mul__, *py__div__, *py__long__, *py__float__, *py__oct__, *py__hex__, *py__getitem__, *py__setitem__, *py__delitem__, *py__getslice__, *py__setslice__, *py__delslice__, *py__contains__, - *py__len__, *py__of__, *py__call__, *py__repr__, *py__str__, *py__cmp__; + *py__len__, *py__of__, *py__call__, *py__repr__, *py__str__, *py__cmp__, + *py__parent__; static PyObject *Acquired=0; @@ -82,7 +83,7 @@ init_py_names(void) INIT_PY_NAME(__repr__); INIT_PY_NAME(__str__); INIT_PY_NAME(__cmp__); - + INIT_PY_NAME(__parent__); #undef INIT_PY_NAME } @@ -414,6 +415,23 @@ static PyObject * Wrapper_findattr(Wrapper *self, PyObject *oname, PyObject *filter, PyObject *extra, PyObject *orig, int sob, int sco, int explicit, int containment) +/* + Parameters: + + sob + Search self->obj for the 'oname' attribute + + sco + Search self->container for the 'oname' attribute + + explicit + Explicitly acquire 'oname' attribute from container (assumed with + implicit acquisition wrapper) + + containment + Use the innermost wrapper ("aq_inner") for looking up the 'oname' + attribute. +*/ { PyObject *r, *v, *tb; char *name=""; @@ -486,6 +504,7 @@ Wrapper_findattr(Wrapper *self, PyObject *oname, Py_XDECREF(r); Py_XDECREF(v); Py_XDECREF(tb); r=NULL; } + /* normal attribute lookup */ else if ((r=PyObject_GetAttr(self->obj,oname))) { if (r==Acquired) @@ -520,6 +539,7 @@ Wrapper_findattr(Wrapper *self, PyObject *oname, PyErr_Clear(); } + /* Lookup has failed, acquire it from parent. */ if (sco && (*name != '_' || explicit)) return Wrapper_acquire(self, oname, filter, extra, orig, explicit, containment); @@ -533,23 +553,34 @@ Wrapper_acquire(Wrapper *self, PyObject *oname, PyObject *filter, PyObject *extra, PyObject *orig, int explicit, int containment) { - PyObject *r; + PyObject *r, *v, *tb; int sob=1, sco=1; if (self->container) { + /* If the container has an acquisition wrapper itself, we'll use + Wrapper_findattr to progress further. */ if (isWrapper(self->container)) { if (self->obj && isWrapper(self->obj)) { - /* Try to optimize search by recognizing repeated obs in path */ + /* Try to optimize search by recognizing repeated + objects in path. */ if (WRAPPER(self->obj)->container== WRAPPER(self->container)->container) sco=0; else if (WRAPPER(self->obj)->container== WRAPPER(self->container)->obj) sob=0; - } + } + + /* Don't search the container when the container of the + container is the same object as 'self'. */ + if (WRAPPER(self->container)->container == WRAPPER(self)->obj) + { + sco=0; + containment=1; + } r=Wrapper_findattr((Wrapper*)self->container, oname, filter, extra, orig, sob, sco, explicit, @@ -558,8 +589,46 @@ Wrapper_acquire(Wrapper *self, PyObject *oname, if (r && has__of__(r)) ASSIGN(r,__of__(r,OBJECT(self))); return r; } + /* If the container has a __parent__ pointer, we create an + acquisition wrapper for it accordingly. Then we can proceed + with Wrapper_findattr, just as if the container had an + acquisition wrapper in the first place (see above). */ + else if ((r = PyObject_GetAttr(self->container, py__parent__))) + { + ASSIGN(self->container, newWrapper(self->container, r, + (PyTypeObject*)&Wrappertype)); + + /* Don't search the container when the parent of the parent + is the same object as 'self' */ + if (WRAPPER(r)->obj == WRAPPER(self)->obj) + sco=0; + + Py_DECREF(r); /* don't need __parent__ anymore */ + + r=Wrapper_findattr((Wrapper*)self->container, + oname, filter, extra, orig, sob, sco, explicit, + containment); + /* There's no need to DECREF the wrapper here because it's + not stored in self->container, thus 'self' owns its + reference now */ + return r; + } + /* The container is the end of the acquisition chain; if we + can't look up the attribute here, we can't look it up at + all. */ else { + /* We need to clean up the AttributeError from the previous + getattr (because it has clearly failed). */ + PyErr_Fetch(&r,&v,&tb); + if (r && (r != PyExc_AttributeError)) + { + PyErr_Restore(r,v,tb); + return NULL; + } + Py_XDECREF(r); Py_XDECREF(v); Py_XDECREF(tb); + r=NULL; + if ((r=PyObject_GetAttr(self->container,oname))) { if (r == Acquired) { Py_DECREF(r); @@ -1341,8 +1410,7 @@ static PyObject * capi_aq_acquire(PyObject *self, PyObject *name, PyObject *filter, PyObject *extra, int explicit, PyObject *defalt, int containment) { - - PyObject *result; + PyObject *result, *v, *tb; if (filter==Py_None) filter=0; @@ -1352,22 +1420,47 @@ capi_aq_acquire(PyObject *self, PyObject *name, PyObject *filter, WRAPPER(self), name, filter, extra, OBJECT(self),1, explicit || WRAPPER(self)->ob_type==(PyTypeObject*)&Wrappertype, - explicit, containment); - - /* Not wrapped and no filter, so just getattr */ - if (! filter) return PyObject_GetAttr(self, name); + explicit, containment); + /* Not wrapped; check if we have a __parent__ pointer. If that's + the case, we create a wrapper and pretend it's business as + usual */ + else if ((result = PyObject_GetAttr(self, py__parent__))) + { + self = newWrapper(self, result, (PyTypeObject*)&Wrappertype); + Py_DECREF(result); /* don't need __parent__ anymore */ + result = Wrapper_findattr(WRAPPER(self), name, filter, extra, + OBJECT(self), 1, 1, explicit, containment); + /* Get rid of temporary wrapper */ + Py_DECREF(self); + return result; + } + /* No wrapper and no __parent__, so just getattr. */ + else + { + /* We need to clean up the AttributeError from the previous + getattr (because it has clearly failed). */ + PyErr_Fetch(&result,&v,&tb); + if (result && (result != PyExc_AttributeError)) + { + PyErr_Restore(result,v,tb); + return NULL; + } + Py_XDECREF(result); Py_XDECREF(v); Py_XDECREF(tb); - /* Crap, we've got to construct a wrapper so we can use Wrapper_findattr */ - UNLESS (self=newWrapper(self, Py_None, (PyTypeObject*)&Wrappertype)) - return NULL; - - result=Wrapper_findattr(WRAPPER(self), name, filter, extra, OBJECT(self), - 1, 1, explicit, containment); + if (! filter) return PyObject_GetAttr(self, name); - /* get rid of temp wrapper */ - Py_DECREF(self); + /* Crap, we've got to construct a wrapper so we can use + Wrapper_findattr */ + UNLESS (self=newWrapper(self, Py_None, (PyTypeObject*)&Wrappertype)) + return NULL; + + result=Wrapper_findattr(WRAPPER(self), name, filter, extra, OBJECT(self), + 1, 1, explicit, containment); - return result; + /* Get rid of temporary wrapper */ + Py_DECREF(self); + return result; + } } static PyObject * diff --git a/tests.py b/tests.py index 7d714fb..506de29 100644 --- a/tests.py +++ b/tests.py @@ -1690,6 +1690,326 @@ def test_proxying(): """ +class Location(object): + __parent__ = None + +class ECLocation(ExtensionClass.Base): + __parent__ = None + +def test___parent__no_wrappers(): + """ + Acquisition also works with objects that aren't wrappers, as long + as they have __parent__ pointers. Let's take a hierarchy like + z --isParent--> y --isParent--> x: + + >>> x = Location() + >>> y = Location() + >>> z = Location() + >>> x.__parent__ = y + >>> y.__parent__ = z + + and some attributes that we want to acquire: + + >>> x.hello = 'world' + >>> y.foo = 42 + >>> z.foo = 43 # this should not be found + >>> z.bar = 3.145 + + ``aq_acquire`` works we know it from implicit/acquisition wrappers: + + >>> Acquisition.aq_acquire(x, 'hello') + 'world' + >>> Acquisition.aq_acquire(x, 'foo') + 42 + >>> Acquisition.aq_acquire(x, 'bar') + 3.145 + + TODO aq_parent, aq_chain + """ + +def test_implicit_wrapper_as___parent__(): + """ + Let's do the same test again, only now not all objects are of the + same kind and link to each other via __parent__ pointers. The + root is a stupid ExtensionClass object: + + >>> class Root(ExtensionClass.Base): + ... bar = 3.145 + >>> z = Root() + + The intermediate parent is an object that supports implicit + acquisition. We bind it to the root via the __of__ protocol: + + >>> class Impl(Acquisition.Implicit): + ... foo = 42 + >>> y = Impl().__of__(z) + + The child object is again a simple object with a simple __parent__ + pointer: + + >>> x = Location() + >>> x.hello = 'world' + >>> x.__parent__ = y + + ``aq_acquire`` works as expected from implicit/acquisition + wrappers: + + >>> Acquisition.aq_acquire(x, 'hello') + 'world' + >>> Acquisition.aq_acquire(x, 'foo') + 42 + >>> Acquisition.aq_acquire(x, 'bar') + 3.145 + + Note that also the (implicit) acquisition wrapper has a __parent__ + pointer, which is automatically computed from the acquisition + container (it's identical to aq_parent): + + >>> y.__parent__ is z + True + + Just as much as you can assign to aq_parent, you can also assign + to __parent__ to change the acquisition context of the wrapper: + + >>> newroot = Root() + >>> y.__parent__ = newroot + >>> y.__parent__ is z + False + >>> y.__parent__ is newroot + True + + Note that messing with the wrapper won't in any way affect the + wrapped object: + + >>> Acquisition.aq_base(y).__parent__ + Traceback (most recent call last): + ... + AttributeError: __parent__ + + TODO aq_parent, aq_chain + """ + +def test_explicit_wrapper_as___parent__(): + """ + Let's do this test yet another time, with an explicit wrapper: + + >>> class Root(ExtensionClass.Base): + ... bar = 3.145 + >>> z = Root() + + The intermediate parent is an object that supports implicit + acquisition. We bind it to the root via the __of__ protocol: + + >>> class Expl(Acquisition.Explicit): + ... foo = 42 + >>> y = Expl().__of__(z) + + The child object is again a simple object with a simple __parent__ + pointer: + + >>> x = Location() + >>> x.hello = 'world' + >>> x.__parent__ = y + + ``aq_acquire`` works as expected from implicit/acquisition + wrappers: + + >>> Acquisition.aq_acquire(x, 'hello') + 'world' + >>> Acquisition.aq_acquire(x, 'foo') + 42 + >>> Acquisition.aq_acquire(x, 'bar') + 3.145 + + Note that also the (explicit) acquisition wrapper has a __parent__ + pointer, which is automatically computed from the acquisition + container (it's identical to aq_parent): + + >>> y.__parent__ is z + True + + Just as much as you can assign to aq_parent, you can also assign + to __parent__ to change the acquisition context of the wrapper: + + >>> newroot = Root() + >>> y.__parent__ = newroot + >>> y.__parent__ is z + False + >>> y.__parent__ is newroot + True + + Note that messing with the wrapper won't in any way affect the + wrapped object: + + >>> Acquisition.aq_base(y).__parent__ + Traceback (most recent call last): + ... + AttributeError: __parent__ + + TODO aq_parent, aq_chain + """ + +def test_implicit_wrapper_has_nonwrapper_as_aq_parent(): + """Let's do this the other way around: The root and the + intermediate parent is an object that doesn't support acquisition, + + >>> y = ECLocation() + >>> z = Location() + >>> y.__parent__ = z + >>> y.foo = 42 + >>> z.foo = 43 # this should not be found + >>> z.bar = 3.145 + + only the outmost object does: + + >>> class Impl(Acquisition.Implicit): + ... hello = 'world' + >>> x = Impl().__of__(y) + + Again, acquiring objects work as usual: + + >>> Acquisition.aq_acquire(x, 'hello') + 'world' + >>> Acquisition.aq_acquire(x, 'foo') + 42 + >>> Acquisition.aq_acquire(x, 'bar') + 3.145 + + Because the outmost object, ``x``, is wrapped in an implicit + acquisition wrapper, we can also use direct attribute access: + + >>> x.hello + 'world' + >>> x.foo + 42 + >>> x.bar + 3.145 + + TODO aq_parent, aq_chain + """ + +def test_explicit_wrapper_has_nonwrapper_as_aq_parent(): + """Let's do this the other way around: The root and the + intermediate parent is an object that doesn't support acquisition, + + >>> y = ECLocation() + >>> z = Location() + >>> y.__parent__ = z + >>> y.foo = 42 + >>> z.foo = 43 # this should not be found + >>> z.bar = 3.145 + + only the outmost object does: + + >>> class Expl(Acquisition.Explicit): + ... hello = 'world' + >>> x = Expl().__of__(y) + + Again, acquiring objects work as usual: + + >>> Acquisition.aq_acquire(x, 'hello') + 'world' + >>> Acquisition.aq_acquire(x, 'foo') + 42 + >>> Acquisition.aq_acquire(x, 'bar') + 3.145 + + TODO aq_parent, aq_chain + """ + +def test___parent__aq_parent_circles(): + """ + As a general safety belt, Acquisition won't follow a mixture of + circular __parent__ pointers and aq_parent wrappers. These can + occurr when code that uses implicit acquisition wrappers meets + code that uses __parent__ pointers. + + >>> class Impl(Acquisition.Implicit): + ... hello = 'world' + + >>> class Impl2(Acquisition.Implicit): + ... hello = 'world2' + ... only = 'here' + + >>> x = Impl() + >>> y = Impl2().__of__(x) + >>> x.__parent__ = y + + >>> x.__parent__.aq_base is y.aq_base + True + + >>> x.__parent__.__parent__ is x + True + + >>> x.hello + 'world' + >>> Acquisition.aq_acquire(x, 'hello') + 'world' + + >>> x.only + Traceback (most recent call last): + ... + AttributeError: only + >>> Acquisition.aq_acquire(x, 'only') + 'here' + + >>> Acquisition.aq_acquire(x, 'non_existant_attr') + Traceback (most recent call last): + ... + AttributeError: non_existant_attr + + >>> Acquisition.aq_acquire(y, 'non_existant_attr') + Traceback (most recent call last): + ... + AttributeError: non_existant_attr + + >>> x.non_existant_attr + Traceback (most recent call last): + ... + AttributeError: non_existant_attr + + >>> y.non_existant_attr + Traceback (most recent call last): + ... + AttributeError: non_existant_attr + + """ + +def test___parent__parent__circles(): + """ + Acquisition won't follow circular __parent__ references: + + >>> class Impl(Acquisition.Implicit): + ... hello = 'world' + + >>> class Impl2(Acquisition.Implicit): + ... hello = 'world2' + ... only = 'here' + + >>> x = Impl() + >>> y = Impl2() + >>> x.__parent__ = y + >>> y.__parent__ = x + + >>> x.__parent__.__parent__ is x + True + + >>> Acquisition.aq_acquire(x, 'hello') + 'world' + >>> Acquisition.aq_acquire(x, 'only') + 'here' + + >>> Acquisition.aq_acquire(x, 'non_existant_attr') + Traceback (most recent call last): + ... + AttributeError: non_existant_attr + + >>> Acquisition.aq_acquire(y, 'non_existant_attr') + Traceback (most recent call last): + ... + AttributeError: non_existant_attr + """ + import unittest from zope.testing.doctest import DocTestSuite