Skip to content

Commit

Permalink
Fixed django#16774 -- Allow view to raise DoesNotResolve to make URL …
Browse files Browse the repository at this point in the history
…Resolver to keep searching for a URL.
  • Loading branch information
meric committed Sep 13, 2012
1 parent 9db7652 commit 2ef7d14
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 14 deletions.
36 changes: 23 additions & 13 deletions django/core/handlers/base.py
Expand Up @@ -100,7 +100,12 @@ def get_response(self, request):
urlconf = request.urlconf
urlresolvers.set_urlconf(urlconf)
resolver = urlresolvers.RegexURLResolver(r'^/', urlconf)
# Use generators and try to resolve matching URL patterns one by
# one
url_patterns = (pattern for pattern in resolver.url_patterns)
resolver = urlresolvers.RegexURLResolver(r'^/', url_patterns)

while response is None:
callback, callback_args, callback_kwargs = resolver.resolve(
request.path_info)

Expand All @@ -110,19 +115,24 @@ def get_response(self, request):
if response:
break

if response is None:
try:
response = callback(request, *callback_args, **callback_kwargs)
except Exception as e:
# If the view raised an exception, run it through exception
# middleware, and if the exception middleware returns a
# response, use that. Otherwise, reraise the exception.
for middleware_method in self._exception_middleware:
response = middleware_method(request, e)
if response:
break
if response is None:
raise
if response is None:

This comment has been minimized.

Copy link
@charettes

charettes Sep 13, 2012

I don't think this check is required anymore since you're already breaking at line 115.

This comment has been minimized.

Copy link
@meric

meric Sep 18, 2012

Author Owner

The break at line 115 was part of a nested loop... I added the check back in the next commit, but now it uses the for-loop else operator.

6ba42de

try:
response = callback(request, *callback_args, **callback_kwargs)
except urlresolvers.DoesNotResolve, e:

This comment has been minimized.

Copy link
@charettes

charettes Sep 13, 2012

No need to assign the exception to e since it's not used.

# Continue resolve URLs if the view raises
# urlresolvers.DoesNotResolve exception to indicate
# the url pattern does not match.
continue
except Exception as e:
# If the view raised an exception, run it through exception
# middleware, and if the exception middleware returns a
# response, use that. Otherwise, reraise the exception.
for middleware_method in self._exception_middleware:
response = middleware_method(request, e)
if response:
break
if response is None:

This comment has been minimized.

Copy link
@charettes

charettes Sep 13, 2012

This should be replaced by an else statement.

raise

# Complain if the view returned None (a common error).
if response is None:
Expand Down
3 changes: 3 additions & 0 deletions django/core/urlresolvers.py
Expand Up @@ -73,6 +73,9 @@ def __repr__(self):
class Resolver404(Http404):
pass

class DoesNotResolve(Http404):
pass

class NoReverseMatch(Exception):
# Don't make this raise an error when used in a template.
silent_variable_failure = True
Expand Down
4 changes: 4 additions & 0 deletions tests/regressiontests/views/generic_urls.py
Expand Up @@ -54,4 +54,8 @@
(r'^shortcuts/render/status/$', 'render_view_with_status'),
(r'^shortcuts/render/current_app/$', 'render_view_with_current_app'),
(r'^shortcuts/render/current_app_conflict/$', 'render_view_with_current_app_conflict'),
url(r'^overlapping_view/(?P<title>[a-z]+)/$', 'overlapping_view1', name='overlapping_view1'),
url(r'^overlapping_view/(?P<author>[a-z]+)/$', 'overlapping_view2', name='overlapping_view2'),
url(r'^overlapping_view/(?P<keywords>[a-z]+)/$', 'overlapping_view3', name='overlapping_view3'),
url(r'^no_overlapping_view/(?P<keywords>[a-z]+)/$', 'no_overlapping_view', name='no_overlapping_view'),
)
14 changes: 14 additions & 0 deletions tests/regressiontests/views/tests/specials.py
Expand Up @@ -37,3 +37,17 @@ def test_permanent_nonascii_redirect(self):
response = self.client.get('/permanent_nonascii_redirect/')
self.assertRedirects(response, self.redirect_target, status_code=301)

def test_overlapping_urls_reverse(self):
from django.core import urlresolvers
url = urlresolvers.reverse('overlapping_view1', kwargs={'title':'sometitle'})
self.assertEqual(url, '/overlapping_view/sometitle/')
url = urlresolvers.reverse('overlapping_view2', kwargs={'author':'someauthor'})
self.assertEqual(url, '/overlapping_view/someauthor/')

def test_overlapping_urls_resolve(self):
response = self.client.get('/overlapping_view/sometitle/')
self.assertContains(response, 'overlapping_view2')

def test_overlapping_urls_not_resolve(self):
response = self.client.get('/no_overlapping_view/sometitle/')
self.assertEqual(response.status_code, 404)
15 changes: 14 additions & 1 deletion tests/regressiontests/views/views.py
Expand Up @@ -3,7 +3,7 @@
import sys

from django.core.exceptions import PermissionDenied
from django.core.urlresolvers import get_resolver
from django.core.urlresolvers import get_resolver, DoesNotResolve

This comment has been minimized.

Copy link
@charettes

charettes Sep 13, 2012

DoesNotResolve should be imported before get_resolver.

from django.http import HttpResponse, HttpResponseRedirect
from django.shortcuts import render_to_response, render
from django.template import Context, RequestContext, TemplateDoesNotExist
Expand Down Expand Up @@ -226,5 +226,18 @@ def method(self, request):
send_log(request, exc_info)
return technical_500_response(request, *exc_info)

def overlapping_view1(request, title=None):
raise DoesNotResolve

def overlapping_view2(request, author=None):
return HttpResponse("overlapping_view2")

def overlapping_view3(request, keywords=None):
return HttpResponse("overlapping_view3")

def no_overlapping_view(request, keywords=None):
raise DoesNotResolve


def sensitive_method_view(request):
return Klass().method(request)

1 comment on commit 2ef7d14

@meric
Copy link
Owner Author

@meric meric commented on 2ef7d14 Sep 13, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @charettes!

I've followed your comments and fixed up the code:

a3905d9

https://github.com/meric/django/compare/ticket_16774

Please sign in to comment.