[Zope-Checkins] SVN: Zope/branches/regebro-traversalfix/ View and attribute lookup order was changed to the following:

Lennart Regebro cvs-admin at zope.org
Thu Jun 15 11:18:44 EDT 2006


Log message for revision 68655:
  View and attribute lookup order was changed to the following:
     1. Unacquired attributes
     2. Views
     3. Acquired attributes
  According to consensus in z3-five mailing list:
  http://codespeak.net/pipermail/z3-five/2006q2/001474.html
  
  

Changed:
  U   Zope/branches/regebro-traversalfix/doc/CHANGES.txt
  U   Zope/branches/regebro-traversalfix/lib/python/OFS/Traversable.py
  U   Zope/branches/regebro-traversalfix/lib/python/OFS/tests/testTraverse.py
  U   Zope/branches/regebro-traversalfix/lib/python/ZPublisher/BaseRequest.py

-=-
Modified: Zope/branches/regebro-traversalfix/doc/CHANGES.txt
===================================================================
--- Zope/branches/regebro-traversalfix/doc/CHANGES.txt	2006-06-15 14:47:15 UTC (rev 68654)
+++ Zope/branches/regebro-traversalfix/doc/CHANGES.txt	2006-06-15 15:18:40 UTC (rev 68655)
@@ -33,4 +33,10 @@
 
       - Collector #2063: cleaned up some mess in MailHost.sendTemplate()
 
-
+      - View and attribute lookup order was changed to the following:
+           1. Unacquired attributes
+           2. Views
+           3. Acquired attributes
+        According to consensus in z3-five mailing list:
+        http://codespeak.net/pipermail/z3-five/2006q2/001474.html
+        

Modified: Zope/branches/regebro-traversalfix/lib/python/OFS/Traversable.py
===================================================================
--- Zope/branches/regebro-traversalfix/lib/python/OFS/Traversable.py	2006-06-15 14:47:15 UTC (rev 68654)
+++ Zope/branches/regebro-traversalfix/lib/python/OFS/Traversable.py	2006-06-15 15:18:40 UTC (rev 68655)
@@ -190,76 +190,93 @@
                         continue
 
                 bobo_traverse = _getattr(obj, '__bobo_traverse__', _none)
-                if name and name[:1] in '@+':
-                    # Process URI segment parameters.
-                    ns, nm = nsParse(name)
-                    if ns:
-                        try:
-                            next = namespaceLookup(ns, nm, obj, 
-                                                   self.REQUEST).__of__(obj)
-                            if restricted and not securityManager.validate(
-                                obj, obj, name, next):
+                try:
+                    if name and name[:1] in '@+':
+                        # Process URI segment parameters.
+                        ns, nm = nsParse(name)
+                        if ns:
+                            try:
+                                next = namespaceLookup(ns, nm, obj, 
+                                                       self.REQUEST).__of__(obj)
+                                if restricted and not securityManager.validate(
+                                    obj, obj, name, next):
+                                    raise Unauthorized, name
+                            except TraversalError:
+                                raise AttributeError(name)
+                    elif bobo_traverse is not _none:
+                        next = bobo_traverse(REQUEST, name)
+                        if restricted:
+                            if aq_base(next) is not next:
+                                # The object is wrapped, so the acquisition
+                                # context is the container.
+                                container = aq_parent(aq_inner(next))
+                            elif _getattr(next, 'im_self', _none) is not _none:
+                                # Bound method, the bound instance
+                                # is the container
+                                container = next.im_self
+                            elif _getattr(aq_base(obj), name, marker) == next:
+                                # Unwrapped direct attribute of the object so
+                                # object is the container
+                                container = obj
+                            else:
+                                # Can't determine container
+                                container = _none
+                            try:
+                                validated = securityManager.validate(
+                                                       obj, container, name, next)
+                            except Unauthorized:
+                                # If next is a simple unwrapped property, it's
+                                # parentage is indeterminate, but it may have been
+                                # acquired safely.  In this case validate will
+                                # raise an error, and we can explicitly check that
+                                # our value was acquired safely.
+                                validated = 0
+                                if container is _none and \
+                                       guarded_getattr(obj, name, marker) is next:
+                                    validated = 1
+                            if not validated:
                                 raise Unauthorized, name
-                        except TraversalError:
-                            raise AttributeError(name)
-                elif bobo_traverse is not _none:
-                    next = bobo_traverse(REQUEST, name)
-                    if restricted:
-                        if aq_base(next) is not next:
-                            # The object is wrapped, so the acquisition
-                            # context is the container.
-                            container = aq_parent(aq_inner(next))
-                        elif _getattr(next, 'im_self', _none) is not _none:
-                            # Bound method, the bound instance
-                            # is the container
-                            container = next.im_self
-                        elif _getattr(aq_base(obj), name, marker) == next:
-                            # Unwrapped direct attribute of the object so
-                            # object is the container
-                            container = obj
+                    else:
+                        if hasattr(aq_base(obj), name):
+                            if restricted:
+                                next = guarded_getattr(obj, name, marker)
+                            else:
+                                next = _getattr(obj, name, marker)
                         else:
-                            # Can't determine container
-                            container = _none
-                        try:
-                            validated = securityManager.validate(
-                                                   obj, container, name, next)
-                        except Unauthorized:
-                            # If next is a simple unwrapped property, it's
-                            # parentage is indeterminate, but it may have been
-                            # acquired safely.  In this case validate will
-                            # raise an error, and we can explicitly check that
-                            # our value was acquired safely.
-                            validated = 0
-                            if container is _none and \
-                                   guarded_getattr(obj, name, marker) is next:
-                                validated = 1
-                        if not validated:
-                            raise Unauthorized, name
-                else:
-                    if restricted:
-                        next = guarded_getattr(obj, name, marker)
-                    else:
-                        next = _getattr(obj, name, marker)
-                    if next is marker:
-                        try:
                             try:
                                 next=obj[name]
                             except AttributeError:
                                 # Raise NotFound for easier debugging
                                 # instead of AttributeError: __getitem__
                                 raise NotFound, name
-                        except (NotFound, KeyError): 
-                            # Try to look for a view
-                            next = queryMultiAdapter((obj, self.REQUEST), 
-                                                     Interface, name)
-                            if next is None:
-                                # Didn't find one, reraise the error:
-                                raise
-                            next = next.__of__(obj)
-                        if restricted and not securityManager.validate(
-                            obj, obj, _none, next):
-                            raise Unauthorized, name
 
+                except (AttributeError, NotFound, KeyError), e: 
+                    # Try to look for a view
+                    next = queryMultiAdapter((obj, self.REQUEST), 
+                                             Interface, name)
+
+                    if next is not None:
+                        next = next.__of__(obj)
+                    elif bobo_traverse is not None:
+                        # Attribute lookup should not be done after 
+                        # __bobo_traverse__:
+                        raise e
+                    else:
+                        # No view, try acquired attributes
+                        try:
+                            if restricted:
+                                next = guarded_getattr(obj, name, marker)
+                            else:
+                                next = _getattr(obj, name, marker)
+                        except AttributeError:
+                            raise e
+                    if next is marker:
+                        # Nothing found re-raise error
+                        raise e
+                
+                if restricted and not securityManager.validate(
+                    obj, obj, _none, next):
+                    raise Unauthorized, name
                 obj = next
 
             return obj

Modified: Zope/branches/regebro-traversalfix/lib/python/OFS/tests/testTraverse.py
===================================================================
--- Zope/branches/regebro-traversalfix/lib/python/OFS/tests/testTraverse.py	2006-06-15 14:47:15 UTC (rev 68654)
+++ Zope/branches/regebro-traversalfix/lib/python/OFS/tests/testTraverse.py	2006-06-15 15:18:40 UTC (rev 68655)
@@ -270,7 +270,7 @@
         bb = BoboTraversableWithAcquisition()
         bb = bb.__of__(self.root)
         self.failUnlessRaises(Unauthorized,
-                              self.root.folder1.restrictedTraverse, 'folder1')
+                              bb.restrictedTraverse, 'folder1')
 
     def testBoboTraverseToAcquiredAttribute(self):
         # Verify it's possible to use __bobo_traverse__ to an acquired
@@ -308,7 +308,7 @@
         newSecurityManager( None, UnitTestUser().__of__( self.root ) )
         self.root.stuff = 'stuff here'
         self.failUnlessRaises(Unauthorized,
-                              self.root.folder1.restrictedTraverse, 'stuff')
+                              self.app.folder1.restrictedTraverse, 'stuff')
 
     def testDefaultValueWhenUnathorized(self):
         # Test that traversing to an unauthorized object returns
@@ -335,9 +335,234 @@
             aq_base(self.root))
 
 
+import os, sys
+if __name__ == '__main__':
+    execfile(os.path.join(sys.path[0], 'framework.py'))
+
+
+class SimpleClass(object):
+    """Class with no __bobo_traverse__."""
+
+
+def test_traversable():
+    """
+    Test the behaviour of unrestrictedTraverse and views. The tests are copies
+    from Five.browser.tests.test_traversable, but instead of publishing they
+    do unrestrictedTraverse.
+
+      >>> import Products.Five
+      >>> from Products.Five import zcml
+      >>> zcml.load_config("configure.zcml", Products.Five)
+      >>> from Testing.makerequest import makerequest
+      >>> self.app = makerequest(self.app)
+      
+    ``SimpleContent`` is a traversable class by default.  Its fallback
+    traverser should raise NotFound when traversal fails.  (Note: If
+    we return None in __fallback_traverse__, this test passes but for
+    the wrong reason: None doesn't have a docstring so BaseRequest
+    raises NotFoundError.)
+
+      >>> from Products.Five.tests.testing.simplecontent import manage_addSimpleContent
+      >>> manage_addSimpleContent(self.folder, 'testoid', 'Testoid')
+      >>> from zExceptions import NotFound
+      >>> try:
+      ...    self.folder.testoid.unrestrictedTraverse('doesntexist')
+      ... except NotFound:
+      ...    pass
+      
+    Now let's take class which already has a __bobo_traverse__ method.
+    Five should correctly use that as a fallback.
+
+      >>> configure_zcml = '''
+      ... <configure xmlns="http://namespaces.zope.org/zope"
+      ...            xmlns:meta="http://namespaces.zope.org/meta"
+      ...            xmlns:browser="http://namespaces.zope.org/browser"
+      ...            xmlns:five="http://namespaces.zope.org/five">
+      ... 
+      ... <!-- make the zope2.Public permission work -->
+      ... <meta:redefinePermission from="zope2.Public" to="zope.Public" />
+      ...
+      ... <!-- this view will never be found -->
+      ... <browser:page
+      ...     for="Products.Five.tests.testing.fancycontent.IFancyContent"
+      ...     class="Products.Five.browser.tests.pages.FancyView"
+      ...     attribute="view"
+      ...     name="fancyview"
+      ...     permission="zope2.Public"
+      ...     />
+      ... <!-- these two will -->
+      ... <browser:page
+      ...     for="Products.Five.tests.testing.fancycontent.IFancyContent"
+      ...     class="Products.Five.browser.tests.pages.FancyView"
+      ...     attribute="view"
+      ...     name="raise-attributeerror"
+      ...     permission="zope2.Public"
+      ...     />
+      ... <browser:page
+      ...     for="Products.Five.tests.testing.fancycontent.IFancyContent"
+      ...     class="Products.Five.browser.tests.pages.FancyView"
+      ...     attribute="view"
+      ...     name="raise-keyerror"
+      ...     permission="zope2.Public"
+      ...     />
+      ... </configure>'''
+      >>> zcml.load_string(configure_zcml)
+
+      >>> from Products.Five.tests.testing.fancycontent import manage_addFancyContent
+      >>> info = manage_addFancyContent(self.folder, 'fancy', '')
+
+    In the following test we let the original __bobo_traverse__ method
+    kick in:
+
+      >>> self.folder.fancy.unrestrictedTraverse('something-else').index_html({})
+      'something-else'
+
+    Once we have a custom __bobo_traverse__ method, though, it always
+    takes over.  Therefore, unless it raises AttributeError or
+    KeyError, it will be the only way traversal is done.
+
+      >>> self.folder.fancy.unrestrictedTraverse('fancyview').index_html({})
+      'fancyview'
+      
+
+    Note that during publishing, if the original __bobo_traverse__ method
+    *does* raise AttributeError or KeyError, we can get normal view look-up.
+    In unrestrictedTraverse, we don't. Maybe we should? Needs discussing.
+
+      >>> self.folder.fancy.unrestrictedTraverse('raise-attributeerror')()
+      u'Fancy, fancy'
+
+      >>> self.folder.fancy.unrestrictedTraverse('raise-keyerror')()
+      u'Fancy, fancy'
+
+      >>> try:
+      ...     self.folder.fancy.unrestrictedTraverse('raise-valueerror')
+      ... except ValueError:
+      ...     pass
+
+    In the Zope 2 ZPublisher, an object with a __bobo_traverse__ will not do
+    attribute lookup unless the __bobo_traverse__ method itself does it (i.e.
+    the __bobo_traverse__ is the only element used for traversal lookup).
+    Let's demonstrate:
+
+      >>> from Products.Five.tests.testing.fancycontent import manage_addNonTraversableFancyContent
+      >>> info = manage_addNonTraversableFancyContent(self.folder, 'fancy_zope2', '')
+      >>> self.folder.fancy_zope2.an_attribute = 'This is an attribute'
+      >>> self.folder.fancy_zope2.unrestrictedTraverse('an_attribute').index_html({})
+      'an_attribute'
+
+    Without a __bobo_traverse__ method this would have returned the attribute
+    value 'This is an attribute'.  Let's make sure the same thing happens for
+    an object that has been marked traversable by Five:
+
+      >>> self.folder.fancy.an_attribute = 'This is an attribute'
+      >>> self.folder.fancy.unrestrictedTraverse('an_attribute').index_html({})
+      'an_attribute'
+
+
+    Clean up:
+
+      >>> from zope.app.testing.placelesssetup import tearDown
+      >>> tearDown()
+
+    Verify that after cleanup, there's no cruft left from five:traversable::
+
+      >>> from Products.Five.browser.tests.test_traversable import SimpleClass
+      >>> hasattr(SimpleClass, '__bobo_traverse__')
+      False
+      >>> hasattr(SimpleClass, '__fallback_traverse__')
+      False
+
+      >>> from Products.Five.tests.testing.fancycontent import FancyContent
+      >>> hasattr(FancyContent, '__bobo_traverse__')
+      True
+      >>> hasattr(FancyContent.__bobo_traverse__, '__five_method__')
+      False
+      >>> hasattr(FancyContent, '__fallback_traverse__')
+      False
+    """
+
+def test_view_doesnt_shadow_attribute():
+    """
+    Test that views don't shadow attributes, e.g. items in a folder.
+
+    Let's first define a browser page for object managers called
+    ``eagle``:
+
+      >>> configure_zcml = '''
+      ... <configure xmlns="http://namespaces.zope.org/zope"
+      ...            xmlns:meta="http://namespaces.zope.org/meta"
+      ...            xmlns:browser="http://namespaces.zope.org/browser"
+      ...            xmlns:five="http://namespaces.zope.org/five">
+      ...   <!-- make the zope2.Public permission work -->
+      ...   <meta:redefinePermission from="zope2.Public" to="zope.Public" />
+      ...   <browser:page
+      ...       name="eagle"
+      ...       for="OFS.interfaces.IObjectManager"
+      ...       class="Products.Five.browser.tests.pages.SimpleView"
+      ...       attribute="eagle"
+      ...       permission="zope2.Public"
+      ...       />
+      ...   <browser:page
+      ...       name="mouse"
+      ...       for="OFS.interfaces.IObjectManager"
+      ...       class="Products.Five.browser.tests.pages.SimpleView"
+      ...       attribute="mouse"
+      ...       permission="zope2.Public"
+      ...       />
+      ... </configure>'''
+      >>> import Products.Five
+      >>> from Products.Five import zcml
+      >>> zcml.load_config("configure.zcml", Products.Five)
+      >>> zcml.load_string(configure_zcml)
+
+    Then we create a traversable folder...
+
+      >>> from Products.Five.tests.testing.folder import manage_addFiveTraversableFolder
+      >>> manage_addFiveTraversableFolder(self.folder, 'ftf')
+
+    and add an object called ``eagle`` to it:
+
+      >>> from Products.Five.tests.testing.simplecontent import manage_addIndexSimpleContent
+      >>> manage_addIndexSimpleContent(self.folder.ftf, 'eagle', 'Eagle')
+
+    When we publish the ``ftf/eagle`` now, we expect the attribute to
+    take precedence over the view during traversal:
+
+      >>> self.folder.ftf.unrestrictedTraverse('eagle').index_html({})
+      'Default index_html called'
+
+    Of course, unless we explicitly want to lookup the view using @@:
+
+      >>> self.folder.ftf.unrestrictedTraverse('@@eagle')()
+      u'The eagle has landed'
+
+
+    Some weird implementations of __bobo_traverse__, like the one
+    found in OFS.Application, raise NotFound.  Five still knows how to
+    deal with this, hence views work there too:
+
+      >>> self.app.unrestrictedTraverse('@@eagle')()
+      u'The eagle has landed'
+
+    However, acquired attributes *should* be shadowed. See discussion on
+    http://codespeak.net/pipermail/z3-five/2006q2/001474.html
+    
+      >>> manage_addIndexSimpleContent(self.folder, 'mouse', 'Mouse')
+      >>> self.folder.ftf.unrestrictedTraverse('mouse')()
+      u'The mouse has been eaten by the eagle'
+      
+    Clean up:
+
+      >>> from zope.app.testing.placelesssetup import tearDown
+      >>> tearDown()
+    """
+
 def test_suite():
     suite = unittest.TestSuite()
     suite.addTest( unittest.makeSuite( TestTraverse ) )
+    from Testing.ZopeTestCase import FunctionalDocTestSuite
+    suite.addTest( FunctionalDocTestSuite() )
     return suite
 
 if __name__ == '__main__':

Modified: Zope/branches/regebro-traversalfix/lib/python/ZPublisher/BaseRequest.py
===================================================================
--- Zope/branches/regebro-traversalfix/lib/python/ZPublisher/BaseRequest.py	2006-06-15 14:47:15 UTC (rev 68654)
+++ Zope/branches/regebro-traversalfix/lib/python/ZPublisher/BaseRequest.py	2006-06-15 15:18:40 UTC (rev 68655)
@@ -17,6 +17,7 @@
 from urllib import quote
 import xmlrpc
 from zExceptions import Forbidden, Unauthorized, NotFound
+from Acquisition import aq_base
 
 from zope.interface import implements, providedBy, Interface
 from zope.component import queryMultiAdapter
@@ -80,15 +81,16 @@
                     parents[-1:] = list(subobject[:-1])
                     object, subobject = subobject[-2:]
             else:
-                try:
-                    subobject=getattr(object, name)
-                except AttributeError:
-                    subobject=object[name]
+                # Try getting unacquired attributes:
+                if hasattr(aq_base(object), name):
+                    subobject = getattr(object, name)
+                else:
+                    subobject=object[name]                    
              
-        except (AttributeError, KeyError, NotFound):
-            # Find a view even if it doesn't start with @@, but only
-            # If nothing else could be found
-            subobject = queryMultiAdapter((object, request), Interface, name)
+        except (AttributeError, KeyError, NotFound), e:
+            # Nothing was found with __bobo_traverse__ or directly on
+            # the object. We try to fall back to a view:
+            subobject = queryMultiAdapter((object, request), Interface, name)                
             if subobject is not None:
                 # OFS.Application.__bobo_traverse__ calls
                 # REQUEST.RESPONSE.notFoundError which sets the HTTP
@@ -96,8 +98,20 @@
                 request.RESPONSE.setStatus(200)
                 # We don't need to do the docstring security check
                 # for views, so lets skip it and return the object here.
-                return subobject.__of__(object) 
-            raise
+                return subobject.__of__(object)
+            
+            # And lastly, of there is no view, try acquired attributes, but
+            # only if there is no __bobo_traverse__:
+            if not hasattr(object,'__bobo_traverse__'):
+                try:
+                    subobject=getattr(object, name)
+                    # Again, clear any error status created by __bobo_traverse__
+                    # because we actually found something:
+                    request.RESPONSE.setStatus(200)
+                    return subobject
+                except AttributeError:
+                    pass
+            raise e
 
         # Ensure that the object has a docstring, or that the parent
         # object has a pseudo-docstring for the object. Objects that



More information about the Zope-Checkins mailing list