Log message for revision 68776: Merge from regebro-traversalfix branch: - 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 - The defaultView directive now only looks up views, not attributes.
Changed: U Zope/branches/2.10/doc/CHANGES.txt U Zope/branches/2.10/lib/python/OFS/Traversable.py U Zope/branches/2.10/lib/python/OFS/tests/testTraverse.py _U Zope/branches/2.10/lib/python/Products/ U Zope/branches/2.10/lib/python/ZPublisher/BaseRequest.py U Zope/branches/2.10/lib/python/ZPublisher/HTTPRequest.py U Zope/branches/2.10/lib/python/ZPublisher/tests/testBaseRequest.py -=- Modified: Zope/branches/2.10/doc/CHANGES.txt =================================================================== --- Zope/branches/2.10/doc/CHANGES.txt 2006-06-20 15:58:06 UTC (rev 68775) +++ Zope/branches/2.10/doc/CHANGES.txt 2006-06-21 11:26:57 UTC (rev 68776) @@ -38,6 +38,15 @@ - 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 + + - The defaultView directive now only looks up views, not attributes. + Zope 2.10.0 beta 1 (2006/05/30) Restructuring Modified: Zope/branches/2.10/lib/python/OFS/Traversable.py =================================================================== --- Zope/branches/2.10/lib/python/OFS/Traversable.py 2006-06-20 15:58:06 UTC (rev 68775) +++ Zope/branches/2.10/lib/python/OFS/Traversable.py 2006-06-21 11:26:57 UTC (rev 68776) @@ -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/2.10/lib/python/OFS/tests/testTraverse.py =================================================================== --- Zope/branches/2.10/lib/python/OFS/tests/testTraverse.py 2006-06-20 15:58:06 UTC (rev 68775) +++ Zope/branches/2.10/lib/python/OFS/tests/testTraverse.py 2006-06-21 11:26:57 UTC (rev 68776) @@ -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__': Property changes on: Zope/branches/2.10/lib/python/Products ___________________________________________________________________ Name: svn:externals - Five -r 68672 svn://svn.zope.org/repos/main/Products.Five/trunk + Five -r 68754 svn://svn.zope.org/repos/main/Products.Five/trunk Modified: Zope/branches/2.10/lib/python/ZPublisher/BaseRequest.py =================================================================== --- Zope/branches/2.10/lib/python/ZPublisher/BaseRequest.py 2006-06-20 15:58:06 UTC (rev 68775) +++ Zope/branches/2.10/lib/python/ZPublisher/BaseRequest.py 2006-06-21 11:26:57 UTC (rev 68776) @@ -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,24 +81,37 @@ 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 # status code to 404 - request.RESPONSE.setStatus(200) + 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 @@ -133,7 +147,9 @@ # deprecated. So we handle that here: default_name = queryDefaultViewName(self.context, request) if default_name is not None: - return self.context, (default_name,) + # Adding '@@' here forces this to be a view. + # A neater solution might be desireable. + return self.context, ('@@' + default_name,) return self.context, () Modified: Zope/branches/2.10/lib/python/ZPublisher/HTTPRequest.py =================================================================== --- Zope/branches/2.10/lib/python/ZPublisher/HTTPRequest.py 2006-06-20 15:58:06 UTC (rev 68775) +++ Zope/branches/2.10/lib/python/ZPublisher/HTTPRequest.py 2006-06-21 11:26:57 UTC (rev 68776) @@ -1498,11 +1498,11 @@ else: # Broken Cookie without = nor value. - broken_p = paramlessre.match(text) - if broken_p: - l = len(broken_p.group(1)) - name = broken_p.group(2) - value = '' + broken_p = paramlessre.match(text) + if broken_p: + l = len(broken_p.group(1)) + name = broken_p.group(2) + value = '' else: return result Modified: Zope/branches/2.10/lib/python/ZPublisher/tests/testBaseRequest.py =================================================================== --- Zope/branches/2.10/lib/python/ZPublisher/tests/testBaseRequest.py 2006-06-20 15:58:06 UTC (rev 68775) +++ Zope/branches/2.10/lib/python/ZPublisher/tests/testBaseRequest.py 2006-06-21 11:26:57 UTC (rev 68776) @@ -247,9 +247,149 @@ self.assertRaises(NotFound, r.traverse, 'folder/simpleSet') self.assertRaises(NotFound, r.traverse, 'folder/simpleFrozenSet') +from ZPublisher import NotFound +import zope.interface +import zope.component +import zope.testing.cleanup +import zope.traversing.namespace +from zope.publisher.browser import IBrowserRequest +from zope.publisher.browser import IDefaultBrowserLayer +from zope.traversing.interfaces import ITraversable + + +class IDummy(zope.interface.Interface): + """IDummy""" + +class DummyObjectZ3(DummyObjectBasic): + zope.interface.implements(IDummy) + def __init__(self, name): + self.name = name + +class DummyObjectZ3WithAttr(DummyObjectZ3): + def meth(self): + """doc""" + return 'meth on %s' % self.name + def methonly(self): + """doc""" + return 'methonly on %s' % self.name + +class DummyView(Implicit): + def __init__(self, content, request): + self.content = content + self.request = request + def __call__(self): + return 'view on %s' % (self.content.name) + +class TestBaseRequestZope3Views(TestCase): + + def setUp(self): + zope.testing.cleanup.cleanUp() + self.root = DummyObjectBasic() + folder = self.root._setObject('folder', DummyObjectZ3('folder')) + folder._setObject('obj', DummyObjectZ3('obj')) + folder._setObject('withattr', DummyObjectZ3WithAttr('withattr')) + folder2 = self.root._setObject('folder2', + DummyObjectZ3WithAttr('folder2')) + folder2._setObject('obj2', DummyObjectZ3('obj2')) + folder2._setObject('withattr2', DummyObjectZ3WithAttr('withattr2')) + gsm = zope.component.getGlobalSiteManager() + + # The request needs to implement the proper interface + zope.interface.classImplements(BaseRequest, IDefaultBrowserLayer) + + # Define our 'meth' view + gsm.registerAdapter(DummyView, (IDummy, IDefaultBrowserLayer), None, + 'meth') + + # Bind the 'view' namespace (for @@ traversal) + gsm.registerAdapter(zope.traversing.namespace.view, + (IDummy, IDefaultBrowserLayer), ITraversable, + 'view') + + def tearDown(self): + zope.testing.cleanup.cleanUp() + + def makeBaseRequest(self): + response = HTTPResponse() + environment = { + 'URL': '', + 'PARENTS': [self.root], + 'steps': [], + '_hacked_path': 0, + '_test_counter': 0, + 'response': response, + } + return BaseRequest(environment) + + def setDefaultViewName(self, name): + from zope.component.interfaces import IDefaultViewName + gsm = zope.component.getGlobalSiteManager() + gsm.registerAdapter(name, (IDummy, IBrowserRequest), IDefaultViewName, + '') + + def test_traverse_view(self): + """simple view""" + r = self.makeBaseRequest() + ob = r.traverse('folder/obj/meth') + self.assertEqual(ob(), 'view on obj') + ob = r.traverse('folder/obj/@@meth') + self.assertEqual(ob(), 'view on obj') + # using default view + self.setDefaultViewName('meth') + ob = r.traverse('folder/obj') + self.assertEqual(ob(), 'view on obj') + + def test_traverse_view_attr_local(self): + """method on object used first""" + r = self.makeBaseRequest() + ob = r.traverse('folder/withattr/meth') + self.assertEqual(ob(), 'meth on withattr') + ob = r.traverse('folder/withattr/@@meth') + self.assertEqual(ob(), 'view on withattr') + # using default view + self.setDefaultViewName('meth') + ob = r.traverse('folder/withattr') + self.assertEqual(ob(), 'view on withattr') + + def test_traverse_view_attr_above(self): + """view takes precedence over acquired attribute""" + r = self.makeBaseRequest() + ob = r.traverse('folder2/obj2/meth') + self.assertEqual(ob(), 'view on obj2') # used to be buggy (acquired) + ob = r.traverse('folder2/obj2/@@meth') + self.assertEqual(ob(), 'view on obj2') + # using default view + self.setDefaultViewName('meth') + ob = r.traverse('folder2/obj2') + self.assertEqual(ob(), 'view on obj2') + + def test_traverse_view_attr_local2(self): + """method with other method above""" + r = self.makeBaseRequest() + ob = r.traverse('folder2/withattr2/meth') + self.assertEqual(ob(), 'meth on withattr2') + ob = r.traverse('folder2/withattr2/@@meth') + self.assertEqual(ob(), 'view on withattr2') + # using default view + self.setDefaultViewName('meth') + ob = r.traverse('folder2/withattr2') + self.assertEqual(ob(), 'view on withattr2') + + def test_traverse_view_attr_acquired(self): + """normal acquired attribute without view""" + r = self.makeBaseRequest() + ob = r.traverse('folder2/obj2/methonly') + self.assertEqual(ob(), 'methonly on folder2') + self.assertRaises(NotFound, r.traverse, 'folder2/obj2/@@methonly') + # using default view + self.setDefaultViewName('methonly') + self.assertRaises(NotFound, r.traverse, 'folder2/obj2') + def test_suite(): - return TestSuite( ( makeSuite(TestBaseRequest), ) ) + return TestSuite( ( makeSuite(TestBaseRequest), + makeSuite(TestBaseRequestZope3Views), + ) ) if __name__ == '__main__': main(defaultTest='test_suite') _______________________________________________ Zope-Checkins maillist - Zope-Checkins@zope.org http://mail.zope.org/mailman/listinfo/zope-checkins