From 89955cc35f3636684ea6f2a6c9504b35a3050f0f Mon Sep 17 00:00:00 2001 From: Jacob Burch Date: Sat, 18 May 2013 12:54:59 +0200 Subject: [PATCH] Fixed #9595 -- Allow non-expiring cache timeouts. Also, streamline the use of 0 and None between cache backends. --- AUTHORS | 1 + django/core/cache/backends/base.py | 10 ++++++--- django/core/cache/backends/db.py | 15 +++++++------ django/core/cache/backends/dummy.py | 8 +++---- django/core/cache/backends/filebased.py | 15 ++++++------- django/core/cache/backends/locmem.py | 19 ++++++++--------- django/core/cache/backends/memcached.py | 22 +++++++++++++------ docs/releases/1.6.txt | 6 ++++++ docs/topics/cache.txt | 13 ++++++++---- tests/cache/tests.py | 28 +++++++++++++++++++++++++ 10 files changed, 97 insertions(+), 40 deletions(-) diff --git a/AUTHORS b/AUTHORS index 70f8bb22d907a..0ad65227253d7 100644 --- a/AUTHORS +++ b/AUTHORS @@ -123,6 +123,7 @@ answer newbie questions, and generally made Django that much better: bthomas btoll@bestweb.net Jonathan Buchanan + Jacob Burch Keith Bussell C8E Chris Cahoon diff --git a/django/core/cache/backends/base.py b/django/core/cache/backends/base.py index 53b0270a57748..deb98e7714e9a 100644 --- a/django/core/cache/backends/base.py +++ b/django/core/cache/backends/base.py @@ -15,6 +15,10 @@ class CacheKeyWarning(DjangoRuntimeWarning): pass +# Stub class to ensure not passing in a `timeout` argument results in +# the default timeout +DEFAULT_TIMEOUT = object() + # Memcached does not accept keys longer than this. MEMCACHE_MAX_KEY_LENGTH = 250 @@ -84,7 +88,7 @@ def make_key(self, key, version=None): new_key = self.key_func(key, self.key_prefix, version) return new_key - def add(self, key, value, timeout=None, version=None): + def add(self, key, value, timeout=DEFAULT_TIMEOUT, version=None): """ Set a value in the cache if the key does not already exist. If timeout is given, that timeout will be used for the key; otherwise @@ -101,7 +105,7 @@ def get(self, key, default=None, version=None): """ raise NotImplementedError - def set(self, key, value, timeout=None, version=None): + def set(self, key, value, timeout=DEFAULT_TIMEOUT, version=None): """ Set a value in the cache. If timeout is given, that timeout will be used for the key; otherwise the default cache timeout will be used. @@ -163,7 +167,7 @@ def __contains__(self, key): # if a subclass overrides it. return self.has_key(key) - def set_many(self, data, timeout=None, version=None): + def set_many(self, data, timeout=DEFAULT_TIMEOUT, version=None): """ Set a bunch of values in the cache at once from a dict of key/value pairs. For certain backends (memcached), this is much more efficient diff --git a/django/core/cache/backends/db.py b/django/core/cache/backends/db.py index 77492841228f7..0e59c6d65efc2 100644 --- a/django/core/cache/backends/db.py +++ b/django/core/cache/backends/db.py @@ -9,7 +9,7 @@ import pickle from django.conf import settings -from django.core.cache.backends.base import BaseCache +from django.core.cache.backends.base import BaseCache, DEFAULT_TIMEOUT from django.db import connections, transaction, router, DatabaseError from django.utils import timezone, six from django.utils.encoding import force_bytes @@ -65,6 +65,7 @@ def get(self, key, default=None, version=None): if row is None: return default now = timezone.now() + if row[2] < now: db = router.db_for_write(self.cache_model_class) cursor = connections[db].cursor() @@ -74,18 +75,18 @@ def get(self, key, default=None, version=None): value = connections[db].ops.process_clob(row[1]) return pickle.loads(base64.b64decode(force_bytes(value))) - def set(self, key, value, timeout=None, version=None): + def set(self, key, value, timeout=DEFAULT_TIMEOUT, version=None): key = self.make_key(key, version=version) self.validate_key(key) self._base_set('set', key, value, timeout) - def add(self, key, value, timeout=None, version=None): + def add(self, key, value, timeout=DEFAULT_TIMEOUT, version=None): key = self.make_key(key, version=version) self.validate_key(key) return self._base_set('add', key, value, timeout) - def _base_set(self, mode, key, value, timeout=None): - if timeout is None: + def _base_set(self, mode, key, value, timeout=DEFAULT_TIMEOUT): + if timeout == DEFAULT_TIMEOUT: timeout = self.default_timeout db = router.db_for_write(self.cache_model_class) table = connections[db].ops.quote_name(self._table) @@ -95,7 +96,9 @@ def _base_set(self, mode, key, value, timeout=None): num = cursor.fetchone()[0] now = timezone.now() now = now.replace(microsecond=0) - if settings.USE_TZ: + if timeout is None: + exp = datetime.max + elif settings.USE_TZ: exp = datetime.utcfromtimestamp(time.time() + timeout) else: exp = datetime.fromtimestamp(time.time() + timeout) diff --git a/django/core/cache/backends/dummy.py b/django/core/cache/backends/dummy.py index 9fe9b3b5f51f4..7ca6114ee474d 100644 --- a/django/core/cache/backends/dummy.py +++ b/django/core/cache/backends/dummy.py @@ -1,12 +1,12 @@ "Dummy cache backend" -from django.core.cache.backends.base import BaseCache +from django.core.cache.backends.base import BaseCache, DEFAULT_TIMEOUT class DummyCache(BaseCache): def __init__(self, host, *args, **kwargs): BaseCache.__init__(self, *args, **kwargs) - def add(self, key, value, timeout=None, version=None): + def add(self, key, value, timeout=DEFAULT_TIMEOUT, version=None): key = self.make_key(key, version=version) self.validate_key(key) return True @@ -16,7 +16,7 @@ def get(self, key, default=None, version=None): self.validate_key(key) return default - def set(self, key, value, timeout=None, version=None): + def set(self, key, value, timeout=DEFAULT_TIMEOUT, version=None): key = self.make_key(key, version=version) self.validate_key(key) @@ -32,7 +32,7 @@ def has_key(self, key, version=None): self.validate_key(key) return False - def set_many(self, data, timeout=0, version=None): + def set_many(self, data, timeout=DEFAULT_TIMEOUT, version=None): pass def delete_many(self, keys, version=None): diff --git a/django/core/cache/backends/filebased.py b/django/core/cache/backends/filebased.py index 96194d458fda9..d19eed4a95052 100644 --- a/django/core/cache/backends/filebased.py +++ b/django/core/cache/backends/filebased.py @@ -9,9 +9,10 @@ except ImportError: import pickle -from django.core.cache.backends.base import BaseCache +from django.core.cache.backends.base import BaseCache, DEFAULT_TIMEOUT from django.utils.encoding import force_bytes + class FileBasedCache(BaseCache): def __init__(self, dir, params): BaseCache.__init__(self, params) @@ -19,7 +20,7 @@ def __init__(self, dir, params): if not os.path.exists(self._dir): self._createdir() - def add(self, key, value, timeout=None, version=None): + def add(self, key, value, timeout=DEFAULT_TIMEOUT, version=None): if self.has_key(key, version=version): return False @@ -35,7 +36,7 @@ def get(self, key, default=None, version=None): with open(fname, 'rb') as f: exp = pickle.load(f) now = time.time() - if exp < now: + if exp is not None and exp < now: self._delete(fname) else: return pickle.load(f) @@ -43,14 +44,14 @@ def get(self, key, default=None, version=None): pass return default - def set(self, key, value, timeout=None, version=None): + def set(self, key, value, timeout=DEFAULT_TIMEOUT, version=None): key = self.make_key(key, version=version) self.validate_key(key) fname = self._key_to_file(key) dirname = os.path.dirname(fname) - if timeout is None: + if timeout == DEFAULT_TIMEOUT: timeout = self.default_timeout self._cull() @@ -60,8 +61,8 @@ def set(self, key, value, timeout=None, version=None): os.makedirs(dirname) with open(fname, 'wb') as f: - now = time.time() - pickle.dump(now + timeout, f, pickle.HIGHEST_PROTOCOL) + expiry = None if timeout is None else time.time() + timeout + pickle.dump(expiry, f, pickle.HIGHEST_PROTOCOL) pickle.dump(value, f, pickle.HIGHEST_PROTOCOL) except (IOError, OSError): pass diff --git a/django/core/cache/backends/locmem.py b/django/core/cache/backends/locmem.py index 76667e9609474..1fa17052fd826 100644 --- a/django/core/cache/backends/locmem.py +++ b/django/core/cache/backends/locmem.py @@ -6,7 +6,7 @@ except ImportError: import pickle -from django.core.cache.backends.base import BaseCache +from django.core.cache.backends.base import BaseCache, DEFAULT_TIMEOUT from django.utils.synch import RWLock # Global in-memory store of cache data. Keyed by name, to provide @@ -23,7 +23,7 @@ def __init__(self, name, params): self._expire_info = _expire_info.setdefault(name, {}) self._lock = _locks.setdefault(name, RWLock()) - def add(self, key, value, timeout=None, version=None): + def add(self, key, value, timeout=DEFAULT_TIMEOUT, version=None): key = self.make_key(key, version=version) self.validate_key(key) with self._lock.writer(): @@ -41,10 +41,8 @@ def get(self, key, default=None, version=None): key = self.make_key(key, version=version) self.validate_key(key) with self._lock.reader(): - exp = self._expire_info.get(key) - if exp is None: - return default - elif exp > time.time(): + exp = self._expire_info.get(key, 0) + if exp is None or exp > time.time(): try: pickled = self._cache[key] return pickle.loads(pickled) @@ -58,15 +56,16 @@ def get(self, key, default=None, version=None): pass return default - def _set(self, key, value, timeout=None): + def _set(self, key, value, timeout=DEFAULT_TIMEOUT): if len(self._cache) >= self._max_entries: self._cull() - if timeout is None: + if timeout == DEFAULT_TIMEOUT: timeout = self.default_timeout + expiry = None if timeout is None else time.time() + timeout self._cache[key] = value - self._expire_info[key] = time.time() + timeout + self._expire_info[key] = expiry - def set(self, key, value, timeout=None, version=None): + def set(self, key, value, timeout=DEFAULT_TIMEOUT, version=None): key = self.make_key(key, version=version) self.validate_key(key) with self._lock.writer(): diff --git a/django/core/cache/backends/memcached.py b/django/core/cache/backends/memcached.py index c942acd52feb0..64d1c41dc5e88 100644 --- a/django/core/cache/backends/memcached.py +++ b/django/core/cache/backends/memcached.py @@ -4,7 +4,7 @@ import pickle from threading import local -from django.core.cache.backends.base import BaseCache, InvalidCacheBackendError +from django.core.cache.backends.base import BaseCache, DEFAULT_TIMEOUT from django.utils import six from django.utils.encoding import force_str @@ -36,12 +36,22 @@ def _cache(self): return self._client - def _get_memcache_timeout(self, timeout): + def _get_memcache_timeout(self, timeout=DEFAULT_TIMEOUT): """ Memcached deals with long (> 30 days) timeouts in a special way. Call this function to obtain a safe value for your timeout. """ - timeout = timeout or self.default_timeout + if timeout == DEFAULT_TIMEOUT: + return self.default_timeout + + if timeout is None: + # Using 0 in memcache sets a non-expiring timeout. + return 0 + elif int(timeout) == 0: + # Other cache backends treat 0 as set-and-expire. To achieve this + # in memcache backends, a negative timeout must be passed. + timeout = -1 + if timeout > 2592000: # 60*60*24*30, 30 days # See http://code.google.com/p/memcached/wiki/FAQ # "You can set expire times up to 30 days in the future. After that @@ -56,7 +66,7 @@ def make_key(self, key, version=None): # Python 2 memcache requires the key to be a byte string. return force_str(super(BaseMemcachedCache, self).make_key(key, version)) - def add(self, key, value, timeout=0, version=None): + def add(self, key, value, timeout=DEFAULT_TIMEOUT, version=None): key = self.make_key(key, version=version) return self._cache.add(key, value, self._get_memcache_timeout(timeout)) @@ -67,7 +77,7 @@ def get(self, key, default=None, version=None): return default return val - def set(self, key, value, timeout=0, version=None): + def set(self, key, value, timeout=DEFAULT_TIMEOUT, version=None): key = self.make_key(key, version=version) self._cache.set(key, value, self._get_memcache_timeout(timeout)) @@ -125,7 +135,7 @@ def decr(self, key, delta=1, version=None): raise ValueError("Key '%s' not found" % key) return val - def set_many(self, data, timeout=0, version=None): + def set_many(self, data, timeout=DEFAULT_TIMEOUT, version=None): safe_data = {} for key, value in data.items(): key = self.make_key(key, version=version) diff --git a/docs/releases/1.6.txt b/docs/releases/1.6.txt index c9b3715c45161..60b3381dd6320 100644 --- a/docs/releases/1.6.txt +++ b/docs/releases/1.6.txt @@ -485,6 +485,12 @@ Miscellaneous changes in 1.6 particularly affect :class:`~django.forms.DecimalField` and :class:`~django.forms.ModelMultipleChoiceField`. +* There have been changes in the way timeouts are handled in cache backends. + Explicitly passing in ``timeout=None`` no longer results in using the + default timeout. It will now set a non-expiring timeout. Passing 0 into the + memcache backend no longer uses the default timeout, and now will + set-and-expire-immediately the value. + Features deprecated in 1.6 ========================== diff --git a/docs/topics/cache.txt b/docs/topics/cache.txt index 6b6d57511a328..a7d54fbeb0a54 100644 --- a/docs/topics/cache.txt +++ b/docs/topics/cache.txt @@ -707,10 +707,15 @@ The basic interface is ``set(key, value, timeout)`` and ``get(key)``:: >>> cache.get('my_key') 'hello, world!' -The ``timeout`` argument is optional and defaults to the ``timeout`` -argument of the appropriate backend in the :setting:`CACHES` setting -(explained above). It's the number of seconds the value should be stored -in the cache. +The ``timeout`` argument is optional and defaults to the ``timeout`` argument +of the appropriate backend in the :setting:`CACHES` setting (explained above). +It's the number of seconds the value should be stored in the cache. Passing in +``None`` for ``timeout`` will cache the value forever. + +.. versionchanged:: 1.6 + + Previously, passing ``None`` explicitly would use the default timeout + value. If the object doesn't exist in the cache, ``cache.get()`` returns ``None``:: diff --git a/tests/cache/tests.py b/tests/cache/tests.py index 00c51638b7220..4f7ee8b52553f 100644 --- a/tests/cache/tests.py +++ b/tests/cache/tests.py @@ -441,6 +441,34 @@ def test_long_timeout(self): self.assertEqual(self.cache.get('key3'), 'sausage') self.assertEqual(self.cache.get('key4'), 'lobster bisque') + def test_forever_timeout(self): + ''' + Passing in None into timeout results in a value that is cached forever + ''' + self.cache.set('key1', 'eggs', None) + self.assertEqual(self.cache.get('key1'), 'eggs') + + self.cache.add('key2', 'ham', None) + self.assertEqual(self.cache.get('key2'), 'ham') + + self.cache.set_many({'key3': 'sausage', 'key4': 'lobster bisque'}, None) + self.assertEqual(self.cache.get('key3'), 'sausage') + self.assertEqual(self.cache.get('key4'), 'lobster bisque') + + def test_zero_timeout(self): + ''' + Passing in None into timeout results in a value that is cached forever + ''' + self.cache.set('key1', 'eggs', 0) + self.assertEqual(self.cache.get('key1'), None) + + self.cache.add('key2', 'ham', 0) + self.assertEqual(self.cache.get('key2'), None) + + self.cache.set_many({'key3': 'sausage', 'key4': 'lobster bisque'}, 0) + self.assertEqual(self.cache.get('key3'), None) + self.assertEqual(self.cache.get('key4'), None) + def test_float_timeout(self): # Make sure a timeout given as a float doesn't crash anything. self.cache.set("key1", "spam", 100.2)