Opened 7 years ago

Closed 7 years ago

#27597 closed Bug (invalid)

UrlResolver doesn't check all possibilities

Reported by: Adrien mille Owned by: nobody
Component: Core (URLs) Version: 1.10
Severity: Normal Keywords:
Cc: Marten Kenbeek Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Adrien mille)

The UrlResolver doesn't check possibilities through multiple namespaces ; It'll look out for the viewname in a namespace to test all possibilities. But the resolver won't go backward to other namespaces to list additional possibilities that might match. I've few difficulties to define that issue so I join this test case to reproduce it.

from django.test import TestCase
from django.conf.urls import url, include
from django.shortcuts import reverse


def my_view(**kwargs):
    pass


class UrlConf(object):
    app_name = 'myapp'

    # those are url patterns and views that I can re-used multiple times
    common_url_patterns = [
        # this is a page like any others
        url(r'^$', my_view, name='page'),
        # here is a namespace to let's say group actions together
        # I clearly set a namespace to make it more logical
        url(r'^actions/', include(([
            url(r'^action/$', my_view, name='action')
        ], app_name), namespace='actions'))
    ]

    urlpatterns = [
        # this is a part of my app into a namespace, nothing special about it
        url(r'^root/', include(([
            # depending of what are my keyword arguments
            # I will use one of these two followings possibilities
            url(r'^without-context/', include(common_url_patterns), kwargs={'additional_arg': -21}),
            url(r'^with-context/(?P<var>[0-9]+)/', include(common_url_patterns)),
            # you may ask why I'm not using nested optional argument
            # this is because I explicitly defined kwargs arguments for my view in one cases
            # but not in the other, so I cannot solve it that way
        ], app_name)))
    ]



class TestUrlResolver(TestCase):
    def _test_reverse_for(self, viewname, kwargs, expected):
        uri = reverse(viewname, kwargs=kwargs, urlconf=UrlConf)
        self.assertEqual(uri, expected)

    def test_page_without_var(self):
        self._test_reverse_for('myapp:page', None, '/root/without-context/')

    def test_page_with_var(self):
        self._test_reverse_for('myapp:page', {'var': 42}, '/root/with-context/42/')

    def test_action_without_var(self):
        self._test_reverse_for('myapp:actions:action', None, '/root/without-context/actions/action/')

    def test_action_with_var(self):
        self._test_reverse_for('myapp:actions:action', {'var': 42}, '/root/with-context/42/actions/action/')

Change History (5)

comment:1 by Tim Graham, 7 years ago

Could you update your example so that it doesn't raise a deprecation warning? I see this:

RemovedInDjango20Warning: Specifying a namespace in django.conf.urls.include() without providing an app_name is deprecated. Set the app_name attribute in the included module, or pass a 2-tuple containing the list of patterns and app_name instead.
  ], namespace='root'))

comment:2 by Adrien mille, 7 years ago

Description: modified (diff)

comment:3 by Tim Graham, 7 years ago

Cc: Marten Kenbeek added

Marten, any input?

comment:4 by Marten Kenbeek, 7 years ago

According to the docs:

instance namespace

This identifies a specific instance of an application. Instance namespaces should be unique across your entire project.

I believe that's what's happening here. You have multiple includes with the exact same instance namespace, while Django expects that there is only one. So I don't think it's a bug in Django.

I'm not sure whether this is actually desirable behaviour, or what kind of impact changing it would have on existing projects. If we keep it as is, it may be a good idea to add a system check that checks if instance namespaces are indeed unique across the project.

comment:5 by Tim Graham, 7 years ago

Resolution: invalid
Status: newclosed

Ok, I guess we'll close this for now. If someone wants to offer an argument for changing the behavior, please write to the DevelopersMailingList. Thanks for investigating, Marten. I created #27612 for your idea about a system check for unique instance namespaces.

Note: See TracTickets for help on using tickets.
Back to Top