Log message for revision 65923: Fix http://codespeak.net/pipermail/z3-five/2006q1/001186.html: Modeled Five's __bobo_traverse__ very much like BaseRequest.traverse: 1. Look for an existing __bobo_traverse__ (in Five's case, it's called __fallback_traverse__) and call it if it exists 2. If not, try attribute lookup (possibly without acquisition wrappers for WebDAV non-POST/non-GET requests) or key item lookup. 3. If all that doesn't work, we try view lookup. This used to be #1 and would therefore shadow existing attributes.
Changed: U Products.Five/branches/philikon-fix-lookup-priorities/browser/tests/test_traversable.py U Products.Five/branches/philikon-fix-lookup-priorities/fiveconfigure.py U Products.Five/branches/philikon-fix-lookup-priorities/traversable.py -=- Modified: Products.Five/branches/philikon-fix-lookup-priorities/browser/tests/test_traversable.py =================================================================== --- Products.Five/branches/philikon-fix-lookup-priorities/browser/tests/test_traversable.py 2006-03-12 18:22:05 UTC (rev 65922) +++ Products.Five/branches/philikon-fix-lookup-priorities/browser/tests/test_traversable.py 2006-03-12 18:27:36 UTC (rev 65923) @@ -67,7 +67,8 @@ ... <five:traversable ... class="Products.Five.tests.testing.FiveTraversableFolder" ... /> - ... + ... + ... <!-- this view will never be found --> ... <browser:page ... for="Products.Five.tests.testing.fancycontent.IFancyContent" ... class="Products.Five.browser.tests.pages.FancyView" @@ -75,7 +76,21 @@ ... 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) @@ -92,23 +107,50 @@ ... something-else - Of course we also need to make sure that Zope 3 style view lookup - actually works: + 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. >>> print http(r''' ... GET /test_folder_1_/fancy/fancyview HTTP/1.1 ... ''') HTTP/1.1 200 OK ... + fancyview + + As said, if the original __bobo_traverse__ method *does* raise + AttributeError or KeyError, we can get normal view look-up. Other + exceptions are passed through just fine: + + >>> print http(r''' + ... GET /test_folder_1_/fancy/raise-attributeerror HTTP/1.1 + ... ''') + HTTP/1.1 200 OK + ... Fancy, fancy - + + >>> print http(r''' + ... GET /test_folder_1_/fancy/raise-keyerror HTTP/1.1 + ... ''') + HTTP/1.1 200 OK + ... + Fancy, fancy + + >>> print http(r''' + ... GET /test_folder_1_/fancy/raise-valueerror HTTP/1.1 + ... ''') + HTTP/1.1 500 Internal Server Error + ... + ...ValueError: raise-valueerror + ... + Five's traversable monkeypatches the __bobo_traverse__ method to do view lookup and then delegates back to the original __bobo_traverse__ or direct attribute/item lookup to do normal lookup. 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' @@ -118,7 +160,7 @@ HTTP/1.1 200 OK ... 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: @@ -154,6 +196,70 @@ 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" + ... /> + ... </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: + + >>> print http(r''' + ... GET /test_folder_1_/ftf/eagle HTTP/1.1 + ... ''') + HTTP/1.1 200 OK + ... + Default index_html called + + Of course, unless we explicitly want to lookup the view using @@: + + >>> print http(r''' + ... GET /test_folder_1_/ftf/@@eagle HTTP/1.1 + ... ''') + HTTP/1.1 200 OK + ... + The eagle has landed + + + Clean up: + + >>> from zope.app.testing.placelesssetup import tearDown + >>> tearDown() + """ + + def test_suite(): from Testing.ZopeTestCase import FunctionalDocTestSuite return FunctionalDocTestSuite() Modified: Products.Five/branches/philikon-fix-lookup-priorities/fiveconfigure.py =================================================================== --- Products.Five/branches/philikon-fix-lookup-priorities/fiveconfigure.py 2006-03-12 18:22:05 UTC (rev 65922) +++ Products.Five/branches/philikon-fix-lookup-priorities/fiveconfigure.py 2006-03-12 18:27:36 UTC (rev 65923) @@ -37,10 +37,10 @@ from zope.app.component.metaconfigure import adapter from zope.app.security.interfaces import IPermission -from viewable import Viewable -from traversable import Traversable -from bridge import fromZ2Interface -from browser.metaconfigure import page +from Products.Five.viewable import Viewable +from Products.Five.traversable import Traversable +from Products.Five.bridge import fromZ2Interface +from Products.Five.browser.metaconfigure import page debug_mode = App.config.getConfiguration().debug_mode @@ -128,14 +128,11 @@ isFiveMethod(class_.__bobo_traverse__)): return - if hasattr(class_, '__bobo_traverse__'): - if not isFiveMethod(class_.__bobo_traverse__): - # if there's an existing bobo_traverse hook already, use that - # as the traversal fallback method - setattr(class_, '__fallback_traverse__', class_.__bobo_traverse__) - if not hasattr(class_, '__fallback_traverse__'): - setattr(class_, '__fallback_traverse__', - Traversable.__fallback_traverse__.im_func) + if (hasattr(class_, '__bobo_traverse__') and + not isFiveMethod(class_.__bobo_traverse__)): + # if there's an existing bobo_traverse hook already, use that + # as the traversal fallback method + setattr(class_, '__fallback_traverse__', class_.__bobo_traverse__) setattr(class_, '__bobo_traverse__', Traversable.__bobo_traverse__.im_func) Modified: Products.Five/branches/philikon-fix-lookup-priorities/traversable.py =================================================================== --- Products.Five/branches/philikon-fix-lookup-priorities/traversable.py 2006-03-12 18:22:05 UTC (rev 65922) +++ Products.Five/branches/philikon-fix-lookup-priorities/traversable.py 2006-03-12 18:27:36 UTC (rev 65923) @@ -15,8 +15,6 @@ $Id$ """ -from zExceptions import NotFound - from zope.component import getMultiAdapter, ComponentLookupError from zope.interface import implements, Interface from zope.publisher.interfaces import ILayer @@ -28,8 +26,9 @@ from zope.app.publication.browser import setDefaultSkin from zope.app.interface import queryType -from AccessControl import getSecurityManager -from Products.Five.security import newInteraction +import Products.Five.security +from zExceptions import NotFound +from ZPublisher import xmlrpc _marker = object @@ -47,18 +46,6 @@ """ __five_traversable__ = True - def __fallback_traverse__(self, REQUEST, name): - """Method hook for fallback traversal - - This method is called by __bobo_traverse___ when Zope3-style - ITraverser traversal fails. - - Just raise a AttributeError to indicate traversal has failed - and let Zope do it's job. - """ - raise NotImplementedError - __fallback_traverse__.__five_method__ = True - def __bobo_traverse__(self, REQUEST, name): """Hook for Zope 2 traversal @@ -66,43 +53,86 @@ It allows us to trick it into faking the Zope 3 traversal system by using an ITraverser adapter. """ + # We are trying to be compatible with Zope 2 and 3 traversal + # behaviour as much as possible. Therefore the first part of + # this method is based on BaseRequest.traverse's behaviour: + # 1. If an object has __bobo_traverse__, use it + # 2. Otherwise do attribute look-up (w/o acquisition if necessary) + # 3. If that doesn't work, try key item lookup. + + if hasattr(self, '__fallback_traverse__'): + try: + return self.__fallback_traverse__(REQUEST, name) + except (AttributeError, KeyError): + pass + else: + # Note - no_acquire_flag is necessary to support things + # like DAV. We have to make sure that the target object + # is not acquired if the request_method is other than GET + # or POST. Otherwise, you could never use PUT to add a new + # object named 'test' if an object 'test' existed above it + # in the heirarchy -- you'd always get the existing object + # :( + no_acquire_flag = False + method = REQUEST.get('REQUEST_METHOD', 'GET').upper() + if ((method not in ('GET', 'POST') or + isinstance(getattr(REQUEST, 'response', {}), xmlrpc.Response)) + and getattr(REQUEST, 'maybe_webdav_client', False)): + # Probably a WebDAV client. + no_acquire_flag = True + + try: + if (no_acquire_flag and + len(REQUEST['TraversalRequestNameStack']) == 0 and + hasattr(self, 'aq_base')): + if hasattr(self.aq_base, name): + return getattr(self, name) + else: + pass # attribute not found + else: + return getattr(self, name) + except AttributeError: + try: + return self[name] + except (KeyError, IndexError, TypeError, AttributeError): + pass # key not found + + # This is the part Five adds: + # 4. If neither __bobo_traverse__ nor attribute/key look-up + # work, we try to find a Zope 3-style view + + # For that we need to make sure we have a good request + # (sometimes __bobo_traverse__ gets a stub request) if not IBrowserRequest.providedBy(REQUEST): # Try to get the REQUEST by acquisition REQUEST = getattr(self, 'REQUEST', None) if not IBrowserRequest.providedBy(REQUEST): REQUEST = FakeRequest() - # set the default skin on the request if it doesn't have any + # Con Zope 3 into using Zope 2's checkPermission + Products.Five.security.newInteraction() + + # Set the default skin on the request if it doesn't have any # layers set on it yet if queryType(REQUEST, ILayer) is None: setDefaultSkin(REQUEST) - # con Zope 3 into using Zope 2's checkPermission - newInteraction() + # Use the ITraverser adapter (which in turn uses ITraversable + # adapters) to traverse to a view. Note that we're mixing + # object-graph and object-publishing traversal here, but Zope + # 2 has no way to tell us when to use which... + # TODO Perhaps we can decide on object-graph vs. + # object-publishing traversal depending on whether REQUEST is + # a stub or not? try: return ITraverser(self).traverse( path=[name], request=REQUEST).__of__(self) except (ComponentLookupError, LookupError, AttributeError, KeyError, NotFound): pass - try: - return self.__fallback_traverse__(REQUEST, name) - except NotImplementedError: - pass - # TODO: This should at least make an attempt to deal with - # potential WebDAV issues, in particular we should not perform - # acquisition for webdav requests. See BaseRequest.traverse for - # details. - try: - return getattr(self, name) - except AttributeError: - pass - try: - return self[name] - except (AttributeError, KeyError): - pass - raise AttributeError, name + raise AttributeError(name) + __bobo_traverse__.__five_method__ = True _______________________________________________ Zope-Checkins maillist - Zope-Checkins@zope.org http://mail.zope.org/mailman/listinfo/zope-checkins