Cache key is invariant of sort or pagenation options

Author: andbag

src/Products/ZCatalog/cache.py

@@ -47,6 +47,17 @@ def make_key(self, query):
 
         catalog = self.catalog
 
+        def skip(name, value):
+            if name in ['b_start', 'b_size']:
+                return True
+            elif catalog._get_sort_attr('on', {name: value}):
+                return True
+            elif catalog._get_sort_attr('limit', {name: value}):
+                return True
+            elif catalog._get_sort_attr('order', {name: value}):
+                return True
+            return False
+
         keys = []
         for name, value in query.items():
             if name in catalog.indexes:
@@ -57,6 +68,10 @@ def make_key(self, query):
                     # cache key invalidation cannot be supported if
                     # any index of query cannot be tested for changes
                     return None
+            elif skip(name, value):
+                # applying the query to indexes is invariant of
+                # sort or pagination options
+                continue
             else:
                 # return None if query has a nonexistent index key
                 return None
@@ -71,7 +86,7 @@ def make_key(self, query):
             else:
                 value = self._convert_datum(index, value)
 
-            keys.append((name, counter, value))
+            keys.append((name, value, counter))
 
         key = frozenset(keys)
         cache_key = (self.cid, key)

src/Products/ZCatalog/tests/test_cache.py

@@ -117,21 +117,32 @@ def _get_cache_key(self, query=None):
 
     def test_make_key(self):
         query = {'big': True}
-        key = (('catalog',),
-               frozenset([('big', self.length, (True,))]))
-        self.assertEquals(self._get_cache_key(query), key)
+        expect = (('catalog',),
+                  frozenset([('big', (True,), self.length)]))
+        self.assertEquals(self._get_cache_key(query), expect)
 
         query = {'start': '2013-07-01'}
-        key = (('catalog',),
-               frozenset([('start', self.length, ('2013-07-01',))]))
-        self.assertEquals(self._get_cache_key(query), key)
+        expect = (('catalog',),
+                  frozenset([('start', ('2013-07-01',), self.length)]))
+        self.assertEquals(self._get_cache_key(query), expect)
 
         query = {'path': '/1', 'date': '2013-07-05', 'numbers': [1, 3]}
-        key = (('catalog',),
-               frozenset([('date', 9, ('2013-07-05',)),
-                          ('numbers', 9, (1, 3)),
-                          ('path', 9, ('/1',))]))
-        self.assertEquals(self._get_cache_key(query), key)
+        expect = (('catalog',),
+                  frozenset([('date', ('2013-07-05',), self.length),
+                             ('numbers', (1, 3), self.length),
+                             ('path', ('/1',), self.length)]))
+        self.assertEquals(self._get_cache_key(query), expect)
+
+        queries = [{'big': True, 'b_start': 0},
+                   {'big': True, 'b_start': 0, 'b_size': 5},
+                   {'big': True, 'sort_on': 'big'},
+                   {'big': True, 'sort_on': 'big', 'sort_limit': 3},
+                   {'big': True, 'sort_on': 'big', 'sort_order': 'descending'},
+                   ]
+        expect = (('catalog',),
+                  frozenset([('big', (True,), self.length)]))
+        for query in queries:
+            self.assertEquals(self._get_cache_key(query), expect)
 
     def test_cache(self):
         query = {'big': True}