Skip to content

Commit

Permalink
Fixed #9595 -- Allow non-expiring cache timeouts.
Browse files Browse the repository at this point in the history
Also, streamline the use of 0 and None between cache backends.
  • Loading branch information
jacobb authored and aaugustin committed May 18, 2013
1 parent e0df647 commit 89955cc
Show file tree
Hide file tree
Showing 10 changed files with 97 additions and 40 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Expand Up @@ -123,6 +123,7 @@ answer newbie questions, and generally made Django that much better:
bthomas
btoll@bestweb.net
Jonathan Buchanan <jonathan.buchanan@gmail.com>
Jacob Burch <jacobburch@gmail.com>
Keith Bussell <kbussell@gmail.com>
C8E
Chris Cahoon <chris.cahoon@gmail.com>
Expand Down
10 changes: 7 additions & 3 deletions django/core/cache/backends/base.py
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down
15 changes: 9 additions & 6 deletions django/core/cache/backends/db.py
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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)
Expand All @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions 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
Expand All @@ -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)

Expand All @@ -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):
Expand Down
15 changes: 8 additions & 7 deletions django/core/cache/backends/filebased.py
Expand Up @@ -9,17 +9,18 @@
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)
self._dir = dir
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

Expand All @@ -35,22 +36,22 @@ 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)
except (IOError, OSError, EOFError, pickle.PickleError):
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()
Expand All @@ -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
Expand Down
19 changes: 9 additions & 10 deletions django/core/cache/backends/locmem.py
Expand Up @@ -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
Expand All @@ -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():
Expand All @@ -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)
Expand All @@ -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():
Expand Down
22 changes: 16 additions & 6 deletions django/core/cache/backends/memcached.py
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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))

Expand All @@ -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))

Expand Down Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions docs/releases/1.6.txt
Expand Up @@ -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
==========================

Expand Down
13 changes: 9 additions & 4 deletions docs/topics/cache.txt
Expand Up @@ -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``::

Expand Down
28 changes: 28 additions & 0 deletions tests/cache/tests.py
Expand Up @@ -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)
Expand Down

0 comments on commit 89955cc

Please sign in to comment.