From dc9b5641ca1ed3edfd0a1b636f5122570fc89d7d Mon Sep 17 00:00:00 2001 From: vagrant Date: Fri, 6 Apr 2018 14:05:45 +0200 Subject: [PATCH] Add transaction awareness for cache keys --- .../DateRangeIndex/DateRangeIndex.py | 5 +- .../PluginIndexes/PathIndex/PathIndex.py | 13 +++++ .../PluginIndexes/TopicIndex/TopicIndex.py | 11 ++++ src/Products/PluginIndexes/interfaces.py | 3 ++ .../PluginIndexes/tests/test_unindex.py | 51 +++++++++++++++++++ src/Products/PluginIndexes/unindex.py | 17 ++++++- src/Products/ZCatalog/cache.py | 4 +- src/Products/ZCatalog/tests/test_cache.py | 31 ++++++++--- 8 files changed, 123 insertions(+), 12 deletions(-) diff --git a/src/Products/PluginIndexes/DateRangeIndex/DateRangeIndex.py b/src/Products/PluginIndexes/DateRangeIndex/DateRangeIndex.py index 53b1a33f..8e376f7b 100644 --- a/src/Products/PluginIndexes/DateRangeIndex/DateRangeIndex.py +++ b/src/Products/PluginIndexes/DateRangeIndex/DateRangeIndex.py @@ -248,8 +248,9 @@ def getRequestCacheKey(self, record, resultset=None): tid = str(term) # unique index identifier - iid = '_%s_%s_%s' % (self.__class__.__name__, - self.id, self.getCounter()) + iid = (self.__class__.__name__, + self.id, self.getCounterKey()) + # record identifier if resultset is None: rid = '_%s' % (tid, ) diff --git a/src/Products/PluginIndexes/PathIndex/PathIndex.py b/src/Products/PluginIndexes/PathIndex/PathIndex.py index 1216542e..c2392525 100644 --- a/src/Products/PluginIndexes/PathIndex/PathIndex.py +++ b/src/Products/PluginIndexes/PathIndex/PathIndex.py @@ -24,8 +24,10 @@ from BTrees.OOBTree import OOBTree from BTrees.Length import Length from Persistence import Persistent +from ZODB.utils import newTid from zope.interface import implementer + from Products.PluginIndexes.interfaces import ( IPathIndex, IQueryIndex, @@ -200,6 +202,17 @@ def getCounter(self): """Return a counter which is increased on index changes""" return self._counter is not None and self._counter() or 0 + def getCounterKey(self): + """Returns an unique key indicating an uniqe state of the index""" + if self._counter is not None: + key = (self.getCounter(), self._counter._p_serial) + else: + # generate new serial for backward compatibility + # if counter is not set + key = (self.getCounter(), newTid(None)) + + return key + def numObjects(self): """ See IPluggableIndex. """ diff --git a/src/Products/PluginIndexes/TopicIndex/TopicIndex.py b/src/Products/PluginIndexes/TopicIndex/TopicIndex.py index 21982031..aac9d2f9 100644 --- a/src/Products/PluginIndexes/TopicIndex/TopicIndex.py +++ b/src/Products/PluginIndexes/TopicIndex/TopicIndex.py @@ -21,6 +21,7 @@ from BTrees.Length import Length from OFS.SimpleItem import SimpleItem from Persistence import Persistent +from ZODB.utils import newTid from zope.interface import implementer from Products.PluginIndexes.interfaces import ( @@ -103,6 +104,16 @@ def getCounter(self): """Return a counter which is increased on index changes""" return self._counter is not None and self._counter() or 0 + def getCounterKey(self): + """Returns an unique key indicating an uniqe state of the index""" + if self._counter is not None: + key = (self.getCounter(), self._counter._p_serial) + else: + # counter is not set, generate new serial + key = (self.getCounter(), newTid(None)) + + return key + def numObjects(self): """Return the number of indexed objects.""" return "n/a" diff --git a/src/Products/PluginIndexes/interfaces.py b/src/Products/PluginIndexes/interfaces.py index fbf18946..3d129ca7 100644 --- a/src/Products/PluginIndexes/interfaces.py +++ b/src/Products/PluginIndexes/interfaces.py @@ -287,3 +287,6 @@ class IIndexCounter(Interface): def getCounter(): """Return a counter which is increased on index changes""" + + def getCounterKey(): + """Returns an unique key indicating an uniqe state of the index""" diff --git a/src/Products/PluginIndexes/tests/test_unindex.py b/src/Products/PluginIndexes/tests/test_unindex.py index cfc3a364..3bddb038 100644 --- a/src/Products/PluginIndexes/tests/test_unindex.py +++ b/src/Products/PluginIndexes/tests/test_unindex.py @@ -16,6 +16,9 @@ from BTrees.IIBTree import difference from OFS.SimpleItem import SimpleItem from Testing.makerequest import makerequest +from Acquisition import aq_base +import ZODB +import transaction class TestUnIndex(unittest.TestCase): @@ -164,3 +167,51 @@ class Dummy(object): # clear changes the index index.clear() self.assertEqual(index.getCounter(), 3) + + def test_getCounterKey(self): + index = self._makeOne('counter') + + class Dummy(object): + def __init__(self, obj_id): + self.id = obj_id + self.counter = 'counter_{0}'.format(obj_id) + + # check counter key of initialized empty index + # counter key is a tuple of the counts of index operations and + # transaction id (tid) of the counter variable + + key0 = index.getCounterKey() + self.assertEqual(key0, (0, b'\x00\x00\x00\x00\x00\x00\x00\x00')) + + connection = ZODB.connection(None) + connection.add(aq_base(index)) + + # first object to index + obj = Dummy(1) + index.index_object(obj.id, obj) + + # indexing of object changes counter but not the tid + key1 = index.getCounterKey() + self.assertEqual(key1, (1, b'\x00\x00\x00\x00\x00\x00\x00\x00')) + + transaction.commit() + + # commit changes the tid but not the counter + key2 = index.getCounterKey() + self.assertEqual(key2[0], key1[0]) + self.assertFalse(key2[1] == key1[1]) + + # second object to index + obj = Dummy(2) + index.index_object(obj.id, obj) + + # indexing of object changes counter but not the tid + key3 = index.getCounterKey() + self.assertFalse(key3[0] == key2[0]) + self.assertEqual(key3[1], key2[1]) + + transaction.abort() + + # abort resets counter key to previos state + key4 = index.getCounterKey() + self.assertEqual(key4, key2) diff --git a/src/Products/PluginIndexes/unindex.py b/src/Products/PluginIndexes/unindex.py index 6ce6b1af..8b94e2b4 100644 --- a/src/Products/PluginIndexes/unindex.py +++ b/src/Products/PluginIndexes/unindex.py @@ -31,6 +31,7 @@ from BTrees.OOBTree import OOBTree from OFS.SimpleItem import SimpleItem from ZODB.POSException import ConflictError +from ZODB.utils import newTid from zope.interface import implementer from Products.PluginIndexes.cache import RequestCache @@ -299,6 +300,17 @@ def getCounter(self): """Return a counter which is increased on index changes""" return self._counter is not None and self._counter() or 0 + def getCounterKey(self): + """Returns an unique key indicating an uniqe state of the index""" + if self._counter is not None: + key = (self.getCounter(), self._counter._p_serial) + else: + # generate new serial for backward compatibility + # if counter is not set + key = (self.getCounter(), newTid(None)) + + return key + def numObjects(self): """Return the number of indexed objects.""" return len(self._unindex) @@ -386,8 +398,9 @@ def getRequestCacheKey(self, record, resultset=None): rid = frozenset(params) # unique index identifier - iid = '_%s_%s_%s' % (self.__class__.__name__, - self.id, self.getCounter()) + iid = (self.__class__.__name__, + self.id, self.getCounterKey()) + return (iid, rid) def _apply_index(self, request, resultset=None): diff --git a/src/Products/ZCatalog/cache.py b/src/Products/ZCatalog/cache.py index f39a3995..82f5d5f1 100644 --- a/src/Products/ZCatalog/cache.py +++ b/src/Products/ZCatalog/cache.py @@ -63,7 +63,7 @@ def skip(name, value): if name in catalog.indexes: index = catalog.getIndex(name) if IIndexCounter.providedBy(index): - counter = index.getCounter() + ck = index.getCounterKey() else: # cache key invalidation cannot be supported if # any index of query cannot be tested for changes @@ -86,7 +86,7 @@ def skip(name, value): else: value = self._convert_datum(index, value) - keys.append((name, value, counter)) + keys.append((name, value, ck)) key = frozenset(keys) cache_key = (self.cid, key) diff --git a/src/Products/ZCatalog/tests/test_cache.py b/src/Products/ZCatalog/tests/test_cache.py index 2b10685e..2c9a905e 100644 --- a/src/Products/ZCatalog/tests/test_cache.py +++ b/src/Products/ZCatalog/tests/test_cache.py @@ -118,19 +118,35 @@ def _get_cache_key(self, query=None): def test_make_key(self): query = {'big': True} expect = (('catalog',), - frozenset([('big', (True,), self.length)])) + frozenset([('big', (True,), + (self.length, + b'\x00\x00\x00\x00\x00\x00\x00\x00'))]) + ) + self.assertEquals(self._get_cache_key(query), expect) query = {'start': '2013-07-01'} expect = (('catalog',), - frozenset([('start', ('2013-07-01',), self.length)])) + frozenset([('start', ('2013-07-01',), + (self.length, + b'\x00\x00\x00\x00\x00\x00\x00\x00'))]) + + ) + self.assertEquals(self._get_cache_key(query), expect) query = {'path': '/1', 'date': '2013-07-05', 'numbers': [1, 3]} expect = (('catalog',), - frozenset([('date', ('2013-07-05',), self.length), - ('numbers', (1, 3), self.length), - ('path', ('/1',), self.length)])) + frozenset([('date', ('2013-07-05',), + (self.length, + b'\x00\x00\x00\x00\x00\x00\x00\x00')), + ('numbers', (1, 3), + (self.length, + b'\x00\x00\x00\x00\x00\x00\x00\x00')), + ('path', ('/1',), + (self.length, + b'\x00\x00\x00\x00\x00\x00\x00\x00'))])) + self.assertEquals(self._get_cache_key(query), expect) queries = [{'big': True, 'b_start': 0}, @@ -140,7 +156,10 @@ def test_make_key(self): {'big': True, 'sort_on': 'big', 'sort_order': 'descending'}, ] expect = (('catalog',), - frozenset([('big', (True,), self.length)])) + frozenset([('big', (True,), + (self.length, + b'\x00\x00\x00\x00\x00\x00\x00\x00'))])) + for query in queries: self.assertEquals(self._get_cache_key(query), expect)