Merge pull request #35453 from theothergraham/fix_CacheDisk

fixes #34279 - disk cache ttl expiry

salt/utils/cache.py

@@ -89,7 +89,18 @@ class CacheDisk(CacheDict):
     def __init__(self, ttl, path, *args, **kwargs):
         super(CacheDisk, self).__init__(ttl, *args, **kwargs)
         self._path = path
-        self._dict = self._read()
+        self._dict = {}
+        self._read()
+
+    def _enforce_ttl_key(self, key):
+        '''
+        Enforce the TTL to a specific key, delete if its past TTL
+        '''
+        if key not in self._key_cache_time:
+            return
+        if time.time() - self._key_cache_time[key] > self._ttl:
+            del self._key_cache_time[key]
+            self._dict.__delitem__(key)
 
     def __contains__(self, key):
         self._enforce_ttl_key(key)
@@ -111,16 +122,33 @@ class CacheDisk(CacheDict):
         # Do the same as the parent but also persist
         self._write()
 
+    def __delitem__(self, key):
+        '''
+        Make sure to remove the key cache time
+        '''
+        del self._key_cache_time[key]
+        self._dict.__delitem__(key)
+        # Do the same as the parent but also persist
+        self._write()
+
     def _read(self):
         '''
         Read in from disk
         '''
         if not HAS_MSGPACK or not os.path.exists(self._path):
-            return {}
+            return
         with salt.utils.fopen(self._path, 'r') as fp_:
             cache = msgpack.load(fp_)
-        log.debug('Disk cache retrieved: {0}'.format(cache))
-        return cache
+        if "CacheDisk_cachetime" in cache:  # new format
+            self._dict = cache["CacheDisk_data"]
+            self._key_cache_time = cache["CacheDisk_cachetime"]
+        else:  # old format
+            self._dict = cache
+            timestamp = os.path.getmtime(self._path)
+            for key in self._dict.keys():
+                self._key_cache_time[key] = timestamp
+        if log.isEnabledFor(logging.DEBUG):
+            log.debug('Disk cache retrieved: {0}'.format(cache))
 
     def _write(self):
         '''
@@ -131,7 +159,11 @@ class CacheDisk(CacheDict):
         # TODO Add check into preflight to ensure dir exists
         # TODO Dir hashing?
         with salt.utils.fopen(self._path, 'w+') as fp_:
-            msgpack.dump(self._dict, fp_)
+            cache = {
+                "CacheDisk_data": self._dict,
+                "CacheDisk_cachetime": self._key_cache_time
+            }
+            msgpack.dump(cache, fp_)
 
 
 class CacheCli(object):

tests/unit/utils/disk_cache_test.py

@@ -0,0 +1,64 @@
+# -*- coding: utf-8 -*-
+'''
+    tests.unit.utils.disk_cache_test
+    ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+    Test the salt disk cache objects
+'''
+
+# Import python libs
+from __future__ import absolute_import
+import os.path
+import shutil
+import tempfile
+import time
+
+# Import Salt Testing libs
+from salttesting import TestCase
+from salttesting.helpers import ensure_in_syspath
+ensure_in_syspath('../../')
+
+# Import salt libs
+from salt.utils import cache
+
+
+class CacheDiskTestCase(TestCase):
+
+    def test_everything(self):
+        '''
+        Make sure you can instantiate, add, update, remove, expire
+        '''
+        try:
+            tmpdir = tempfile.mkdtemp()
+            path = os.path.join(tmpdir, 'CacheDisk_test')
+
+            # test instantiation
+            cd = cache.CacheDisk(0.1, path)
+            self.assertIsInstance(cd, cache.CacheDisk)
+
+            # test to make sure it looks like a dict
+            self.assertNotIn('foo', cd)
+            cd['foo'] = 'bar'
+            self.assertIn('foo', cd)
+            self.assertEqual(cd['foo'], 'bar')
+            del cd['foo']
+            self.assertNotIn('foo', cd)
+
+            # test persistence
+            cd['foo'] = 'bar'
+            cd2 = cache.CacheDisk(0.1, path)
+            self.assertIn('foo', cd2)
+            self.assertEqual(cd2['foo'], 'bar')
+
+            # test ttl
+            time.sleep(0.2)
+            self.assertNotIn('foo', cd)
+            self.assertNotIn('foo', cd2)
+
+        finally:
+            shutil.rmtree(tmpdir, ignore_errors=True)
+
+
+if __name__ == '__main__':
+    from integration import run_tests
+    run_tests(CacheDiskTestCase, needs_daemon=False)