From 0339844b70895d6162b4595ae615e6edf843c6cd Mon Sep 17 00:00:00 2001 From: Bas Peschier Date: Sun, 15 Mar 2015 11:26:47 +0100 Subject: [PATCH] Fixed #24476 -- Added context manager/decorator for overriding script prefix. Tests were using an undocumented keyword argument for easily overriding script prefix while reversing. This is now changed into a test utility which can be used as decorator or context manager. --- django/core/urlresolvers.py | 5 ++--- django/test/utils.py | 21 +++++++++++++++++++ tests/admin_views/tests.py | 12 +++-------- tests/flatpages_tests/test_models.py | 9 +++----- tests/i18n/patterns/tests.py | 21 ++++++------------- tests/urlpatterns_reverse/tests.py | 31 +++++++++++++++++----------- 6 files changed, 54 insertions(+), 45 deletions(-) diff --git a/django/core/urlresolvers.py b/django/core/urlresolvers.py index cf42768add..563d4a7ade 100644 --- a/django/core/urlresolvers.py +++ b/django/core/urlresolvers.py @@ -527,15 +527,14 @@ def resolve(path, urlconf=None): return get_resolver(urlconf).resolve(path) -def reverse(viewname, urlconf=None, args=None, kwargs=None, prefix=None, current_app=None): +def reverse(viewname, urlconf=None, args=None, kwargs=None, current_app=None): if urlconf is None: urlconf = get_urlconf() resolver = get_resolver(urlconf) args = args or [] kwargs = kwargs or {} - if prefix is None: - prefix = get_script_prefix() + prefix = get_script_prefix() if not isinstance(viewname, six.string_types): view = viewname diff --git a/django/test/utils.py b/django/test/utils.py index 5224bcfb71..0101919d11 100644 --- a/django/test/utils.py +++ b/django/test/utils.py @@ -12,11 +12,13 @@ from django.apps import apps from django.conf import UserSettingsHolder, settings from django.core import mail from django.core.signals import request_started +from django.core.urlresolvers import get_script_prefix, set_script_prefix from django.db import reset_queries from django.http import request from django.template import Template from django.test.signals import setting_changed, template_rendered from django.utils import six +from django.utils.decorators import ContextDecorator from django.utils.encoding import force_str from django.utils.translation import deactivate @@ -595,3 +597,22 @@ def require_jinja2(test_func): 'OPTIONS': {'keep_trailing_newline': True}, }])(test_func) return test_func + + +class ScriptPrefix(ContextDecorator): + def __enter__(self): + set_script_prefix(self.prefix) + + def __exit__(self, exc_type, exc_val, traceback): + set_script_prefix(self.old_prefix) + + def __init__(self, prefix): + self.prefix = prefix + self.old_prefix = get_script_prefix() + + +def override_script_prefix(prefix): + """ + Decorator or context manager to temporary override the script prefix. + """ + return ScriptPrefix(prefix) diff --git a/tests/admin_views/tests.py b/tests/admin_views/tests.py index 44490f7a7d..429f9d7b6c 100644 --- a/tests/admin_views/tests.py +++ b/tests/admin_views/tests.py @@ -22,15 +22,13 @@ from django.contrib.staticfiles.storage import staticfiles_storage from django.core import mail from django.core.checks import Error from django.core.files import temp as tempfile -from django.core.urlresolvers import ( - NoReverseMatch, get_script_prefix, resolve, reverse, set_script_prefix, -) +from django.core.urlresolvers import NoReverseMatch, resolve, reverse from django.forms.utils import ErrorList from django.template.response import TemplateResponse from django.test import ( TestCase, modify_settings, override_settings, skipUnlessDBFeature, ) -from django.test.utils import patch_logger +from django.test.utils import override_script_prefix, patch_logger from django.utils import formats, six, translation from django.utils._os import upath from django.utils.cache import get_max_age @@ -5822,16 +5820,12 @@ class AdminKeepChangeListFiltersTests(TestCase): add_preserved_filters(context, url), ) - original_prefix = get_script_prefix() - try: - set_script_prefix('/prefix/') + with override_script_prefix('/prefix/'): url = reverse('admin:auth_user_changelist', current_app=self.admin_site.name) self.assertURLEqual( self.get_changelist_url(), add_preserved_filters(context, url), ) - finally: - set_script_prefix(original_prefix) class NamespacedAdminKeepChangeListFiltersTests(AdminKeepChangeListFiltersTests): diff --git a/tests/flatpages_tests/test_models.py b/tests/flatpages_tests/test_models.py index c48dc38637..8c67450f40 100644 --- a/tests/flatpages_tests/test_models.py +++ b/tests/flatpages_tests/test_models.py @@ -3,8 +3,8 @@ from __future__ import unicode_literals from django.contrib.flatpages.models import FlatPage -from django.core.urlresolvers import clear_script_prefix, set_script_prefix from django.test import TestCase +from django.test.utils import override_script_prefix class FlatpageModelTests(TestCase): @@ -13,10 +13,7 @@ class FlatpageModelTests(TestCase): pf = FlatPage(title="Café!", url='/café/') self.assertEqual(pf.get_absolute_url(), '/caf%C3%A9/') + @override_script_prefix('/beverages/') def test_get_absolute_url_honors_script_prefix(self): pf = FlatPage(title="Tea!", url='/tea/') - set_script_prefix('/beverages/') - try: - self.assertEqual(pf.get_absolute_url(), '/beverages/tea/') - finally: - clear_script_prefix() + self.assertEqual(pf.get_absolute_url(), '/beverages/tea/') diff --git a/tests/i18n/patterns/tests.py b/tests/i18n/patterns/tests.py index 62ca49b67d..07a01f6d9b 100644 --- a/tests/i18n/patterns/tests.py +++ b/tests/i18n/patterns/tests.py @@ -3,13 +3,12 @@ from __future__ import unicode_literals import os from django.core.exceptions import ImproperlyConfigured -from django.core.urlresolvers import ( - clear_url_caches, reverse, set_script_prefix, translate_url, -) +from django.core.urlresolvers import clear_url_caches, reverse, translate_url from django.http import HttpResponsePermanentRedirect from django.middleware.locale import LocaleMiddleware from django.template import Context, Template from django.test import TestCase, override_settings +from django.test.utils import override_script_prefix from django.utils import translation from django.utils._os import upath @@ -314,19 +313,11 @@ class URLRedirectWithScriptAliasTests(URLTestCaseBase): """ #21579 - LocaleMiddleware should respect the script prefix. """ - def setUp(self): - super(URLRedirectWithScriptAliasTests, self).setUp() - self.script_prefix = '/script_prefix' - set_script_prefix(self.script_prefix) - - def tearDown(self): - super(URLRedirectWithScriptAliasTests, self).tearDown() - # reset script prefix - set_script_prefix('') - def test_language_prefix_with_script_prefix(self): - response = self.client.get('/prefixed/', HTTP_ACCEPT_LANGUAGE='en', SCRIPT_NAME=self.script_prefix) - self.assertRedirects(response, '%s/en/prefixed/' % self.script_prefix, target_status_code=404) + prefix = '/script_prefix' + with override_script_prefix(prefix): + response = self.client.get('/prefixed/', HTTP_ACCEPT_LANGUAGE='en', SCRIPT_NAME=prefix) + self.assertRedirects(response, '%s/en/prefixed/' % prefix, target_status_code=404) class URLTagTests(URLTestCaseBase): diff --git a/tests/urlpatterns_reverse/tests.py b/tests/urlpatterns_reverse/tests.py index 28f4ff4d0b..4e2fcd3a31 100644 --- a/tests/urlpatterns_reverse/tests.py +++ b/tests/urlpatterns_reverse/tests.py @@ -24,6 +24,7 @@ from django.shortcuts import redirect from django.test import ( SimpleTestCase, TestCase, ignore_warnings, override_settings, ) +from django.test.utils import override_script_prefix from django.utils import six from django.utils.deprecation import RemovedInDjango20Warning @@ -199,33 +200,38 @@ class URLPatternReverse(TestCase): # Reversing None should raise an error, not return the last un-named view. self.assertRaises(NoReverseMatch, reverse, None) + @override_script_prefix('/{{invalid}}/') def test_prefix_braces(self): self.assertEqual( '/%7B%7Binvalid%7D%7D/includes/non_path_include/', - reverse('non_path_include', prefix='/{{invalid}}/') + reverse('non_path_include') ) def test_prefix_parenthesis(self): # Parentheses are allowed and should not cause errors or be escaped - self.assertEqual( - '/bogus)/includes/non_path_include/', - reverse('non_path_include', prefix='/bogus)/') - ) - self.assertEqual( - '/(bogus)/includes/non_path_include/', - reverse('non_path_include', prefix='/(bogus)/') - ) + with override_script_prefix('/bogus)/'): + self.assertEqual( + '/bogus)/includes/non_path_include/', + reverse('non_path_include') + ) + with override_script_prefix('/(bogus)/'): + self.assertEqual( + '/(bogus)/includes/non_path_include/', + reverse('non_path_include') + ) + @override_script_prefix('/bump%20map/') def test_prefix_format_char(self): self.assertEqual( '/bump%2520map/includes/non_path_include/', - reverse('non_path_include', prefix='/bump%20map/') + reverse('non_path_include') ) + @override_script_prefix('/%7Eme/') def test_non_urlsafe_prefix_with_args(self): # Regression for #20022, adjusted for #24013 because ~ is an unreserved # character. Tests whether % is escaped. - self.assertEqual('/%257Eme/places/1/', reverse('places', args=[1], prefix='/%7Eme/')) + self.assertEqual('/%257Eme/places/1/', reverse('places', args=[1])) def test_patterns_reported(self): # Regression for #17076 @@ -240,9 +246,10 @@ class URLPatternReverse(TestCase): # exception self.fail("Expected a NoReverseMatch, but none occurred.") + @override_script_prefix('/script:name/') def test_script_name_escaping(self): self.assertEqual( - reverse('optional', args=['foo:bar'], prefix='/script:name/'), + reverse('optional', args=['foo:bar']), '/script:name/optional/foo:bar/' )