From 6fc5ce868da102b8d3206552925a513b2f26cb75 Mon Sep 17 00:00:00 2001 From: Aman Pandey Date: Tue, 20 Aug 2024 18:08:39 +0530 Subject: [PATCH] Async compatible `StaticFilesPanel` (#1983) * incoperate signals and contextvars to record staticfiles for concurrent requests * remove used_static_files contextvar and its dependencies allow on app ready monkey patching * async static files panel test * update doc * Code review changes * suggested changes --- debug_toolbar/panels/staticfiles.py | 47 ++++++++++++------- docs/architecture.rst | 2 +- tests/panels/test_staticfiles.py | 13 +++++ tests/templates/base.html | 1 + tests/templates/staticfiles/async_static.html | 6 +++ 5 files changed, 52 insertions(+), 17 deletions(-) create mode 100644 tests/templates/staticfiles/async_static.html diff --git a/debug_toolbar/panels/staticfiles.py b/debug_toolbar/panels/staticfiles.py index 2eed2efa0..b0997404c 100644 --- a/debug_toolbar/panels/staticfiles.py +++ b/debug_toolbar/panels/staticfiles.py @@ -1,9 +1,11 @@ import contextlib +import uuid from contextvars import ContextVar from os.path import join, normpath from django.conf import settings from django.contrib.staticfiles import finders, storage +from django.dispatch import Signal from django.utils.functional import LazyObject from django.utils.translation import gettext_lazy as _, ngettext @@ -28,8 +30,10 @@ def url(self): return storage.staticfiles_storage.url(self.path) -# This will collect the StaticFile instances across threads. -used_static_files = ContextVar("djdt_static_used_static_files") +# This will record and map the StaticFile instances with its associated +# request across threads and async concurrent requests state. +request_id_context_var = ContextVar("djdt_request_id_store") +record_static_file_signal = Signal() class DebugConfiguredStorage(LazyObject): @@ -59,7 +63,12 @@ def url(self, path): # The ContextVar wasn't set yet. Since the toolbar wasn't properly # configured to handle this request, we don't need to capture # the static file. - used_static_files.get().append(StaticFile(path)) + request_id = request_id_context_var.get() + record_static_file_signal.send( + sender=self, + staticfile=StaticFile(path), + request_id=request_id, + ) return super().url(path) self._wrapped = DebugStaticFilesStorage() @@ -73,6 +82,7 @@ class StaticFilesPanel(panels.Panel): A panel to display the found staticfiles. """ + is_async = True name = "Static files" template = "debug_toolbar/panels/staticfiles.html" @@ -87,12 +97,28 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.num_found = 0 self.used_paths = [] + self.request_id = str(uuid.uuid4()) - def enable_instrumentation(self): + @classmethod + def ready(cls): storage.staticfiles_storage = DebugConfiguredStorage() + def _store_static_files_signal_handler(self, sender, staticfile, **kwargs): + # Only record the static file if the request_id matches the one + # that was used to create the panel. + # as sender of the signal and this handler will have multiple + # concurrent connections and we want to avoid storing of same + # staticfile from other connections as well. + if request_id_context_var.get() == self.request_id: + self.used_paths.append(staticfile) + + def enable_instrumentation(self): + self.ctx_token = request_id_context_var.set(self.request_id) + record_static_file_signal.connect(self._store_static_files_signal_handler) + def disable_instrumentation(self): - storage.staticfiles_storage = _original_storage + record_static_file_signal.disconnect(self._store_static_files_signal_handler) + request_id_context_var.reset(self.ctx_token) @property def num_used(self): @@ -108,17 +134,6 @@ def nav_subtitle(self): "%(num_used)s file used", "%(num_used)s files used", num_used ) % {"num_used": num_used} - def process_request(self, request): - reset_token = used_static_files.set([]) - response = super().process_request(request) - # Make a copy of the used paths so that when the - # ContextVar is reset, our panel still has the data. - self.used_paths = used_static_files.get().copy() - # Reset the ContextVar to be empty again, removing the reference - # to the list of used files. - used_static_files.reset(reset_token) - return response - def generate_stats(self, request, response): self.record_stats( { diff --git a/docs/architecture.rst b/docs/architecture.rst index 0c267c806..0043f5153 100644 --- a/docs/architecture.rst +++ b/docs/architecture.rst @@ -81,7 +81,7 @@ Problematic Parts the main benefit of the toolbar - Support for async and multi-threading: ``debug_toolbar.middleware.DebugToolbarMiddleware`` is now async compatible and can process async requests. However certain - panels such as ``SQLPanel``, ``TimerPanel``, ``StaticFilesPanel``, + panels such as ``SQLPanel``, ``TimerPanel``, ``RequestPanel``, ``HistoryPanel`` and ``ProfilingPanel`` aren't fully compatible and currently being worked on. For now, these panels are disabled by default when running in async environment. diff --git a/tests/panels/test_staticfiles.py b/tests/panels/test_staticfiles.py index 0736d86ed..3caedc4eb 100644 --- a/tests/panels/test_staticfiles.py +++ b/tests/panels/test_staticfiles.py @@ -1,5 +1,7 @@ from django.conf import settings from django.contrib.staticfiles import finders +from django.shortcuts import render +from django.test import AsyncRequestFactory from ..base import BaseTestCase @@ -27,6 +29,17 @@ def test_default_case(self): self.panel.get_staticfiles_dirs(), finders.FileSystemFinder().locations ) + async def test_store_staticfiles_with_async_context(self): + async def get_response(request): + # template contains one static file + return render(request, "staticfiles/async_static.html") + + self._get_response = get_response + async_request = AsyncRequestFactory().get("/") + response = await self.panel.process_request(async_request) + self.panel.generate_stats(self.request, response) + self.assertEqual(self.panel.num_used, 1) + def test_insert_content(self): """ Test that the panel only inserts content after generate_stats and diff --git a/tests/templates/base.html b/tests/templates/base.html index ea0d773ac..272c316f0 100644 --- a/tests/templates/base.html +++ b/tests/templates/base.html @@ -2,6 +2,7 @@ {{ title }} + {% block head %}{% endblock %} {% block content %}{% endblock %} diff --git a/tests/templates/staticfiles/async_static.html b/tests/templates/staticfiles/async_static.html new file mode 100644 index 000000000..fc0c9b885 --- /dev/null +++ b/tests/templates/staticfiles/async_static.html @@ -0,0 +1,6 @@ +{% extends "base.html" %} +{% load static %} + +{% block head %} + +{% endblock %}