Collector #1832: UnIndex swallowed ConflictErrors (bare 'except:' is evil).

lib/python/Products/PluginIndexes/common/UnIndex.py

@@ -24,6 +24,7 @@ from BTrees.IOBTree import IOBTree
 import BTrees.Length
 from BTrees.OOBTree import OOBTree
 from OFS.SimpleItem import SimpleItem
+from ZODB.POSException import ConflictError
 from zope.interface import implements
 
 from Products.PluginIndexes import PluggableIndex
@@ -169,6 +170,9 @@ class UnIndex(SimpleItem):
                     del self._index[entry]
                     self._length.change(-1)
 
+            except ConflictError:
+                raise
+
             except AttributeError:
                 # index row is an int
                 del self._index[entry]
@@ -209,9 +213,7 @@ class UnIndex(SimpleItem):
     def index_object(self, documentId, obj, threshold=None):
         """ wrapper to handle indexing of multiple attributes """
 
-        # needed for backward compatibility
-        try: fields = self.indexed_attrs
-        except: fields  = [ self.id ]
+        fields = self.getIndexSourceNames()
 
         res = 0
         for attr in fields:
@@ -235,6 +237,8 @@ class UnIndex(SimpleItem):
                 if datum is _marker:
                     try:
                         del self._unindex[documentId]
+                    except ConflictError:
+                        raise
                     except:
                         LOG.error('Should not happen: oldDatum was there, now its not,'
                                   'for document with id %s' % documentId)
@@ -279,6 +283,8 @@ class UnIndex(SimpleItem):
 
         try:
             del self._unindex[documentId]
+        except ConflictError:
+            raise
         except:
             LOG.error('Attempt to unindex nonexistent document'
                       ' with id %s' % documentId)
@@ -391,10 +397,8 @@ class UnIndex(SimpleItem):
 
     def getIndexSourceNames(self):
         """ return sequence of indexed attributes """
-        try:
-            return self.indexed_attrs
-        except:
-            return [ self.id ]
+        # BBB:  older indexes didn't have 'indexed_attrs'
+        return getattr(self, 'indexed_attrs', [self.id])
 
     def uniqueValues(self, name=None, withLengths=0):
         """returns the unique values for name

lib/python/Products/PluginIndexes/common/tests/__init__.py

+##############################################################################
+#
+# Copyright (c) 2001 Zope Corporation and Contributors. All Rights Reserved.
+#
+# This software is subject to the provisions of the Zope Public License,
+# Version 2.0 (ZPL).  A copy of the ZPL should accompany this distribution.
+# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
+# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
+# FOR A PARTICULAR PURPOSE
+#
+#############################################################################
+

lib/python/Products/PluginIndexes/common/tests/test_UnIndex.py

+##############################################################################
+#
+# Copyright (c) 2001 Zope Corporation and Contributors. All Rights Reserved.
+#
+# This software is subject to the provisions of the Zope Public License,
+# Version 2.0 (ZPL).  A copy of the ZPL should accompany this distribution.
+# THIS SOFTWARE IS PROVIDED "AS IS" AND ANY AND ALL EXPRESS OR IMPLIED
+# WARRANTIES ARE DISCLAIMED, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+# WARRANTIES OF TITLE, MERCHANTABILITY, AGAINST INFRINGEMENT, AND FITNESS
+# FOR A PARTICULAR PURPOSE
+#
+#############################################################################
+""" Tests for common UnIndex features.
+
+$Id$
+"""
+import unittest
+
+class UnIndexTests(unittest.TestCase):
+
+    def _getTargetClass(self):
+        from Products.PluginIndexes.common.UnIndex import UnIndex
+        return UnIndex
+
+    def _makeOne(self, *args, **kw):
+        return self._getTargetClass()(*args, **kw)
+
+    def _makeConflicted(self):
+        from ZODB.POSException import ConflictError
+        class Conflicted:
+            def __str__(self):
+                return 'Conflicted'
+            __repr__ = __str__
+            def __getattr__(self, id, default=object()):
+                raise ConflictError, 'testing'
+        return Conflicted()
+
+    def test_empty(self):
+        unindex = self._makeOne(id='empty')
+        self.assertEqual(unindex.indexed_attrs, ['empty'])
+
+    def test_removeForwardIndexEntry_with_ConflictError(self):
+        from ZODB.POSException import ConflictError
+        unindex = self._makeOne(id='conflicted')
+        unindex._index['conflicts'] = self._makeConflicted()
+        self.assertRaises(ConflictError, unindex.removeForwardIndexEntry,
+                          'conflicts', 42)
+
+def test_suite():
+    suite = unittest.TestSuite()
+    suite.addTest(unittest.makeSuite(UnIndexTests))
+    return suite
+
+if __name__ == '__main__':
+    unittest.main(defaultTest='test_suite')