Log message for revision 29797: CatalogBrains' 'getObject' now raises by default. o Old "return None" behavior can be restored with a deprecated ZConfig option.
Changed: U Zope/branches/tseaver-catalog_getObject_raises/doc/CHANGES.txt U Zope/branches/tseaver-catalog_getObject_raises/lib/python/OFS/Traversable.py U Zope/branches/tseaver-catalog_getObject_raises/lib/python/Products/ZCatalog/CatalogBrains.py U Zope/branches/tseaver-catalog_getObject_raises/lib/python/Products/ZCatalog/tests/testBrains.py U Zope/branches/tseaver-catalog_getObject_raises/lib/python/Products/ZCatalog/tests/testCatalog.py U Zope/branches/tseaver-catalog_getObject_raises/lib/python/Zope2/Startup/handlers.py U Zope/branches/tseaver-catalog_getObject_raises/lib/python/Zope2/Startup/zopeschema.xml -=- Modified: Zope/branches/tseaver-catalog_getObject_raises/doc/CHANGES.txt =================================================================== --- Zope/branches/tseaver-catalog_getObject_raises/doc/CHANGES.txt 2005-04-01 17:17:35 UTC (rev 29796) +++ Zope/branches/tseaver-catalog_getObject_raises/doc/CHANGES.txt 2005-04-01 18:25:18 UTC (rev 29797) @@ -28,6 +28,15 @@ Features added + - ZCatalog.CatalogBrains: 'getObject' now raises errors, rather than + returning None, in cases where the path points either to a nonexistent + object (in which case it raises NotFound) or to one which the user + cannot access (raising Unauthorized). Sites which rely on the old + behavior can restore setting a new zope.conf option, + 'catalog-getObject-raises', to "off". + + This compatibility option will be removed in Zope 2.10. + - PluginIndexes: the ZCatalog's "Indexes" tab now show the number of distinct values indexed by each index instead of a mixture of indexed objects versus number of distinct values. Indexes derived from UnIndex @@ -63,6 +72,8 @@ Bugs fixed + - OFS.Traversable still used a string 'NotFound' exception. + - ZPublisher would fail to recognize a XML-RPC request if the content-type header included a 'charset' parameter. Modified: Zope/branches/tseaver-catalog_getObject_raises/lib/python/OFS/Traversable.py =================================================================== --- Zope/branches/tseaver-catalog_getObject_raises/lib/python/OFS/Traversable.py 2005-04-01 17:17:35 UTC (rev 29796) +++ Zope/branches/tseaver-catalog_getObject_raises/lib/python/OFS/Traversable.py 2005-04-01 18:25:18 UTC (rev 29797) @@ -21,7 +21,7 @@ from ZODB.POSException import ConflictError from urllib import quote -NotFound = 'NotFound' +from zExceptions import NotFound _marker = object() Modified: Zope/branches/tseaver-catalog_getObject_raises/lib/python/Products/ZCatalog/CatalogBrains.py =================================================================== --- Zope/branches/tseaver-catalog_getObject_raises/lib/python/Products/ZCatalog/CatalogBrains.py 2005-04-01 17:17:35 UTC (rev 29796) +++ Zope/branches/tseaver-catalog_getObject_raises/lib/python/Products/ZCatalog/CatalogBrains.py 2005-04-01 18:25:18 UTC (rev 29797) @@ -14,7 +14,14 @@ __version__ = "$Revision$"[11:-2] import Acquisition, Record +from zExceptions import NotFound +from zExceptions import Unauthorized +from ZODB.POSException import ConflictError +# Switch for new behavior, raise NotFound instead of returning None. +# Use 'catalog-getOb-raises off' in zope.conf to restore old behavior. +GETOBJECT_RAISES = True + class AbstractCatalogBrain(Record.Record, Acquisition.Implicit): """Abstract base brain that handles looking up attributes as required, and provides just enough smarts to let us get the URL, path, @@ -54,11 +61,26 @@ return None parent = self.aq_parent if len(path) > 1: - parent = parent.unrestrictedTraverse('/'.join(path[:-1]), None) - if parent is None: + try: + parent = parent.unrestrictedTraverse(path[:-1]) + except ConflictError: + raise + except: + if GETOBJECT_RAISES: + raise return None - return parent.restrictedTraverse(path[-1], None) + try: + target = parent.restrictedTraverse(path[-1]) + except ConflictError: + raise + except: + if GETOBJECT_RAISES: + raise + return None + + return target + def getRID(self): """Return the record ID for this object.""" return self.data_record_id_ Modified: Zope/branches/tseaver-catalog_getObject_raises/lib/python/Products/ZCatalog/tests/testBrains.py =================================================================== --- Zope/branches/tseaver-catalog_getObject_raises/lib/python/Products/ZCatalog/tests/testBrains.py 2005-04-01 17:17:35 UTC (rev 29796) +++ Zope/branches/tseaver-catalog_getObject_raises/lib/python/Products/ZCatalog/tests/testBrains.py 2005-04-01 18:25:18 UTC (rev 29797) @@ -56,7 +56,8 @@ # This is sooooo ugly def unrestrictedTraverse(self, path, default=None): - assert path == '' # for these tests... + # for these tests... + assert path == '' or path == ('') or path == [''], path return self def restrictedTraverse(self, path, default=_marker): @@ -66,7 +67,7 @@ ob = self._objs[path].__of__(self) ob.check() return ob - except (KeyError, Unauthorized): + except KeyError: if default is not _marker: return default raise @@ -86,60 +87,99 @@ def getpath(self, rid): raise ConflictError -class TestBrains(unittest.TestCase): +class BrainsTestBase: + + _old_flag = None def setUp(self): self.cat = DummyCatalog() self.cat.REQUEST = DummyRequest() + self._init_getOb_flag() + + def tearDown(self): + if self._old_flag is not None: + self._restore_getOb_flag() + + def _init_getOb_flag(self): + from Products.ZCatalog import CatalogBrains + self._old_flag = CatalogBrains.GETOBJECT_RAISES + CatalogBrains.GETOBJECT_RAISES = self._flag_value() + + def _restore_getOb_flag(self): + from Products.ZCatalog import CatalogBrains + CatalogBrains.GETOBJECT_RAISES = self._old_flag - def makeBrain(self, rid): + def _makeBrain(self, rid): from Products.ZCatalog.CatalogBrains import AbstractCatalogBrain class Brain(AbstractCatalogBrain): __record_schema__ = {'test_field': 0, 'data_record_id_':1} return Brain(('test', rid)).__of__(self.cat) - + def testHasKey(self): - b = self.makeBrain(1) + b = self._makeBrain(1) self.failUnless(b.has_key('test_field')) self.failUnless(b.has_key('data_record_id_')) self.failIf(b.has_key('godel')) def testGetPath(self): - b = [self.makeBrain(rid) for rid in range(3)] + b = [self._makeBrain(rid) for rid in range(3)] self.assertEqual(b[0].getPath(), '/conflicter') self.assertEqual(b[1].getPath(), '/happy') self.assertEqual(b[2].getPath(), '/secret') def testGetPathPropagatesConflictErrors(self): self.cat = ConflictingCatalog() - b = self.makeBrain(0) + b = self._makeBrain(0) self.assertRaises(ConflictError, b.getPath) def testGetURL(self): - b = self.makeBrain(0) + b = self._makeBrain(0) self.assertEqual(b.getURL(), 'http://superbad.com/conflicter') def testGetRID(self): - b = self.makeBrain(42) + b = self._makeBrain(42) self.assertEqual(b.getRID(), 42) def testGetObjectHappy(self): - b = self.makeBrain(1) + b = self._makeBrain(1) self.assertEqual(b.getPath(), '/happy') self.failUnless(b.getObject().aq_base is self.cat.getobject(1).aq_base) def testGetObjectPropagatesConflictErrors(self): - b = self.makeBrain(0) + b = self._makeBrain(0) self.assertEqual(b.getPath(), '/conflicter') self.assertRaises(ConflictError, b.getObject) + +class TestBrains(BrainsTestBase, unittest.TestCase): + def _flag_value(self): + return True + + def testGetObjectRaisesUnauthorized(self): + from zExceptions import Unauthorized + b = self._makeBrain(2) + self.assertEqual(b.getPath(), '/secret') + self.assertRaises(Unauthorized, b.getObject) + + def testGetObjectRaisesNotFoundForMissing(self): + from zExceptions import NotFound + b = self._makeBrain(3) + self.assertEqual(b.getPath(), '/zonked') + self.assertRaises(KeyError, self.cat.getobject, 3) + self.assertRaises((NotFound, AttributeError, KeyError), b.getObject) + +class TestBrainsOldBehavior(BrainsTestBase, unittest.TestCase): + + def _flag_value(self): + return False + def testGetObjectReturnsNoneForUnauthorized(self): - b = self.makeBrain(2) + b = self._makeBrain(2) self.assertEqual(b.getPath(), '/secret') self.assertEqual(b.getObject(), None) def testGetObjectReturnsNoneForMissing(self): - b = self.makeBrain(3) + b = self._makeBrain(3) self.assertEqual(b.getPath(), '/zonked') self.assertRaises(KeyError, self.cat.getobject, 3) self.assertEqual(b.getObject(), None) @@ -147,6 +187,7 @@ def test_suite(): suite = unittest.TestSuite() suite.addTest(unittest.makeSuite(TestBrains)) + suite.addTest(unittest.makeSuite(TestBrainsOldBehavior)) return suite if __name__ == '__main__': Modified: Zope/branches/tseaver-catalog_getObject_raises/lib/python/Products/ZCatalog/tests/testCatalog.py =================================================================== --- Zope/branches/tseaver-catalog_getObject_raises/lib/python/Products/ZCatalog/tests/testCatalog.py 2005-04-01 17:17:35 UTC (rev 29796) +++ Zope/branches/tseaver-catalog_getObject_raises/lib/python/Products/ZCatalog/tests/testCatalog.py 2005-04-01 18:25:18 UTC (rev 29797) @@ -590,6 +590,8 @@ class TestZCatalogGetObject(unittest.TestCase): # Test what objects are returned by brain.getObject() + _old_flag = None + def setUp(self): from Products.ZCatalog.ZCatalog import ZCatalog catalog = ZCatalog('catalog') @@ -601,7 +603,18 @@ def tearDown(self): noSecurityManager() + if self._old_flag is not None: + self._restore_getObject_flag() + + def _init_getObject_flag(self, flag): + from Products.ZCatalog import CatalogBrains + self._old_flag = CatalogBrains.GETOBJECT_RAISES + CatalogBrains.GETOBJECT_RAISES = flag + def _restore_getObject_flag(self): + from Products.ZCatalog import CatalogBrains + CatalogBrains.GETOBJECT_RAISES = self._old_flag + def test_getObject_found(self): # Check normal traversal root = self.root @@ -612,19 +625,59 @@ self.assertEqual(brain.getPath(), '/ob') self.assertEqual(brain.getObject().getId(), 'ob') - def test_getObject_missing(self): + def test_getObject_missing_raises_NotFound(self): # Check that if the object is missing None is returned + from zExceptions import NotFound + self._init_getObject_flag(True) root = self.root catalog = root.catalog root.ob = Folder('ob') catalog.catalog_object(root.ob) brain = catalog.searchResults()[0] del root.ob + self.assertRaises((NotFound, AttributeError, KeyError), brain.getObject) + + def test_getObject_restricted_raises_Unauthorized(self): + # Check that if the object's security does not allow traversal, + # None is returned + from zExceptions import NotFound + self._init_getObject_flag(True) + root = self.root + catalog = root.catalog + root.fold = Folder('fold') + root.fold.ob = Folder('ob') + catalog.catalog_object(root.fold.ob) + brain = catalog.searchResults()[0] + # allow all accesses + pickySecurityManager = PickySecurityManager() + setSecurityManager(pickySecurityManager) + self.assertEqual(brain.getObject().getId(), 'ob') + # disallow just 'ob' access + pickySecurityManager = PickySecurityManager(['ob']) + setSecurityManager(pickySecurityManager) + self.assertRaises(Unauthorized, brain.getObject) + # disallow just 'fold' access + pickySecurityManager = PickySecurityManager(['fold']) + setSecurityManager(pickySecurityManager) + ob = brain.getObject() + self.failIf(ob is None) + self.assertEqual(ob.getId(), 'ob') + + def test_getObject_missing_returns_None(self): + # Check that if the object is missing None is returned + self._init_getObject_flag(False) + root = self.root + catalog = root.catalog + root.ob = Folder('ob') + catalog.catalog_object(root.ob) + brain = catalog.searchResults()[0] + del root.ob self.assertEqual(brain.getObject(), None) - def test_getObject_restricted(self): + def test_getObject_restricted_returns_None(self): # Check that if the object's security does not allow traversal, # None is returned + self._init_getObject_flag(False) root = self.root catalog = root.catalog root.fold = Folder('fold') Modified: Zope/branches/tseaver-catalog_getObject_raises/lib/python/Zope2/Startup/handlers.py =================================================================== --- Zope/branches/tseaver-catalog_getObject_raises/lib/python/Zope2/Startup/handlers.py 2005-04-01 17:17:35 UTC (rev 29796) +++ Zope/branches/tseaver-catalog_getObject_raises/lib/python/Zope2/Startup/handlers.py 2005-04-01 18:25:18 UTC (rev 29797) @@ -124,6 +124,20 @@ def http_header_max_length(value): return value +def catalog_getObject_raises(value): + + if value is not None: + + import warnings + warnings.warn( + "'catalog-getObject-raises' option will be removed in Zope 2.10:\n", + DeprecationWarning) + + from Products.ZCatalog import CatalogBrains + CatalogBrains.GETOBJECT_RAISES = bool(value) + + return value + # server handlers def root_handler(config): Modified: Zope/branches/tseaver-catalog_getObject_raises/lib/python/Zope2/Startup/zopeschema.xml =================================================================== --- Zope/branches/tseaver-catalog_getObject_raises/lib/python/Zope2/Startup/zopeschema.xml 2005-04-01 17:17:35 UTC (rev 29796) +++ Zope/branches/tseaver-catalog_getObject_raises/lib/python/Zope2/Startup/zopeschema.xml 2005-04-01 18:25:18 UTC (rev 29797) @@ -768,6 +768,17 @@ </description> </key> + <key name="catalog-getObject-raises" datatype="boolean" + handler="catalog_getObject_raises"> + <description> + If this directive is set to "on" (the deafult), ZCatalog brains objects + will raise NotFound exceptions from 'getObject' for unreachable objects, + and Unauthorized for disallowed objects. If the option is "off", they + will return None in such cases (which was the old behavior) + </description> + <metadefault>off</metadefault> + </key> + <multisection type="ZServer.server" name="*" attribute="servers"/> <key name="port-base" datatype="integer" default="0"> <description> _______________________________________________ Zope-Checkins maillist - Zope-Checkins@zope.org http://mail.zope.org/mailman/listinfo/zope-checkins