From 6cb639645415b7e1288e9d5becb92c53e6f06b56 Mon Sep 17 00:00:00 2001 From: Aman Pandey Date: Tue, 20 Aug 2024 23:58:07 +0530 Subject: [PATCH 1/7] ASGI check approach with added test and docs --- debug_toolbar/toolbar.py | 15 +++++++++------ docs/architecture.rst | 2 +- tests/test_integration.py | 18 +++++++++++++++++- 3 files changed, 27 insertions(+), 8 deletions(-) diff --git a/debug_toolbar/toolbar.py b/debug_toolbar/toolbar.py index 35d789a53..921546aaf 100644 --- a/debug_toolbar/toolbar.py +++ b/debug_toolbar/toolbar.py @@ -12,6 +12,7 @@ from django.apps import apps from django.conf import settings from django.core.exceptions import ImproperlyConfigured +from django.core.handlers.asgi import ASGIRequest from django.dispatch import Signal from django.template import TemplateSyntaxError from django.template.loader import render_to_string @@ -101,12 +102,14 @@ def should_render_panels(self): If False, the panels will be loaded via Ajax. """ if (render_panels := self.config["RENDER_PANELS"]) is None: - # If wsgi.multiprocess isn't in the headers, then it's likely - # being served by ASGI. This type of set up is most likely - # incompatible with the toolbar until - # https://github.com/jazzband/django-debug-toolbar/issues/1430 - # is resolved. - render_panels = self.request.META.get("wsgi.multiprocess", True) + # If wsgi.multiprocess is true then it is either being served + # from ASGI or multithreaded third-party WSGI server eg gunicorn. + # we need to make special handling for ASGI for supporting + # async context based requests. + if isinstance(self.request, ASGIRequest): + render_panels = False + else: + render_panels = self.request.META.get("wsgi.multiprocess", True) return render_panels # Handle storing toolbars in memory and fetching them later on diff --git a/docs/architecture.rst b/docs/architecture.rst index 0043f5153..cf5c54951 100644 --- a/docs/architecture.rst +++ b/docs/architecture.rst @@ -82,7 +82,7 @@ Problematic Parts - 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``, - ``RequestPanel``, ``HistoryPanel`` and ``ProfilingPanel`` aren't fully + ``RequestPanel`` and ``ProfilingPanel`` aren't fully compatible and currently being worked on. For now, these panels are disabled by default when running in async environment. follow the progress of this issue in `Async compatible toolbar project `_. diff --git a/tests/test_integration.py b/tests/test_integration.py index df276d90c..ca31a294c 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -11,7 +11,7 @@ from django.db import connection from django.http import HttpResponse from django.template.loader import get_template -from django.test import RequestFactory +from django.test import AsyncRequestFactory, RequestFactory from django.test.utils import override_settings from debug_toolbar.forms import SignedDataForm @@ -126,6 +126,22 @@ def test_should_render_panels_multiprocess(self): request.META.pop("wsgi.multiprocess") self.assertTrue(toolbar.should_render_panels()) + def test_should_render_panels_asgi(self): + """ + The toolbar not should render the panels on each request when wsgi.multiprocess + is True or missing in case of async context rather than multithreaded + wsgi. + """ + async_request = AsyncRequestFactory().get("/") + # by default ASGIRequest will have wsgi.multiprocess set to True + # but we are still assigning this to true cause this could change + # and we specifically need to check that method returns false even with + # wsgi.multiprocess set to true + async_request.META["wsgi.multiprocess"] = True + toolbar = DebugToolbar(async_request, self.get_response) + toolbar.config["RENDER_PANELS"] = None + self.assertFalse(toolbar.should_render_panels()) + def _resolve_stats(self, path): # takes stats from Request panel request = rf.get(path) From 830c9641d1b4b9b196198d53eca8accee8d3e319 Mon Sep 17 00:00:00 2001 From: Aman Pandey Date: Wed, 21 Aug 2024 00:02:52 +0530 Subject: [PATCH 2/7] revert changes --- debug_toolbar/toolbar.py | 15 ++++++--------- docs/architecture.rst | 2 +- tests/test_integration.py | 18 +----------------- 3 files changed, 8 insertions(+), 27 deletions(-) diff --git a/debug_toolbar/toolbar.py b/debug_toolbar/toolbar.py index 921546aaf..35d789a53 100644 --- a/debug_toolbar/toolbar.py +++ b/debug_toolbar/toolbar.py @@ -12,7 +12,6 @@ from django.apps import apps from django.conf import settings from django.core.exceptions import ImproperlyConfigured -from django.core.handlers.asgi import ASGIRequest from django.dispatch import Signal from django.template import TemplateSyntaxError from django.template.loader import render_to_string @@ -102,14 +101,12 @@ def should_render_panels(self): If False, the panels will be loaded via Ajax. """ if (render_panels := self.config["RENDER_PANELS"]) is None: - # If wsgi.multiprocess is true then it is either being served - # from ASGI or multithreaded third-party WSGI server eg gunicorn. - # we need to make special handling for ASGI for supporting - # async context based requests. - if isinstance(self.request, ASGIRequest): - render_panels = False - else: - render_panels = self.request.META.get("wsgi.multiprocess", True) + # If wsgi.multiprocess isn't in the headers, then it's likely + # being served by ASGI. This type of set up is most likely + # incompatible with the toolbar until + # https://github.com/jazzband/django-debug-toolbar/issues/1430 + # is resolved. + render_panels = self.request.META.get("wsgi.multiprocess", True) return render_panels # Handle storing toolbars in memory and fetching them later on diff --git a/docs/architecture.rst b/docs/architecture.rst index cf5c54951..0043f5153 100644 --- a/docs/architecture.rst +++ b/docs/architecture.rst @@ -82,7 +82,7 @@ Problematic Parts - 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``, - ``RequestPanel`` and ``ProfilingPanel`` aren't fully + ``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. follow the progress of this issue in `Async compatible toolbar project `_. diff --git a/tests/test_integration.py b/tests/test_integration.py index ca31a294c..df276d90c 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -11,7 +11,7 @@ from django.db import connection from django.http import HttpResponse from django.template.loader import get_template -from django.test import AsyncRequestFactory, RequestFactory +from django.test import RequestFactory from django.test.utils import override_settings from debug_toolbar.forms import SignedDataForm @@ -126,22 +126,6 @@ def test_should_render_panels_multiprocess(self): request.META.pop("wsgi.multiprocess") self.assertTrue(toolbar.should_render_panels()) - def test_should_render_panels_asgi(self): - """ - The toolbar not should render the panels on each request when wsgi.multiprocess - is True or missing in case of async context rather than multithreaded - wsgi. - """ - async_request = AsyncRequestFactory().get("/") - # by default ASGIRequest will have wsgi.multiprocess set to True - # but we are still assigning this to true cause this could change - # and we specifically need to check that method returns false even with - # wsgi.multiprocess set to true - async_request.META["wsgi.multiprocess"] = True - toolbar = DebugToolbar(async_request, self.get_response) - toolbar.config["RENDER_PANELS"] = None - self.assertFalse(toolbar.should_render_panels()) - def _resolve_stats(self, path): # takes stats from Request panel request = rf.get(path) From 7171dbbaa149f4481d78764cb4f335de2ac14ff8 Mon Sep 17 00:00:00 2001 From: Tim Schilling Date: Wed, 29 Jan 2025 19:29:21 -0600 Subject: [PATCH 3/7] Make show toolbar callback function async/sync compatible. This checks if the SHOW_TOOLBAR_CALLBACK is a coroutine if we're in async mode and the reverse if it's not. It will automatically wrap the function with sync_to_async or async_to_sync when necessary. --- debug_toolbar/decorators.py | 2 +- debug_toolbar/middleware.py | 35 +++++++++-- docs/changes.rst | 2 + tests/test_middleware.py | 85 ++++++++++++++++++++++++++ tests/test_middleware_compatibility.py | 46 -------------- 5 files changed, 117 insertions(+), 53 deletions(-) create mode 100644 tests/test_middleware.py delete mode 100644 tests/test_middleware_compatibility.py diff --git a/debug_toolbar/decorators.py b/debug_toolbar/decorators.py index 787282706..aafb8511c 100644 --- a/debug_toolbar/decorators.py +++ b/debug_toolbar/decorators.py @@ -11,7 +11,7 @@ def require_show_toolbar(view): def inner(request, *args, **kwargs): from debug_toolbar.middleware import get_show_toolbar - show_toolbar = get_show_toolbar() + show_toolbar = get_show_toolbar(async_mode=False) if not show_toolbar(request): raise Http404 diff --git a/debug_toolbar/middleware.py b/debug_toolbar/middleware.py index 9986d9106..f4c1a8e1b 100644 --- a/debug_toolbar/middleware.py +++ b/debug_toolbar/middleware.py @@ -6,7 +6,7 @@ import socket from functools import cache -from asgiref.sync import iscoroutinefunction, markcoroutinefunction +from asgiref.sync import iscoroutinefunction, markcoroutinefunction, sync_to_async, async_to_sync from django.conf import settings from django.utils.module_loading import import_string @@ -45,9 +45,13 @@ def show_toolbar(request): # No test passed return False - @cache -def get_show_toolbar(): +def show_toolbar_func_or_path(): + """ + Fetch the show toolbar callback from settings + + Cached to avoid importing multiple times. + """ # If SHOW_TOOLBAR_CALLBACK is a string, which is the recommended # setup, resolve it to the corresponding callable. func_or_path = dt_settings.get_config()["SHOW_TOOLBAR_CALLBACK"] @@ -57,6 +61,23 @@ def get_show_toolbar(): return func_or_path +def get_show_toolbar(async_mode): + """ + Get the callback function to show the toolbar. + + Will wrap the function with sync_to_async or + async_to_sync depending on the status of async_mode + and whether the underlying function is a coroutine. + """ + show_toolbar = show_toolbar_func_or_path() + is_coroutine = iscoroutinefunction(show_toolbar) + if is_coroutine and not async_mode: + show_toolbar = async_to_sync(show_toolbar) + elif not is_coroutine and async_mode: + show_toolbar = sync_to_async(show_toolbar) + return show_toolbar + + class DebugToolbarMiddleware: """ Middleware to set up Debug Toolbar on incoming request and render toolbar @@ -82,7 +103,8 @@ def __call__(self, request): if self.async_mode: return self.__acall__(request) # Decide whether the toolbar is active for this request. - show_toolbar = get_show_toolbar() + show_toolbar = get_show_toolbar(async_mode=self.async_mode) + if not show_toolbar(request) or DebugToolbar.is_toolbar_request(request): return self.get_response(request) toolbar = DebugToolbar(request, self.get_response) @@ -103,8 +125,9 @@ def __call__(self, request): async def __acall__(self, request): # Decide whether the toolbar is active for this request. - show_toolbar = get_show_toolbar() - if not show_toolbar(request) or DebugToolbar.is_toolbar_request(request): + show_toolbar = get_show_toolbar(async_mode=self.async_mode) + + if not await show_toolbar(request) or DebugToolbar.is_toolbar_request(request): response = await self.get_response(request) return response diff --git a/docs/changes.rst b/docs/changes.rst index 9010650ba..a30f62dc3 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -5,6 +5,8 @@ Pending ------- * Added Django 5.2 to the tox matrix. +* Wrap ``SHOW_TOOLBAR_CALLBACK`` function with ``sync_to_async`` + or ``async_to_sync`` to allow sync/async compatibility. 5.0.1 (2025-01-13) ------------------ diff --git a/tests/test_middleware.py b/tests/test_middleware.py new file mode 100644 index 000000000..e4df5df9e --- /dev/null +++ b/tests/test_middleware.py @@ -0,0 +1,85 @@ +import asyncio +from unittest.mock import patch + +from django.contrib.auth.models import User +from django.http import HttpResponse +from django.test import AsyncRequestFactory, RequestFactory, TestCase, override_settings + +from debug_toolbar.middleware import DebugToolbarMiddleware + + +def show_toolbar_if_staff(request): + # Hit the database, but always return True + return User.objects.exists() or True + + +async def ashow_toolbar_if_staff(request): + # Hit the database, but always return True + has_users = await User.objects.afirst() + return has_users or True + + +class MiddlewareSyncAsyncCompatibilityTestCase(TestCase): + def setUp(self): + self.factory = RequestFactory() + self.async_factory = AsyncRequestFactory() + + @override_settings(DEBUG=True) + def test_sync_mode(self): + """ + test middleware switches to sync (__call__) based on get_response type + """ + + request = self.factory.get("/") + middleware = DebugToolbarMiddleware( + lambda x: HttpResponse("Test app") + ) + + self.assertFalse(asyncio.iscoroutinefunction(middleware)) + + response = middleware(request) + self.assertEqual(response.status_code, 200) + self.assertIn(b"djdt", response.content) + + @override_settings(DEBUG=True) + async def test_async_mode(self): + """ + test middleware switches to async (__acall__) based on get_response type + and returns a coroutine + """ + + async def get_response(request): + return HttpResponse("Test app") + + middleware = DebugToolbarMiddleware(get_response) + request = self.async_factory.get("/") + + self.assertTrue(asyncio.iscoroutinefunction(middleware)) + + response = await middleware(request) + self.assertEqual(response.status_code, 200) + self.assertIn(b"djdt", response.content) + + @override_settings(DEBUG=True) + @patch("debug_toolbar.middleware.show_toolbar_func_or_path", return_value=ashow_toolbar_if_staff) + def test_async_show_toolbar_callback_sync_middleware(self, mocked_show): + def get_response(request): + return HttpResponse("Hello world") + middleware = DebugToolbarMiddleware(get_response) + + request = self.factory.get("/") + response = middleware(request) + self.assertEqual(response.status_code, 200) + self.assertIn(b"djdt", response.content) + + @override_settings(DEBUG=True) + @patch("debug_toolbar.middleware.show_toolbar_func_or_path", return_value=show_toolbar_if_staff) + async def test_sync_show_toolbar_callback_async_middleware(self, mocked_show): + async def get_response(request): + return HttpResponse("Hello world") + middleware = DebugToolbarMiddleware(get_response) + + request = self.async_factory.get("/") + response = await middleware(request) + self.assertEqual(response.status_code, 200) + self.assertIn(b"djdt", response.content) diff --git a/tests/test_middleware_compatibility.py b/tests/test_middleware_compatibility.py deleted file mode 100644 index 1337864b1..000000000 --- a/tests/test_middleware_compatibility.py +++ /dev/null @@ -1,46 +0,0 @@ -import asyncio - -from django.http import HttpResponse -from django.test import AsyncRequestFactory, RequestFactory, TestCase, override_settings - -from debug_toolbar.middleware import DebugToolbarMiddleware - - -class MiddlewareSyncAsyncCompatibilityTestCase(TestCase): - def setUp(self): - self.factory = RequestFactory() - self.async_factory = AsyncRequestFactory() - - @override_settings(DEBUG=True) - def test_sync_mode(self): - """ - test middleware switches to sync (__call__) based on get_response type - """ - - request = self.factory.get("/") - middleware = DebugToolbarMiddleware( - lambda x: HttpResponse("Django debug toolbar") - ) - - self.assertFalse(asyncio.iscoroutinefunction(middleware)) - - response = middleware(request) - self.assertEqual(response.status_code, 200) - - @override_settings(DEBUG=True) - async def test_async_mode(self): - """ - test middleware switches to async (__acall__) based on get_response type - and returns a coroutine - """ - - async def get_response(request): - return HttpResponse("Django debug toolbar") - - middleware = DebugToolbarMiddleware(get_response) - request = self.async_factory.get("/") - - self.assertTrue(asyncio.iscoroutinefunction(middleware)) - - response = await middleware(request) - self.assertEqual(response.status_code, 200) From ec3308a917982026aacb9c7ee4087b22a870652d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Thu, 30 Jan 2025 01:34:41 +0000 Subject: [PATCH 4/7] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- debug_toolbar/middleware.py | 8 +++++++- tests/test_middleware.py | 12 ++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/debug_toolbar/middleware.py b/debug_toolbar/middleware.py index f4c1a8e1b..598ff3eef 100644 --- a/debug_toolbar/middleware.py +++ b/debug_toolbar/middleware.py @@ -6,7 +6,12 @@ import socket from functools import cache -from asgiref.sync import iscoroutinefunction, markcoroutinefunction, sync_to_async, async_to_sync +from asgiref.sync import ( + async_to_sync, + iscoroutinefunction, + markcoroutinefunction, + sync_to_async, +) from django.conf import settings from django.utils.module_loading import import_string @@ -45,6 +50,7 @@ def show_toolbar(request): # No test passed return False + @cache def show_toolbar_func_or_path(): """ diff --git a/tests/test_middleware.py b/tests/test_middleware.py index e4df5df9e..56081ce56 100644 --- a/tests/test_middleware.py +++ b/tests/test_middleware.py @@ -61,10 +61,14 @@ async def get_response(request): self.assertIn(b"djdt", response.content) @override_settings(DEBUG=True) - @patch("debug_toolbar.middleware.show_toolbar_func_or_path", return_value=ashow_toolbar_if_staff) + @patch( + "debug_toolbar.middleware.show_toolbar_func_or_path", + return_value=ashow_toolbar_if_staff, + ) def test_async_show_toolbar_callback_sync_middleware(self, mocked_show): def get_response(request): return HttpResponse("Hello world") + middleware = DebugToolbarMiddleware(get_response) request = self.factory.get("/") @@ -73,10 +77,14 @@ def get_response(request): self.assertIn(b"djdt", response.content) @override_settings(DEBUG=True) - @patch("debug_toolbar.middleware.show_toolbar_func_or_path", return_value=show_toolbar_if_staff) + @patch( + "debug_toolbar.middleware.show_toolbar_func_or_path", + return_value=show_toolbar_if_staff, + ) async def test_sync_show_toolbar_callback_async_middleware(self, mocked_show): async def get_response(request): return HttpResponse("Hello world") + middleware = DebugToolbarMiddleware(get_response) request = self.async_factory.get("/") From e18301206ee69c1dd28bb1899424ec48ae3669fa Mon Sep 17 00:00:00 2001 From: Aman Pandey Date: Tue, 25 Feb 2025 06:02:18 +0530 Subject: [PATCH 5/7] async compatible require_toolbar and tests --- debug_toolbar/decorators.py | 30 ++++++++++++++++++------ tests/test_decorators.py | 46 ++++++++++++++++++++++++++++++++++--- 2 files changed, 66 insertions(+), 10 deletions(-) diff --git a/debug_toolbar/decorators.py b/debug_toolbar/decorators.py index aafb8511c..61e46490d 100644 --- a/debug_toolbar/decorators.py +++ b/debug_toolbar/decorators.py @@ -1,5 +1,6 @@ import functools +from asgiref.sync import iscoroutinefunction from django.http import Http404 from django.utils.translation import get_language, override as language_override @@ -7,15 +8,30 @@ def require_show_toolbar(view): - @functools.wraps(view) - def inner(request, *args, **kwargs): - from debug_toolbar.middleware import get_show_toolbar + """ + Async compatible decorator to restrict access to a view + based on the Debug Toolbar's visibility settings. + """ + from debug_toolbar.middleware import get_show_toolbar + + if iscoroutinefunction(view): - show_toolbar = get_show_toolbar(async_mode=False) - if not show_toolbar(request): - raise Http404 + @functools.wraps(view) + async def inner(request, *args, **kwargs): + show_toolbar = get_show_toolbar(async_mode=True) + if not await show_toolbar(request): + raise Http404 - return view(request, *args, **kwargs) + return await view(request, *args, **kwargs) + else: + + @functools.wraps(view) + def inner(request, *args, **kwargs): + show_toolbar = get_show_toolbar(async_mode=False) + if not show_toolbar(request): + raise Http404 + + return view(request, *args, **kwargs) return inner diff --git a/tests/test_decorators.py b/tests/test_decorators.py index 5e7c8523b..9840a6390 100644 --- a/tests/test_decorators.py +++ b/tests/test_decorators.py @@ -1,10 +1,10 @@ from unittest.mock import patch -from django.http import HttpResponse -from django.test import RequestFactory, TestCase +from django.http import Http404, HttpResponse +from django.test import AsyncRequestFactory, RequestFactory, TestCase from django.test.utils import override_settings -from debug_toolbar.decorators import render_with_toolbar_language +from debug_toolbar.decorators import render_with_toolbar_language, require_show_toolbar @render_with_toolbar_language @@ -12,6 +12,46 @@ def stub_view(request): return HttpResponse(200) +@require_show_toolbar +def stub_require_toolbar_view(request): + return HttpResponse(200) + + +@require_show_toolbar +async def stub_require_toolbar_async_view(request): + return HttpResponse(200) + + +class TestRequireToolbar(TestCase): + """ + Tests require_toolbar functionality and async compatibility. + """ + + def setUp(self): + self.factory = RequestFactory() + self.async_factory = AsyncRequestFactory() + + @override_settings(DEBUG=True) + def test_require_toolbar_debug_true(self): + response = stub_require_toolbar_view(self.factory.get("/")) + self.assertEqual(response.status_code, 200) + + def test_require_toolbar_debug_false(self): + with self.assertRaises(Http404): + stub_require_toolbar_view(self.factory.get("/")) + + # Following tests additionally tests async compatibility + # of require_toolbar decorator + @override_settings(DEBUG=True) + async def test_require_toolbar_async_debug_true(self): + response = await stub_require_toolbar_async_view(self.async_factory.get("/")) + self.assertEqual(response.status_code, 200) + + async def test_require_toolbar_async_debug_false(self): + with self.assertRaises(Http404): + await stub_require_toolbar_async_view(self.async_factory.get("/")) + + @override_settings(DEBUG=True, LANGUAGE_CODE="fr") class RenderWithToolbarLanguageTestCase(TestCase): @override_settings(DEBUG_TOOLBAR_CONFIG={"TOOLBAR_LANGUAGE": "de"}) From 588a71b1b57c5dea65a658c0b2c2c8eaae758627 Mon Sep 17 00:00:00 2001 From: Aman Pandey Date: Tue, 25 Feb 2025 16:03:17 +0530 Subject: [PATCH 6/7] changes.rst --- docs/changes.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/changes.rst b/docs/changes.rst index 30d974a92..d8c6d60e7 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -8,6 +8,7 @@ Pending * Updated package metadata to include well-known labels. * Wrap ``SHOW_TOOLBAR_CALLBACK`` function with ``sync_to_async`` or ``async_to_sync`` to allow sync/async compatibility. +* Make ``require_toolbar`` decorator compatible to async views. 5.0.1 (2025-01-13) ------------------ From fc2130fe2b8910cfc491df523be6820e2ff62cc4 Mon Sep 17 00:00:00 2001 From: Aman Pandey Date: Tue, 25 Feb 2025 16:15:30 +0530 Subject: [PATCH 7/7] add docs for async require_toolbar --- docs/panels.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/panels.rst b/docs/panels.rst index 7892dcf94..be481fb6e 100644 --- a/docs/panels.rst +++ b/docs/panels.rst @@ -321,7 +321,7 @@ Panels can ship their own templates, static files and views. Any views defined for the third-party panel use the following decorators: - ``debug_toolbar.decorators.require_show_toolbar`` - Prevents unauthorized - access to the view. + access to the view. This decorator is compatible with async views. - ``debug_toolbar.decorators.render_with_toolbar_language`` - Supports internationalization for any content rendered by the view. This will render the response with the :ref:`TOOLBAR_LANGUAGE ` rather than