From 974942a75039ba43e618f6a5ff95e08b5d5176fd Mon Sep 17 00:00:00 2001 From: Mariusz Felisiak Date: Thu, 1 Sep 2022 21:09:16 +0200 Subject: [PATCH] Fixed #33955, Fixed #33971 -- Reverted "Fixed #32565 -- Moved internal URLResolver view-strings mapping to admindocs." This reverts commit 7f3cfaa12b28d15c0ca78bb692bfd6e59d17bff1. Thanks Tom Carrick and Greg Kaleka for reports. --- django/contrib/admindocs/apps.py | 8 ------ django/contrib/admindocs/utils.py | 44 ------------------------------ django/contrib/admindocs/views.py | 5 ++-- django/urls/resolvers.py | 25 +++++++++++++++++ docs/releases/4.1.1.txt | 4 +++ tests/admin_docs/test_utils.py | 29 +------------------- tests/urlpatterns_reverse/tests.py | 21 ++++++++++++++ 7 files changed, 54 insertions(+), 82 deletions(-) diff --git a/django/contrib/admindocs/apps.py b/django/contrib/admindocs/apps.py index 2fda5fb4fb0..e79dc892cb4 100644 --- a/django/contrib/admindocs/apps.py +++ b/django/contrib/admindocs/apps.py @@ -1,15 +1,7 @@ from django.apps import AppConfig -from django.urls import get_resolver, get_urlconf from django.utils.translation import gettext_lazy as _ -from .utils import _active, register_callback - class AdminDocsConfig(AppConfig): name = "django.contrib.admindocs" verbose_name = _("Administrative Documentation") - - def ready(self): - urlconf = get_urlconf() - urlresolver = get_resolver(urlconf) - register_callback(urlresolver, _active.local_value) diff --git a/django/contrib/admindocs/utils.py b/django/contrib/admindocs/utils.py index a18dbe12cd2..6edff502ecd 100644 --- a/django/contrib/admindocs/utils.py +++ b/django/contrib/admindocs/utils.py @@ -1,15 +1,11 @@ "Misc. utility functions/classes for admin documentation generator." -import functools import re from email.errors import HeaderParseError from email.parser import HeaderParser from inspect import cleandoc -from asgiref.local import Local - from django.urls import reverse -from django.urls.resolvers import URLPattern from django.utils.regex_helper import _lazy_re_compile from django.utils.safestring import mark_safe @@ -243,43 +239,3 @@ def remove_non_capturing_groups(pattern): final_pattern += pattern[prev_end:start] prev_end = end return final_pattern + pattern[prev_end:] - - -# Callback strings are cached in a dictionary for every urlconf. -# The active calback_strs are stored by thread id to make them thread local. -_callback_strs = set() -_active = Local() -_active.local_value = _callback_strs - - -def _is_callback(name, urlresolver=None): - if urlresolver and not urlresolver._populated: - register_callback(urlresolver, _active.local_value) - return name in _active.local_value - - -@functools.lru_cache(maxsize=None) -def lookup_str(urlpattern): - """ - A string that identifies the view (e.g. 'path.to.view_function' or - 'path.to.ClassBasedView'). - """ - callback = urlpattern.callback - if isinstance(callback, functools.partial): - callback = callback.func - if hasattr(callback, "view_class"): - callback = callback.view_class - elif not hasattr(callback, "__name__"): - return callback.__module__ + "." + callback.__class__.__name__ - return callback.__module__ + "." + callback.__qualname__ - - -def register_callback(urlresolver, thread): - for url_pattern in reversed(urlresolver.url_patterns): - if isinstance(url_pattern, URLPattern): - thread.add(lookup_str(url_pattern)) - else: # url_pattern is a URLResolver. - _active.url_pattern_value = _callback_strs - register_callback(url_pattern, _active.url_pattern_value) - thread.update(_active.url_pattern_value) - urlresolver._populated = True diff --git a/django/contrib/admindocs/views.py b/django/contrib/admindocs/views.py index f771e8f3912..1d085410577 100644 --- a/django/contrib/admindocs/views.py +++ b/django/contrib/admindocs/views.py @@ -30,7 +30,7 @@ from django.utils.inspect import ( from django.utils.translation import gettext as _ from django.views.generic import TemplateView -from .utils import _is_callback, get_view_name +from .utils import get_view_name # Exclude methods starting with these strings from documentation MODEL_METHODS_EXCLUDE = ("_", "add_", "delete", "save", "set_") @@ -166,7 +166,8 @@ class ViewDetailView(BaseAdminDocsView): @staticmethod def _get_view_func(view): - if _is_callback(view): + urlconf = get_urlconf() + if get_resolver(urlconf)._is_callback(view): mod, func = get_mod_func(view) try: # Separate the module and function, e.g. diff --git a/django/urls/resolvers.py b/django/urls/resolvers.py index 7c0d4fd85af..9f42e2738cf 100644 --- a/django/urls/resolvers.py +++ b/django/urls/resolvers.py @@ -437,6 +437,21 @@ class URLPattern: extra_kwargs=self.default_args, ) + @cached_property + def lookup_str(self): + """ + A string that identifies the view (e.g. 'path.to.view_function' or + 'path.to.ClassBasedView'). + """ + callback = self.callback + if isinstance(callback, functools.partial): + callback = callback.func + if hasattr(callback, "view_class"): + callback = callback.view_class + elif not hasattr(callback, "__name__"): + return callback.__module__ + "." + callback.__class__.__name__ + return callback.__module__ + "." + callback.__qualname__ + class URLResolver: def __init__( @@ -454,6 +469,9 @@ class URLResolver: self._reverse_dict = {} self._namespace_dict = {} self._app_dict = {} + # set of dotted paths to all functions and classes that are used in + # urlpatterns + self._callback_strs = set() self._populated = False self._local = Local() @@ -527,6 +545,7 @@ class URLResolver: if p_pattern.startswith("^"): p_pattern = p_pattern[1:] if isinstance(url_pattern, URLPattern): + self._callback_strs.add(url_pattern.lookup_str) bits = normalize(url_pattern.pattern.regex.pattern) lookups.appendlist( url_pattern.callback, @@ -585,6 +604,7 @@ class URLResolver: namespaces[namespace] = (p_pattern + prefix, sub_pattern) for app_name, namespace_list in url_pattern.app_dict.items(): apps.setdefault(app_name, []).extend(namespace_list) + self._callback_strs.update(url_pattern._callback_strs) self._namespace_dict[language_code] = namespaces self._app_dict[language_code] = apps self._reverse_dict[language_code] = lookups @@ -629,6 +649,11 @@ class URLResolver: route2 = route2[1:] return route1 + route2 + def _is_callback(self, name): + if not self._populated: + self._populate() + return name in self._callback_strs + def resolve(self, path): path = str(path) # path may be a reverse_lazy object tried = [] diff --git a/docs/releases/4.1.1.txt b/docs/releases/4.1.1.txt index 2e4782f9040..193741762e9 100644 --- a/docs/releases/4.1.1.txt +++ b/docs/releases/4.1.1.txt @@ -55,3 +55,7 @@ Bugfixes * Fixed a regression in Django 4.1 that caused a migration crash on SQLite < 3.20 (:ticket:`33960`). + +* Fixed a regression in Django 4.1 that caused an admin crash when the + :mod:`~django.contrib.admindocs` app was used (:ticket:`33955`, + :ticket:`33971`). diff --git a/tests/admin_docs/test_utils.py b/tests/admin_docs/test_utils.py index e26a4ca1f30..18c6769fade 100644 --- a/tests/admin_docs/test_utils.py +++ b/tests/admin_docs/test_utils.py @@ -1,15 +1,13 @@ import unittest from django.contrib.admindocs.utils import ( - _is_callback, docutils_is_available, parse_docstring, parse_rst, ) from django.test.utils import captured_stderr -from django.urls import get_resolver -from .tests import AdminDocsSimpleTestCase, SimpleTestCase +from .tests import AdminDocsSimpleTestCase @unittest.skipUnless(docutils_is_available, "no docutils installed.") @@ -121,28 +119,3 @@ class TestUtils(AdminDocsSimpleTestCase): markup = "

reST, interpreted text, default role.

\n" parts = docutils.core.publish_parts(source=source, writer_name="html4css1") self.assertEqual(parts["fragment"], markup) - - -class TestResolver(SimpleTestCase): - def test_namespaced_view_detail(self): - resolver = get_resolver("urlpatterns_reverse.nested_urls") - self.assertTrue(_is_callback("urlpatterns_reverse.nested_urls.view1", resolver)) - self.assertTrue(_is_callback("urlpatterns_reverse.nested_urls.view2", resolver)) - self.assertTrue(_is_callback("urlpatterns_reverse.nested_urls.View3", resolver)) - self.assertFalse(_is_callback("urlpatterns_reverse.nested_urls.blub", resolver)) - - def test_view_detail_as_method(self): - # Views which have a class name as part of their path. - resolver = get_resolver("urlpatterns_reverse.method_view_urls") - self.assertTrue( - _is_callback( - "urlpatterns_reverse.method_view_urls.ViewContainer.method_view", - resolver, - ) - ) - self.assertTrue( - _is_callback( - "urlpatterns_reverse.method_view_urls.ViewContainer.classmethod_view", - resolver, - ) - ) diff --git a/tests/urlpatterns_reverse/tests.py b/tests/urlpatterns_reverse/tests.py index 73159b6bb82..89dfd0deba4 100644 --- a/tests/urlpatterns_reverse/tests.py +++ b/tests/urlpatterns_reverse/tests.py @@ -640,6 +640,27 @@ class ResolverTests(SimpleTestCase): % (e["name"], t.name), ) + def test_namespaced_view_detail(self): + resolver = get_resolver("urlpatterns_reverse.nested_urls") + self.assertTrue(resolver._is_callback("urlpatterns_reverse.nested_urls.view1")) + self.assertTrue(resolver._is_callback("urlpatterns_reverse.nested_urls.view2")) + self.assertTrue(resolver._is_callback("urlpatterns_reverse.nested_urls.View3")) + self.assertFalse(resolver._is_callback("urlpatterns_reverse.nested_urls.blub")) + + def test_view_detail_as_method(self): + # Views which have a class name as part of their path. + resolver = get_resolver("urlpatterns_reverse.method_view_urls") + self.assertTrue( + resolver._is_callback( + "urlpatterns_reverse.method_view_urls.ViewContainer.method_view" + ) + ) + self.assertTrue( + resolver._is_callback( + "urlpatterns_reverse.method_view_urls.ViewContainer.classmethod_view" + ) + ) + def test_populate_concurrency(self): """ URLResolver._populate() can be called concurrently, but not more