From 3e73c65ffcf263d5ccd107589452a4615281a0e8 Mon Sep 17 00:00:00 2001 From: Angus Holder Date: Sat, 14 Nov 2020 17:25:57 +0000 Subject: [PATCH] Fixed #32195 -- Added system check for invalid view in path() and improved error messages. --- django/urls/conf.py | 8 ++++++++ django/urls/resolvers.py | 17 +++++++++++++++++ docs/ref/checks.txt | 2 ++ tests/check_framework/test_urls.py | 10 ++++++++++ tests/check_framework/urls/cbv_as_view.py | 19 +++++++++++++++++++ tests/urlpatterns/tests.py | 9 +++++++++ 6 files changed, 65 insertions(+) create mode 100644 tests/check_framework/urls/cbv_as_view.py diff --git a/django/urls/conf.py b/django/urls/conf.py index 119e95df41..b3937d5512 100644 --- a/django/urls/conf.py +++ b/django/urls/conf.py @@ -55,6 +55,8 @@ def include(arg, namespace=None): def _path(route, view, kwargs=None, name=None, Pattern=None): + from django.views import View + if isinstance(view, (list, tuple)): # For include(...) processing. pattern = Pattern(route, is_endpoint=False) @@ -69,6 +71,12 @@ def _path(route, view, kwargs=None, name=None, Pattern=None): elif callable(view): pattern = Pattern(route, name=name, is_endpoint=True) return URLPattern(pattern, view, kwargs, name) + elif isinstance(view, View): + view_cls_name = view.__class__.__name__ + raise TypeError( + f'view must be a callable, pass {view_cls_name}.as_view(), not ' + f'{view_cls_name}().' + ) else: raise TypeError('view must be a callable or a list/tuple in the case of include().') diff --git a/django/urls/resolvers.py b/django/urls/resolvers.py index fac77fc4bc..674fd0c58e 100644 --- a/django/urls/resolvers.py +++ b/django/urls/resolvers.py @@ -345,6 +345,7 @@ class URLPattern: def check(self): warnings = self._check_pattern_name() warnings.extend(self.pattern.check()) + warnings.extend(self._check_callback()) return warnings def _check_pattern_name(self): @@ -361,6 +362,22 @@ class URLPattern: else: return [] + def _check_callback(self): + from django.views import View + + view = self.callback + if inspect.isclass(view) and issubclass(view, View): + return [Error( + 'Your URL pattern %s has an invalid view, pass %s.as_view() ' + 'instead of %s.' % ( + self.pattern.describe(), + view.__name__, + view.__name__, + ), + id='urls.E009', + )] + return [] + def resolve(self, path): match = self.pattern.match(path) if match: diff --git a/docs/ref/checks.txt b/docs/ref/checks.txt index f304da7e11..3147114f8d 100644 --- a/docs/ref/checks.txt +++ b/docs/ref/checks.txt @@ -584,6 +584,8 @@ The following checks are performed on your URL configuration: take the correct number of arguments (…). * **urls.E008**: The custom ``handlerXXX`` view ``'path.to.view'`` could not be imported. +* **urls.E009**: Your URL pattern ```` has an invalid view, pass + ``.as_view()`` instead of ````. ``contrib`` app checks ====================== diff --git a/tests/check_framework/test_urls.py b/tests/check_framework/test_urls.py index 2ef3d5b07f..663c7a299c 100644 --- a/tests/check_framework/test_urls.py +++ b/tests/check_framework/test_urls.py @@ -134,6 +134,16 @@ class CheckUrlConfigTests(SimpleTestCase): result = check_url_namespaces_unique(None) self.assertEqual(result, []) + @override_settings(ROOT_URLCONF='check_framework.urls.cbv_as_view') + def test_check_view_not_class(self): + self.assertEqual(check_url_config(None), [ + Error( + "Your URL pattern 'missing_as_view' has an invalid view, pass " + "EmptyCBV.as_view() instead of EmptyCBV.", + id='urls.E009', + ), + ]) + class UpdatedToPathTests(SimpleTestCase): diff --git a/tests/check_framework/urls/cbv_as_view.py b/tests/check_framework/urls/cbv_as_view.py new file mode 100644 index 0000000000..932a2bfcc9 --- /dev/null +++ b/tests/check_framework/urls/cbv_as_view.py @@ -0,0 +1,19 @@ +from django.http import HttpResponse +from django.urls import path +from django.views import View + + +class EmptyCBV(View): + pass + + +class EmptyCallableView: + def __call__(self, request, *args, **kwargs): + return HttpResponse() + + +urlpatterns = [ + path('missing_as_view', EmptyCBV), + path('has_as_view', EmptyCBV.as_view()), + path('callable_class', EmptyCallableView()), +] diff --git a/tests/urlpatterns/tests.py b/tests/urlpatterns/tests.py index aa593294af..dca9f63086 100644 --- a/tests/urlpatterns/tests.py +++ b/tests/urlpatterns/tests.py @@ -5,6 +5,7 @@ from django.core.exceptions import ImproperlyConfigured from django.test import SimpleTestCase from django.test.utils import override_settings from django.urls import NoReverseMatch, Resolver404, path, resolve, reverse +from django.views import View from .converters import DynamicConverter from .views import empty_view @@ -146,6 +147,14 @@ class SimplifiedURLTests(SimpleTestCase): with self.assertRaisesMessage(TypeError, msg): path('articles/', 'invalid_view') + def test_invalid_view_instance(self): + class EmptyCBV(View): + pass + + msg = 'view must be a callable, pass EmptyCBV.as_view(), not EmptyCBV().' + with self.assertRaisesMessage(TypeError, msg): + path('foo', EmptyCBV()) + def test_whitespace_in_route(self): msg = ( "URL route 'space//extra/' cannot contain "