From baf9604ed8fed3e6e7ddfaca2d83c377c81399ae Mon Sep 17 00:00:00 2001 From: Alokik Vijay Date: Mon, 28 Mar 2022 21:26:20 +0530 Subject: [PATCH] Fixed #16406 -- Added ResolveMatch.captured_kwargs and extra_kwargs. Thanks Florian Apolloner for the review and implementation idea. --- django/urls/resolvers.py | 38 +++++++++++++++++---- docs/ref/urlresolvers.txt | 17 ++++++++- docs/releases/4.1.txt | 6 +++- tests/i18n/patterns/tests.py | 19 ++++++++++- tests/i18n/patterns/urls/default.py | 5 ++- tests/urlpatterns/more_urls.py | 7 +++- tests/urlpatterns/path_urls.py | 9 ++++- tests/urlpatterns/tests.py | 37 ++++++++++++++++++++ tests/urlpatterns_reverse/namespace_urls.py | 5 ++- tests/urlpatterns_reverse/tests.py | 27 ++++++++++++++- tests/urlpatterns_reverse/urls.py | 7 ++++ 11 files changed, 163 insertions(+), 14 deletions(-) diff --git a/django/urls/resolvers.py b/django/urls/resolvers.py index 26c9884e18..9f42e2738c 100644 --- a/django/urls/resolvers.py +++ b/django/urls/resolvers.py @@ -41,6 +41,8 @@ class ResolverMatch: namespaces=None, route=None, tried=None, + captured_kwargs=None, + extra_kwargs=None, ): self.func = func self.args = args @@ -48,6 +50,8 @@ class ResolverMatch: self.url_name = url_name self.route = route self.tried = tried + self.captured_kwargs = captured_kwargs + self.extra_kwargs = extra_kwargs # If a URLRegexResolver doesn't have a namespace or app_name, it passes # in an empty value. @@ -78,7 +82,7 @@ class ResolverMatch: func = self._func_path return ( "ResolverMatch(func=%s, args=%r, kwargs=%r, url_name=%r, " - "app_names=%r, namespaces=%r, route=%r)" + "app_names=%r, namespaces=%r, route=%r%s%s)" % ( func, self.args, @@ -87,6 +91,10 @@ class ResolverMatch: self.app_names, self.namespaces, self.route, + f", captured_kwargs={self.captured_kwargs!r}" + if self.captured_kwargs + else "", + f", extra_kwargs={self.extra_kwargs!r}" if self.extra_kwargs else "", ) ) @@ -416,11 +424,17 @@ class URLPattern: def resolve(self, path): match = self.pattern.match(path) if match: - new_path, args, kwargs = match - # Pass any extra_kwargs as **kwargs. - kwargs.update(self.default_args) + new_path, args, captured_kwargs = match + # Pass any default args as **kwargs. + kwargs = {**captured_kwargs, **self.default_args} return ResolverMatch( - self.callback, args, kwargs, self.pattern.name, route=str(self.pattern) + self.callback, + args, + kwargs, + self.pattern.name, + route=str(self.pattern), + captured_kwargs=captured_kwargs, + extra_kwargs=self.default_args, ) @cached_property @@ -678,6 +692,11 @@ class URLResolver: [self.namespace] + sub_match.namespaces, self._join_route(current_route, sub_match.route), tried, + captured_kwargs=sub_match.captured_kwargs, + extra_kwargs={ + **self.default_kwargs, + **sub_match.extra_kwargs, + }, ) tried.append([pattern]) raise Resolver404({"tried": tried, "path": new_path}) @@ -737,7 +756,14 @@ class URLResolver: else: if set(kwargs).symmetric_difference(params).difference(defaults): continue - if any(kwargs.get(k, v) != v for k, v in defaults.items()): + matches = True + for k, v in defaults.items(): + if k in params: + continue + if kwargs.get(k, v) != v: + matches = False + break + if not matches: continue candidate_subs = kwargs # Convert the candidate subs to text using Converter.to_url(). diff --git a/docs/ref/urlresolvers.txt b/docs/ref/urlresolvers.txt index d39345b478..2bde4d73ea 100644 --- a/docs/ref/urlresolvers.txt +++ b/docs/ref/urlresolvers.txt @@ -123,9 +123,24 @@ If the URL does not resolve, the function raises a .. attribute:: ResolverMatch.kwargs - The keyword arguments that would be passed to the view + All keyword arguments that would be passed to the view function, i.e. + :attr:`~ResolverMatch.captured_kwargs` and + :attr:`~ResolverMatch.extra_kwargs`. + + .. attribute:: ResolverMatch.captured_kwargs + + .. versionadded:: 4.1 + + The captured keyword arguments that would be passed to the view function, as parsed from the URL. + .. attribute:: ResolverMatch.extra_kwargs + + .. versionadded:: 4.1 + + The additional keyword arguments that would be passed to the view + function. + .. attribute:: ResolverMatch.url_name The name of the URL pattern that matches the URL. diff --git a/docs/releases/4.1.txt b/docs/releases/4.1.txt index c7159bd4f2..eb4bde652b 100644 --- a/docs/releases/4.1.txt +++ b/docs/releases/4.1.txt @@ -326,7 +326,11 @@ Tests URLs ~~~~ -* ... +* The new :attr:`.ResolverMatch.captured_kwargs` attribute stores the captured + keyword arguments, as parsed from the URL. + +* The new :attr:`.ResolverMatch.extra_kwargs` attribute stores the additional + keyword arguments passed to the view function. Utilities ~~~~~~~~~ diff --git a/tests/i18n/patterns/tests.py b/tests/i18n/patterns/tests.py index db04e9a1a4..43d95ff668 100644 --- a/tests/i18n/patterns/tests.py +++ b/tests/i18n/patterns/tests.py @@ -8,7 +8,7 @@ from django.template import Context, Template from django.test import SimpleTestCase, override_settings from django.test.client import RequestFactory from django.test.utils import override_script_prefix -from django.urls import clear_url_caches, reverse, translate_url +from django.urls import clear_url_caches, resolve, reverse, translate_url from django.utils import translation @@ -198,6 +198,23 @@ class URLTranslationTests(URLTestCaseBase): self.assertEqual(translate_url("/nl/gebruikers/", "en"), "/en/users/") self.assertEqual(translation.get_language(), "nl") + def test_reverse_translated_with_captured_kwargs(self): + with translation.override("en"): + match = resolve("/translated/apo/") + # Links to the same page in other languages. + tests = [ + ("nl", "/vertaald/apo/"), + ("pt-br", "/traduzidos/apo/"), + ] + for lang, expected_link in tests: + with translation.override(lang): + self.assertEqual( + reverse( + match.url_name, args=match.args, kwargs=match.captured_kwargs + ), + expected_link, + ) + class URLNamespaceTests(URLTestCaseBase): """ diff --git a/tests/i18n/patterns/urls/default.py b/tests/i18n/patterns/urls/default.py index c77bf98c73..418e9f5685 100644 --- a/tests/i18n/patterns/urls/default.py +++ b/tests/i18n/patterns/urls/default.py @@ -10,7 +10,10 @@ urlpatterns = [ path("not-prefixed-include/", include("i18n.patterns.urls.included")), re_path(_(r"^translated/$"), view, name="no-prefix-translated"), re_path( - _(r"^translated/(?P[\w-]+)/$"), view, name="no-prefix-translated-slug" + _(r"^translated/(?P[\w-]+)/$"), + view, + {"slug": "default-slug"}, + name="no-prefix-translated-slug", ), ] diff --git a/tests/urlpatterns/more_urls.py b/tests/urlpatterns/more_urls.py index bbfd962c4e..8fc3abd518 100644 --- a/tests/urlpatterns/more_urls.py +++ b/tests/urlpatterns/more_urls.py @@ -3,5 +3,10 @@ from django.urls import re_path from . import views urlpatterns = [ - re_path(r"^more/(?P\w+)/$", views.empty_view, name="inner-more"), + re_path( + r"^more/(?P\w+)/$", + views.empty_view, + {"sub-extra": True}, + name="inner-more", + ), ] diff --git a/tests/urlpatterns/path_urls.py b/tests/urlpatterns/path_urls.py index 5ebdfe1b16..8a356b5463 100644 --- a/tests/urlpatterns/path_urls.py +++ b/tests/urlpatterns/path_urls.py @@ -13,6 +13,13 @@ urlpatterns = [ views.empty_view, name="articles-year-month-day", ), + path("books/2007/", views.empty_view, {"extra": True}, name="books-2007"), + path( + "books////", + views.empty_view, + {"extra": True}, + name="books-year-month-day", + ), path("users/", views.empty_view, name="users"), path("users//", views.empty_view, name="user-with-id"), path("included_urls/", include("urlpatterns.included_urls")), @@ -27,6 +34,6 @@ urlpatterns = [ views.empty_view, name="regex_only_optional", ), - path("", include("urlpatterns.more_urls")), + path("", include("urlpatterns.more_urls"), {"sub-extra": False}), path("//", views.empty_view, name="lang-and-path"), ] diff --git a/tests/urlpatterns/tests.py b/tests/urlpatterns/tests.py index cbe61278da..f8d73fdb4a 100644 --- a/tests/urlpatterns/tests.py +++ b/tests/urlpatterns/tests.py @@ -34,6 +34,8 @@ class SimplifiedURLTests(SimpleTestCase): self.assertEqual(match.args, ()) self.assertEqual(match.kwargs, {}) self.assertEqual(match.route, "articles/2003/") + self.assertEqual(match.captured_kwargs, {}) + self.assertEqual(match.extra_kwargs, {}) def test_path_lookup_with_typed_parameters(self): match = resolve("/articles/2015/") @@ -41,6 +43,8 @@ class SimplifiedURLTests(SimpleTestCase): self.assertEqual(match.args, ()) self.assertEqual(match.kwargs, {"year": 2015}) self.assertEqual(match.route, "articles//") + self.assertEqual(match.captured_kwargs, {"year": 2015}) + self.assertEqual(match.extra_kwargs, {}) def test_path_lookup_with_multiple_parameters(self): match = resolve("/articles/2015/04/12/") @@ -48,18 +52,44 @@ class SimplifiedURLTests(SimpleTestCase): self.assertEqual(match.args, ()) self.assertEqual(match.kwargs, {"year": 2015, "month": 4, "day": 12}) self.assertEqual(match.route, "articles////") + self.assertEqual(match.captured_kwargs, {"year": 2015, "month": 4, "day": 12}) + self.assertEqual(match.extra_kwargs, {}) + + def test_path_lookup_with_multiple_parameters_and_extra_kwarg(self): + match = resolve("/books/2015/04/12/") + self.assertEqual(match.url_name, "books-year-month-day") + self.assertEqual(match.args, ()) + self.assertEqual( + match.kwargs, {"year": 2015, "month": 4, "day": 12, "extra": True} + ) + self.assertEqual(match.route, "books////") + self.assertEqual(match.captured_kwargs, {"year": 2015, "month": 4, "day": 12}) + self.assertEqual(match.extra_kwargs, {"extra": True}) + + def test_path_lookup_with_extra_kwarg(self): + match = resolve("/books/2007/") + self.assertEqual(match.url_name, "books-2007") + self.assertEqual(match.args, ()) + self.assertEqual(match.kwargs, {"extra": True}) + self.assertEqual(match.route, "books/2007/") + self.assertEqual(match.captured_kwargs, {}) + self.assertEqual(match.extra_kwargs, {"extra": True}) def test_two_variable_at_start_of_path_pattern(self): match = resolve("/en/foo/") self.assertEqual(match.url_name, "lang-and-path") self.assertEqual(match.kwargs, {"lang": "en", "url": "foo"}) self.assertEqual(match.route, "//") + self.assertEqual(match.captured_kwargs, {"lang": "en", "url": "foo"}) + self.assertEqual(match.extra_kwargs, {}) def test_re_path(self): match = resolve("/regex/1/") self.assertEqual(match.url_name, "regex") self.assertEqual(match.kwargs, {"pk": "1"}) self.assertEqual(match.route, "^regex/(?P[0-9]+)/$") + self.assertEqual(match.captured_kwargs, {"pk": "1"}) + self.assertEqual(match.extra_kwargs, {}) def test_re_path_with_optional_parameter(self): for url, kwargs in ( @@ -74,6 +104,8 @@ class SimplifiedURLTests(SimpleTestCase): match.route, r"^regex_optional/(?P\d+)/(?:(?P\d+)/)?", ) + self.assertEqual(match.captured_kwargs, kwargs) + self.assertEqual(match.extra_kwargs, {}) def test_re_path_with_missing_optional_parameter(self): match = resolve("/regex_only_optional/") @@ -84,6 +116,8 @@ class SimplifiedURLTests(SimpleTestCase): match.route, r"^regex_only_optional/(?:(?P\d+)/)?", ) + self.assertEqual(match.captured_kwargs, {}) + self.assertEqual(match.extra_kwargs, {}) def test_path_lookup_with_inclusion(self): match = resolve("/included_urls/extra/something/") @@ -94,6 +128,9 @@ class SimplifiedURLTests(SimpleTestCase): match = resolve("/more/99/") self.assertEqual(match.url_name, "inner-more") self.assertEqual(match.route, r"^more/(?P\w+)/$") + self.assertEqual(match.kwargs, {"extra": "99", "sub-extra": True}) + self.assertEqual(match.captured_kwargs, {"extra": "99"}) + self.assertEqual(match.extra_kwargs, {"sub-extra": True}) def test_path_lookup_with_double_inclusion(self): match = resolve("/included_urls/more/some_value/") diff --git a/tests/urlpatterns_reverse/namespace_urls.py b/tests/urlpatterns_reverse/namespace_urls.py index 2e3db3ecc3..4a39ab2290 100644 --- a/tests/urlpatterns_reverse/namespace_urls.py +++ b/tests/urlpatterns_reverse/namespace_urls.py @@ -23,7 +23,10 @@ urlpatterns = [ path("resolver_match/", views.pass_resolver_match_view, name="test-resolver-match"), re_path(r"^\+\\\$\*/$", views.empty_view, name="special-view"), re_path( - r"^mixed_args/([0-9]+)/(?P[0-9]+)/$", views.empty_view, name="mixed-args" + r"^mixed_args/([0-9]+)/(?P[0-9]+)/$", + views.empty_view, + {"extra": True}, + name="mixed-args", ), re_path(r"^no_kwargs/([0-9]+)/([0-9]+)/$", views.empty_view, name="no-kwargs"), re_path( diff --git a/tests/urlpatterns_reverse/tests.py b/tests/urlpatterns_reverse/tests.py index 59c22dd589..89dfd0deba 100644 --- a/tests/urlpatterns_reverse/tests.py +++ b/tests/urlpatterns_reverse/tests.py @@ -89,7 +89,7 @@ resolve_test_data = ( "mixed-args", views.empty_view, (), - {"arg2": "37"}, + {"extra": True, "arg2": "37"}, ), ( "/included/mixed_args/42/37/", @@ -1554,6 +1554,16 @@ class ResolverMatchTests(SimpleTestCase): "namespaces=[], route='^no_kwargs/([0-9]+)/([0-9]+)/$')", ) + def test_repr_extra_kwargs(self): + self.assertEqual( + repr(resolve("/mixed_args/1986/11/")), + "ResolverMatch(func=urlpatterns_reverse.views.empty_view, args=(), " + "kwargs={'arg2': '11', 'extra': True}, url_name='mixed-args', " + "app_names=[], namespaces=[], " + "route='^mixed_args/([0-9]+)/(?P[0-9]+)/$', " + "captured_kwargs={'arg2': '11'}, extra_kwargs={'extra': True})", + ) + @override_settings(ROOT_URLCONF="urlpatterns_reverse.reverse_lazy_urls") def test_classbased_repr(self): self.assertEqual( @@ -1758,3 +1768,18 @@ class LookaheadTests(SimpleTestCase): with self.subTest(name=name, kwargs=kwargs): with self.assertRaises(NoReverseMatch): reverse(name, kwargs=kwargs) + + +@override_settings(ROOT_URLCONF="urlpatterns_reverse.urls") +class ReverseResolvedTests(SimpleTestCase): + def test_rereverse(self): + match = resolve("/resolved/12/") + self.assertEqual( + reverse(match.url_name, args=match.args, kwargs=match.kwargs), + "/resolved/12/", + ) + match = resolve("/resolved-overridden/12/url/") + self.assertEqual( + reverse(match.url_name, args=match.args, kwargs=match.captured_kwargs), + "/resolved-overridden/12/url/", + ) diff --git a/tests/urlpatterns_reverse/urls.py b/tests/urlpatterns_reverse/urls.py index ba3f51b563..c745331483 100644 --- a/tests/urlpatterns_reverse/urls.py +++ b/tests/urlpatterns_reverse/urls.py @@ -78,6 +78,13 @@ urlpatterns = [ name="windows", ), re_path(r"^special_chars/(?P.+)/$", empty_view, name="special"), + re_path(r"^resolved/(?P\d+)/$", empty_view, {"extra": True}, name="resolved"), + re_path( + r"^resolved-overridden/(?P\d+)/(?P\w+)/$", + empty_view, + {"extra": True, "overridden": "default"}, + name="resolved-overridden", + ), re_path(r"^(?P.+)/[0-9]+/$", empty_view, name="mixed"), re_path(r"^repeats/a{1,2}/$", empty_view, name="repeats"), re_path(r"^repeats/a{2,4}/$", empty_view, name="repeats2"),