Author: mtredinnick
Date: 2010-09-12 14:07:09 -0500 (Sun, 12 Sep 2010)
New Revision: 13767
Added:
django/branches/releases/1.2.X/tests/regressiontests/cache/liberal_backend.py
Modified:
django/branches/releases/1.2.X/django/core/cache/__init__.py
django/branches/releases/1.2.X/django/core/cache/backends/base.py
django/branches/releases/1.2.X/django/core/cache/backends/db.py
django/branches/releases/1.2.X/django/core/cache/backends/dummy.py
django/branches/releases/1.2.X/django/core/cache/backends/filebased.py
django/branches/releases/1.2.X/django/core/cache/backends/locmem.py
django/branches/releases/1.2.X/django/core/exceptions.py
django/branches/releases/1.2.X/docs/topics/cache.txt
django/branches/releases/1.2.X/tests/regressiontests/cache/tests.py
Log:
[1.2.X] Add warning when using cache keys that might not work with
memcached.
This means testing with local dev caches (not memcache) will warn
developers if they are introducing inadvertent importabilities. There
is also the ability to silence the warning if a dev is not planning to
use memcache and knows what they are doing with their keys.
Thanks to Carl Meyer for the patch. Fixed #6447.
Backport of r13766 from trunk.
Modified: django/branches/releases/1.2.X/django/core/cache/__init__.py
===================================================================
--- django/branches/releases/1.2.X/django/core/cache/__init__.py
2010-09-12 18:45:26 UTC (rev 13766)
+++ django/branches/releases/1.2.X/django/core/cache/__init__.py
2010-09-12 19:07:09 UTC (rev 13767)
@@ -18,7 +18,7 @@
from cgi import parse_qsl
from django.conf import settings
from django.core import signals
-from django.core.cache.backends.base import InvalidCacheBackendError
+from django.core.cache.backends.base import InvalidCacheBackendError,
CacheKeyWarning
from django.utils import importlib
# Name for use in settings file --> name of module in "backends" directory.
Modified: django/branches/releases/1.2.X/django/core/cache/backends/base.py
===================================================================
--- django/branches/releases/1.2.X/django/core/cache/backends/base.py
2010-09-12 18:45:26 UTC (rev 13766)
+++ django/branches/releases/1.2.X/django/core/cache/backends/base.py
2010-09-12 19:07:09 UTC (rev 13767)
@@ -1,10 +1,18 @@
"Base Cache class."
-from django.core.exceptions import ImproperlyConfigured
+import warnings
+from django.core.exceptions import ImproperlyConfigured, DjangoRuntimeWarning
+
class InvalidCacheBackendError(ImproperlyConfigured):
pass
+class CacheKeyWarning(DjangoRuntimeWarning):
+ pass
+
+# Memcached does not accept keys longer than this.
+MEMCACHE_MAX_KEY_LENGTH = 250
+
class BaseCache(object):
def __init__(self, params):
timeout = params.get('timeout', 300)
@@ -116,3 +124,21 @@
def clear(self):
"""Remove *all* values from the cache at once."""
raise NotImplementedError
+
+ def validate_key(self, key):
+ """
+ Warn about keys that would not be portable to the memcached
+ backend. This encourages (but does not force) writing backend-portable
+ cache code.
+
+ """
+ if len(key) > MEMCACHE_MAX_KEY_LENGTH:
+ warnings.warn('Cache key will cause errors if used with memcached:
'
+ '%s (longer than %s)' % (key, MEMCACHE_MAX_KEY_LENGTH),
+ CacheKeyWarning)
+ for char in key:
+ if ord(char) < 33 or ord(char) == 127:
+ warnings.warn('Cache key contains characters that will cause '
+ 'errors if used with memcached: %r' % key,
+ CacheKeyWarning)
+
Modified: django/branches/releases/1.2.X/django/core/cache/backends/db.py
===================================================================
--- django/branches/releases/1.2.X/django/core/cache/backends/db.py
2010-09-12 18:45:26 UTC (rev 13766)
+++ django/branches/releases/1.2.X/django/core/cache/backends/db.py
2010-09-12 19:07:09 UTC (rev 13767)
@@ -46,6 +46,7 @@
self._cull_frequency = 3
def get(self, key, default=None):
+ self.validate_key(key)
db = router.db_for_read(self.cache_model_class)
table = connections[db].ops.quote_name(self._table)
cursor = connections[db].cursor()
@@ -65,9 +66,11 @@
return pickle.loads(base64.decodestring(value))
def set(self, key, value, timeout=None):
+ self.validate_key(key)
self._base_set('set', key, value, timeout)
def add(self, key, value, timeout=None):
+ self.validate_key(key)
return self._base_set('add', key, value, timeout)
def _base_set(self, mode, key, value, timeout=None):
@@ -103,6 +106,7 @@
return True
def delete(self, key):
+ self.validate_key(key)
db = router.db_for_write(self.cache_model_class)
table = connections[db].ops.quote_name(self._table)
cursor = connections[db].cursor()
@@ -111,6 +115,7 @@
transaction.commit_unless_managed(using=db)
def has_key(self, key):
+ self.validate_key(key)
db = router.db_for_read(self.cache_model_class)
table = connections[db].ops.quote_name(self._table)
cursor = connections[db].cursor()
Modified: django/branches/releases/1.2.X/django/core/cache/backends/dummy.py
===================================================================
--- django/branches/releases/1.2.X/django/core/cache/backends/dummy.py
2010-09-12 18:45:26 UTC (rev 13766)
+++ django/branches/releases/1.2.X/django/core/cache/backends/dummy.py
2010-09-12 19:07:09 UTC (rev 13767)
@@ -6,22 +6,25 @@
def __init__(self, *args, **kwargs):
pass
- def add(self, *args, **kwargs):
+ def add(self, key, *args, **kwargs):
+ self.validate_key(key)
return True
def get(self, key, default=None):
+ self.validate_key(key)
return default
- def set(self, *args, **kwargs):
- pass
+ def set(self, key, *args, **kwargs):
+ self.validate_key(key)
- def delete(self, *args, **kwargs):
- pass
+ def delete(self, key, *args, **kwargs):
+ self.validate_key(key)
def get_many(self, *args, **kwargs):
return {}
- def has_key(self, *args, **kwargs):
+ def has_key(self, key, *args, **kwargs):
+ self.validate_key(key)
return False
def set_many(self, *args, **kwargs):
Modified: django/branches/releases/1.2.X/django/core/cache/backends/filebased.py
===================================================================
--- django/branches/releases/1.2.X/django/core/cache/backends/filebased.py
2010-09-12 18:45:26 UTC (rev 13766)
+++ django/branches/releases/1.2.X/django/core/cache/backends/filebased.py
2010-09-12 19:07:09 UTC (rev 13767)
@@ -32,6 +32,7 @@
self._createdir()
def add(self, key, value, timeout=None):
+ self.validate_key(key)
if self.has_key(key):
return False
@@ -39,6 +40,7 @@
return True
def get(self, key, default=None):
+ self.validate_key(key)
fname = self._key_to_file(key)
try:
f = open(fname, 'rb')
@@ -56,6 +58,7 @@
return default
def set(self, key, value, timeout=None):
+ self.validate_key(key)
fname = self._key_to_file(key)
dirname = os.path.dirname(fname)
@@ -79,6 +82,7 @@
pass
def delete(self, key):
+ self.validate_key(key)
try:
self._delete(self._key_to_file(key))
except (IOError, OSError):
@@ -95,6 +99,7 @@
pass
def has_key(self, key):
+ self.validate_key(key)
fname = self._key_to_file(key)
try:
f = open(fname, 'rb')
Modified: django/branches/releases/1.2.X/django/core/cache/backends/locmem.py
===================================================================
--- django/branches/releases/1.2.X/django/core/cache/backends/locmem.py
2010-09-12 18:45:26 UTC (rev 13766)
+++ django/branches/releases/1.2.X/django/core/cache/backends/locmem.py
2010-09-12 19:07:09 UTC (rev 13767)
@@ -30,6 +30,7 @@
self._lock = RWLock()
def add(self, key, value, timeout=None):
+ self.validate_key(key)
self._lock.writer_enters()
try:
exp = self._expire_info.get(key)
@@ -44,6 +45,7 @@
self._lock.writer_leaves()
def get(self, key, default=None):
+ self.validate_key(key)
self._lock.reader_enters()
try:
exp = self._expire_info.get(key)
@@ -76,6 +78,7 @@
self._expire_info[key] = time.time() + timeout
def set(self, key, value, timeout=None):
+ self.validate_key(key)
self._lock.writer_enters()
# Python 2.4 doesn't allow combined try-except-finally blocks.
try:
@@ -87,6 +90,7 @@
self._lock.writer_leaves()
def has_key(self, key):
+ self.validate_key(key)
self._lock.reader_enters()
try:
exp = self._expire_info.get(key)
@@ -127,6 +131,7 @@
pass
def delete(self, key):
+ self.validate_key(key)
self._lock.writer_enters()
try:
self._delete(key)
Modified: django/branches/releases/1.2.X/django/core/exceptions.py
===================================================================
--- django/branches/releases/1.2.X/django/core/exceptions.py 2010-09-12
18:45:26 UTC (rev 13766)
+++ django/branches/releases/1.2.X/django/core/exceptions.py 2010-09-12
19:07:09 UTC (rev 13767)
@@ -1,5 +1,10 @@
-"Global Django exceptions"
+"""
+Global Django exception and warning classes.
+"""
+class DjangoRuntimeWarning(RuntimeWarning):
+ pass
+
class ObjectDoesNotExist(Exception):
"The requested object does not exist"
silent_variable_failure = True
Modified: django/branches/releases/1.2.X/docs/topics/cache.txt
===================================================================
--- django/branches/releases/1.2.X/docs/topics/cache.txt 2010-09-12
18:45:26 UTC (rev 13766)
+++ django/branches/releases/1.2.X/docs/topics/cache.txt 2010-09-12
19:07:09 UTC (rev 13767)
@@ -641,6 +641,45 @@
However, if the backend doesn't natively provide an increment/decrement
operation, it will be implemented using a two-step retrieve/update.
+Cache key warnings
+------------------
+
+.. versionadded:: 1.3
+
+Memcached, the most commonly-used production cache backend, does not allow
+cache keys longer than 250 characters or containing whitespace or control
+characters, and using such keys will cause an exception. To encourage
+cache-portable code and minimize unpleasant surprises, the other built-in cache
+backends issue a warning (``django.core.cache.backends.base.CacheKeyWarning``)
+if a key is used that would cause an error on memcached.
+
+If you are using a production backend that can accept a wider range of keys (a
+custom backend, or one of the non-memcached built-in backends), and want to use
+this wider range without warnings, you can silence ``CacheKeyWarning`` with
+this code in the ``management`` module of one of your
+:setting:`INSTALLED_APPS`::
+
+ import warnings
+
+ from django.core.cache import CacheKeyWarning
+
+ warnings.simplefilter("ignore", CacheKeyWarning)
+
+If you want to instead provide custom key validation logic for one of the
+built-in backends, you can subclass it, override just the ``validate_key``
+method, and follow the instructions for `using a custom cache backend`_. For
+instance, to do this for the ``locmem`` backend, put this code in a module::
+
+ from django.core.cache.backends.locmem import CacheClass as
LocMemCacheClass
+
+ class CacheClass(LocMemCacheClass):
+ def validate_key(self, key):
+ """Custom validation, raising exceptions or warnings as needed."""
+ # ...
+
+...and use the dotted Python path to this module as the scheme portion of your
+:setting:`CACHE_BACKEND`.
+
Upstream caches
===============
Added:
django/branches/releases/1.2.X/tests/regressiontests/cache/liberal_backend.py
===================================================================
---
django/branches/releases/1.2.X/tests/regressiontests/cache/liberal_backend.py
(rev 0)
+++
django/branches/releases/1.2.X/tests/regressiontests/cache/liberal_backend.py
2010-09-12 19:07:09 UTC (rev 13767)
@@ -0,0 +1,9 @@
+from django.core.cache.backends.locmem import CacheClass as LocMemCacheClass
+
+class LiberalKeyValidationMixin(object):
+ def validate_key(self, key):
+ pass
+
+class CacheClass(LiberalKeyValidationMixin, LocMemCacheClass):
+ pass
+
Modified: django/branches/releases/1.2.X/tests/regressiontests/cache/tests.py
===================================================================
--- django/branches/releases/1.2.X/tests/regressiontests/cache/tests.py
2010-09-12 18:45:26 UTC (rev 13766)
+++ django/branches/releases/1.2.X/tests/regressiontests/cache/tests.py
2010-09-12 19:07:09 UTC (rev 13767)
@@ -8,11 +8,12 @@
import tempfile
import time
import unittest
+import warnings
from django.conf import settings
from django.core import management
from django.core.cache import get_cache
-from django.core.cache.backends.base import InvalidCacheBackendError
+from django.core.cache.backends.base import InvalidCacheBackendError,
CacheKeyWarning
from django.http import HttpResponse, HttpRequest
from django.middleware.cache import FetchFromCacheMiddleware,
UpdateCacheMiddleware
from django.utils import translation
@@ -366,6 +367,33 @@
count = count + 1
self.assertEqual(count, final_count)
+ def test_invalid_keys(self):
+ """
+ All the builtin backends (except memcached, see below) should warn on
+ keys that would be refused by memcached. This encourages portable
+ caching code without making it too difficult to use production backends
+ with more liberal key rules. Refs #6447.
+
+ """
+ # On Python 2.6+ we could use the catch_warnings context
+ # manager to test this warning nicely. Since we can't do that
+ # yet, the cleanest option is to temporarily ask for
+ # CacheKeyWarning to be raised as an exception.
+ warnings.simplefilter("error", CacheKeyWarning)
+
+ # memcached does not allow whitespace or control characters in keys
+ self.assertRaises(CacheKeyWarning, self.cache.set, 'key with spaces',
'value')
+ # memcached limits key length to 250
+ self.assertRaises(CacheKeyWarning, self.cache.set, 'a' * 251, 'value')
+
+ # The warnings module has no public API for getting the
+ # current list of warning filters, so we can't save that off
+ # and reset to the previous value, we have to globally reset
+ # it. The effect will be the same, as long as the Django test
+ # runner doesn't add any global warning filters (it currently
+ # does not).
+ warnings.resetwarnings()
+
class DBCacheTests(unittest.TestCase, BaseCacheTests):
def setUp(self):
# Spaces are used in the table name to ensure quoting/escaping is
working
@@ -397,6 +425,22 @@
def setUp(self):
self.cache = get_cache(settings.CACHE_BACKEND)
+ def test_invalid_keys(self):
+ """
+ On memcached, we don't introduce a duplicate key validation
+ step (for speed reasons), we just let the memcached API
+ library raise its own exception on bad keys. Refs #6447.
+
+ In order to be memcached-API-library agnostic, we only assert
+ that a generic exception of some kind is raised.
+
+ """
+ # memcached does not allow whitespace or control characters in keys
+ self.assertRaises(Exception, self.cache.set, 'key with spaces',
'value')
+ # memcached limits key length to 250
+ self.assertRaises(Exception, self.cache.set, 'a' * 251, 'value')
+
+
class FileBasedCacheTests(unittest.TestCase, BaseCacheTests):
"""
Specific test cases for the file-based cache.
@@ -429,6 +473,22 @@
def test_cull(self):
self.perform_cull_test(50, 28)
+class CustomCacheKeyValidationTests(unittest.TestCase):
+ """
+ Tests for the ability to mixin a custom ``validate_key`` method to
+ a custom cache backend that otherwise inherits from a builtin
+ backend, and override the default key validation. Refs #6447.
+
+ """
+ def test_custom_key_validation(self):
+ cache = get_cache('regressiontests.cache.liberal_backend://')
+
+ # this key is both longer than 250 characters, and has spaces
+ key = 'some key with spaces' * 15
+ val = 'a value'
+ cache.set(key, val)
+ self.assertEqual(cache.get(key), val)
+
class CacheUtils(unittest.TestCase):
"""TestCase for django.utils.cache functions."""
--
You received this message because you are subscribed to the Google Groups
"Django updates" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en.
