Fix 100 (#103)

* proposed fix for #100, case where index value is changed to None after previously being indexed

* remove pycharm junk

* remove old comment as no longer relevant in BTrees. But we still want
to not index None values

* use assertRaises instead of try/except for expected KeyError

Co-authored-by: wohnlice <[email protected]>

CHANGES.rst

@@ -4,7 +4,8 @@ Changelog
 5.2 (unreleased)
 ----------------
 
-- Nothing changed yet.
+- Fix case where index value is changed to None after previously being indexed.
+  (`#100 <https://github.com/zopefoundation/Products.ZCatalog/issues/100>`_)
 
 
 5.1 (2020-04-20)

src/Products/PluginIndexes/FieldIndex/tests.py

@@ -216,6 +216,17 @@ def testNone(self):
         self.assertFalse(None in self._index.uniqueValues('foo'))
         self._checkApply({'foo': None}, [])
 
+    def testReindexNone(self):
+        d = Dummy('abc')
+        self._index.index_object(2, d)
+        self._checkApply({'foo': 'abc'}, [self._values[2], ])
+        assert self._index.keyForDocument(2) == 'abc'
+        d._foo = None
+        self._index.index_object(2, d)
+        self._checkApply({'foo': 'abc'}, [])
+        with self.assertRaises(KeyError):
+            self._index.keyForDocument(2)
+
     def testReindex(self):
         self._populateIndex()
         self._checkApply({'foo': 'abc'}, [self._values[2], ])

src/Products/PluginIndexes/unindex.py

@@ -251,10 +251,11 @@ def _index_object(self, documentId, obj, threshold=None, attr=''):
         # First we need to see if there's anything interesting to look at
         datum = self._get_object_datum(obj, attr)
         if datum is None:
-            # Prevent None from being indexed. None doesn't have a valid
-            # ordering definition compared to any other object.
-            # BTrees 4.0+ will throw a TypeError
-            # "object has default comparison" and won't let it be indexed.
+            # Remove previous index if it exists
+            oldDatum = self._unindex.get(documentId, _marker)
+            if oldDatum:
+                self.removeForwardIndexEntry(oldDatum, documentId)
+                del self._unindex[documentId]
             return 0
 
         datum = self._convert(datum, default=_marker)