From 68039c6833410a8f27ccd0d5b574c14c2cb792e8 Mon Sep 17 00:00:00 2001 From: Daniel Harding Date: Mon, 11 Mar 2024 19:47:26 +0300 Subject: [PATCH 1/2] Simplify default OBSERVE_REQUEST_CALLBACK behavior The previous implementation of observe_request(), the default callback for OBSERVE_REQUEST_CALLBACK, was checking for DebugToolbar.is_toolbar_request() and returning True only if that method returned False. However, this is actually unneeded, because DebugToolbarMiddleware never instruments its own requests. If DebugToolbar.is_toolbar_request() returns True for a given request, DebugToolbarMiddleware will return early without performing any instrumentation or processing on the request [1]. In this case, the OBSERVE_REQUEST_CALLBACK callback never gets called, because it only gets called via the HistoryPanel.get_headers() method [2]. The .get_headers() method is only called by DebugToolbarMiddleware after the early return mentioned above [3]. Thus if OBSERVE_REQUEST_CALLBACK is called, it must be the case that DebugToolbar.is_toolbar_request() is False for the current request. Therefore observe_request() can be simplified to always return True without changing its behavior. [1] https://github.com/jazzband/django-debug-toolbar/blob/c688ce4ad7d18c5ecb800869298ca8cf6c08be1d/debug_toolbar/middleware.py#L48-L49 [2] https://github.com/jazzband/django-debug-toolbar/blob/c688ce4ad7d18c5ecb800869298ca8cf6c08be1d/debug_toolbar/panels/history/panel.py#L23-L29 [3] https://github.com/jazzband/django-debug-toolbar/blob/c688ce4ad7d18c5ecb800869298ca8cf6c08be1d/debug_toolbar/middleware.py#L76-L77 --- debug_toolbar/toolbar.py | 2 +- docs/configuration.rst | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/debug_toolbar/toolbar.py b/debug_toolbar/toolbar.py index 11f8a1daa..fc07543b5 100644 --- a/debug_toolbar/toolbar.py +++ b/debug_toolbar/toolbar.py @@ -185,4 +185,4 @@ def observe_request(request): """ Determine whether to update the toolbar from a client side request. """ - return not DebugToolbar.is_toolbar_request(request) + return True diff --git a/docs/configuration.rst b/docs/configuration.rst index 8271092ca..2109e7c52 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -146,9 +146,8 @@ Toolbar options Default: ``'debug_toolbar.toolbar.observe_request'`` This is the dotted path to a function used for determining whether the - toolbar should update on AJAX requests or not. The default checks are that - the request doesn't originate from the toolbar itself, EG that - ``is_toolbar_request`` is false for a given request. + toolbar should update on AJAX requests or not. The default implementation + always returns ``True``. .. _TOOLBAR_LANGUAGE: From cfbad48862e2bbff2a614206ed77dbfd91ec315e Mon Sep 17 00:00:00 2001 From: Daniel Harding Date: Mon, 11 Mar 2024 16:43:23 +0300 Subject: [PATCH 2/2] Deprecate OBSERVE_REQUEST_CALLBACK With the addition of the UPDATE_ON_FETCH setting, the OBSERVE_REQUEST_CALLBACK setting is redundant. Thus add a check debug_toolbar.W008 to warn if it is present in DEBUG_TOOLBAR_CONFIG, but do not remove it yet. See #1886 --- debug_toolbar/apps.py | 15 +++++++++++++++ docs/changes.rst | 3 +++ docs/checks.rst | 3 +++ docs/configuration.rst | 5 +++++ tests/test_checks.py | 8 ++++++++ 5 files changed, 34 insertions(+) diff --git a/debug_toolbar/apps.py b/debug_toolbar/apps.py index 0a10a4b08..05cd35ae3 100644 --- a/debug_toolbar/apps.py +++ b/debug_toolbar/apps.py @@ -206,3 +206,18 @@ def js_mimetype_check(app_configs, **kwargs): ) ] return [] + + +@register() +def check_settings(app_configs, **kwargs): + errors = [] + USER_CONFIG = getattr(settings, "DEBUG_TOOLBAR_CONFIG", {}) + if "OBSERVE_REQUEST_CALLBACK" in USER_CONFIG: + errors.append( + Warning( + "The deprecated OBSERVE_REQUEST_CALLBACK setting is present in DEBUG_TOOLBAR_CONFIG.", + hint="Use the UPDATE_ON_FETCH and/or SHOW_TOOLBAR_CALLBACK settings instead.", + id="debug_toolbar.W008", + ) + ) + return errors diff --git a/docs/changes.rst b/docs/changes.rst index 5e6fbb24d..b461fbb40 100644 --- a/docs/changes.rst +++ b/docs/changes.rst @@ -17,6 +17,9 @@ Pending :class:`StaticFilesPanel ` since that check is made redundant by a similar check in Django 4.0 and later. +* Deprecated the ``OBSERVE_REQUEST_CALLBACK`` setting and added check + ``debug_toolbar.W008`` to warn when it is present in + ``DEBUG_TOOLBAR_SETTINGS``. 4.3.0 (2024-02-01) ------------------ diff --git a/docs/checks.rst b/docs/checks.rst index 6ed1e88f4..1c41d04fc 100644 --- a/docs/checks.rst +++ b/docs/checks.rst @@ -21,3 +21,6 @@ Django Debug Toolbar setup and configuration: * **debug_toolbar.W007**: JavaScript files are resolving to the wrong content type. Refer to :external:ref:`Django's explanation of mimetypes on Windows `. +* **debug_toolbar.W008**: The deprecated ``OBSERVE_REQUEST_CALLBACK`` setting + is present in ``DEBUG_TOOLBAR_CONFIG``. Use the ``UPDATE_ON_FETCH`` and/or + ``SHOW_TOOLBAR_CALLBACK`` settings instead. diff --git a/docs/configuration.rst b/docs/configuration.rst index 2109e7c52..b7db25900 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -145,6 +145,11 @@ Toolbar options Default: ``'debug_toolbar.toolbar.observe_request'`` + .. note:: + + This setting is deprecated in favor of the ``UPDATE_ON_FETCH`` and + ``SHOW_TOOLBAR_CALLBACK`` settings. + This is the dotted path to a function used for determining whether the toolbar should update on AJAX requests or not. The default implementation always returns ``True``. diff --git a/tests/test_checks.py b/tests/test_checks.py index b88787f9d..d04f8fc87 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -235,3 +235,11 @@ def test_check_w007_invalid(self, mocked_guess_type): ) ], ) + + @override_settings( + DEBUG_TOOLBAR_CONFIG={"OBSERVE_REQUEST_CALLBACK": lambda request: False} + ) + def test_observe_request_callback_specified(self): + errors = run_checks() + self.assertEqual(len(errors), 1) + self.assertEqual(errors[0].id, "debug_toolbar.W008")