Florent Guillaume wrote:

So here's a proposal: how about having the following order: - __bobo_traverse__ - unacquired attribute - zope 3 views - acquired attributes

Attached is the current diff I'm working with (for Zope 2.10). Please review the unit test (which pass as is), to check if you agree with the semantics. I've decided that if you traverse ".../foo/@@something", then only the zope 3 views will be consulted and never an attribute. I hope everyone agrees with that. The remaining important question is: if a *default* view is specified using the zope 3 mechanism, should we always treat it as a zope 3 view, and refuse to lookup an attribute with that name? Currently the traverse code is not completely coherent with respect to that, there's a NotFound in the tests I added which isn't coherent with the rest. So we should decide on one semantic. To explicit the question, if you have: <browser:defaultView name="myview" for=".IFoo"/> and the publisher decides it has to use the default view (when ".../foo/" is traversed), should it try to lookup "myview" as an attribute? Or should only the zope 3 view be looked up? I'd be inclined to not use attributes, after all that's zope 3 directives we're talking about. Florent -- Florent Guillaume, Nuxeo (Paris, France) Director of R&D +33 1 40 33 71 59 http://nuxeo.com fg@nuxeo.com Index: lib/python/ZPublisher/tests/testBaseRequest.py =================================================================== --- lib/python/ZPublisher/tests/testBaseRequest.py (revision 68718) +++ lib/python/ZPublisher/tests/testBaseRequest.py (working copy) @@ -3,8 +3,15 @@ from Acquisition import Implicit from ZPublisher.BaseRequest import BaseRequest from ZPublisher.HTTPResponse import HTTPResponse +from ZPublisher import NotFound +import zope.interface +import zope.testing.cleanup +import zope.traversing.namespace +from zope.app.testing import ztapi +from zope.publisher.interfaces.browser import IDefaultBrowserLayer + class DummyObjectBasic(Implicit): """Dummy class with docstring.""" @@ -166,7 +173,6 @@ def test_traverse_withBDEmpty(self): # Collector 1079 (infinite loop 2) - from ZPublisher import NotFound r = self.makeBaseRequest() self.f1.objWithBD._default_path = [''] self.failUnlessRaises(NotFound, r.traverse, 'folder/objWithBD') @@ -174,7 +180,6 @@ def test_traverse_withBBT_handles_AttributeError(self): # Test that if __bobo_traverse__ raises AttributeError # that we get a NotFound - from ZPublisher import NotFound r = self.makeBaseRequest() self.failUnlessRaises(NotFound, r.traverse, 'folder/objWithBBT/bbt_foo') @@ -194,7 +199,6 @@ # and __bobo_traverse__ # __bobo_traverse__ should raise an AttributeError, which will # raise a NotFound - from ZPublisher import NotFound r = self.makeBaseRequest() self.f1.objWithBDBBT._default_path = ['xxx'] r = self.makeBaseRequest() @@ -214,27 +218,22 @@ self.assertEqual(r.response.base, '') def test_traverse_attribute_without_docstring(self): - from ZPublisher import NotFound r = self.makeBaseRequest() self.assertRaises(NotFound, r.traverse, 'folder/objBasic/noview') def test_traverse_class_without_docstring(self): - from ZPublisher import NotFound r = self.makeBaseRequest() self.assertRaises(NotFound, r.traverse, 'folder/objWithoutDocstring') def test_traverse_attribute_of_class_without_docstring(self): - from ZPublisher import NotFound r = self.makeBaseRequest() self.assertRaises(NotFound, r.traverse, 'folder/objWithoutDocstring/view') def test_traverse_attribute_and_class_without_docstring(self): - from ZPublisher import NotFound r = self.makeBaseRequest() self.assertRaises(NotFound, r.traverse, 'folder/objWithoutDocstring/noview') def test_traverse_simple_type(self): - from ZPublisher import NotFound r = self.makeBaseRequest() self.assertRaises(NotFound, r.traverse, 'folder/simpleString') self.assertRaises(NotFound, r.traverse, 'folder/simpleList') @@ -242,14 +241,144 @@ self.assertRaises(NotFound, r.traverse, 'folder/simpleComplex') def test_traverse_set_type(self): - from ZPublisher import NotFound r = self.makeBaseRequest() self.assertRaises(NotFound, r.traverse, 'folder/simpleSet') self.assertRaises(NotFound, r.traverse, 'folder/simpleFrozenSet') +class IDummy(zope.interface.Interface): + """IDummy""" + +class DummyObjectBasicZ3(DummyObjectBasic): + zope.interface.implements(IDummy) + def __init__(self, name): + self.name = name + +class DummyObjectBasicZ3WithAttr(DummyObjectBasicZ3): + 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() + self.f1 = self.root._setObject('folder', DummyObjectBasicZ3('folder')) + self.f1._setObject('obj', DummyObjectBasicZ3('obj')) + self.f1._setObject('withattr', DummyObjectBasicZ3WithAttr('withattr')) + self.f2 = self.root._setObject('folder2', + DummyObjectBasicZ3WithAttr('folder2')) + self.f2._setObject('obj2', DummyObjectBasicZ3('obj2')) + self.f2._setObject('withattr2', DummyObjectBasicZ3WithAttr('withattr2')) + + # This is needed to make "zope 3" traversing work + zope.interface.classImplements(BaseRequest, IDefaultBrowserLayer) + + # Define our 'meth' view + ztapi.browserView(IDummy, 'meth', DummyView) + + # Bind @@ to the view namespace + ztapi.provideNamespaceHandler('view', zope.traversing.namespace.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 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 + ztapi.setDefaultViewName(IDummy, 'meth') + ob = r.traverse('folder/obj/') + self.assertEqual(ob(), 'view on obj') + 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 + ztapi.setDefaultViewName(IDummy, 'meth') + ob = r.traverse('folder/withattr/') + self.assertEqual(ob(), 'meth on withattr') + ob = r.traverse('folder/withattr') + self.assertEqual(ob(), 'meth 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 + ztapi.setDefaultViewName(IDummy, 'meth') + ob = r.traverse('folder2/obj2/') + self.assertEqual(ob(), 'view on obj2') + 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 + ztapi.setDefaultViewName(IDummy, 'meth') + ob = r.traverse('folder2/withattr2/') + self.assertEqual(ob(), 'meth on withattr2') + ob = r.traverse('folder2/withattr2') + self.assertEqual(ob(), 'meth 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 + ztapi.setDefaultViewName(IDummy, 'methonly') + ob = r.traverse('folder2/obj2/') + self.assertEqual(ob(), 'methonly on folder2') + ob = r.traverse('folder2/obj2') + self.assertEqual(ob(), 'methonly on folder2') + + def test_suite(): - return TestSuite( ( makeSuite(TestBaseRequest), ) ) + return TestSuite(( + makeSuite(TestBaseRequest), + makeSuite(TestBaseRequestZope3Views), + )) if __name__ == '__main__': main(defaultTest='test_suite') Index: lib/python/ZPublisher/BaseRequest.py =================================================================== --- lib/python/ZPublisher/BaseRequest.py (revision 68718) +++ lib/python/ZPublisher/BaseRequest.py (working copy) @@ -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 @@ -72,6 +73,7 @@ raise Forbidden("Object name begins with an underscore at: %s" % URL) try: + # 1. Try __bobo_traverse__ if hasattr(object,'__bobo_traverse__'): subobject=object.__bobo_traverse__(request, name) if type(subobject) is type(()) and len(subobject) > 1: @@ -81,24 +83,28 @@ object, subobject = subobject[-2:] else: try: + # 2. Try as attribute on base object + getattr(aq_base(object), name) subobject=getattr(object, name) except AttributeError: + # 2.5 Try as item 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 + # 4. Try as 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 + # 5. Try as acquired attribute, may raise + subobject = getattr(object, name) + # Ensure that the object has a docstring, or that the parent # object has a pseudo-docstring for the object. Objects that # have an empty or missing docstring are not published.