Log message for revision 78314:
  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.
  

Changed:
  U   Zope/branches/philikon-aq/lib/python/Acquisition/_Acquisition.c
  U   Zope/branches/philikon-aq/lib/python/Acquisition/tests.py

-=-
Modified: Zope/branches/philikon-aq/lib/python/Acquisition/_Acquisition.c
===================================================================
--- Zope/branches/philikon-aq/lib/python/Acquisition/_Acquisition.c     
2007-07-24 14:41:47 UTC (rev 78313)
+++ Zope/branches/philikon-aq/lib/python/Acquisition/_Acquisition.c     
2007-07-24 19:40:28 UTC (rev 78314)
@@ -38,7 +38,8 @@
   *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_NAME(__repr__);
   INIT_PY_NAME(__str__);
   INIT_PY_NAME(__cmp__);
-  
+  INIT_PY_NAME(__parent__);
 #undef INIT_PY_NAME
 }
 
@@ -414,6 +415,23 @@
 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 @@
          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 @@
       PyErr_Clear();
     }
 
+  /* Lookup has failed, acquire it from parent. */
   if (sco && (*name != '_' || explicit)) 
     return Wrapper_acquire(self, oname, filter, extra, orig, explicit, 
                           containment);
@@ -533,24 +553,35 @@
                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, 
                             containment);
@@ -558,8 +589,46 @@
          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 @@
 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 @@
              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;
+      if (! filter) return PyObject_GetAttr(self, name);
+
+      /* 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);
+      result=Wrapper_findattr(WRAPPER(self), name, filter, extra, OBJECT(self),
+                              1, 1, explicit, containment);
 
-  /* get rid of temp wrapper */
-  Py_DECREF(self);
-
-  return result;
+      /* Get rid of temporary wrapper */
+      Py_DECREF(self);
+      return result;
+    }
 }
 
 static PyObject *

Modified: Zope/branches/philikon-aq/lib/python/Acquisition/tests.py
===================================================================
--- Zope/branches/philikon-aq/lib/python/Acquisition/tests.py   2007-07-24 
14:41:47 UTC (rev 78313)
+++ Zope/branches/philikon-aq/lib/python/Acquisition/tests.py   2007-07-24 
19:40:28 UTC (rev 78314)
@@ -1690,6 +1690,326 @@
     """
 
 
+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
 

_______________________________________________
Zope-Checkins maillist  -  Zope-Checkins@zope.org
http://mail.zope.org/mailman/listinfo/zope-checkins

Reply via email to