From 24fa728a476a6da3e565dbe33959ea62c02c250b Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 22 Dec 2016 00:54:15 +0300 Subject: [PATCH] Fixed #27612 -- Added a check for duplicate URL instance namespaces. --- django/core/checks/urls.py | 41 ++++++++++++++++++- docs/ref/checks.txt | 2 + tests/check_framework/test_urls.py | 23 ++++++++++- .../urls/non_unique_namespaces.py | 13 ++++++ .../check_framework/urls/unique_namespaces.py | 11 +++++ 5 files changed, 88 insertions(+), 2 deletions(-) create mode 100644 tests/check_framework/urls/non_unique_namespaces.py create mode 100644 tests/check_framework/urls/unique_namespaces.py diff --git a/django/core/checks/urls.py b/django/core/checks/urls.py index a1e9929084..8187a4e561 100644 --- a/django/core/checks/urls.py +++ b/django/core/checks/urls.py @@ -1,9 +1,11 @@ from __future__ import unicode_literals +from collections import Counter + from django.conf import settings from django.utils import six -from . import Error, Tags, register +from . import Error, Tags, Warning, register @register(Tags.urls) @@ -28,6 +30,43 @@ def check_resolver(resolver): return [] +@register(Tags.urls) +def check_url_namespaces_unique(app_configs, **kwargs): + """ + Warn if URL namespaces used in applications aren't unique. + """ + if not getattr(settings, 'ROOT_URLCONF', None): + return [] + + from django.urls import get_resolver + resolver = get_resolver() + all_namespaces = _load_all_namespaces(resolver) + counter = Counter(all_namespaces) + non_unique_namespaces = [n for n, count in counter.items() if count > 1] + errors = [] + for namespace in non_unique_namespaces: + errors.append(Warning( + "URL namespace '{}' isn't unique. You may not be able to reverse " + "all URLs in this namespace".format(namespace), + id="urls.W005", + )) + return errors + + +def _load_all_namespaces(resolver): + """ + Recursively load all namespaces from URL patterns. + """ + url_patterns = getattr(resolver, 'url_patterns', []) + namespaces = [ + url.namespace for url in url_patterns + if getattr(url, 'namespace', None) is not None + ] + for pattern in url_patterns: + namespaces.extend(_load_all_namespaces(pattern)) + return namespaces + + def get_warning_for_invalid_pattern(pattern): """ Return a list containing a warning that the pattern is invalid. diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index c6f7e518ef..ef6bbcee0e 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -673,3 +673,5 @@ The following checks are performed on your URL configuration: references. * **urls.E004**: Your URL pattern ```` is invalid. Ensure that ``urlpatterns`` is a list of :func:`~django.conf.urls.url()` instances. +* **urls.W005**: URL namespace ```` isn't unique. You may not be + able to reverse all URLs in this namespace. diff --git a/tests/check_framework/test_urls.py b/tests/check_framework/test_urls.py index 159c41e7a9..5e7c952587 100644 --- a/tests/check_framework/test_urls.py +++ b/tests/check_framework/test_urls.py @@ -1,6 +1,7 @@ from django.conf import settings +from django.core.checks.messages import Warning from django.core.checks.urls import ( - check_url_config, get_warning_for_invalid_pattern, + check_url_config, check_url_namespaces_unique, get_warning_for_invalid_pattern, ) from django.test import SimpleTestCase from django.test.utils import override_settings @@ -94,3 +95,23 @@ class CheckUrlsTest(SimpleTestCase): def test_get_warning_for_invalid_pattern_other(self): warning = get_warning_for_invalid_pattern(object())[0] self.assertIsNone(warning.hint) + + @override_settings(ROOT_URLCONF='check_framework.urls.non_unique_namespaces') + def test_check_non_unique_namespaces(self): + result = check_url_namespaces_unique(None) + self.assertEqual(len(result), 2) + non_unique_namespaces = ['app-ns1', 'app-1'] + warning_messages = [ + "URL namespace '{}' isn't unique. You may not be able to reverse " + "all URLs in this namespace".format(namespace) + for namespace in non_unique_namespaces + ] + for warning in result: + self.assertIsInstance(warning, Warning) + self.assertEqual('urls.W005', warning.id) + self.assertIn(warning.msg, warning_messages) + + @override_settings(ROOT_URLCONF='check_framework.urls.unique_namespaces') + def test_check_unique_namespaces(self): + result = check_url_namespaces_unique(None) + self.assertEqual(result, []) diff --git a/tests/check_framework/urls/non_unique_namespaces.py b/tests/check_framework/urls/non_unique_namespaces.py new file mode 100644 index 0000000000..781be4c6d0 --- /dev/null +++ b/tests/check_framework/urls/non_unique_namespaces.py @@ -0,0 +1,13 @@ +from django.conf.urls import include, url + +common_url_patterns = ([ + url(r'^app-ns1/', include([])), + url(r'^app-url/', include([])), +], 'app-ns1') + +urlpatterns = [ + url(r'^app-ns1-0/', include(common_url_patterns)), + url(r'^app-ns1-1/', include(common_url_patterns)), + url(r'^app-some-url/', include(([], 'app'), namespace='app-1')), + url(r'^app-some-url-2/', include(([], 'app'), namespace='app-1')) +] diff --git a/tests/check_framework/urls/unique_namespaces.py b/tests/check_framework/urls/unique_namespaces.py new file mode 100644 index 0000000000..897f27757e --- /dev/null +++ b/tests/check_framework/urls/unique_namespaces.py @@ -0,0 +1,11 @@ +from django.conf.urls import include, url + +common_url_patterns = ([ + url(r'^app-ns1/', include([])), + url(r'^app-url/', include([])), +], 'common') + +urlpatterns = [ + url(r'^app-ns1-0/', include(common_url_patterns, namespace='app-include-1')), + url(r'^app-ns1-1/', include(common_url_patterns, namespace='app-include-2')) +]