Browse Source

Fixed #16406 -- Added ResolveMatch.captured_kwargs and extra_kwargs.

Thanks Florian Apolloner for the review and implementation idea.
Alokik Vijay 3 years ago
parent
commit
baf9604ed8

+ 32 - 6
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().

+ 16 - 1
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.

+ 5 - 1
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
 ~~~~~~~~~

+ 18 - 1
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):
     """

+ 4 - 1
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<slug>[\w-]+)/$"), view, name="no-prefix-translated-slug"
+        _(r"^translated/(?P<slug>[\w-]+)/$"),
+        view,
+        {"slug": "default-slug"},
+        name="no-prefix-translated-slug",
     ),
 ]
 

+ 6 - 1
tests/urlpatterns/more_urls.py

@@ -3,5 +3,10 @@ from django.urls import re_path
 from . import views
 
 urlpatterns = [
-    re_path(r"^more/(?P<extra>\w+)/$", views.empty_view, name="inner-more"),
+    re_path(
+        r"^more/(?P<extra>\w+)/$",
+        views.empty_view,
+        {"sub-extra": True},
+        name="inner-more",
+    ),
 ]

+ 8 - 1
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/<int:year>/<int:month>/<int:day>/",
+        views.empty_view,
+        {"extra": True},
+        name="books-year-month-day",
+    ),
     path("users/", views.empty_view, name="users"),
     path("users/<id>/", 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("<lang>/<path:url>/", views.empty_view, name="lang-and-path"),
 ]

+ 37 - 0
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/<int:year>/")
+        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/<int:year>/<int:month>/<int:day>/")
+        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/<int:year>/<int:month>/<int:day>/")
+        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, "<lang>/<path:url>/")
+        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<pk>[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<arg1>\d+)/(?:(?P<arg2>\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<arg1>\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<extra>\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/")

+ 4 - 1
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<arg2>[0-9]+)/$", views.empty_view, name="mixed-args"
+        r"^mixed_args/([0-9]+)/(?P<arg2>[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(

+ 26 - 1
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<arg2>[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/",
+        )

+ 7 - 0
tests/urlpatterns_reverse/urls.py

@@ -78,6 +78,13 @@ urlpatterns = [
         name="windows",
     ),
     re_path(r"^special_chars/(?P<chars>.+)/$", empty_view, name="special"),
+    re_path(r"^resolved/(?P<arg>\d+)/$", empty_view, {"extra": True}, name="resolved"),
+    re_path(
+        r"^resolved-overridden/(?P<arg>\d+)/(?P<overridden>\w+)/$",
+        empty_view,
+        {"extra": True, "overridden": "default"},
+        name="resolved-overridden",
+    ),
     re_path(r"^(?P<name>.+)/[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"),