Consistent use of _convert method (#69)

* Consolidate code for consistent use of `_convert` method
* Add more information to the log entries

CHANGES.rst

@@ -18,6 +18,10 @@ Bug fixes
 - Fix rewriting of query to avoid wrong optimization of CompositeIndex.
   (`#59 <https://github.com/zopefoundation/Products.ZCatalog/issues/59>`_)
 
+- Consolidate code for general usage of ``UnIndex._convert`` method to
+  avoid unnecessary doubling of code.
+  (`#69 <https://github.com/zopefoundation/Products.ZCatalog/issues/69>`_)
+
 
 4.4 (2019-03-08)
 ----------------

src/Products/PluginIndexes/DateIndex/DateIndex.py

@@ -22,12 +22,10 @@
 from BTrees.Length import Length
 from DateTime.DateTime import DateTime
 from OFS.PropertyManager import PropertyManager
-from ZODB.POSException import ConflictError
 from zope.interface import implementer
 
 from Products.PluginIndexes.interfaces import IDateIndex
 from Products.PluginIndexes.unindex import UnIndex
-from Products.PluginIndexes.util import safe_callable
 
 LOG = getLogger('DateIndex')
 _marker = []
@@ -112,51 +110,6 @@ def clear(self):
         else:
             self._increment_counter()
 
-    def index_object(self, documentId, obj, threshold=None):
-        """index an object, normalizing the indexed value to an integer
-
-           o Normalized value has granularity of one minute.
-
-           o Objects which have 'None' as indexed value are *omitted*,
-             by design.
-        """
-        returnStatus = 0
-
-        try:
-            date_attr = getattr(obj, self.id)
-            if safe_callable(date_attr):
-                date_attr = date_attr()
-
-            ConvertedDate = self._convert(value=date_attr, default=_marker)
-        except AttributeError:
-            ConvertedDate = _marker
-
-        oldConvertedDate = self._unindex.get(documentId, _marker)
-
-        if ConvertedDate != oldConvertedDate:
-            if oldConvertedDate is not _marker:
-                self.removeForwardIndexEntry(oldConvertedDate, documentId)
-                if ConvertedDate is _marker:
-                    try:
-                        del self._unindex[documentId]
-                    except ConflictError:
-                        raise
-                    except Exception:
-                        LOG.error('Should not happen: ConvertedDate was there,'
-                                  ' now it\'s not, for document'
-                                  ' with id %s', documentId)
-
-            if ConvertedDate is not _marker:
-                self.insertForwardIndexEntry(ConvertedDate, documentId)
-                self._unindex[documentId] = ConvertedDate
-
-            returnStatus = 1
-
-        if returnStatus > 0:
-            self._increment_counter()
-
-        return returnStatus
-
     def _convert(self, value, default=None):
         """Convert Date/Time value to our internal representation"""
         if isinstance(value, DateTime):
@@ -186,7 +139,7 @@ def _convert(self, value, default=None):
 
         # flatten to precision
         if self.precision > 1:
-            t_val = t_val - (t_val % self.precision)
+            t_val = t_val - (int(t_val) % self.precision)
 
         t_val = int(t_val)
 

src/Products/PluginIndexes/unindex.py

@@ -257,6 +257,8 @@ def _index_object(self, documentId, obj, threshold=None, attr=''):
             # "object has default comparison" and won't let it be indexed.
             return 0
 
+        datum = self._convert(datum, default=_marker)
+
         # We don't want to do anything that we don't have to here, so we'll
         # check to see if the new and existing information is the same.
         oldDatum = self._unindex.get(documentId, _marker)
@@ -269,8 +271,14 @@ def _index_object(self, documentId, obj, threshold=None, attr=''):
                     except ConflictError:
                         raise
                     except Exception:
-                        LOG.error('Should not happen: oldDatum was there, '
-                                  'now its not, for document: %s', documentId)
+                        LOG.error('%(context)s: oldDatum was there, '
+                                  'now it\'s not for documentId %(doc_id)s '
+                                  'from index %(index)r.  This '
+                                  'should not happen.', dict(
+                                      context=self.__class__.__name__,
+                                      doc_id=documentId,
+                                      index=self.id),
+                                  exc_info=sys.exc_info())
 
             if datum is not _marker:
                 self.insertForwardIndexEntry(datum, documentId)
@@ -325,8 +333,13 @@ def unindex_object(self, documentId):
         except ConflictError:
             raise
         except Exception:
-            LOG.debug('Attempt to unindex nonexistent document'
-                      ' with id %s', documentId, exc_info=True)
+            LOG.debug('%(context)s: attempt to unindex nonexistent '
+                      'documentId %(doc_id)s from index %(index)r. This '
+                      'should not happen.', dict(
+                          context=self.__class__.__name__,
+                          doc_id=documentId,
+                          index=self.id),
+                      exc_info=True)
 
     def _apply_not(self, not_parm, resultset=None):
         index = self._index