Leonardo Rochael Almeida wrote:
- obj = self.aq_parent.unrestrictedTraverse(self.getpath(rid)) - if not obj: + obj = self.aq_parent.unrestrictedTraverse(self.getpath(rid), None) + if obj is None: Please revert this change. You've changed the semantics of this statement and it will now mask errors in traversing to the path.
This is bad...
I agree with you the semantics have changed, but I argue that they've changed to what they were intended to be in the first place :-)
This argument has been had many times before, please google zope-dev's archives for the numerous discussions on this and then check the current implementation in Products/ZCatalog/CatalogBrains.py:AbstractCatalogBrains.getOject.
This is the full method before my changes:
def getobject(self, rid, REQUEST=None): """Return a cataloged object given a 'data_record_id_' """ obj = self.aq_parent.unrestrictedTraverse(self.getpath(rid)) if not obj: if REQUEST is None: REQUEST=self.REQUEST obj = self.resolve_url(self.getpath(rid), REQUEST) return obj
This code is just plain rubbish, there's no single bug here. I'd like someone to give a use case where unrestrictedTraverse fails and yet where resolve_url works.
Traversing with a default argument will only mask traversal errors, not any other errors that might happen during traversing.
Sure it will, see previous discussions...
In the version with my changes, but with the old unrestrictedTraverse call, the "if obj is None" would only ever be reached in the pathological situation where the attribute at the end of the path is None. Which means the ".resolve_url(self.getpath(rid), REQUEST)" would hardly ever be called at all.
Correct. .resolve_url(self.getpath(rid), REQUEST) hardly ever gets called.
A) Keep my changes, but use a marker "object()" instead of None for the "default" parameter of the .unrestrictedTraverse() call.
B) Reverse my changes, but also remove the "if not obj" block, which is buggy whichever way you look at it with the old unrestrictedTraverse call.
Which one should it be?
I'd go for B. cheers, Chris -- Simplistix - Content Management, Zope & Python Consulting - http://www.simplistix.co.uk