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

[Leonardo Rochael Almeida]

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

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

-=-
Modified: Zope/branches/2.9/doc/CHANGES.txt
===================================================================
--- Zope/branches/2.9/doc/CHANGES.txt	2006-11-15 08:00:29 UTC (rev 71131)
+++ Zope/branches/2.9/doc/CHANGES.txt	2006-11-15 08:05:34 UTC (rev 71132)
@@ -9,6 +9,11 @@
 
    Bugs fixed
 
+      - 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 not None".
+
       - Collector #2218: fixed wrong logger argument in OFS/Cache.py
 
       - Collector #2205: fixed wrong logger argument in ZRDB/Connection.py

Modified: Zope/branches/2.9/lib/python/Products/ZCatalog/ZCatalog.py
===================================================================
--- Zope/branches/2.9/lib/python/Products/ZCatalog/ZCatalog.py	2006-11-15 08:00:29 UTC (rev 71131)
+++ Zope/branches/2.9/lib/python/Products/ZCatalog/ZCatalog.py	2006-11-15 08:05:34 UTC (rev 71132)
@@ -234,7 +234,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)
@@ -298,7 +298,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:
@@ -615,8 +615,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/branches/2.9/lib/python/Products/ZCatalog/tests/testCatalog.py
===================================================================
--- Zope/branches/2.9/lib/python/Products/ZCatalog/tests/testCatalog.py	2006-11-15 08:00:29 UTC (rev 71131)
+++ Zope/branches/2.9/lib/python/Products/ZCatalog/tests/testCatalog.py	2006-11-15 08:05:34 UTC (rev 71132)
@@ -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'