[Zope-Checkins] SVN: Zope/trunk/ fix for #2235: ZCatalog triggering boolean evaluation of objects

[Leonardo Rochael Almeida]

Log message for revision 71136:
  fix for #2235: ZCatalog triggering boolean evaluation of objects
  

Changed:
  U   Zope/trunk/doc/CHANGES.txt
  U   Zope/trunk/lib/python/Products/ZCatalog/ZCatalog.py
  U   Zope/trunk/lib/python/Products/ZCatalog/tests/testCatalog.py

-=-
Modified: Zope/trunk/doc/CHANGES.txt
===================================================================
--- Zope/trunk/doc/CHANGES.txt	2006-11-15 09:19:33 UTC (rev 71135)
+++ Zope/trunk/doc/CHANGES.txt	2006-11-15 09:26:18 UTC (rev 71136)
@@ -13,6 +13,11 @@
 
       - Collector #2213: Can't edit "old" ZopePageTemplate instances.
 
+      - Collector #2235: A number of ZCatalog methods were doing boolean
+        evaluation of objects that implemented __len__ instead of checking
+        them against None. Replaced a number of "if not obj" with
+        "if obj is None".
+
       - reStructuredText/ZReST: setting raw_enabled to 0 for security
         reasons
 

Modified: Zope/trunk/lib/python/Products/ZCatalog/ZCatalog.py
===================================================================
--- Zope/trunk/lib/python/Products/ZCatalog/ZCatalog.py	2006-11-15 09:19:33 UTC (rev 71135)
+++ Zope/trunk/lib/python/Products/ZCatalog/ZCatalog.py	2006-11-15 09:26:18 UTC (rev 71136)
@@ -221,7 +221,7 @@
 
             for url in urls:
                 obj = self.resolve_path(url)
-                if not obj and hasattr(self, 'REQUEST'):
+                if obj is None and hasattr(self, 'REQUEST'):
                     obj = self.resolve_url(url, REQUEST)
                 if obj is not None:
                     self.catalog_object(obj, url)
@@ -289,7 +289,7 @@
 
             p = paths[i]
             obj = self.resolve_path(p)
-            if not obj:
+            if obj is None:
                 obj = self.resolve_url(p, self.REQUEST)
             if obj is not None:
                 try:
@@ -587,8 +587,8 @@
     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:
+        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)

Modified: Zope/trunk/lib/python/Products/ZCatalog/tests/testCatalog.py
===================================================================
--- Zope/trunk/lib/python/Products/ZCatalog/tests/testCatalog.py	2006-11-15 09:19:33 UTC (rev 71135)
+++ Zope/trunk/lib/python/Products/ZCatalog/tests/testCatalog.py	2006-11-15 09:26:18 UTC (rev 71136)
@@ -28,6 +28,7 @@
 from AccessControl.SecurityManagement import setSecurityManager
 from AccessControl.SecurityManagement import noSecurityManager
 from AccessControl import Unauthorized
+from Acquisition import Implicit
 from Products.ZCatalog import Vocabulary
 from Products.ZCatalog.Catalog import Catalog
 from Products.ZCatalog.Catalog import CatalogError
@@ -159,7 +160,32 @@
     def __nonzero__(self):
         return False
 
+# make objects with failing __len__ and __nonzero__
+class dummyLenFail(zdummy):
+    def __init__(self, num, fail):
+        zdummy.__init__(self, num)
+        self.fail = fail
 
+    def __len__(self):
+        self.fail("__len__() was called")
+
+class dummyNonzeroFail(zdummy):
+    def __init__(self, num, fail):
+        zdummy.__init__(self, num)
+        self.fail = fail
+
+    def __nonzero__(self):
+        self.fail("__nonzero__() was called")
+
+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)
+
 class TestZCatalog(unittest.TestCase):
 
     def setUp(self):
@@ -246,7 +272,30 @@
         result = self._catalog(title='9999')
         self.assertEquals(1, len(result))
 
+    def testBooleanEvalOn_manage_catalogObject(self):
+        self.d['11'] = dummyLenFail(11, self.fail)
+        self.d['12'] = dummyNonzeroFail(12, self.fail)
+        # create a fake response that doesn't bomb on manage_catalogObject()
+        class myresponse:
+            def redirect(self, url):
+                pass
+        # this next call should not fail
+        self._catalog.manage_catalogObject(None, myresponse(), 'URL1', urls=('11', '12'))
 
+    def testBooleanEvalOn_refreshCatalog_getobject(self):
+        # wrap catalog under the fake parent
+        catalog = self._catalog.__of__(fakeparent(self.d))
+        # replace entries to test refreshCatalog
+        self.d['0'] = dummyLenFail(0, self.fail)
+        self.d['1'] = dummyNonzeroFail(1, self.fail)
+        # this next call should not fail
+        catalog.refreshCatalog()
+
+        for uid in ('0', '1'):
+            rid = self._catalog.getrid(uid)
+            # neither should these
+            catalog.getobject(rid)
+
 class dummy(ExtensionClass.Base):
     att1 = 'att1'
     att2 = 'att2'