Fixed #21157 -- Fixed problems with ResolverMatch
- Fixed bug in get_callable() that caused resolve() to put a string in ResolverMatch.func. - Made ResolverMatch.url_name match the actual url name (or None). - Updated tests that used the string value in ResolverMatch.func, and added regression tests for this bug. - Corrected test urls whose dummy view paths caused failures (behavior that was previously masked by this bug).
This commit is contained in:
Kevin Christopher Henry
Tim Graham
parent
0aa4c6c391
commit
91afc00513
|
|
@ -39,34 +39,31 @@ class ResolverMatch(object):
|
|||
self.func = func
|
||||
self.args = args
|
||||
self.kwargs = kwargs
|
||||
self.url_name = url_name
|
||||
self.app_name = app_name
|
||||
|
||||
if namespaces:
|
||||
self.namespaces = [x for x in namespaces if x]
|
||||
else:
|
||||
self.namespaces = []
|
||||
if not url_name:
|
||||
if not hasattr(func, '__name__'):
|
||||
# An instance of a callable class
|
||||
url_name = '.'.join([func.__class__.__module__, func.__class__.__name__])
|
||||
else:
|
||||
# A function
|
||||
url_name = '.'.join([func.__module__, func.__name__])
|
||||
self.url_name = url_name
|
||||
self.namespace = ':'.join(self.namespaces)
|
||||
|
||||
@property
|
||||
def namespace(self):
|
||||
return ':'.join(self.namespaces)
|
||||
if not hasattr(func, '__name__'):
|
||||
# A class-based view
|
||||
self._func_path = '.'.join([func.__class__.__module__, func.__class__.__name__])
|
||||
else:
|
||||
# A function-based view
|
||||
self._func_path = '.'.join([func.__module__, func.__name__])
|
||||
|
||||
@property
|
||||
def view_name(self):
|
||||
return ':'.join(filter(bool, (self.namespace, self.url_name)))
|
||||
view_path = url_name or self._func_path
|
||||
self.view_name = ':'.join(self.namespaces + [view_path])
|
||||
|
||||
def __getitem__(self, index):
|
||||
return (self.func, self.args, self.kwargs)[index]
|
||||
|
||||
def __repr__(self):
|
||||
return "ResolverMatch(func=%s, args=%s, kwargs=%s, url_name='%s', app_name='%s', namespace='%s')" % (
|
||||
self.func, self.args, self.kwargs, self.url_name, self.app_name, self.namespace)
|
||||
return "ResolverMatch(func=%s, args=%s, kwargs=%s, url_name=%s, app_name=%s, namespaces=%s)" % (
|
||||
self._func_path, self.args, self.kwargs, self.url_name, self.app_name, self.namespaces)
|
||||
|
||||
|
||||
class Resolver404(Http404):
|
||||
|
|
@ -80,43 +77,61 @@ class NoReverseMatch(Exception):
|
|||
@lru_cache.lru_cache(maxsize=None)
|
||||
def get_callable(lookup_view, can_fail=False):
|
||||
"""
|
||||
Convert a string version of a function name to the callable object.
|
||||
Return a callable corresponding to lookup_view. This function is used
|
||||
by both resolve() and reverse(), so can_fail allows the caller to choose
|
||||
between returning the input as is and raising an exception when the input
|
||||
string can't be interpreted as an import path.
|
||||
|
||||
If the lookup_view is not an import path, it is assumed to be a URL pattern
|
||||
label and the original string is returned.
|
||||
|
||||
If can_fail is True, lookup_view might be a URL pattern label, so errors
|
||||
during the import fail and the string is returned.
|
||||
If lookup_view is already a callable, return it.
|
||||
If lookup_view is a string import path that can be resolved to a callable,
|
||||
import that callable and return it.
|
||||
If lookup_view is some other kind of string and can_fail is True, the string
|
||||
is returned as is. If can_fail is False, an exception is raised (either
|
||||
ImportError or ViewDoesNotExist).
|
||||
"""
|
||||
if not callable(lookup_view):
|
||||
mod_name, func_name = get_mod_func(lookup_view)
|
||||
if func_name == '':
|
||||
return lookup_view
|
||||
if callable(lookup_view):
|
||||
return lookup_view
|
||||
|
||||
try:
|
||||
mod = import_module(mod_name)
|
||||
except ImportError:
|
||||
parentmod, submod = get_mod_func(mod_name)
|
||||
if (not can_fail and submod != '' and
|
||||
not module_has_submodule(import_module(parentmod), submod)):
|
||||
raise ViewDoesNotExist(
|
||||
"Could not import %s. Parent module %s does not exist." %
|
||||
(lookup_view, mod_name))
|
||||
if not can_fail:
|
||||
raise
|
||||
mod_name, func_name = get_mod_func(lookup_view)
|
||||
if not func_name: # No '.' in lookup_view
|
||||
if can_fail:
|
||||
return lookup_view
|
||||
else:
|
||||
try:
|
||||
lookup_view = getattr(mod, func_name)
|
||||
if not callable(lookup_view):
|
||||
raise ViewDoesNotExist(
|
||||
"Could not import %s.%s. View is not callable." %
|
||||
(mod_name, func_name))
|
||||
except AttributeError:
|
||||
if not can_fail:
|
||||
raise ViewDoesNotExist(
|
||||
"Could not import %s. View does not exist in module %s." %
|
||||
(lookup_view, mod_name))
|
||||
return lookup_view
|
||||
raise ImportError(
|
||||
"Could not import '%s'. The path must be fully qualified." %
|
||||
lookup_view)
|
||||
|
||||
try:
|
||||
mod = import_module(mod_name)
|
||||
except ImportError:
|
||||
if can_fail:
|
||||
return lookup_view
|
||||
else:
|
||||
parentmod, submod = get_mod_func(mod_name)
|
||||
if submod and not module_has_submodule(import_module(parentmod), submod):
|
||||
raise ViewDoesNotExist(
|
||||
"Could not import '%s'. Parent module %s does not exist." %
|
||||
(lookup_view, mod_name))
|
||||
else:
|
||||
raise
|
||||
else:
|
||||
try:
|
||||
view_func = getattr(mod, func_name)
|
||||
except AttributeError:
|
||||
if can_fail:
|
||||
return lookup_view
|
||||
else:
|
||||
raise ViewDoesNotExist(
|
||||
"Could not import '%s'. View does not exist in module %s." %
|
||||
(lookup_view, mod_name))
|
||||
else:
|
||||
if not callable(view_func):
|
||||
# For backwards compatibility this is raised regardless of can_fail
|
||||
raise ViewDoesNotExist(
|
||||
"Could not import '%s.%s'. View is not callable." %
|
||||
(mod_name, func_name))
|
||||
|
||||
return view_func
|
||||
|
||||
|
||||
@lru_cache.lru_cache(maxsize=None)
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
from django.conf.urls import url
|
||||
|
||||
urlpatterns = [
|
||||
url(r'^customurlconf/noslash$', 'view'),
|
||||
url(r'^customurlconf/slash/$', 'view'),
|
||||
url(r'^customurlconf/needsquoting#/$', 'view'),
|
||||
url(r'^customurlconf/noslash$', 'middleware.views.empty_view'),
|
||||
url(r'^customurlconf/slash/$', 'middleware.views.empty_view'),
|
||||
url(r'^customurlconf/needsquoting#/$', 'middleware.views.empty_view'),
|
||||
]
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
from django.conf.urls import url
|
||||
|
||||
urlpatterns = [
|
||||
url(r'^noslash$', 'view'),
|
||||
url(r'^slash/$', 'view'),
|
||||
url(r'^needsquoting#/$', 'view'),
|
||||
url(r'^noslash$', 'middleware.views.empty_view'),
|
||||
url(r'^slash/$', 'middleware.views.empty_view'),
|
||||
url(r'^needsquoting#/$', 'middleware.views.empty_view'),
|
||||
]
|
||||
|
|
|
|||
|
|
@ -0,0 +1,5 @@
|
|||
from django.http import HttpResponse
|
||||
|
||||
|
||||
def empty_view(request, *args, **kwargs):
|
||||
return HttpResponse('')
|
||||
|
|
@ -1,5 +1,5 @@
|
|||
from django.conf.urls import url
|
||||
|
||||
urlpatterns = [
|
||||
url(r'^guitarists/(\w{1,50})/$', 'unimplemented_view_placeholder', name='guitarist_detail'),
|
||||
url(r'^guitarists/(\w{1,50})/$', 'model_permalink.views.empty_view', name='guitarist_detail'),
|
||||
]
|
||||
|
|
|
|||
|
|
@ -0,0 +1,5 @@
|
|||
from django.http import HttpResponse
|
||||
|
||||
|
||||
def empty_view(request, *args, **kwargs):
|
||||
return HttpResponse('')
|
||||
|
|
@ -5,6 +5,8 @@ urlpatterns = [
|
|||
url(r'erroneous_inner/$', 'urlpatterns_reverse.views.erroneous_view'),
|
||||
# Module has erroneous import
|
||||
url(r'erroneous_outer/$', 'urlpatterns_reverse.erroneous_views_module.erroneous_view'),
|
||||
# Module is an unqualified string
|
||||
url(r'erroneous_unqualified/$', 'unqualified_view'),
|
||||
# View does not exist
|
||||
url(r'missing_inner/$', 'urlpatterns_reverse.views.missing_view'),
|
||||
# View is not callable
|
||||
|
|
|
|||
|
|
@ -10,9 +10,9 @@ class URLObject(object):
|
|||
|
||||
def urls(self):
|
||||
return ([
|
||||
url(r'^inner/$', 'empty_view', name='urlobject-view'),
|
||||
url(r'^inner/(?P<arg1>[0-9]+)/(?P<arg2>[0-9]+)/$', 'empty_view', name='urlobject-view'),
|
||||
url(r'^inner/\+\\\$\*/$', 'empty_view', name='urlobject-special-view'),
|
||||
url(r'^inner/$', 'urlpatterns_reverse.views.empty_view', name='urlobject-view'),
|
||||
url(r'^inner/(?P<arg1>[0-9]+)/(?P<arg2>[0-9]+)/$', 'urlpatterns_reverse.views.empty_view', name='urlobject-view'),
|
||||
url(r'^inner/\+\\\$\*/$', 'urlpatterns_reverse.views.empty_view', name='urlobject-special-view'),
|
||||
], self.app_name, self.namespace)
|
||||
urls = property(urls)
|
||||
|
||||
|
|
|
|||
|
|
@ -25,41 +25,41 @@ from .views import empty_view
|
|||
|
||||
|
||||
resolve_test_data = (
|
||||
# These entries are in the format: (path, url_name, app_name, namespace, view_func, args, kwargs)
|
||||
# These entries are in the format: (path, url_name, app_name, namespace, view_name, func, args, kwargs)
|
||||
# Simple case
|
||||
('/normal/42/37/', 'normal-view', None, '', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/view_class/42/37/', 'view-class', None, '', views.view_class_instance, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/included/normal/42/37/', 'inc-normal-view', None, '', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/included/view_class/42/37/', 'inc-view-class', None, '', views.view_class_instance, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/normal/42/37/', 'normal-view', None, '', 'normal-view', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/view_class/42/37/', 'view-class', None, '', 'view-class', views.view_class_instance, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/included/normal/42/37/', 'inc-normal-view', None, '', 'inc-normal-view', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/included/view_class/42/37/', 'inc-view-class', None, '', 'inc-view-class', views.view_class_instance, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
|
||||
# Unnamed args are dropped if you have *any* kwargs in a pattern
|
||||
('/mixed_args/42/37/', 'mixed-args', None, '', views.empty_view, tuple(), {'arg2': '37'}),
|
||||
('/included/mixed_args/42/37/', 'inc-mixed-args', None, '', views.empty_view, tuple(), {'arg2': '37'}),
|
||||
('/mixed_args/42/37/', 'mixed-args', None, '', 'mixed-args', views.empty_view, tuple(), {'arg2': '37'}),
|
||||
('/included/mixed_args/42/37/', 'inc-mixed-args', None, '', 'inc-mixed-args', views.empty_view, tuple(), {'arg2': '37'}),
|
||||
|
||||
# Unnamed views will be resolved to the function/class name
|
||||
('/unnamed/normal/42/37/', 'urlpatterns_reverse.views.empty_view', None, '', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/unnamed/view_class/42/37/', 'urlpatterns_reverse.views.ViewClass', None, '', views.view_class_instance, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
# Unnamed views should have None as the url_name. Regression data for #21157.
|
||||
('/unnamed/normal/42/37/', None, None, '', 'urlpatterns_reverse.views.empty_view', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/unnamed/view_class/42/37/', None, None, '', 'urlpatterns_reverse.views.ViewClass', views.view_class_instance, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
|
||||
# If you have no kwargs, you get an args list.
|
||||
('/no_kwargs/42/37/', 'no-kwargs', None, '', views.empty_view, ('42', '37'), {}),
|
||||
('/included/no_kwargs/42/37/', 'inc-no-kwargs', None, '', views.empty_view, ('42', '37'), {}),
|
||||
('/no_kwargs/42/37/', 'no-kwargs', None, '', 'no-kwargs', views.empty_view, ('42', '37'), {}),
|
||||
('/included/no_kwargs/42/37/', 'inc-no-kwargs', None, '', 'inc-no-kwargs', views.empty_view, ('42', '37'), {}),
|
||||
|
||||
# Namespaces
|
||||
('/test1/inner/42/37/', 'urlobject-view', 'testapp', 'test-ns1', 'empty_view', tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/included/test3/inner/42/37/', 'urlobject-view', 'testapp', 'test-ns3', 'empty_view', tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/ns-included1/normal/42/37/', 'inc-normal-view', None, 'inc-ns1', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/included/test3/inner/42/37/', 'urlobject-view', 'testapp', 'test-ns3', 'empty_view', tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/default/inner/42/37/', 'urlobject-view', 'testapp', 'testapp', 'empty_view', tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/other2/inner/42/37/', 'urlobject-view', 'nodefault', 'other-ns2', 'empty_view', tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/other1/inner/42/37/', 'urlobject-view', 'nodefault', 'other-ns1', 'empty_view', tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/test1/inner/42/37/', 'urlobject-view', 'testapp', 'test-ns1', 'test-ns1:urlobject-view', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/included/test3/inner/42/37/', 'urlobject-view', 'testapp', 'test-ns3', 'test-ns3:urlobject-view', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/ns-included1/normal/42/37/', 'inc-normal-view', None, 'inc-ns1', 'inc-ns1:inc-normal-view', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/included/test3/inner/42/37/', 'urlobject-view', 'testapp', 'test-ns3', 'test-ns3:urlobject-view', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/default/inner/42/37/', 'urlobject-view', 'testapp', 'testapp', 'testapp:urlobject-view', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/other2/inner/42/37/', 'urlobject-view', 'nodefault', 'other-ns2', 'other-ns2:urlobject-view', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/other1/inner/42/37/', 'urlobject-view', 'nodefault', 'other-ns1', 'other-ns1:urlobject-view', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
|
||||
# Nested namespaces
|
||||
('/ns-included1/test3/inner/42/37/', 'urlobject-view', 'testapp', 'inc-ns1:test-ns3', 'empty_view', tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/ns-included1/ns-included4/ns-included2/test3/inner/42/37/', 'urlobject-view', 'testapp', 'inc-ns1:inc-ns4:inc-ns2:test-ns3', 'empty_view', tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/ns-included1/test3/inner/42/37/', 'urlobject-view', 'testapp', 'inc-ns1:test-ns3', 'inc-ns1:test-ns3:urlobject-view', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
('/ns-included1/ns-included4/ns-included2/test3/inner/42/37/', 'urlobject-view', 'testapp', 'inc-ns1:inc-ns4:inc-ns2:test-ns3', 'inc-ns1:inc-ns4:inc-ns2:test-ns3:urlobject-view', views.empty_view, tuple(), {'arg1': '42', 'arg2': '37'}),
|
||||
|
||||
# Namespaces capturing variables
|
||||
('/inc70/', 'inner-nothing', None, 'inc-ns5', views.empty_view, tuple(), {'outer': '70'}),
|
||||
('/inc78/extra/foobar/', 'inner-extra', None, 'inc-ns5', views.empty_view, tuple(), {'outer': '78', 'extra': 'foobar'}),
|
||||
('/inc70/', 'inner-nothing', None, 'inc-ns5', 'inc-ns5:inner-nothing', views.empty_view, tuple(), {'outer': '70'}),
|
||||
('/inc78/extra/foobar/', 'inner-extra', None, 'inc-ns5', 'inc-ns5:inner-extra', views.empty_view, tuple(), {'outer': '78', 'extra': 'foobar'}),
|
||||
)
|
||||
|
||||
test_data = (
|
||||
|
|
@ -141,8 +141,6 @@ test_data = (
|
|||
# once with an explicit argument, and once using the default value on
|
||||
# the method. This is potentially ambiguous, as you have to pick the
|
||||
# correct view for the arguments provided.
|
||||
('kwargs_view', '/arg_view/', [], {}),
|
||||
('kwargs_view', '/arg_view/10/', [], {'arg1': 10}),
|
||||
('urlpatterns_reverse.views.absolute_kwargs_view', '/absolute_arg_view/', [], {}),
|
||||
('urlpatterns_reverse.views.absolute_kwargs_view', '/absolute_arg_view/10/', [], {'arg1': 10}),
|
||||
('non_path_include', '/includes/non_path_include/', [], {}),
|
||||
|
|
@ -603,7 +601,6 @@ class ErrorHandlerResolutionTests(TestCase):
|
|||
"""Tests for handler400, handler404 and handler500"""
|
||||
|
||||
def setUp(self):
|
||||
from django.core.urlresolvers import RegexURLResolver
|
||||
urlconf = 'urlpatterns_reverse.urls_error_handlers'
|
||||
urlconf_callables = 'urlpatterns_reverse.urls_error_handlers_callables'
|
||||
self.resolver = RegexURLResolver(r'^$', urlconf)
|
||||
|
|
@ -651,7 +648,7 @@ class NoRootUrlConfTests(TestCase):
|
|||
class ResolverMatchTests(TestCase):
|
||||
|
||||
def test_urlpattern_resolve(self):
|
||||
for path, name, app_name, namespace, func, args, kwargs in resolve_test_data:
|
||||
for path, url_name, app_name, namespace, view_name, func, args, kwargs in resolve_test_data:
|
||||
# Test legacy support for extracting "function, args, kwargs"
|
||||
match_func, match_args, match_kwargs = resolve(path)
|
||||
self.assertEqual(match_func, func)
|
||||
|
|
@ -661,12 +658,13 @@ class ResolverMatchTests(TestCase):
|
|||
# Test ResolverMatch capabilities.
|
||||
match = resolve(path)
|
||||
self.assertEqual(match.__class__, ResolverMatch)
|
||||
self.assertEqual(match.url_name, name)
|
||||
self.assertEqual(match.args, args)
|
||||
self.assertEqual(match.kwargs, kwargs)
|
||||
self.assertEqual(match.url_name, url_name)
|
||||
self.assertEqual(match.app_name, app_name)
|
||||
self.assertEqual(match.namespace, namespace)
|
||||
self.assertEqual(match.view_name, view_name)
|
||||
self.assertEqual(match.func, func)
|
||||
self.assertEqual(match.args, args)
|
||||
self.assertEqual(match.kwargs, kwargs)
|
||||
|
||||
# ... and for legacy purposes:
|
||||
self.assertEqual(match[0], func)
|
||||
|
|
@ -693,6 +691,9 @@ class ErroneousViewTests(TestCase):
|
|||
self.assertRaises(ViewDoesNotExist, self.client.get, '/missing_outer/')
|
||||
self.assertRaises(ViewDoesNotExist, self.client.get, '/uncallable/')
|
||||
|
||||
# Regression test for #21157
|
||||
self.assertRaises(ImportError, self.client.get, '/erroneous_unqualified/')
|
||||
|
||||
def test_erroneous_reverse(self):
|
||||
"""
|
||||
Ensure that a useful exception is raised when a regex is invalid in the
|
||||
|
|
|
|||
|
|
@ -62,14 +62,14 @@ with warnings.catch_warnings(record=True):
|
|||
url(r'^(?:foo|bar)(\w+)/$', empty_view, name="disjunction"),
|
||||
|
||||
# Regression views for #9038. See tests for more details
|
||||
url(r'arg_view/$', 'kwargs_view'),
|
||||
url(r'arg_view/(?P<arg1>[0-9]+)/$', 'kwargs_view'),
|
||||
url(r'arg_view/$', 'urlpatterns_reverse.views.kwargs_view'),
|
||||
url(r'arg_view/(?P<arg1>[0-9]+)/$', 'urlpatterns_reverse.views.kwargs_view'),
|
||||
url(r'absolute_arg_view/(?P<arg1>[0-9]+)/$', absolute_kwargs_view),
|
||||
url(r'absolute_arg_view/$', absolute_kwargs_view),
|
||||
|
||||
# Tests for #13154. Mixed syntax to test both ways of defining URLs.
|
||||
url(r'defaults_view1/(?P<arg1>[0-9]+)/', 'defaults_view', {'arg2': 1}, name='defaults'),
|
||||
(r'defaults_view2/(?P<arg1>[0-9]+)/', 'defaults_view', {'arg2': 2}, 'defaults'),
|
||||
url(r'defaults_view1/(?P<arg1>[0-9]+)/', 'urlpatterns_reverse.views.defaults_view', {'arg2': 1}, name='defaults'),
|
||||
(r'defaults_view2/(?P<arg1>[0-9]+)/', 'urlpatterns_reverse.views.defaults_view', {'arg2': 2}, 'defaults'),
|
||||
|
||||
url('^includes/', include(other_patterns)),
|
||||
)
|
||||
|
|
|
|||
Loading…
Reference in New Issue