Post-mortem [Was: Zope 2.7.0 b3 regressions]
After causing all this noise, let me summarize what I now understand is the intended behavior of Zope's URL methods. Given an object is accessed by /VirtualHostBase/http/example.com:80/VirtualHostRoot/_vh_foo/folder/doc 1) doc.absolute_url() Returns the full URL of doc including protocol and host. e.g. http://example.com/foo/folder/doc 2) doc.absolute_url(relative=1) Returns the path of doc, relative to the virtual root. e.g. folder/doc 3) REQUEST.BASEPATH1 + '/' + doc.absolute_url(1) Results in the absolute path of doc, starting at the server root. e.g. /foo/folder/doc Comments: ad 2) This may or may not be RFC-2396 compliant, I must admit I fail to parse the RFC here. It certainly appears necessary for b/w compatibility at least. ad 3) It may still be a good idea to add an API method for this (e.g. 'absolute_path') like I proposed in February (#809). If only to increase the visibility of this requirement. I understand that it may be too late for 2.7. Whoever is going to unroll the fix, please also adapt the tests in Products/SiteAccess/tests. Finally, please accept my apologies for the tone of some replies; I do certainly need a vacation. Stefan -- The time has come to start talking about whether the emperor is as well dressed as we are supposed to think he is. /Pete McBreen/
I'd like to suggest that any new methods created to handle paths or urls do not require parameters for different behaviours (which should be implemented as different methods), so that they can be more easily used from plain ZPT path expressions. Cheers, Leo
Summary: absolute_url(1) didn't include the path to the virtual root, which broke code that assumed that it could just prepend "/" in all cases. I changed it to include the base path, and broke code that prepends BASEPATH1. Since the old behavior existed for two years (including part of the 2.7 beta), is arguably easier to work with, and works with existing CMF code, I will revert to it. This will leave "/"-prepending code broken. In line with Stefan's original suggestion, I will add a new method with alternate behavior. It's not hard to grep for broken code and use the new method to make it correct -- Product authors should do so. def absolute_url_path(self): '''Return the entire path of the absolute URL for this object. This includes the leading slash, and can be used as an "absolute-path reference" as defined in RFC 2396. ''' Lennart proposed additional methods, but I'm going to work on that in the post-2.7 branches, since I really want to clean up this mess properly, and beta 3 isn't the time. Cheers, Evan @ 4-am
On Tue, Dec 09, 2003 at 10:24:47AM -0600, Evan Simpson wrote:
def absolute_url_path(self): '''Return the entire path of the absolute URL for this object. This includes the leading slash, and can be used as an "absolute-path reference" as defined in RFC 2396. '''
OK. But maybe the docstrings could be more clear about BASEPATH1? IIUC, absolute_url(1) will leave out BASEPATH1, continuing the behavior from zope < 2.7, but absolute_url_path() will include BASEPATH1? I propose this, in OFSP/help/ObjectManagerItem.py: def absolute_url_path(self): ''' Return the entire path portion of the absolute URL for this object. This includes the leading slash and BASEPATH1, and can be used as an "absolute-path reference" as defined in RFC 2396. Thus, this method is always safe to use with VirtualHostMonster. ''' def absolute_url(relative=None): """ Return the absolute url to the object. If the relative argument is provided with a true value, then the URL returned is relative to the site object. Does not include a leading slash or BASEPATH1, thus is not always safe to use with VirtualHostMonster. Code that uses '/'+absolute_url(1) should be changed to use absolute_url_path instead. Permission -- Always available """ I removed the "logical rather than physical" note from absolute_url because I don't think it was actually true before Evan's change, which IIUC is being reverted. Maybe I misunderstood.
Lennart proposed additional methods, but I'm going to work on that in the post-2.7 branches, since I really want to clean up this mess properly, and beta 3 isn't the time.
Great. -- Paul Winkler http://www.slinkp.com Look! Up in the sky! It's HYPO-SUPERCALIFRAGILISTICEXPIALADOCIOUS THIGH MAN! (random hero from isometric.spaceninja.com)
Summary: absolute_url(1) didn't include the path to the virtual root, which broke code that assumed that it could just prepend "/" in all cases. I changed it to include the base path, and broke code that prepends BASEPATH1.
Since the old behavior existed for two years (including part of the 2.7 beta), is arguably easier to work with, and works with existing CMF code, I will revert to it. This will leave "/"-prepending code broken.
In line with Stefan's original suggestion, I will add a new method with alternate behavior. It's not hard to grep for broken code and use the new method to make it correct -- Product authors should do so.
Good call. I think it would be best to make sure the docstring of the new method is clear on its reason for being. I think somewhere there is an interface file that is used to generate some of the api docs - ideally that can get updated too. Brian Lloyd brian@zope.com V.P. Engineering 540.361.1716 Zope Corporation http://www.zope.com
Hi! Brian Lloyd wrote:
Good call. I think it would be best to make sure the docstring of the new method is clear on its reason for being. I think somewhere there is an interface file that is used to generate some of the api docs - ideally that can get updated too.
What is the Right Way to keep api docs in sync? Example 1: ZCatalog/IZCatalog.py seems to be more up to date, but ZCatalog/help/ZCatalog.py is used by the Zope Help System and I guess also to generate api docs. Example 2: I did add OFS/IOrderSupport.py, but how will it become part of help and api docs? Is there a better way than copying it to OFSP/help? I hate to add redundant code because - as example 1 shows - it is hard to keep the files in sync. CMF uses the interface files also as help files. As interface files become more and more common in Zope it might make sense to do the same in Zope. Cheers, Yuppie
What is the Right Way to keep api docs in sync?
Example 1:
ZCatalog/IZCatalog.py seems to be more up to date, but ZCatalog/help/ZCatalog.py is used by the Zope Help System and I guess also to generate api docs.
Example 2:
I did add OFS/IOrderSupport.py, but how will it become part of help and api docs? Is there a better way than copying it to OFSP/help? I hate to add redundant code because - as example 1 shows - it is hard to keep the files in sync.
CMF uses the interface files also as help files. As interface files become more and more common in Zope it might make sense to do the same in Zope.
Ghaaa... :( API docs come from OFSP/help, so you'll probably have to copy it for now. As you rightly pointed out, this is problematic for keeping things in sync, but we should hold our nose for now and do it, since it's too late in the 2.7 cycle to contemplate that big a restructuring. Making this sane would be a good potential project for an enterprising zopista for 2.8... Brian Lloyd brian@zope.com V.P. Engineering 540.361.1716 Zope Corporation http://www.zope.com
On Wed, Dec 10, 2003 at 10:20:22AM -0500, Brian Lloyd wrote:
Ghaaa... :( API docs come from OFSP/help, so you'll probably have to copy it for now. As you rightly pointed out, this is problematic for keeping things in sync, but we should hold our nose for now and do it, since it's too late in the 2.7 cycle to contemplate that big a restructuring.
Making this sane would be a good potential project for an enterprising zopista for 2.8...
What would be "sane" in your view? Using interfaces for help, as CMF does? I was curious how CMF sets this up, and found the following in CMFCore/__init__.py: try: context.registerHelpTitle( 'CMF Core Help' ) context.registerHelp(directory='interfaces') except: # AARGH!! pass I love comments like that :-) -- Paul Winkler http://www.slinkp.com Look! Up in the sky! It's THE EXPECTANT MOLE RAT! (random hero from isometric.spaceninja.com)
Hi! Paul Winkler wrote:
What would be "sane" in your view? Using interfaces for help, as CMF does?
I was curious how CMF sets this up, and found the following in CMFCore/__init__.py:
try: context.registerHelpTitle( 'CMF Core Help' ) context.registerHelp(directory='interfaces') except: # AARGH!! pass
I love comments like that :-)
This bare except is removed in CMF HEAD, it's not necessary for current Zope versions. <http://cvs.zope.org/CMF/CMFCore/__init__.py.diff?r1=1.24&r2=1.25> But there are other issues with this approach. I never got a reply to this mail: <http://mail.zope.org/pipermail/zope-dev/2002-November/018103.html> Maybe the Zope X3 Introspector is a sane solution? Cheers, Yuppie
Brian Lloyd wrote:
Good call. I think it would be best to make sure the docstring of the new method is clear on its reason for being. I think somewhere there is an interface file that is used to generate some of the api docs - ideally that can get updated too.
Done and done! Cheerios, Evan
Hi Evan! Evan Simpson wrote:
Done and done!
??? Are you sure? Today I built Zope 2.7 from a new checkout. And all icons in the ZMI are broken :-( They have src paths starting with // I didn't have a closer look at your checkin, but I'm afraid something went wrong. Cheers, Yuppie
Yuppie wrote:
Today I built Zope 2.7 from a new checkout. And all icons in the ZMI are broken :-(
They have src paths starting with //
Argh. A bug in my refactoring of the tests made them all pass without really testing. I *think* it's fixed now. Sorry, Evan @ 4-am
From: "Evan Simpson" <evan@4-am.com>
Lennart proposed additional methods, but I'm going to work on that in the post-2.7 branches, since I really want to clean up this mess properly, and beta 3 isn't the time.
I totally agree. This is new features, introducing that in a beta3 seems very bad. I proposed introducing them in 2.7 only to: 1. Get things moving by scaring people. :-) 2. A way of fixing absolute_url by making it's bugs less important. If absolute_url can be fixed so it doesn't break backwards compatibility, then number 2 is of no importance. //Lennart
Stefan H. Holek wrote at 2003-12-9 15:47 +0100:
... Given an object is accessed by /VirtualHostBase/http/example.com:80/VirtualHostRoot/_vh_foo/folder/doc ... 2) doc.absolute_url(relative=1)
Returns the path of doc, relative to the virtual root. e.g. folder/doc ... Comments:
ad 2) This may or may not be RFC-2396 compliant,
It is not as "/folder/doc" would not allow to locate the correct object but likely result in an "Resource not found". -- Dieter
participants (9)
-
Brian Lloyd -
Dieter Maurer -
Evan Simpson -
Lennart Regebro -
Leonardo Rochael Almeida -
Paul Winkler -
Stefan H. Holek -
Toby Dickenson -
Yuppie