Reorder to match interfaces; comply with interface semantics.

src/Products/PluginIndexes/PathIndex/PathIndex.py

@@ -11,8 +11,6 @@
 #
 ##############################################################################
 """Path index.
-
-$Id$
 """
 
 from logging import getLogger
@@ -35,7 +33,6 @@ from Products.PluginIndexes.common.util import parseIndexRequest
 from Products.PluginIndexes.interfaces import IPathIndex
 from Products.PluginIndexes.interfaces import IUniqueValueIndex
 
-_marker = []
 LOG = getLogger('Zope.PathIndex')
 
 
@@ -71,34 +68,29 @@ class PathIndex(Persistent, SimpleItem):
         self.useOperator = 'or'
         self.clear()
 
-    def clear(self):
-        self._depth = 0
-        self._index = OOBTree()
-        self._unindex = IOBTree()
-        self._length = Length(0)
+    def __len__(self):
+        return self._length()
 
-    def insertEntry(self, comp, id, level):
-        """Insert an entry.
+    # IPluggableIndex implementation
 
-           comp is a path component
-           id is the docid
-           level is the level of the component inside the path
+    def getEntryForObject(self, docid, default=None):
+        """ See IPluggableIndex.
         """
+        try:
+            return self._unindex[docid]
+        except KeyError:
+            return default
 
-        if not self._index.has_key(comp):
-            self._index[comp] = IOBTree()
-
-        if not self._index[comp].has_key(level):
-            self._index[comp][level] = IITreeSet()
-
-        self._index[comp][level].insert(id)
-        if level > self._depth:
-            self._depth = level
+    def getIndexSourceNames(self):
+        """ See IPluggableIndex.
+        """
+        return (self.id, 'getPhysicalPath', )
 
     def index_object(self, docid, obj ,threshold=100):
-        """ hook for (Z)Catalog """
-
+        """ See IPluggableIndex.
+        """
         f = getattr(obj, self.id, None)
+
         if f is not None:
             if safe_callable(f):
                 try:
@@ -118,20 +110,21 @@ class PathIndex(Persistent, SimpleItem):
 
         if isinstance(path, (list, tuple)):
             path = '/'+ '/'.join(path[1:])
+
         comps = filter(None, path.split('/'))
 
         if not self._unindex.has_key(docid):
             self._length.change(1)
 
         for i in range(len(comps)):
-            self.insertEntry(comps[i], docid, i)
+            self._insertEntry(comps[i], docid, i)
         self._unindex[docid] = path
         return 1
 
     def unindex_object(self, docid):
-        """ hook for (Z)Catalog """
-
-        if not self._unindex.has_key(docid):
+        """ See IPluggableIndex.
+        """
+        if docid not in self._unindex:
             LOG.debug('Attempt to unindex nonexistent document with id %s'
                       % docid)
             return
@@ -156,14 +149,106 @@ class PathIndex(Persistent, SimpleItem):
         self._length.change(-1)
         del self._unindex[docid]
 
-    def search(self, path, default_level=0):
+    def _apply_index(self, request):
+        """ See IPluggableIndex.
+
+        o Unpacks args from catalog and mapps onto '_search'.
+        """
+        record = parseIndexRequest(request, self.id, self.query_options)
+        if record.keys is None:
+            return None
+
+        level = record.get("level", 0)
+        operator = record.get('operator', self.useOperator).lower()
+
+        # depending on the operator we use intersection of union
+        if operator == "or":
+            set_func = union
+        else:
+            set_func = intersection
+
+        res = None
+        for k in record.keys:
+            rows = self._search(k,level)
+            res = set_func(res,rows)
+
+        if res:
+            return res, (self.id,)
+        else:
+            return IISet(), (self.id,)
+
+    def numObjects(self):
+        """ See IPluggableIndex.
         """
-        path is either a string representing a
-        relative URL or a part of a relative URL or
-        a tuple (path,level).
+        return len(self._unindex)
+
+    def indexSize(self):
+        """ See IPluggableIndex.
+        """
+        return len(self)
+
+    def clear(self):
+        """ See IPluggableIndex.
+        """
+        self._depth = 0
+        self._index = OOBTree()
+        self._unindex = IOBTree()
+        self._length = Length(0)
 
-        level >= 0  starts searching at the given level
-        level <  0  match at *any* level
+    # IUniqueValueIndex implementation
+
+    def hasUniqueValuesFor(self, name):
+        """ See IUniqueValueIndex.
+        """
+        return name == self.id
+
+    def uniqueValues(self, name=None, withLength=0):
+        """  See IUniqueValueIndex.
+        """
+        if name in (None, self.id, 'getPhysicalPath'):
+            if withLength:
+                for key in self._index:
+                    yield key, len(self._search(key, -1))
+            else:
+                for key in self._index.keys():
+                    yield key
+
+    # Helper methods
+
+    def _insertEntry(self, comp, id, level):
+        """ Insert an entry.
+
+        'comp' is an individual path component
+
+        'id' is the docid
+
+        .level'is the level of the component inside the path
+        """
+
+        if not self._index.has_key(comp):
+            self._index[comp] = IOBTree()
+
+        if not self._index[comp].has_key(level):
+            self._index[comp][level] = IITreeSet()
+
+        self._index[comp][level].insert(id)
+        if level > self._depth:
+            self._depth = level
+
+    def _search(self, path, default_level=0):
+        """ Perform the actual search.
+
+        ``path``
+            a string representing a relative URL, or a part of a relative URL,
+            or a tuple ``(path, level)``.  In the first two cases, use
+            ``default_level`` as the level for the search.
+
+        ``default_level``
+            the level to use for non-tuple queries.
+
+        ``level >= 0`` =>  match ``path`` only at the given level.
+
+        ``level <  0`` =>  match ``path`` at *any* level
         """
         if isinstance(path, str):
             level = default_level
@@ -174,7 +259,7 @@ class PathIndex(Persistent, SimpleItem):
         if level < 0:
             # Search at every level, return the union of all results
             return multiunion(
-                [self.search(path, level) 
+                [self._search(path, level) 
                  for level in xrange(self._depth + 1)])
 
         comps = filter(None, path.split('/'))
@@ -192,66 +277,6 @@ class PathIndex(Persistent, SimpleItem):
             results = intersection(results, self._index[comp][level+i])
         return results
 
-    def numObjects(self):
-        """Return the number of indexed objects."""
-        return len(self._unindex)
-
-    def indexSize(self):
-        """Return the size of the index in terms of distinct values."""
-        return len(self)
-
-    def __len__(self):
-        return self._length()
-
-    def _apply_index(self, request):
-        """ hook for (Z)Catalog
-            'request' --  mapping type (usually {"path": "..." }
-             additionaly a parameter "path_level" might be passed
-             to specify the level (see search())
-        """
-        record = parseIndexRequest(request, self.id, self.query_options)
-        if record.keys is None:
-            return None
-
-        level    = record.get("level",0)
-        operator = record.get('operator',self.useOperator).lower()
-
-        # depending on the operator we use intersection of union
-        if operator == "or":  set_func = union
-        else: set_func = intersection
-
-        res = None
-        for k in record.keys:
-            rows = self.search(k,level)
-            res = set_func(res,rows)
-
-        if res:
-            return res, (self.id,)
-        else:
-            return IISet(), (self.id,)
-
-    def hasUniqueValuesFor(self, name):
-        """has unique values for column name"""
-        return name == self.id
-
-    def uniqueValues(self, name=None, withLength=0):
-        """ needed to be consistent with the interface """
-        return self._index.keys()
-
-    def getIndexSourceNames(self):
-        """ return names of indexed attributes """
-        return ('getPhysicalPath', )
-
-    def getEntryForObject(self, docid, default=_marker):
-        """ Takes a document ID and returns all the information
-            we have on that specific object.
-        """
-        try:
-            return self._unindex[docid]
-        except KeyError:
-            # XXX Why is default ignored?
-            return None
-
     manage = manage_main = DTMLFile('dtml/managePathIndex', globals())
     manage_main._setName('manage_main')
 

src/Products/PluginIndexes/PathIndex/tests/testPathIndex.py

@@ -94,15 +94,24 @@ class PathIndexTests(unittest.TestCase):
         self.assertEqual(len(index._unindex), 0)
         self.assertEqual(index._length(), 0)
 
-    def test_clear(self):
+    def test_getEntryForObject_miss_no_default(self):
+        index = self._makeOne()
+        self.assertEqual(index.getEntryForObject(1234), None)
+
+    def test_getEntryForObject_miss_w_default(self):
+        index = self._makeOne()
+        default = object()
+        self.failUnless(index.getEntryForObject(1234, default) is default)
+
+    def test_getEntryForObject_hit(self):
         index = self._makeOne()
         _populateIndex(index)
-        index.clear()
-        self.assertEqual(len(index), 0)
-        self.assertEqual(index._depth, 0)
-        self.assertEqual(len(index._index), 0)
-        self.assertEqual(len(index._unindex), 0)
-        self.assertEqual(index._length(), 0)
+        self.assertEqual(index.getEntryForObject(1), DUMMIES[1].path)
+
+    def test_getIndexSourceNames(self):
+        index = self._makeOne('foo')
+        self.assertEqual(list(index.getIndexSourceNames()),
+                         ['foo', 'getPhysicalPath'])
 
     def test_index_object_broken_path_raises_TypeError(self):
         index = self._makeOne()
@@ -234,71 +243,6 @@ class PathIndexTests(unittest.TestCase):
         self.assertEqual(len(index._index), 0)
         self.assertEqual(len(index._unindex), 0)
 
-    def test_search_empty_index_string_query(self):
-        index = self._makeOne()
-        self.assertEqual(list(index.search('/xxx')), [])
-
-    def test_search_empty_index_tuple_query(self):
-        index = self._makeOne()
-        self.assertEqual(list(index.search(('/xxx', 0))), [])
-
-    def test_search_empty_path(self):
-        index = self._makeOne()
-        doc = Dummy('/aa')
-        index.index_object(1, doc)
-        self.assertEqual(list(index.search('/')), [1])
-
-    def test_search_matching_path(self):
-        index = self._makeOne()
-        doc = Dummy('/aa')
-        index.index_object(1, doc)
-        self.assertEqual(list(index.search('/aa')), [1])
-
-    def test_search_mismatched_path(self):
-        index = self._makeOne()
-        doc = Dummy('/aa')
-        index.index_object(1, doc)
-        self.assertEqual(list(index.search('/bb')), [])
-
-    def test_search_w_level_0(self):
-        index = self._makeOne()
-        doc = Dummy('/aa/bb')
-        index.index_object(1, doc)
-        self.assertEqual(list(index.search('aa', 0)), [1])
-        self.assertEqual(list(index.search('aa', 1)), [])
-        self.assertEqual(list(index.search('bb', 1)), [1])
-        self.assertEqual(list(index.search('aa/bb', 0)), [1])
-        self.assertEqual(list(index.search('aa/bb', 1)), [])
-
-    def test_numObjects_empty(self):
-        index = self._makeOne()
-        self.assertEqual(index.numObjects(), 0)
-
-    def test_numObjects_filled(self):
-        index = self._makeOne()
-        _populateIndex(index)
-        self.assertEqual(index.numObjects(), len(DUMMIES))
-
-    def test_indexSize_empty(self):
-        index = self._makeOne()
-        self.assertEqual(index.indexSize(), 0)
-
-    def test_indexSize_filled(self):
-        index = self._makeOne()
-        _populateIndex(index)
-        self.assertEqual(index.indexSize(), len(DUMMIES))
-
-    def test_indexSize_multiple_items_same_path(self):
-        index = self._makeOne()
-        doc1 = Dummy('/shared')
-        doc2 = Dummy('/shared')
-        index.index_object(1, doc1)
-        index.index_object(2, doc2)
-        self.assertEqual(len(index._index), 1)
-        self.assertEqual(len(index), 2)
-        self.assertEqual(index.numObjects(), 2)
-        self.assertEqual(index.indexSize(), 2)
-
     def test__apply_index_no_match_in_query(self):
         index = self._makeOne()
         self.assertEqual(index._apply_index({'foo': 'xxx'}), None)
@@ -397,6 +341,45 @@ class PathIndexTests(unittest.TestCase):
         lst = list(res[0].keys())
         self.assertEqual(lst, [2, 3, 4])
 
+    def test_numObjects_empty(self):
+        index = self._makeOne()
+        self.assertEqual(index.numObjects(), 0)
+
+    def test_numObjects_filled(self):
+        index = self._makeOne()
+        _populateIndex(index)
+        self.assertEqual(index.numObjects(), len(DUMMIES))
+
+    def test_indexSize_empty(self):
+        index = self._makeOne()
+        self.assertEqual(index.indexSize(), 0)
+
+    def test_indexSize_filled(self):
+        index = self._makeOne()
+        _populateIndex(index)
+        self.assertEqual(index.indexSize(), len(DUMMIES))
+
+    def test_indexSize_multiple_items_same_path(self):
+        index = self._makeOne()
+        doc1 = Dummy('/shared')
+        doc2 = Dummy('/shared')
+        index.index_object(1, doc1)
+        index.index_object(2, doc2)
+        self.assertEqual(len(index._index), 1)
+        self.assertEqual(len(index), 2)
+        self.assertEqual(index.numObjects(), 2)
+        self.assertEqual(index.indexSize(), 2)
+
+    def test_clear(self):
+        index = self._makeOne()
+        _populateIndex(index)
+        index.clear()
+        self.assertEqual(len(index), 0)
+        self.assertEqual(index._depth, 0)
+        self.assertEqual(len(index._index), 0)
+        self.assertEqual(len(index._unindex), 0)
+        self.assertEqual(index._length(), 0)
+
     def test_hasUniqueValuesFor_miss(self):
         index = self._makeOne()
         self.failIf(index.hasUniqueValuesFor('miss'))
@@ -407,33 +390,68 @@ class PathIndexTests(unittest.TestCase):
 
     def test_uniqueValues_empty(self):
         index = self._makeOne()
-        self.assertEqual(len(index.uniqueValues()), 0)
+        self.assertEqual(len(list(index.uniqueValues())), 0)
 
-    def test_uniqueValues_filled(self):
-        index = self._makeOne()
+    def test_uniqueValues_miss(self):
+        index = self._makeOne('foo')
         _populateIndex(index)
-        self.assertEqual(len(index.uniqueValues()), len(DUMMIES) + 3)
+        self.assertEqual(len(list(index.uniqueValues('bar'))), 0)
 
-    def test_getEntryForObject_miss_no_default(self):
+    def test_uniqueValues_hit(self):
+        index = self._makeOne('foo')
+        _populateIndex(index)
+        self.assertEqual(len(list(index.uniqueValues('foo'))),
+                         len(DUMMIES) + 3)
+
+    def test_uniqueValues_hit_w_withLength(self):
+        index = self._makeOne('foo')
+        _populateIndex(index)
+        results = dict(index.uniqueValues('foo', True))
+        self.assertEqual(len(results), len(DUMMIES) + 3)
+        for i in range(1, 19):
+            self.assertEqual(results['%s.html' % i], 1)
+        self.assertEqual(results['aa'],
+                         len([x for x in DUMMIES.values() if 'aa' in x.path]))
+        self.assertEqual(results['bb'],
+                         len([x for x in DUMMIES.values() if 'bb' in x.path]))
+        self.assertEqual(results['cc'],
+                         len([x for x in DUMMIES.values() if 'cc' in x.path]))
+
+    def test__search_empty_index_string_query(self):
         index = self._makeOne()
-        self.assertEqual(index.getEntryForObject(1234), None)
+        self.assertEqual(list(index._search('/xxx')), [])
 
-    def test_getEntryForObject_miss_w_default(self):
+    def test__search_empty_index_tuple_query(self):
         index = self._makeOne()
-        default = object()
-        # XXX  this is wrong:  should return the default
-        self.assertEqual(index.getEntryForObject(1234, default), None)
+        self.assertEqual(list(index._search(('/xxx', 0))), [])
 
-    def test_getEntryForObject_hit(self):
+    def test__search_empty_path(self):
         index = self._makeOne()
-        _populateIndex(index)
-        self.assertEqual(index.getEntryForObject(1), DUMMIES[1].path)
+        doc = Dummy('/aa')
+        index.index_object(1, doc)
+        self.assertEqual(list(index._search('/')), [1])
 
-    def test_getIndexSourceNames(self):
+    def test__search_matching_path(self):
         index = self._makeOne()
-        # XXX  this is wrong:  should include the index ID as well
-        self.assertEqual(list(index.getIndexSourceNames()),
-                         ['getPhysicalPath'])
+        doc = Dummy('/aa')
+        index.index_object(1, doc)
+        self.assertEqual(list(index._search('/aa')), [1])
+
+    def test__search_mismatched_path(self):
+        index = self._makeOne()
+        doc = Dummy('/aa')
+        index.index_object(1, doc)
+        self.assertEqual(list(index._search('/bb')), [])
+
+    def test__search_w_level_0(self):
+        index = self._makeOne()
+        doc = Dummy('/aa/bb')
+        index.index_object(1, doc)
+        self.assertEqual(list(index._search('aa', 0)), [1])
+        self.assertEqual(list(index._search('aa', 1)), [])
+        self.assertEqual(list(index._search('bb', 1)), [1])
+        self.assertEqual(list(index._search('aa/bb', 0)), [1])
+        self.assertEqual(list(index._search('aa/bb', 1)), [])
 
 
 def test_suite():