Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

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.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@13766 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information...
commit fc26da645aea2361d78cf5a5543214a240d82527 1 parent bfbc259
Malcolm Tredinnick authored September 12, 2010
2  django/core/cache/__init__.py
@@ -18,7 +18,7 @@
18 18
 from cgi import parse_qsl
19 19
 from django.conf import settings
20 20
 from django.core import signals
21  
-from django.core.cache.backends.base import InvalidCacheBackendError
  21
+from django.core.cache.backends.base import InvalidCacheBackendError, CacheKeyWarning
22 22
 from django.utils import importlib
23 23
 
24 24
 # Name for use in settings file --> name of module in "backends" directory.
28  django/core/cache/backends/base.py
... ...
@@ -1,10 +1,18 @@
1 1
 "Base Cache class."
2 2
 
3  
-from django.core.exceptions import ImproperlyConfigured
  3
+import warnings
  4
+
  5
+from django.core.exceptions import ImproperlyConfigured, DjangoRuntimeWarning
4 6
 
5 7
 class InvalidCacheBackendError(ImproperlyConfigured):
6 8
     pass
7 9
 
  10
+class CacheKeyWarning(DjangoRuntimeWarning):
  11
+    pass
  12
+
  13
+# Memcached does not accept keys longer than this.
  14
+MEMCACHE_MAX_KEY_LENGTH = 250
  15
+
8 16
 class BaseCache(object):
9 17
     def __init__(self, params):
10 18
         timeout = params.get('timeout', 300)
@@ -116,3 +124,21 @@ def delete_many(self, keys):
116 124
     def clear(self):
117 125
         """Remove *all* values from the cache at once."""
118 126
         raise NotImplementedError
  127
+
  128
+    def validate_key(self, key):
  129
+        """
  130
+        Warn about keys that would not be portable to the memcached
  131
+        backend. This encourages (but does not force) writing backend-portable
  132
+        cache code.
  133
+
  134
+        """
  135
+        if len(key) > MEMCACHE_MAX_KEY_LENGTH:
  136
+            warnings.warn('Cache key will cause errors if used with memcached: '
  137
+                    '%s (longer than %s)' % (key, MEMCACHE_MAX_KEY_LENGTH),
  138
+                    CacheKeyWarning)
  139
+        for char in key:
  140
+            if ord(char) < 33 or ord(char) == 127:
  141
+                warnings.warn('Cache key contains characters that will cause '
  142
+                        'errors if used with memcached: %r' % key,
  143
+                              CacheKeyWarning)
  144
+
5  django/core/cache/backends/db.py
@@ -46,6 +46,7 @@ class CacheEntry(object):
46 46
             self._cull_frequency = 3
47 47
 
48 48
     def get(self, key, default=None):
  49
+        self.validate_key(key)
49 50
         db = router.db_for_read(self.cache_model_class)
50 51
         table = connections[db].ops.quote_name(self._table)
51 52
         cursor = connections[db].cursor()
@@ -65,9 +66,11 @@ def get(self, key, default=None):
65 66
         return pickle.loads(base64.decodestring(value))
66 67
 
67 68
     def set(self, key, value, timeout=None):
  69
+        self.validate_key(key)
68 70
         self._base_set('set', key, value, timeout)
69 71
 
70 72
     def add(self, key, value, timeout=None):
  73
+        self.validate_key(key)
71 74
         return self._base_set('add', key, value, timeout)
72 75
 
73 76
     def _base_set(self, mode, key, value, timeout=None):
@@ -103,6 +106,7 @@ def _base_set(self, mode, key, value, timeout=None):
103 106
             return True
104 107
 
105 108
     def delete(self, key):
  109
+        self.validate_key(key)
106 110
         db = router.db_for_write(self.cache_model_class)
107 111
         table = connections[db].ops.quote_name(self._table)
108 112
         cursor = connections[db].cursor()
@@ -111,6 +115,7 @@ def delete(self, key):
111 115
         transaction.commit_unless_managed(using=db)
112 116
 
113 117
     def has_key(self, key):
  118
+        self.validate_key(key)
114 119
         db = router.db_for_read(self.cache_model_class)
115 120
         table = connections[db].ops.quote_name(self._table)
116 121
         cursor = connections[db].cursor()
15  django/core/cache/backends/dummy.py
@@ -6,22 +6,25 @@ class CacheClass(BaseCache):
6 6
     def __init__(self, *args, **kwargs):
7 7
         pass
8 8
 
9  
-    def add(self, *args, **kwargs):
  9
+    def add(self, key, *args, **kwargs):
  10
+        self.validate_key(key)
10 11
         return True
11 12
 
12 13
     def get(self, key, default=None):
  14
+        self.validate_key(key)
13 15
         return default
14 16
 
15  
-    def set(self, *args, **kwargs):
16  
-        pass
  17
+    def set(self, key, *args, **kwargs):
  18
+        self.validate_key(key)
17 19
 
18  
-    def delete(self, *args, **kwargs):
19  
-        pass
  20
+    def delete(self, key, *args, **kwargs):
  21
+        self.validate_key(key)
20 22
 
21 23
     def get_many(self, *args, **kwargs):
22 24
         return {}
23 25
 
24  
-    def has_key(self, *args, **kwargs):
  26
+    def has_key(self, key, *args, **kwargs):
  27
+        self.validate_key(key)
25 28
         return False
26 29
 
27 30
     def set_many(self, *args, **kwargs):
5  django/core/cache/backends/filebased.py
@@ -32,6 +32,7 @@ def __init__(self, dir, params):
32 32
             self._createdir()
33 33
 
34 34
     def add(self, key, value, timeout=None):
  35
+        self.validate_key(key)
35 36
         if self.has_key(key):
36 37
             return False
37 38
 
@@ -39,6 +40,7 @@ def add(self, key, value, timeout=None):
39 40
         return True
40 41
 
41 42
     def get(self, key, default=None):
  43
+        self.validate_key(key)
42 44
         fname = self._key_to_file(key)
43 45
         try:
44 46
             f = open(fname, 'rb')
@@ -56,6 +58,7 @@ def get(self, key, default=None):
56 58
         return default
57 59
 
58 60
     def set(self, key, value, timeout=None):
  61
+        self.validate_key(key)
59 62
         fname = self._key_to_file(key)
60 63
         dirname = os.path.dirname(fname)
61 64
 
@@ -79,6 +82,7 @@ def set(self, key, value, timeout=None):
79 82
             pass
80 83
 
81 84
     def delete(self, key):
  85
+        self.validate_key(key)
82 86
         try:
83 87
             self._delete(self._key_to_file(key))
84 88
         except (IOError, OSError):
@@ -95,6 +99,7 @@ def _delete(self, fname):
95 99
             pass
96 100
 
97 101
     def has_key(self, key):
  102
+        self.validate_key(key)
98 103
         fname = self._key_to_file(key)
99 104
         try:
100 105
             f = open(fname, 'rb')
5  django/core/cache/backends/locmem.py
@@ -30,6 +30,7 @@ def __init__(self, _, params):
30 30
         self._lock = RWLock()
31 31
 
32 32
     def add(self, key, value, timeout=None):
  33
+        self.validate_key(key)
33 34
         self._lock.writer_enters()
34 35
         try:
35 36
             exp = self._expire_info.get(key)
@@ -44,6 +45,7 @@ def add(self, key, value, timeout=None):
44 45
             self._lock.writer_leaves()
45 46
 
46 47
     def get(self, key, default=None):
  48
+        self.validate_key(key)
47 49
         self._lock.reader_enters()
48 50
         try:
49 51
             exp = self._expire_info.get(key)
@@ -76,6 +78,7 @@ def _set(self, key, value, timeout=None):
76 78
         self._expire_info[key] = time.time() + timeout
77 79
 
78 80
     def set(self, key, value, timeout=None):
  81
+        self.validate_key(key)
79 82
         self._lock.writer_enters()
80 83
         # Python 2.4 doesn't allow combined try-except-finally blocks.
81 84
         try:
@@ -87,6 +90,7 @@ def set(self, key, value, timeout=None):
87 90
             self._lock.writer_leaves()
88 91
 
89 92
     def has_key(self, key):
  93
+        self.validate_key(key)
90 94
         self._lock.reader_enters()
91 95
         try:
92 96
             exp = self._expire_info.get(key)
@@ -127,6 +131,7 @@ def _delete(self, key):
127 131
             pass
128 132
 
129 133
     def delete(self, key):
  134
+        self.validate_key(key)
130 135
         self._lock.writer_enters()
131 136
         try:
132 137
             self._delete(key)
7  django/core/exceptions.py
... ...
@@ -1,4 +1,9 @@
1  
-"Global Django exceptions"
  1
+"""
  2
+Global Django exception and warning classes.
  3
+"""
  4
+
  5
+class DjangoRuntimeWarning(RuntimeWarning):
  6
+   pass
2 7
 
3 8
 class ObjectDoesNotExist(Exception):
4 9
     "The requested object does not exist"
39  docs/topics/cache.txt
@@ -641,6 +641,45 @@ nonexistent cache key.::
641 641
     However, if the backend doesn't natively provide an increment/decrement
642 642
     operation, it will be implemented using a two-step retrieve/update.
643 643
 
  644
+Cache key warnings
  645
+------------------
  646
+
  647
+.. versionadded:: 1.3
  648
+
  649
+Memcached, the most commonly-used production cache backend, does not allow
  650
+cache keys longer than 250 characters or containing whitespace or control
  651
+characters, and using such keys will cause an exception. To encourage
  652
+cache-portable code and minimize unpleasant surprises, the other built-in cache
  653
+backends issue a warning (``django.core.cache.backends.base.CacheKeyWarning``)
  654
+if a key is used that would cause an error on memcached.
  655
+
  656
+If you are using a production backend that can accept a wider range of keys (a
  657
+custom backend, or one of the non-memcached built-in backends), and want to use
  658
+this wider range without warnings, you can silence ``CacheKeyWarning`` with
  659
+this code in the ``management`` module of one of your
  660
+:setting:`INSTALLED_APPS`::
  661
+
  662
+     import warnings
  663
+
  664
+     from django.core.cache import CacheKeyWarning
  665
+
  666
+     warnings.simplefilter("ignore", CacheKeyWarning)
  667
+
  668
+If you want to instead provide custom key validation logic for one of the
  669
+built-in backends, you can subclass it, override just the ``validate_key``
  670
+method, and follow the instructions for `using a custom cache backend`_. For
  671
+instance, to do this for the ``locmem`` backend, put this code in a module::
  672
+
  673
+    from django.core.cache.backends.locmem import CacheClass as LocMemCacheClass
  674
+
  675
+    class CacheClass(LocMemCacheClass):
  676
+        def validate_key(self, key):
  677
+            """Custom validation, raising exceptions or warnings as needed."""
  678
+            # ...
  679
+
  680
+...and use the dotted Python path to this module as the scheme portion of your
  681
+:setting:`CACHE_BACKEND`.
  682
+
644 683
 Upstream caches
645 684
 ===============
646 685
 
9  tests/regressiontests/cache/liberal_backend.py
... ...
@@ -0,0 +1,9 @@
  1
+from django.core.cache.backends.locmem import CacheClass as LocMemCacheClass
  2
+
  3
+class LiberalKeyValidationMixin(object):
  4
+    def validate_key(self, key):
  5
+        pass
  6
+
  7
+class CacheClass(LiberalKeyValidationMixin, LocMemCacheClass):
  8
+    pass
  9
+
62  tests/regressiontests/cache/tests.py
@@ -8,11 +8,12 @@
8 8
 import tempfile
9 9
 import time
10 10
 import unittest
  11
+import warnings
11 12
 
12 13
 from django.conf import settings
13 14
 from django.core import management
14 15
 from django.core.cache import get_cache
15  
-from django.core.cache.backends.base import InvalidCacheBackendError
  16
+from django.core.cache.backends.base import InvalidCacheBackendError, CacheKeyWarning
16 17
 from django.http import HttpResponse, HttpRequest
17 18
 from django.middleware.cache import FetchFromCacheMiddleware, UpdateCacheMiddleware
18 19
 from django.utils import translation
@@ -366,6 +367,33 @@ def perform_cull_test(self, initial_count, final_count):
366 367
                 count = count + 1
367 368
         self.assertEqual(count, final_count)
368 369
 
  370
+    def test_invalid_keys(self):
  371
+        """
  372
+        All the builtin backends (except memcached, see below) should warn on
  373
+        keys that would be refused by memcached. This encourages portable
  374
+        caching code without making it too difficult to use production backends
  375
+        with more liberal key rules. Refs #6447.
  376
+
  377
+        """
  378
+        # On Python 2.6+ we could use the catch_warnings context
  379
+        # manager to test this warning nicely. Since we can't do that
  380
+        # yet, the cleanest option is to temporarily ask for
  381
+        # CacheKeyWarning to be raised as an exception.
  382
+        warnings.simplefilter("error", CacheKeyWarning)
  383
+
  384
+        # memcached does not allow whitespace or control characters in keys
  385
+        self.assertRaises(CacheKeyWarning, self.cache.set, 'key with spaces', 'value')
  386
+        # memcached limits key length to 250
  387
+        self.assertRaises(CacheKeyWarning, self.cache.set, 'a' * 251, 'value')
  388
+
  389
+        # The warnings module has no public API for getting the
  390
+        # current list of warning filters, so we can't save that off
  391
+        # and reset to the previous value, we have to globally reset
  392
+        # it. The effect will be the same, as long as the Django test
  393
+        # runner doesn't add any global warning filters (it currently
  394
+        # does not).
  395
+        warnings.resetwarnings()
  396
+
369 397
 class DBCacheTests(unittest.TestCase, BaseCacheTests):
370 398
     def setUp(self):
371 399
         # Spaces are used in the table name to ensure quoting/escaping is working
@@ -397,6 +425,22 @@ class MemcachedCacheTests(unittest.TestCase, BaseCacheTests):
397 425
         def setUp(self):
398 426
             self.cache = get_cache(settings.CACHE_BACKEND)
399 427
 
  428
+        def test_invalid_keys(self):
  429
+            """
  430
+            On memcached, we don't introduce a duplicate key validation
  431
+            step (for speed reasons), we just let the memcached API
  432
+            library raise its own exception on bad keys. Refs #6447.
  433
+
  434
+            In order to be memcached-API-library agnostic, we only assert
  435
+            that a generic exception of some kind is raised.
  436
+
  437
+            """
  438
+            # memcached does not allow whitespace or control characters in keys
  439
+            self.assertRaises(Exception, self.cache.set, 'key with spaces', 'value')
  440
+            # memcached limits key length to 250
  441
+            self.assertRaises(Exception, self.cache.set, 'a' * 251, 'value')
  442
+
  443
+
400 444
 class FileBasedCacheTests(unittest.TestCase, BaseCacheTests):
401 445
     """
402 446
     Specific test cases for the file-based cache.
@@ -429,6 +473,22 @@ def test_subdirectory_removal(self):
429 473
     def test_cull(self):
430 474
         self.perform_cull_test(50, 28)
431 475
 
  476
+class CustomCacheKeyValidationTests(unittest.TestCase):
  477
+    """
  478
+    Tests for the ability to mixin a custom ``validate_key`` method to
  479
+    a custom cache backend that otherwise inherits from a builtin
  480
+    backend, and override the default key validation. Refs #6447.
  481
+
  482
+    """
  483
+    def test_custom_key_validation(self):
  484
+        cache = get_cache('regressiontests.cache.liberal_backend://')
  485
+
  486
+        # this key is both longer than 250 characters, and has spaces
  487
+        key = 'some key with spaces' * 15
  488
+        val = 'a value'
  489
+        cache.set(key, val)
  490
+        self.assertEqual(cache.get(key), val)
  491
+
432 492
 class CacheUtils(unittest.TestCase):
433 493
     """TestCase for django.utils.cache functions."""
434 494
 

0 notes on commit fc26da6

Please sign in to comment.
Something went wrong with that request. Please try again.