From 015a9b928af233ca036b93a4d7eecf18f15cbc2b Mon Sep 17 00:00:00 2001 From: Bas Peschier Date: Thu, 19 Mar 2015 08:56:38 +0100 Subject: [PATCH] [1.8.x] Fixed #13525 -- Added tests and docs for nested parameters in URL patterns. When reversing, only outer parameters are used if captured parameters are nested. Added tests to check the edge cases and documentation for the behavior with an example to avoid it. Backport of 23a5d64f40b0f4a3fbfef7427ca793cb1df1034e from master --- docs/topics/http/urls.txt | 36 ++++++++++++++++++++++++++++++ tests/urlpatterns_reverse/tests.py | 17 ++++++++++++++ tests/urlpatterns_reverse/urls.py | 6 +++++ 3 files changed, 59 insertions(+) diff --git a/docs/topics/http/urls.txt b/docs/topics/http/urls.txt index a7dbb72462..f4cf4b6029 100644 --- a/docs/topics/http/urls.txt +++ b/docs/topics/http/urls.txt @@ -368,6 +368,42 @@ the following example is valid:: In the above example, the captured ``"username"`` variable is passed to the included URLconf, as expected. +Nested arguments +================ + +Regular expressions allow nested arguments, and Django will resolve them and +pass them to the view. When reversing, Django will try to fill in all outer +captured arguments, ignoring any nested captured arguments. Consider the +following URL patterns which optionally take a page argument:: + + from django.conf.urls import url + + urlpatterns = [ + url(r'blog/(page-(\d+)/)?$', blog_articles), # bad + url(r'comments/(?:page-(?P\d+)/)?$', comments), # good + ] + +Both patterns use nested arguments and will resolve: for example, +``blog/page-2/`` will result in a match to ``blog_articles`` with two +positional arguments: ``page-2/`` and ``2``. The second pattern for +``comments`` will match ``comments/page-2/`` with keyword argument +``page_number`` set to 2. The outer argument in this case is a non-capturing +argument ``(?:...)``. + +The ``blog_articles`` view needs the outermost captured argument to be reversed, +``page-2/`` or no arguments in this case, while ``comments`` can be reversed +with either no arguments or a value for ``page_number``. + +Nested captured arguments create a strong coupling between the view arguments +and the URL as illustrated by ``blog_articles``: the view receives part of the +URL (``page-2/``) instead of only the value the view is interested in. This +coupling is even more pronounced when reversing, since to reverse the view we +need to pass the piece of URL instead of the page number. + +As a rule of thumb, only capture the values the view needs to work with and +use non-capturing arguments when the regular expression needs an argument but +the view ignores it. + .. _views-extra-options: Passing extra options to view functions diff --git a/tests/urlpatterns_reverse/tests.py b/tests/urlpatterns_reverse/tests.py index 7c8c17a928..8f8496a515 100644 --- a/tests/urlpatterns_reverse/tests.py +++ b/tests/urlpatterns_reverse/tests.py @@ -94,6 +94,12 @@ test_data = ( ('people_backref', '/people/nate-nate/', [], {'name': 'nate'}), ('optional', '/optional/fred/', [], {'name': 'fred'}), ('optional', '/optional/fred/', ['fred'], {}), + ('named_optional', '/optional/1/', [1], {}), + ('named_optional', '/optional/1/', [], {'arg1': 1}), + ('named_optional', '/optional/1/2/', [1, 2], {}), + ('named_optional', '/optional/1/2/', [], {'arg1': 1, 'arg2': 2}), + ('named_optional_terminated', '/optional/1/2/', [1, 2], {}), + ('named_optional_terminated', '/optional/1/2/', [], {'arg1': 1, 'arg2': 2}), ('hardcoded', '/hardcoded/', [], {}), ('hardcoded2', '/hardcoded/doc.pdf', [], {}), ('people3', '/people/il/adrian/', [], {'state': 'il', 'name': 'adrian'}), @@ -142,6 +148,17 @@ test_data = ( ('part2', '/prefix/xx/part2/one/', [], {'value': 'one', 'prefix': 'xx'}), ('part2', '/prefix/xx/part2/', [], {'prefix': 'xx'}), + # Tests for nested groups. Nested capturing groups will only work if you + # *only* supply the correct outer group. + ('nested-noncapture', '/nested/noncapture/opt', [], {'p': 'opt'}), + ('nested-capture', '/nested/capture/opt/', ['opt/'], {}), + ('nested-capture', NoReverseMatch, [], {'p': 'opt'}), + ('nested-mixedcapture', '/nested/capture/mixed/opt', ['opt'], {}), + ('nested-mixedcapture', NoReverseMatch, [], {'p': 'opt'}), + ('nested-namedcapture', '/nested/capture/named/opt/', [], {'outer': 'opt/'}), + ('nested-namedcapture', NoReverseMatch, [], {'outer': 'opt/', 'inner': 'opt'}), + ('nested-namedcapture', NoReverseMatch, [], {'inner': 'opt'}), + # Regression for #9038 # These views are resolved by method name. Each method is deployed twice - # once with an explicit argument, and once using the default value on diff --git a/tests/urlpatterns_reverse/urls.py b/tests/urlpatterns_reverse/urls.py index 9b5a5f29fa..d6b692d220 100644 --- a/tests/urlpatterns_reverse/urls.py +++ b/tests/urlpatterns_reverse/urls.py @@ -32,6 +32,12 @@ with warnings.catch_warnings(): url(r'^people/(?:name/(\w+)/)?', empty_view, name="people2a"), url(r'^people/(?P\w+)-(?P=name)/$', empty_view, name="people_backref"), url(r'^optional/(?P.*)/(?:.+/)?', empty_view, name="optional"), + url(r'^optional/(?P\d+)/(?:(?P\d+)/)?', absolute_kwargs_view, name="named_optional"), + url(r'^optional/(?P\d+)/(?:(?P\d+)/)?$', absolute_kwargs_view, name="named_optional_terminated"), + url(r'^nested/noncapture/(?:(?P

\w+))$', empty_view, name='nested-noncapture'), + url(r'^nested/capture/((\w+)/)?$', empty_view, name='nested-capture'), + url(r'^nested/capture/mixed/((?P

\w+))$', empty_view, name='nested-mixedcapture'), + url(r'^nested/capture/named/(?P(?P\w+)/)?$', empty_view, name='nested-namedcapture'), url(r'^hardcoded/$', empty_view, name="hardcoded"), url(r'^hardcoded/doc\.pdf$', empty_view, name="hardcoded2"), url(r'^people/(?P\w\w)/(?P\w+)/$', empty_view, name="people3"),