bug in CatalogAwareness?
I just checked out the latest Zope 2 CVS. In lib/python/Products/ZCatalog/CatalogAwareness.py, the method "url": def url(self, ftype=urllib.splittype, fhost=urllib.splithost): """Return a SCRIPT_NAME-based url for an object.""" if hasattr(self, 'DestinationURL') and \ callable(self.DestinationURL): url='%s/%s' % (self.DestinationURL(), self.id) else: url=self.absolute_url() type, uri=ftype(url) host, uri=fhost(uri) script_name=self.REQUEST['SCRIPT_NAME'] __traceback_info__=(`uri`, `script_name`) if script_name: uri=filter(None, string.split(uri, script_name))[0] * if uri[0] != '/': uri = '/' + uri * uri=uri or '/' return urllib.unquote(uri) I'm suspicious of the two lines marked with a "*". The line uri=uri or '/' seems to imply that url can be the empty string, or possibly None. However, the line before if uri[0] != '/': uri = '/' + uri Will fail with an IndexError if uri is ''. Perhaps the two lines ought to be replaced with: uri=uri and uri[0]!='/' and '/'+uri or uri or '/' and perhaps a comment # input output # None '/' # '' '/' # '/' '/' # 'foo' '/foo' # '/foo' '/foo' This is now in the Collector: http://classic.zope.org:8080/Collector/1551/view -- Steve Alexander Software Engineer Cat-Box limited http://www.cat-box.net
Steve Alexander wrote:
In lib/python/Products/ZCatalog/CatalogAwareness.py, the method "url":
def url(self, ftype=urllib.splittype, fhost=urllib.splithost): """Return a SCRIPT_NAME-based url for an object.""" if hasattr(self, 'DestinationURL') and \ callable(self.DestinationURL): url='%s/%s' % (self.DestinationURL(), self.id) else: url=self.absolute_url() type, uri=ftype(url) host, uri=fhost(uri) script_name=self.REQUEST['SCRIPT_NAME'] __traceback_info__=(`uri`, `script_name`) if script_name: uri=filter(None, string.split(uri, script_name))[0] * if uri[0] != '/': uri = '/' + uri * uri=uri or '/' return urllib.unquote(uri)
I'm suspicious of the two lines marked with a "*".
I was suspicious of the entire method so I wrote a replacement which is based on a new interface available in Zope. I *think* it's correct, but it's a drastic change so it's only in the new PTK and nowhere else. (This is slightly modified from PTK CVS.) def __url(self): return string.join(self.getPhysicalPath(), '/') Shane
Shane Hathaway wrote:
Steve Alexander wrote:
In lib/python/Products/ZCatalog/CatalogAwareness.py, the method "url":
def url(self, ftype=urllib.splittype, fhost=urllib.splithost): """Return a SCRIPT_NAME-based url for an object.""" if hasattr(self, 'DestinationURL') and \ callable(self.DestinationURL): url='%s/%s' % (self.DestinationURL(), self.id) else: url=self.absolute_url() type, uri=ftype(url) host, uri=fhost(uri) script_name=self.REQUEST['SCRIPT_NAME'] __traceback_info__=(`uri`, `script_name`) if script_name: uri=filter(None, string.split(uri, script_name))[0] * if uri[0] != '/': uri = '/' + uri * uri=uri or '/' return urllib.unquote(uri)
I'm suspicious of the two lines marked with a "*".
I was suspicious of the entire method so I wrote a replacement which is based on a new interface available in Zope. I *think* it's correct, but it's a drastic change so it's only in the new PTK and nowhere else. (This is slightly modified from PTK CVS.)
def __url(self): return string.join(self.getPhysicalPath(), '/')
That certainly looks a lot cleaner and more obvious. Does it work with SiteAccess? What's with all the DestinationURL stuff in the original url method? -- Steve Alexander Software Engineer Cat-Box limited http://www.cat-box.net
Steve Alexander wrote:
Shane Hathaway wrote:
Steve Alexander wrote:
In lib/python/Products/ZCatalog/CatalogAwareness.py, the method "url":
def url(self, ftype=urllib.splittype, fhost=urllib.splithost): """Return a SCRIPT_NAME-based url for an object.""" if hasattr(self, 'DestinationURL') and \ callable(self.DestinationURL): url='%s/%s' % (self.DestinationURL(), self.id) else: url=self.absolute_url() type, uri=ftype(url) host, uri=fhost(uri) script_name=self.REQUEST['SCRIPT_NAME'] __traceback_info__=(`uri`, `script_name`) if script_name: uri=filter(None, string.split(uri, script_name))[0] * if uri[0] != '/': uri = '/' + uri * uri=uri or '/' return urllib.unquote(uri)
I'm suspicious of the two lines marked with a "*".
I was suspicious of the entire method so I wrote a replacement which is based on a new interface available in Zope. I *think* it's correct, but it's a drastic change so it's only in the new PTK and nowhere else. (This is slightly modified from PTK CVS.)
def __url(self): return string.join(self.getPhysicalPath(), '/')
That certainly looks a lot cleaner and more obvious.
Does it work with SiteAccess?
Untested, but it should work, yes.
What's with all the DestinationURL stuff in the original url method?
That would be related to product factories. When you're accessing "http://someserver/somepath/manage_addProduct/productName/factoryName", the path is supposed to simplify down to "http://someserver/somepath/someid". I don't know whether getPhysicalPath() deals with that situation the same way, but it probably should if it doesn't. Shane
Shane Hathaway wrote:
I was suspicious of the entire method so I wrote a replacement which is based on a new interface available in Zope. I *think* it's correct, but it's a drastic change so it's only in the new PTK and nowhere else. (This is slightly modified from PTK CVS.)
def __url(self): return string.join(self.getPhysicalPath(), '/')
The implementation of url() in CatalogAwareness.py in Zope 2.2.1 seems to ensure that there is a '/' at the start of the url that is given to the ZCatalog. I don't know how important that really is, though. Perhaps your method should be changed to this: def __url(self): return '/'+string.join(self.getPhysicalPath(), '/') -- Steve Alexander Software Engineer Cat-Box limited http://www.cat-box.net
Steve Alexander wrote:
Shane Hathaway wrote:
I was suspicious of the entire method so I wrote a replacement which is based on a new interface available in Zope. I *think* it's correct, but it's a drastic change so it's only in the new PTK and nowhere else. (This is slightly modified from PTK CVS.)
def __url(self): return string.join(self.getPhysicalPath(), '/')
The implementation of url() in CatalogAwareness.py in Zope 2.2.1 seems to ensure that there is a '/' at the start of the url that is given to the ZCatalog. I don't know how important that really is, though.
Perhaps your method should be changed to this:
def __url(self): return '/'+string.join(self.getPhysicalPath(), '/')
My mistake -- I just found out that getPhysiscalPath() returns a list with an empty string as the first element. So, Shane's original version is just fine. -- Steve Alexander Software Engineer Cat-Box limited http://www.cat-box.net
participants (3)
-
Chris Withers -
Shane Hathaway -
Steve Alexander