Skip to content

Changed cache monkey-patching for Django 3.2+ #1497

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 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)
------------------
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
22 changes: 21 additions & 1 deletion 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,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))
Expand Down Expand Up @@ -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.
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