From bf9782883bf143748d4b08fb42d11502fd7b21f6 Mon Sep 17 00:00:00 2001 From: Eric Wohnlich Date: Fri, 8 May 2020 11:37:37 -0400 Subject: [PATCH] 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 --- CHANGES.rst | 3 ++- src/Products/PluginIndexes/FieldIndex/tests.py | 11 +++++++++++ src/Products/PluginIndexes/unindex.py | 9 +++++---- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 6527d8f8..a114bded 100644 --- a/CHANGES.rst +++ b/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 `_) 5.1 (2020-04-20) diff --git a/src/Products/PluginIndexes/FieldIndex/tests.py b/src/Products/PluginIndexes/FieldIndex/tests.py index d8bfc329..226adf4d 100644 --- a/src/Products/PluginIndexes/FieldIndex/tests.py +++ b/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], ]) diff --git a/src/Products/PluginIndexes/unindex.py b/src/Products/PluginIndexes/unindex.py index 620f252f..12d5f2c7 100644 --- a/src/Products/PluginIndexes/unindex.py +++ b/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)