Skip to content

Commit

Permalink
Changed cache monkey-patching for Django 3.2+
Browse files Browse the repository at this point in the history
Changed caching monkey-patching for Django3.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. Fixes #1496

Test cache panel is diabled for middleware before it.
  • Loading branch information
tim-schilling committed Aug 28, 2021
1 parent 3ed59cb commit f282050
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 13 deletions.
62 changes: 49 additions & 13 deletions debug_toolbar/panels/cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 6 additions & 0 deletions docs/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ Change log
Next version
------------

* Changed caching monkey-patching for Django3.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. Fixes #1496


3.2.2 (2021-08-14)
------------------
Expand Down
17 changes: 17 additions & 0 deletions tests/middleware.py
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions tests/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
20 changes: 20 additions & 0 deletions tests/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -102,6 +103,13 @@ 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_is_toolbar_request(self):
self.request.path = "/__debug__/render_panel/"
self.assertTrue(self.toolbar.is_toolbar_request(self.request))
Expand Down Expand Up @@ -553,6 +561,18 @@ def test_django_cached_template_loader(self):
)
)

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.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_sql_action_and_go_back(self):
self.get("/execute_sql/")
sql_panel = self.selenium.find_element_by_id("SQLPanel")
Expand Down
1 change: 1 addition & 0 deletions tests/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down
10 changes: 10 additions & 0 deletions tests/views.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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"})

Expand Down

0 comments on commit f282050

Please sign in to comment.