Remove CatalogSearchArgumentsMap and support for request based queries.

CHANGES.rst

@@ -4,6 +4,9 @@ Changelog
 4.0 (unreleased)
 ----------------
 
+- Remove CatalogSearchArgumentsMap and support for using requests
+  objects as queries.
+
 - Empty catalog queries now return no results.
 
 - No longer special-case empty strings in catalog queries.

src/Products/ZCatalog/Catalog.py

@@ -13,7 +13,6 @@
 
 import types
 import logging
-import warnings
 from bisect import bisect
 from collections import defaultdict
 from operator import itemgetter
@@ -474,46 +473,18 @@ def getIndexDataForRID(self, rid):
             result[name] = self.getIndex(name).getEntryForObject(rid, "")
         return result
 
-    # This is the Catalog search engine. Most of the heavy lifting happens
-    # below
-
-    def make_query(self, request):
-        # This is a bit of a mess, but the ZCatalog API has traditionally
-        # supported passing in query restrictions in almost arbitrary ways.
-        real_req = None
-        if isinstance(request, dict):
-            query = request.copy()
-        elif isinstance(request, CatalogSearchArgumentsMap):
-            query = {}
-            query.update(request.keywords)
-            real_req = request.request
-            if isinstance(real_req, dict):
-                query.update(real_req)
-                real_req = None
-        else:
-            real_req = request
-
-        if real_req:
-            warnings.warn('You have specified a query using either a request '
-                          'object or a mixture of a query dict and keyword '
-                          'arguments. Please use only a simple query dict. '
-                          'Your query contained "%s". This support is '
-                          'deprecated and will be removed in Zope 4.' %
-                          repr(real_req), DeprecationWarning, stacklevel=4)
-
-            known_keys = query.keys()
-            # The request has too many places where an index restriction
-            # might be specified. Putting all of request.form,
-            # request.other, ... into the query isn't what we want.
-            # So we iterate over all known indexes instead and see if they
-            # are in the request.
-            for iid in self.indexes.keys():
-                if iid in known_keys:
-                    continue
-                value = real_req.get(iid)
-                if value:
-                    query[iid] = value
+    def merge_query_args(self, query=None, **kw):
+        if not kw and isinstance(query, dict):
+            # Short cut for the best practice.
+            return query
+
+        merged_query = {}
+        if isinstance(query, dict):
+            merged_query.update(query)
+        merged_query.update(kw)
+        return merged_query
 
+    def make_query(self, query):
         for iid in self.indexes.keys():
             index = self.getIndex(iid)
             if ITransposeQuery.providedBy(index):
@@ -579,7 +550,7 @@ def search(self, query,
         """Iterate through the indexes, applying the query to each one. If
         merge is true then return a lazy result set (sorted if appropriate)
         otherwise return the raw (possibly scored) results for later merging.
-        Limit is used in conjuntion with sorting or scored results to inform
+        Limit is used in conjunction with sorting or scored results to inform
         the catalog how many results you are really interested in. The catalog
         can then use optimizations to save time and memory. The number of
         results is not guaranteed to fall within the limit however, you should
@@ -1052,28 +1023,15 @@ def _getSortIndex(self, args):
             return sort_indexes
         return None
 
-    def searchResults(self, REQUEST=None, used=None, _merge=True, **kw):
-        # You should pass in a simple dictionary as the request argument,
+    def searchResults(self, query=None, _merge=True, **kw):
+        # You should pass in a simple dictionary as the first argument,
         # which only contains the relevant query.
-        # The used argument is deprecated and is ignored
-        if REQUEST is None and not kw:
-            # Try to acquire request if we get no args for bw compat
-            warnings.warn('Calling searchResults without a query argument nor '
-                          'keyword arguments is deprecated. In Zope 4 the '
-                          'query will no longer be automatically taken from '
-                          'the acquired request.',
-                          DeprecationWarning, stacklevel=3)
-            REQUEST = getattr(self, 'REQUEST', None)
-        if isinstance(REQUEST, dict) and not kw:
-            # short cut for the best practice
-            args = REQUEST
-        else:
-            args = CatalogSearchArgumentsMap(REQUEST, kw)
-        sort_indexes = self._getSortIndex(args)
-        sort_limit = self._get_sort_attr('limit', args)
+        query = self.merge_query_args(query, **kw)
+        sort_indexes = self._getSortIndex(query)
+        sort_limit = self._get_sort_attr('limit', query)
         reverse = False
         if sort_indexes is not None:
-            order = self._get_sort_attr("order", args)
+            order = self._get_sort_attr("order", query)
             reverse = []
             if order is None:
                 order = ['']
@@ -1085,7 +1043,7 @@ def searchResults(self, REQUEST=None, used=None, _merge=True, **kw):
                 # be nice and keep the old API intact for single sort_order
                 reverse = reverse[0]
         # Perform searches with indexes and sort_index
-        return self.search(args, sort_indexes, reverse, sort_limit, _merge)
+        return self.search(query, sort_indexes, reverse, sort_limit, _merge)
 
     __call__ = searchResults
 
@@ -1097,42 +1055,6 @@ def getCatalogPlan(self, query=None):
         return CatalogPlan(self, query, threshold)
 
 
-class CatalogSearchArgumentsMap(object):
-    """Multimap catalog arguments coming simultaneously from keywords
-    and request.
-    """
-
-    def __init__(self, request, keywords):
-        self.request = request or {}
-        self.keywords = keywords or {}
-
-    def __getitem__(self, key):
-        marker = []
-        v = self.keywords.get(key, marker)
-        if v is marker:
-            v = self.request[key]
-        return v
-
-    def get(self, key, default=None):
-        try:
-            v = self[key]
-        except KeyError:
-            return default
-        else:
-            return v
-
-    def has_key(self, key):
-        try:
-            self[key]
-        except KeyError:
-            return False
-        else:
-            return True
-
-    def __contains__(self, name):
-        return self.has_key(name)  # NOQA
-
-
 def mergeResults(results, has_sort_keys, reverse):
     """Sort/merge sub-results, generating a flat sequence.
 

src/Products/ZCatalog/ZCatalog.py

@@ -610,37 +610,33 @@ def _searchable_result_columns(self):
         return r
 
     security.declareProtected(search_zcatalog, 'searchResults')
-    def searchResults(self, REQUEST=None, used=None, **kw):
-        """Search the catalog
+    def searchResults(self, query=None, **kw):
+        """Search the catalog.
 
-        Search terms can be passed in the REQUEST or as keyword
-        arguments.
-
-        The used argument is now deprecated and ignored
+        Search terms can be passed as a query or as keyword arguments.
         """
-
-        return self._catalog.searchResults(REQUEST, used, **kw)
+        return self._catalog.searchResults(query, **kw)
 
     security.declareProtected(search_zcatalog, '__call__')
     __call__ = searchResults
 
     security.declareProtected(search_zcatalog, 'search')
-    def search(self, query_request,
+    def search(self, query,
                sort_index=None, reverse=0, limit=None, merge=1):
         """Programmatic search interface, use for searching the catalog from
         scripts.
 
-        query_request: Dictionary containing catalog query
-        sort_index:    Name of sort index
-        reverse:       Reverse sort order?
-        limit:         Limit sorted result count (optimization hint)
-        merge:         Return merged results (like searchResults) or raw
-                       results for later merging.
+        query:      Dictionary containing catalog query
+        sort_index: Name of sort index
+        reverse:    Reverse sort order?
+        limit:      Limit sorted result count (optimization hint)
+        merge:      Return merged results (like searchResults) or raw
+                    results for later merging.
         """
         if sort_index is not None:
             sort_index = self._catalog.indexes[sort_index]
         return self._catalog.search(
-            query_request, sort_index, reverse, limit, merge)
+            query, sort_index, reverse, limit, merge)
 
     security.declareProtected(search_zcatalog, 'valid_roles')
     def valid_roles(self):

src/Products/ZCatalog/interfaces.py

@@ -123,15 +123,15 @@ def getIndexObjects():
         """Returns a list of acquisition wrapped index objects
         """
 
-    def searchResults(REQUEST=None, **kw):
+    def searchResults(query=None, **kw):
         """Search the catalog.
 
-        Search terms can be passed in the REQUEST or as keyword
+        Search terms can be passed in the query or as keyword
         arguments.
 
         Search queries consist of a mapping of index names to search
-        parameters.  You can either pass a mapping to searchResults as
-        the variable 'REQUEST' or you can use index names and search
+        parameters. You can either pass a mapping to searchResults as
+        the variable 'query' or you can use index names and search
         parameters as keyword arguments to the method, in other words::
 
           searchResults(title='Elvis Exposed',
@@ -142,15 +142,15 @@ def searchResults(REQUEST=None, **kw):
           searchResults({'title' : 'Elvis Exposed',
                          'author : 'The Great Elvonso'})
 
-        In these examples, 'title' and 'author' are indexes.  This
+        In these examples, 'title' and 'author' are indexes. This
         query will return any objects that have the title *Elvis
-        Exposed* AND also are authored by *The Great Elvonso*.  Terms
+        Exposed* AND also are authored by *The Great Elvonso*. Terms
         that are passed as keys and values in a searchResults() call
         are implicitly ANDed together. To OR two search results, call
         searchResults() twice and add concatenate the results like this::
 
-          results = ( searchResults(title='Elvis Exposed') +
-                      searchResults(author='The Great Elvonso') )
+          results = (searchResults(title='Elvis Exposed') +
+                     searchResults(author='The Great Elvonso'))
 
         This will return all objects that have the specified title OR
         the specified author.
@@ -170,39 +170,37 @@ def searchResults(REQUEST=None, **kw):
 
         There are some rules to consider when querying this method:
 
-            - an empty query mapping (or a bogus REQUEST) returns all
-              items in the catalog.
+            - an empty query mapping returns an empty result.
 
             - results from a query involving only field/keyword
-              indexes, e.g.  {'id':'foo'} and no 'sort_on' will be
+              indexes, e.g. {'id':'foo'} and no 'sort_on' will be
               returned unsorted.
 
             - results from a complex query involving a field/keyword
               index *and* a text index,
-              e.g. {'id':'foo','PrincipiaSearchSource':'bar'} and no
+              e.g. {'id':'foo','SearchableText':'bar'} and no
               'sort_on' will be returned unsorted.
 
             - results from a simple text index query
-              e.g.{'PrincipiaSearchSource':'foo'} will be returned
-              sorted in descending order by 'score'.  A text index
-              cannot beused as a 'sort_on' parameter, and attempting
+              e.g.{'SearchableText':'foo'} will be returned
+              sorted in descending order by 'score'. A text index
+              cannot be used as a 'sort_on' parameter, and attempting
               to do so will raise an error.
 
         Depending on the type of index you are querying, you may be
         able to provide more advanced search parameters that can
-        specify range searches or wildcards.  These features are
-        documented in The Zope Book.
+        specify range searches or wildcards.
         """
 
-    def __call__(REQUEST=None, **kw):
+    def __call__(query=None, **kw):
         """Search the catalog, the same way as 'searchResults'.
         """
 
-    def search(query_request, sort_index=None, reverse=0, limit=None, merge=1):
+    def search(query, sort_index=None, reverse=0, limit=None, merge=1):
         """Programmatic search interface, use for searching the catalog from
         scripts.
 
-        query_request -- Dictionary containing catalog query. This uses the
+        query -- Dictionary containing catalog query. This uses the
         same format as searchResults.
 
         sort_index -- Name of sort index

src/Products/ZCatalog/tests/test_catalog.py

@@ -310,7 +310,7 @@ def testResultLength(self):
         self.assertEqual(len(a), self.upper,
                          'length should be %s, its %s' % (self.upper, len(a)))
 
-    def testEmptyMapping(self):
+    def test_query_empty(self):
         # Queries with empty mappings used to return all.
         def extra(catalog):
             col1 = FieldIndex('col1')
@@ -320,10 +320,9 @@ def extra(catalog):
         all_data = catalog({})
         self.assertEqual(len(all_data), 0)
 
-    def testMappingWithEmptyKeysDoesntReturnAll(self):
-        # Queries with empty keys used to return all, because of a bug in the
-        # parseIndexRequest function, mistaking a CatalogSearchArgumentsMap
-        # for a Record class
+    def test_query_empty_keys(self):
+        # Queries with empty keys used to return all, because of a bug
+        # in the parseIndexRequest function.
         def extra(catalog):
             col1 = FieldIndex('col1')
             catalog.addIndex('col1', col1)
@@ -922,61 +921,6 @@ def test_range_search(self):
                              "%d vs [%d,%d]" % (r.number, m, n))
 
 
-class TestCatalogSearchArgumentsMap(unittest.TestCase):
-
-    def _make_one(self, request=None, keywords=None):
-        from Products.ZCatalog.Catalog import CatalogSearchArgumentsMap
-        return CatalogSearchArgumentsMap(request, keywords)
-
-    def test_init_empty(self):
-        argmap = self._make_one()
-        self.assert_(argmap)
-
-    def test_init_request(self):
-        argmap = self._make_one(dict(foo='bar'), None)
-        self.assertEquals(argmap.get('foo'), 'bar')
-
-    def test_init_keywords(self):
-        argmap = self._make_one(None, dict(foo='bar'))
-        self.assertEquals(argmap.get('foo'), 'bar')
-
-    def test_getitem(self):
-        argmap = self._make_one(dict(a='a'), dict(b='b'))
-        self.assertEquals(argmap['a'], 'a')
-        self.assertEquals(argmap['b'], 'b')
-        self.assertRaises(KeyError, argmap.__getitem__, 'c')
-
-    def test_getitem_emptystring(self):
-        argmap = self._make_one(dict(a='', c='c'), dict(b='', c=''))
-        self.assertEqual(argmap['a'], '')
-        self.assertEqual(argmap['b'], '')
-        self.assertEquals(argmap['c'], '')
-
-    def test_get(self):
-        argmap = self._make_one(dict(a='a'), dict(b='b'))
-        self.assertEquals(argmap.get('a'), 'a')
-        self.assertEquals(argmap.get('b'), 'b')
-        self.assertEquals(argmap.get('c'), None)
-        self.assertEquals(argmap.get('c', 'default'), 'default')
-
-    def test_keywords_precedence(self):
-        argmap = self._make_one(dict(a='a', c='r'), dict(b='b', c='k'))
-        self.assertEquals(argmap.get('c'), 'k')
-        self.assertEquals(argmap['c'], 'k')
-
-    def test_haskey(self):
-        argmap = self._make_one(dict(a='a'), dict(b='b'))
-        self.assert_(argmap.has_key('a'))  # NOQA
-        self.assert_(argmap.has_key('b'))  # NOQA
-        self.assert_(not argmap.has_key('c'))  # NOQA
-
-    def test_contains(self):
-        argmap = self._make_one(dict(a='a'), dict(b='b'))
-        self.assert_('a' in argmap)
-        self.assert_('b' in argmap)
-        self.assert_('c' not in argmap)
-
-
 class TestMergeResults(unittest.TestCase):
 
     def _make_one(self):