Fixed cache.obj_modified when checking annotations

CHANGES.rst

@@ -4,6 +4,12 @@ Changelog
 0.74 (unreleased)
 -----------------
 
+- Fixed `cache.obj_modified` when checking annotations, take care that `_p_mtime`
+  is not changed on `__annotations__` when a value changes in a stored annotation
+  that is a `PersistentMapping`.
+  Also removed parameter `asstring=False`, when `asdatetime=False`, returned
+  value is float which is convenient to be used in a cachekey.
+  [gbastien]
 - Add `catalog` parameter on `content.uuidsToObjects`, `content.uuidsToObject`,
   `content.uuidsToCatalogBrains` and `uuidsToCatalogBrain` to allow query on
   other catalogs (e.g. uid_catalog)

src/imio/helpers/cache.py

@@ -175,18 +175,26 @@ def cache(get_key):
     return replacement
 
 
-def obj_modified(obj, asdatetime=True, check_annotation=True, asstring=False):
-    """Returns max value between obj.modified(), obj._p_mtime and __anotations__._p_mtime.
-
-       to check also attribute modification and annotation modification."""
+def obj_modified(obj, asdatetime=True, check_annotation=True):
+    """Returns max value between obj.modified(), obj._p_mtime and __anotations__._p_mtime
+       to check also attribute modification and annotation modification.
+       Returns a float when asdatetime=False.
+    """
+    modified = max(float(obj.modified()), obj._p_mtime)
     if check_annotation and base_hasattr(obj, '__annotations__'):
-        modified = max(float(obj.modified()), obj._p_mtime, obj.__annotations__._p_mtime)
-    else:
-        modified = max(float(obj.modified()), obj._p_mtime)
+        # when stored annotation is a PersistentMapping, we need to check _p_mtime
+        # of stored annotation because a change in the PersistentMapping will not change
+        # the __annotations__ _p_mtime
+        ann_max_time = max(
+            [obj.__annotations__._p_mtime] +
+            [obj.__annotations__[k]._p_mtime
+             for k in obj.__annotations__.keys()
+             if hasattr(obj.__annotations__[k], '_p_mtime')])
+        modified = max(float(obj.modified()),
+                       obj._p_mtime,
+                       ann_max_time)
     if asdatetime:
         modified = datetime.fromtimestamp(modified)
-    elif asstring:
-        modified = datetime.fromtimestamp(modified).strftime('%Y%m%d-%H%M%S-%f')
     return modified
 
 

src/imio/helpers/testing.py

@@ -94,6 +94,7 @@ def setUp(self):
         super(IntegrationTestCase, self).setUp()
         self.portal = self.layer['portal']
         self.portal_url = self.portal.absolute_url()
+        self.request = self.portal.REQUEST
         self.catalog = self.portal.portal_catalog
 
         sml = getSiteManager(self.portal)

src/imio/helpers/tests/test_cache.py

@@ -9,6 +9,7 @@
 from imio.helpers.cache import generate_key
 from imio.helpers.cache import get_cachekey_volatile
 from imio.helpers.cache import invalidate_cachekey_volatile_for
+from imio.helpers.cache import obj_modified
 from imio.helpers.cache import VOLATILE_ATTR
 from imio.helpers.cache import volatile_cache_with_parameters
 from imio.helpers.cache import volatile_cache_without_parameters
@@ -23,13 +24,15 @@
 from plone.memoize.interfaces import ICacheChooser
 from Products.CMFCore.utils import _getAuthenticatedUser
 from Products.PlonePAS.plugins.ufactory import PloneUser
+from zope.annotation import IAnnotations
 from zope.component import getUtility
 from zope.component import queryUtility
 from zope.ramcache.interfaces.ram import IRAMCache
 from zope.schema.interfaces import IVocabularyFactory
 
 import os
 import time
+import transaction
 
 
 memPropName = Memojito.propname
@@ -75,7 +78,7 @@ def test_cleanVocabularyCacheFor(self):
         """
         This helper method cleans instance.memoize cache defined on a vocabulary.
         """
-        self.portal.REQUEST.set('vocab_values', ('1', '2'))
+        self.request.set('vocab_values', ('1', '2'))
         vocab = queryUtility(IVocabularyFactory,
                              "imio.helpers.testing.testingvocabulary")
         # not cached for now
@@ -87,7 +90,7 @@ def test_cleanVocabularyCacheFor(self):
                           ['1', '2'])
 
         # change value but do not clean cache
-        self.portal.REQUEST.set('vocab_values', ('1', '2', '3'))
+        self.request.set('vocab_values', ('1', '2', '3'))
         self.assertEquals([term.token for term in vocab(self.portal)._terms],
                           ['1', '2'])
         # clean vocabulary cache
@@ -96,7 +99,7 @@ def test_cleanVocabularyCacheFor(self):
                           ['1', '2', '3'])
 
         # every existing vocabularies can also be cleaned if nothing passed to cleanVocabularyCacheFor
-        self.portal.REQUEST.set('vocab_values', ('1', '2', '3', '4'))
+        self.request.set('vocab_values', ('1', '2', '3', '4'))
         self.assertEquals([term.token for term in vocab(self.portal)._terms],
                           ['1', '2', '3'])
         # clean vocabulary cache
@@ -106,7 +109,7 @@ def test_cleanVocabularyCacheFor(self):
 
         # if cleanVocabularyCacheFor is called without parameter,
         # every registered vocabularies cache is cleaned
-        self.portal.REQUEST.set('vocab_values', ('1', '2', '3', '4', '5'))
+        self.request.set('vocab_values', ('1', '2', '3', '4', '5'))
         self.assertEquals([term.token for term in vocab(self.portal)._terms],
                           ['1', '2', '3', '4'])
         cleanVocabularyCacheFor()
@@ -117,10 +120,10 @@ def test_cleanRamCache(self):
         """
         This helper method invalidates all ram.cache.
         """
-        self.portal.REQUEST.set('ramcached', 'a')
+        self.request.set('ramcached', 'a')
         self.assertEquals(ramCachedMethod(self.portal, param='1'), 'a')
         # change value in REQUEST, as it is ram cached, it will still return 'a'
-        self.portal.REQUEST.set('ramcached', 'b')
+        self.request.set('ramcached', 'b')
         self.assertEquals(ramCachedMethod(self.portal, param='1'), 'a')
         # ram.cache works as expected if param changes
         self.assertEquals(ramCachedMethod(self.portal, param='2'), 'b')
@@ -134,10 +137,10 @@ def test_cleanRamCacheFor(self):
         """
         This helper method invalidates ram.cache for given method.
         """
-        self.portal.REQUEST.set('ramcached', 'a')
+        self.request.set('ramcached', 'a')
         self.assertEquals(ramCachedMethod(self.portal, param='1'), 'a')
         # change value in REQUEST, as it is ram cached, it will still return 'a'
-        self.portal.REQUEST.set('ramcached', 'b')
+        self.request.set('ramcached', 'b')
         self.assertEquals(ramCachedMethod(self.portal, param='1'), 'a')
         # ram.cache works as expected if param changes
         self.assertEquals(ramCachedMethod(self.portal, param='2'), 'b')
@@ -255,9 +258,9 @@ def bar(self):
     def test_volatile_cache_without_parameters(self):
         """Helper cache @volatile_cache_without_parameters"""
         generate_key(volatile_without_parameters_cached)
-        self.portal.REQUEST.set('volatile_without_parameters_cached', 'a')
+        self.request.set('volatile_without_parameters_cached', 'a')
         self.assertEqual('a', volatile_without_parameters_cached(self.portal))
-        self.portal.REQUEST.set('volatile_without_parameters_cached', 'b')
+        self.request.set('volatile_without_parameters_cached', 'b')
         self.assertEqual('a', volatile_without_parameters_cached(self.portal))
         # Test invalidation
         invalidate_cachekey_volatile_for(
@@ -267,12 +270,12 @@ def test_volatile_cache_without_parameters(self):
 
     def test_volatile_cache_with_parameters(self):
         """Helper cache @volatile_cache_with_parameters"""
-        self.portal.REQUEST.set('volatile_with_parameters_cached', 'a')
+        self.request.set('volatile_with_parameters_cached', 'a')
         self.assertEqual(
             'a',
             volatile_with_parameters_cached(self.portal, 'a'),
         )
-        self.portal.REQUEST.set('volatile_with_parameters_cached', 'b')
+        self.request.set('volatile_with_parameters_cached', 'b')
         self.assertEqual(
             'a',
             volatile_with_parameters_cached(self.portal, 'a'),
@@ -281,7 +284,7 @@ def test_volatile_cache_with_parameters(self):
             'b',
             volatile_with_parameters_cached(self.portal, 'b'),
         )
-        self.portal.REQUEST.set('volatile_with_parameters_cached', 'c')
+        self.request.set('volatile_with_parameters_cached', 'c')
         self.assertEqual(
             'b',
             volatile_with_parameters_cached(self.portal, 'b'),
@@ -299,6 +302,37 @@ def test_volatile_cache_with_parameters(self):
             volatile_with_parameters_cached(self.portal, 'b'),
         )
 
+    def test_obj_modified(self):
+        """ """
+        # init annotations
+        folder = self.portal.folder
+        ann = IAnnotations(folder)
+        ann['test'] = 0
+        transaction.commit()
+        ann = folder.__annotations__
+        modified = obj_modified(folder)
+        # changed when modified
+        folder.notifyModified()
+        modified2 = obj_modified(folder)
+        self.assertNotEqual(modified, modified2)
+        # changed when annotation changed
+        ann['test'] = 1
+        transaction.commit()
+        modified3 = obj_modified(folder)
+        self.assertNotEqual(modified2, modified3)
+        # also working when stored annotation is a dict and something changed into it
+        ann['test2'] = PersistentMapping({'key': 'value1'})
+        transaction.commit()
+        modified4 = obj_modified(folder)
+        self.assertNotEqual(modified3, modified4)
+        ann['test2']['key'] = 'value2'
+        transaction.commit()
+        modified5 = obj_modified(folder)
+        self.assertNotEqual(modified4, modified5)
+        # by default result is a datetime, can be float
+        self.assertEqual(type(obj_modified(folder)), datetime)
+        self.assertEqual(type(obj_modified(folder, asdatetime=False)), float)
+
 
 class TestCachedMethods(IntegrationTestCase):
     """

src/imio/helpers/tests/test_fancytree.py

@@ -32,32 +32,32 @@ class TestBaseRenderFancyTree(IntegrationTestCase):
     """Test BaseRenderFancyTree view."""
 
     def test_label(self):
-        view = BaseRenderFancyTree(self.portal, self.portal.REQUEST)
+        view = BaseRenderFancyTree(self.portal, self.request)
         self.assertEqual(view.label(), 'Plone site')
 
     def test_redirect_url(self):
-        view = BaseRenderFancyTree(self.portal, self.portal.REQUEST)
+        view = BaseRenderFancyTree(self.portal, self.request)
         with self.assertRaises(NotImplementedError):
             view.redirect_url('some_uid')
 
     def test_get_query(self):
-        view = BaseRenderFancyTree(self.portal, self.portal.REQUEST)
+        view = BaseRenderFancyTree(self.portal, self.request)
         with self.assertRaises(NotImplementedError):
             view.get_query()
 
     def test_call(self):
-        view = RenderFancyTreeExample(self.portal, self.portal.REQUEST)
+        view = RenderFancyTreeExample(self.portal, self.request)
         self.assertEqual(view(), "index")
 
-        request = self.portal.REQUEST
+        request = self.request
         request.method = 'POST'
         request['uid'] = 'myuid'
         self.assertEqual(view(), '')
         self.assertEqual(view.request.response.headers['location'], 'myuid')
 
     def test_get_data(self):
         """Test get_data method."""
-        view = RenderFancyTreeExample(self.portal, self.portal.REQUEST)
+        view = RenderFancyTreeExample(self.portal, self.request)
         self.assertEqual(view.get_data(), '[]')
 
         # add some folders and documents