[Zope-Checkins] SVN: Zope/branches/2.12/ - LP #142410: Do not index documents in a KeywordIndex if the document

[Jens Vagelpohl]

Log message for revision 112542:
  - LP #142410: Do not index documents in a KeywordIndex if the document
    is missing the indexed attribute, if determining the value raises
    AttributeError, or of the indexed attribute is empty.
  

Changed:
  U   Zope/branches/2.12/doc/CHANGES.rst
  U   Zope/branches/2.12/src/Products/PluginIndexes/KeywordIndex/KeywordIndex.py
  U   Zope/branches/2.12/src/Products/PluginIndexes/KeywordIndex/tests/testKeywordIndex.py
  U   Zope/branches/2.12/src/Products/PluginIndexes/interfaces.py

-=-
Modified: Zope/branches/2.12/doc/CHANGES.rst
===================================================================
--- Zope/branches/2.12/doc/CHANGES.rst	2010-05-19 13:42:24 UTC (rev 112541)
+++ Zope/branches/2.12/doc/CHANGES.rst	2010-05-19 14:01:19 UTC (rev 112542)
@@ -11,6 +11,10 @@
 Bugs Fixed
 ++++++++++
 
+- LP #142410: Do not index documents in a KeywordIndex if the document
+  is missing the indexed attribute, if determining the value raises
+  AttributeError, or of the indexed attribute is empty.
+
 - LP #142590: The ``DTMLMethod`` and ``DTMLDocument`` ``manage_edit``
   methods could not deal with ``TaintedString`` instances. Removed the
   entirely redundant ``DTMLDocument.manage_edit`` method at the same time.

Modified: Zope/branches/2.12/src/Products/PluginIndexes/KeywordIndex/KeywordIndex.py
===================================================================
--- Zope/branches/2.12/src/Products/PluginIndexes/KeywordIndex/KeywordIndex.py	2010-05-19 13:42:24 UTC (rev 112541)
+++ Zope/branches/2.12/src/Products/PluginIndexes/KeywordIndex/KeywordIndex.py	2010-05-19 14:01:19 UTC (rev 112542)
@@ -70,7 +70,8 @@
             try:
                 for kw in newKeywords:
                     self.insertForwardIndexEntry(kw, documentId)
-                self._unindex[documentId] = list(newKeywords)
+                if newKeywords:
+                    self._unindex[documentId] = list(newKeywords)
             except TypeError:
                 return 0
         else:
@@ -83,7 +84,10 @@
             rdiff = difference(newKeywords, oldKeywords)
             if fdiff or rdiff:
                 # if we've got forward or reverse changes
-                self._unindex[documentId] = list(newKeywords)
+                if newKeywords:
+                    self._unindex[documentId] = list(newKeywords)
+                else:
+                    del self._unindex[documentId]
                 if fdiff:
                     self.unindex_objectKeywords(documentId, fdiff)
                 if rdiff:
@@ -94,8 +98,13 @@
     def _get_object_keywords(self, obj, attr):
         newKeywords = getattr(obj, attr, ())
         if safe_callable(newKeywords):
-            newKeywords = newKeywords()
-        if isinstance(newKeywords, basestring): #Python 2.1 compat isinstance
+            try:
+                newKeywords = newKeywords()
+            except AttributeError:
+                return ()
+        if not newKeywords:
+            return ()
+        elif isinstance(newKeywords, basestring): #Python 2.1 compat isinstance
             return (newKeywords,)
         else:
             unique = {}

Modified: Zope/branches/2.12/src/Products/PluginIndexes/KeywordIndex/tests/testKeywordIndex.py
===================================================================
--- Zope/branches/2.12/src/Products/PluginIndexes/KeywordIndex/tests/testKeywordIndex.py	2010-05-19 13:42:24 UTC (rev 112541)
+++ Zope/branches/2.12/src/Products/PluginIndexes/KeywordIndex/tests/testKeywordIndex.py	2010-05-19 14:01:19 UTC (rev 112542)
@@ -243,7 +243,32 @@
                  }
         self._checkApply(record, values[5:7])
 
+    def test_noindexing_when_noattribute(self):
+        to_index = Dummy(['hello'])
+        self._index._index_object(10, to_index, attr='UNKNOWN')
+        self.failIf(self._index._unindex.get(10))
+        self.failIf(self._index.getEntryForObject(10))
 
+    def test_noindexing_when_raising_attribute(self):
+        class FauxObject:
+            def foo(self):
+                raise AttributeError
+        to_index = FauxObject()
+        self._index._index_object(10, to_index, attr='foo')
+        self.failIf(self._index._unindex.get(10))
+        self.failIf(self._index.getEntryForObject(10))
+
+    def test_value_removes(self):
+        
+        to_index = Dummy(['hello'])
+        self._index._index_object(10, to_index, attr='foo')
+        self.failUnless(self._index._unindex.get(10))
+
+        to_index = Dummy('')
+        self._index._index_object(10, to_index, attr='foo')
+        self.failIf(self._index._unindex.get(10))
+
+
 def test_suite():
     suite = unittest.TestSuite()
     suite.addTest( unittest.makeSuite( TestKeywordIndex ) )

Modified: Zope/branches/2.12/src/Products/PluginIndexes/interfaces.py
===================================================================
--- Zope/branches/2.12/src/Products/PluginIndexes/interfaces.py	2010-05-19 13:42:24 UTC (rev 112541)
+++ Zope/branches/2.12/src/Products/PluginIndexes/interfaces.py	2010-05-19 14:01:19 UTC (rev 112542)
@@ -34,10 +34,22 @@
     def index_object(documentId, obj, threshold=None):
         """Index an object.
 
-        'documentId' is the integer ID of the document.
-        'obj' is the object to be indexed.
-        'threshold' is the number of words to process between committing
-        subtransactions.  If None, subtransactions are disabled.
+        - ``documentId`` is the integer ID of the document.
+
+        - ``obj`` is the object to be indexed.
+
+        - ``threshold`` is the number of words to process between committing
+          subtransactions.  If None, subtransactions are disabled.
+
+        For each name in ``getIndexSourceNames``, try to get the named
+        attribute from ``obj``.
+
+        - If the object does not have the attribute, do not add it to the
+          index for that name.
+
+        - If the attribute is a callable, call it to get the value.  If
+          calling it raises an AttributeError, do not add it to the index.
+          for that name.
         """
 
     def unindex_object(documentId):