Fixed #32195 -- Added system check for invalid view in path() and improved error messages.

This commit is contained in:
Angus Holder 2020-11-14 17:25:57 +00:00 committed by Mariusz Felisiak
parent 8f89454bbc
commit 3e73c65ffc
6 changed files with 65 additions and 0 deletions

View File

@ -55,6 +55,8 @@ def include(arg, namespace=None):
def _path(route, view, kwargs=None, name=None, Pattern=None): def _path(route, view, kwargs=None, name=None, Pattern=None):
from django.views import View
if isinstance(view, (list, tuple)): if isinstance(view, (list, tuple)):
# For include(...) processing. # For include(...) processing.
pattern = Pattern(route, is_endpoint=False) pattern = Pattern(route, is_endpoint=False)
@ -69,6 +71,12 @@ def _path(route, view, kwargs=None, name=None, Pattern=None):
elif callable(view): elif callable(view):
pattern = Pattern(route, name=name, is_endpoint=True) pattern = Pattern(route, name=name, is_endpoint=True)
return URLPattern(pattern, view, kwargs, name) 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: else:
raise TypeError('view must be a callable or a list/tuple in the case of include().') raise TypeError('view must be a callable or a list/tuple in the case of include().')

View File

@ -345,6 +345,7 @@ class URLPattern:
def check(self): def check(self):
warnings = self._check_pattern_name() warnings = self._check_pattern_name()
warnings.extend(self.pattern.check()) warnings.extend(self.pattern.check())
warnings.extend(self._check_callback())
return warnings return warnings
def _check_pattern_name(self): def _check_pattern_name(self):
@ -361,6 +362,22 @@ class URLPattern:
else: else:
return [] 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): def resolve(self, path):
match = self.pattern.match(path) match = self.pattern.match(path)
if match: if match:

View File

@ -584,6 +584,8 @@ The following checks are performed on your URL configuration:
take the correct number of arguments (…). take the correct number of arguments (…).
* **urls.E008**: The custom ``handlerXXX`` view ``'path.to.view'`` could not be * **urls.E008**: The custom ``handlerXXX`` view ``'path.to.view'`` could not be
imported. imported.
* **urls.E009**: Your URL pattern ``<pattern>`` has an invalid view, pass
``<view>.as_view()`` instead of ``<view>``.
``contrib`` app checks ``contrib`` app checks
====================== ======================

View File

@ -134,6 +134,16 @@ class CheckUrlConfigTests(SimpleTestCase):
result = check_url_namespaces_unique(None) result = check_url_namespaces_unique(None)
self.assertEqual(result, []) 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): class UpdatedToPathTests(SimpleTestCase):

View File

@ -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()),
]

View File

@ -5,6 +5,7 @@ from django.core.exceptions import ImproperlyConfigured
from django.test import SimpleTestCase from django.test import SimpleTestCase
from django.test.utils import override_settings from django.test.utils import override_settings
from django.urls import NoReverseMatch, Resolver404, path, resolve, reverse from django.urls import NoReverseMatch, Resolver404, path, resolve, reverse
from django.views import View
from .converters import DynamicConverter from .converters import DynamicConverter
from .views import empty_view from .views import empty_view
@ -146,6 +147,14 @@ class SimplifiedURLTests(SimpleTestCase):
with self.assertRaisesMessage(TypeError, msg): with self.assertRaisesMessage(TypeError, msg):
path('articles/', 'invalid_view') 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): def test_whitespace_in_route(self):
msg = ( msg = (
"URL route 'space/<int:num>/extra/<str:%stest>' cannot contain " "URL route 'space/<int:num>/extra/<str:%stest>' cannot contain "