Just because that's what it did before doesn't mean we should leave it like that. I can see absolutely no benefit in returning None over raising a specific error.
Also, the original behaviour of getObject, prior to Casey's drastic and unexpected switch to restrictedTraverse(path,None) was to raise Unauthorized, NOT to return None.
You're mistaken. The old code did:
def getObject(self, REQUEST=None):
"""Try to return the object for this record"""
obj = self.aq_parent.unrestrictedTraverse(self.getPath())
if not obj:
if REQUEST is None:
REQUEST = self.REQUEST
obj = self.aq_parent.resolve_url(self.getPath(), REQUEST)
zLOG.LOG('CatalogBrains', zLOG.INFO, 'getObject raised an error',
Which effectively returns None.
I first became aware of Casey's changes when I upgraded a production Zope instance and started getting loads of random attribute errors where I'd happilly been dealing with the Unauthorized's before.
You probably had the unauthorized *after* the getObject, when it returned to you an object you weren't actually supposed to try to access.
So, for me, returning None is just plain evil. All it serves to do is mask an exception that's likely to be useful. If people are relying on it returning None, then it's a one line change in their code.
All robust old code had to be able to test for None, because it was returned in many cases (when indexes got desynchronised, due to transaction bugs for instance, or manage_beforeDelete swallowing stuff, or conflict errors happening...). I know I had to add lots in my code.
Now, the other problem I have with this fix is no tests were checked in for this. We should have tests that verify this behaves as it should, since those tests are likely to be the only formal documentation this issue ever receives ;-)
Tests were checked in: http://cvs.zope.org/Zope/lib/python/Products/ZCatalog/tests/Attic/testCatalog.py.diff?r1=188.8.131.52&r2=184.108.40.206
(but zope-checkins list had problems that day, I don't know why, and the checkin mail never appeared).
Would anyone object if I wrote tests and changed the implementation to raise exceptions, including Unauthorized, instead of returning None?
Unauthorized in getObject is out of the question, that would be new behaviour.
Florent Guillaume, Nuxeo (Paris, France) CTO, Director of R&D
+33 1 40 33 71 59 http://nuxeo.com [EMAIL PROTECTED]
Zope-Dev maillist - Zope-Dev@zope.org
** No cross posts or HTML encoding! **
(Related lists - http://mail.zope.org/mailman/listinfo/zope-announce