Log message for revision 71170: revert ZCatalog.getobject() semantics not to mask traversal errors and not to fallback to .resolve_url() when the traversal result is None
Changed: U Zope/trunk/lib/python/Products/ZCatalog/ZCatalog.py U Zope/trunk/lib/python/Products/ZCatalog/tests/testCatalog.py -=- Modified: Zope/trunk/lib/python/Products/ZCatalog/ZCatalog.py =================================================================== --- Zope/trunk/lib/python/Products/ZCatalog/ZCatalog.py 2006-11-17 20:11:22 UTC (rev 71169) +++ Zope/trunk/lib/python/Products/ZCatalog/ZCatalog.py 2006-11-17 20:17:28 UTC (rev 71170) @@ -587,12 +587,7 @@ def getobject(self, rid, REQUEST=None): """Return a cataloged object given a 'data_record_id_' """ - obj = self.aq_parent.unrestrictedTraverse(self.getpath(rid), None) - if obj is None: - if REQUEST is None: - REQUEST=self.REQUEST - obj = self.resolve_url(self.getpath(rid), REQUEST) - return obj + return self.aq_parent.unrestrictedTraverse(self.getpath(rid)) def getMetadataForUID(self, uid): """return the correct metadata given the uid, usually the path""" Modified: Zope/trunk/lib/python/Products/ZCatalog/tests/testCatalog.py =================================================================== --- Zope/trunk/lib/python/Products/ZCatalog/tests/testCatalog.py 2006-11-17 20:11:22 UTC (rev 71169) +++ Zope/trunk/lib/python/Products/ZCatalog/tests/testCatalog.py 2006-11-17 20:17:28 UTC (rev 71170) @@ -177,15 +177,23 @@ def __nonzero__(self): self.fail("__nonzero__() was called") +class FakeTraversalError(KeyError): + """fake traversal exception for testing""" + class fakeparent(Implicit): # fake parent mapping unrestrictedTraverse to # catalog.resolve_path as simulated by TestZCatalog def __init__(self, d): self.d = d - def unrestrictedTraverse(self, path, default=None): - return self.d.get(path, default) + marker = object() + def unrestrictedTraverse(self, path, default=marker): + result = self.d.get(path, default) + if result is self.marker: + raise FakeTraversalError(path) + return result + class TestZCatalog(unittest.TestCase): def setUp(self): @@ -283,7 +291,7 @@ self._catalog.manage_catalogObject(None, myresponse(), 'URL1', urls=('11', '12')) def testBooleanEvalOn_refreshCatalog_getobject(self): - # wrap catalog under the fake parent + # wrap catalog under the fake parent providing unrestrictedTraverse() catalog = self._catalog.__of__(fakeparent(self.d)) # replace entries to test refreshCatalog self.d['0'] = dummyLenFail(0, self.fail) @@ -292,10 +300,27 @@ catalog.refreshCatalog() for uid in ('0', '1'): - rid = self._catalog.getrid(uid) + rid = catalog.getrid(uid) # neither should these catalog.getobject(rid) + def test_getobject_doesntMaskTraversalErrorsAndDoesntDelegateTo_resolve_url(self): + # wrap catalog under the fake parent providing unrestrictedTraverse() + catalog = self._catalog.__of__(fakeparent(self.d)) + # make resolve_url fail if ZCatalog falls back on it + def resolve_url(path, REQUEST): + self.fail(".resolve_url() should not be called by .getobject()") + catalog.resolve_url = resolve_url + + # traversal should work at first + rid0 = catalog.getrid('0') + # lets set it up so the traversal fails + del self.d['0'] + self.assertRaises(FakeTraversalError, catalog.getobject, rid0, REQUEST=object()) + # and if there is a None at the traversal point, that's where it should return + self.d['0'] = None + self.assertEquals(catalog.getobject(rid0), None) + class dummy(ExtensionClass.Base): att1 = 'att1' att2 = 'att2' _______________________________________________ Zope-Checkins maillist - Zope-Checkins@zope.org http://mail.zope.org/mailman/listinfo/zope-checkins