[Zope-Checkins]
SVN: Zope/branches/tseaver-catalog_getObject_raises/
CatalogBrains' 'getObject' now raises by default.
Tres Seaver
tseaver at zope.com
Fri Apr 1 13:25:19 EST 2005
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>
More information about the Zope-Checkins
mailing list