Changed cache monkey-patching for Django 3.2+ (#1497)

* Changed cache monkey-patching for Django 3.2+

Changed cache monkey-patching for Django 3.2+ to iterate over existing
caches and patch them individually rather than attempting to patch
django.core.caches as a whole. The middleware.cache is still
being patched as a whole in order to attempt to catch any cache
usages before enable_instrumentation is called.

Test cache panel is disabled for middleware before it.

* Apply suggestions from code review

Co-authored-by: Matthias Kestenholz <[email protected]>

Author: tim-schilling

debug_toolbar/panels/cache.py

@@ -8,6 +8,7 @@
 except ImportError:
     ConnectionProxy = None
 
+import django
 from django.conf import settings
 from django.core import cache
 from django.core.cache import (
@@ -140,10 +141,27 @@ def decr_version(self, *args, **kwargs):
         return self.cache.decr_version(*args, **kwargs)
 
 
-class CacheHandlerPatch(CacheHandler):
-    def __getitem__(self, alias):
-        actual_cache = super().__getitem__(alias)
-        return CacheStatTracker(actual_cache)
+if django.VERSION < (3, 2):
+
+    class CacheHandlerPatch(CacheHandler):
+        def __getitem__(self, alias):
+            actual_cache = super().__getitem__(alias)
+            return CacheStatTracker(actual_cache)
+
+
+else:
+
+    class CacheHandlerPatch(CacheHandler):
+        def __init__(self, settings=None):
+            self._djdt_wrap = True
+            super().__init__(settings=settings)
+
+        def create_connection(self, alias):
+            actual_cache = super().create_connection(alias)
+            if self._djdt_wrap:
+                return CacheStatTracker(actual_cache)
+            else:
+                return actual_cache
 
 
 middleware_cache.caches = CacheHandlerPatch()
@@ -251,22 +269,40 @@ def title(self):
         )
 
     def enable_instrumentation(self):
-        if isinstance(middleware_cache.caches, CacheHandlerPatch):
-            cache.caches = middleware_cache.caches
+        if django.VERSION < (3, 2):
+            if isinstance(middleware_cache.caches, CacheHandlerPatch):
+                cache.caches = middleware_cache.caches
+            else:
+                cache.caches = CacheHandlerPatch()
         else:
-            cache.caches = CacheHandlerPatch()
+            for alias in cache.caches:
+                if not isinstance(cache.caches[alias], CacheStatTracker):
+                    cache.caches[alias] = CacheStatTracker(cache.caches[alias])
+
+            if not isinstance(middleware_cache.caches, CacheHandlerPatch):
+                middleware_cache.caches = cache.caches
 
         # Wrap the patched cache inside Django's ConnectionProxy
         if ConnectionProxy:
             cache.cache = ConnectionProxy(cache.caches, DEFAULT_CACHE_ALIAS)
 
     def disable_instrumentation(self):
-        cache.caches = original_caches
-        cache.cache = original_cache
-        # While it can be restored to the original, any views that were
-        # wrapped with the cache_page decorator will continue to use a
-        # monkey patched cache.
-        middleware_cache.caches = original_caches
+        if django.VERSION < (3, 2):
+            cache.caches = original_caches
+            cache.cache = original_cache
+            # While it can be restored to the original, any views that were
+            # wrapped with the cache_page decorator will continue to use a
+            # monkey patched cache.
+            middleware_cache.caches = original_caches
+        else:
+            for alias in cache.caches:
+                if isinstance(cache.caches[alias], CacheStatTracker):
+                    cache.caches[alias] = cache.caches[alias].cache
+            if ConnectionProxy:
+                cache.cache = ConnectionProxy(cache.caches, DEFAULT_CACHE_ALIAS)
+            # While it can be restored to the original, any views that were
+            # wrapped with the cache_page decorator will continue to use a
+            # monkey patched cache.
 
     def generate_stats(self, request, response):
         self.record_stats(

docs/changes.rst

@@ -4,6 +4,12 @@ Change log
 Next version
 ------------
 
+* Changed cache monkey-patching for Django 3.2+ to iterate over existing
+  caches and patch them individually rather than attempting to patch
+  ``django.core.caches`` as a whole. The ``middleware.cache`` is still
+  being patched as a whole in order to attempt to catch any cache
+  usages before ``enable_instrumentation`` is called.
+
 
 3.2.2 (2021-08-14)
 ------------------

tests/middleware.py

@@ -0,0 +1,17 @@
+from django.core.cache import cache
+
+
+class UseCacheAfterToolbar:
+    """
+    This middleware exists to use the cache before and after
+    the toolbar is setup.
+    """
+
+    def __init__(self, get_response):
+        self.get_response = get_response
+
+    def __call__(self, request):
+        cache.set("UseCacheAfterToolbar.before", 1)
+        response = self.get_response(request)
+        cache.set("UseCacheAfterToolbar.after", 1)
+        return response

tests/settings.py

@@ -30,6 +30,7 @@
 MEDIA_URL = "/media/"  # Avoids https://code.djangoproject.com/ticket/21451
 
 MIDDLEWARE = [
+    "tests.middleware.UseCacheAfterToolbar",
     "debug_toolbar.middleware.DebugToolbarMiddleware",
     "django.middleware.security.SecurityMiddleware",
     "django.contrib.sessions.middleware.SessionMiddleware",

tests/test_integration.py

@@ -6,6 +6,7 @@
 import html5lib
 from django.contrib.staticfiles.testing import StaticLiveServerTestCase
 from django.core import signing
+from django.core.cache import cache
 from django.db import connection
 from django.http import HttpResponse
 from django.template.loader import get_template
@@ -102,6 +103,25 @@ def test_cache_page(self):
         self.client.get("/cached_view/")
         self.assertEqual(len(self.toolbar.get_panel_by_id("CachePanel").calls), 5)
 
+    def test_low_level_cache_view(self):
+        """Test cases when low level caching API is used within a request."""
+        self.client.get("/cached_low_level_view/")
+        self.assertEqual(len(self.toolbar.get_panel_by_id("CachePanel").calls), 2)
+        self.client.get("/cached_low_level_view/")
+        self.assertEqual(len(self.toolbar.get_panel_by_id("CachePanel").calls), 3)
+
+    def test_cache_disable_instrumentation(self):
+        """
+        Verify that middleware cache usages before and after
+        DebugToolbarMiddleware are not counted.
+        """
+        self.assertIsNone(cache.set("UseCacheAfterToolbar.before", None))
+        self.assertIsNone(cache.set("UseCacheAfterToolbar.after", None))
+        self.client.get("/execute_sql/")
+        self.assertEqual(cache.get("UseCacheAfterToolbar.before"), 1)
+        self.assertEqual(cache.get("UseCacheAfterToolbar.after"), 1)
+        self.assertEqual(len(self.toolbar.get_panel_by_id("CachePanel").calls), 0)
+
     def test_is_toolbar_request(self):
         self.request.path = "/__debug__/render_panel/"
         self.assertTrue(self.toolbar.is_toolbar_request(self.request))
@@ -376,7 +396,7 @@ def test_view_returns_template_response(self):
         self.assertEqual(response.status_code, 200)
 
     @override_settings(DEBUG_TOOLBAR_CONFIG={"DISABLE_PANELS": set()})
-    def test_incercept_redirects(self):
+    def test_intcercept_redirects(self):
         response = self.client.get("/redirect/")
         self.assertEqual(response.status_code, 200)
         # Link to LOCATION header.

tests/urls.py

@@ -20,6 +20,7 @@
     path("new_user/", views.new_user),
     path("execute_sql/", views.execute_sql),
     path("cached_view/", views.cached_view),
+    path("cached_low_level_view/", views.cached_low_level_view),
     path("json_view/", views.json_view),
     path("redirect/", views.redirect_view),
     path("login_without_redirect/", LoginView.as_view(redirect_field_name=None)),

tests/views.py

@@ -1,4 +1,5 @@
 from django.contrib.auth.models import User
+from django.core.cache import cache
 from django.http import HttpResponseRedirect, JsonResponse
 from django.shortcuts import render
 from django.template.response import TemplateResponse
@@ -33,6 +34,15 @@ def cached_view(request):
     return render(request, "base.html")
 
 
+def cached_low_level_view(request):
+    key = "spam"
+    value = cache.get(key)
+    if not value:
+        value = "eggs"
+        cache.set(key, value, 60)
+    return render(request, "base.html")
+
+
 def json_view(request):
     return JsonResponse({"foo": "bar"})