From 9a30acad8a1996c914351bad981d937de4db29a4 Mon Sep 17 00:00:00 2001 From: Berker Peksag Date: Fri, 21 Nov 2014 18:55:58 +0200 Subject: [PATCH] Fixed #21587 -- Added a warning for changing default of RedirectView.permanent. --- django/views/generic/base.py | 18 +++++++- docs/internals/deprecation.txt | 4 ++ docs/ref/class-based-views/base.txt | 5 +++ docs/releases/1.8.txt | 7 +++ tests/generic_views/test_base.py | 69 ++++++++++++++++++++++++++++- tests/test_client/tests.py | 45 ++++++++++++------- tests/urlpatterns_reverse/tests.py | 8 ++-- 7 files changed, 135 insertions(+), 21 deletions(-) diff --git a/django/views/generic/base.py b/django/views/generic/base.py index d66f45b599..594122ef77 100644 --- a/django/views/generic/base.py +++ b/django/views/generic/base.py @@ -1,6 +1,7 @@ from __future__ import unicode_literals import logging +import warnings from functools import update_wrapper from django import http @@ -8,8 +9,10 @@ from django.core.exceptions import ImproperlyConfigured from django.core.urlresolvers import reverse, NoReverseMatch from django.template.response import TemplateResponse from django.utils.decorators import classonlymethod +from django.utils.deprecation import RemovedInDjango19Warning from django.utils import six +_sentinel = object() logger = logging.getLogger('django.request') @@ -48,7 +51,6 @@ class View(object): """ Main entry point for a request-response process. """ - # sanitize keyword arguments for key in initkwargs: if key in cls.http_method_names: raise TypeError("You tried to pass in the %s method name as a " @@ -159,11 +161,23 @@ class RedirectView(View): """ A view that provides a redirect on any GET request. """ - permanent = True + permanent = _sentinel url = None pattern_name = None query_string = False + def __init__(self, *args, **kwargs): + if 'permanent' not in kwargs and self.permanent is _sentinel: + warnings.warn( + "Default value of 'RedirectView.permanent' will change " + "from True to False in Django 1.9. Set an explicit value " + "to silence this warning.", + RemovedInDjango19Warning, + stacklevel=3 + ) + self.permanent = True + super(RedirectView, self).__init__(*args, **kwargs) + def get_redirect_url(self, *args, **kwargs): """ Return the URL redirect to. Keyword arguments from the diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index 1f4aca4cf3..3750d469dd 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -214,6 +214,10 @@ details on these changes. * The `cache_choices` option to :class:`~django.forms.ModelChoiceField` and :class:`~django.forms.ModelMultipleChoiceField` will be removed. +* The default value of the + :attr:`RedirectView.permanent ` + attribute will change from ``True`` to ``False``. + .. _deprecation-removed-in-1.8: 1.8 diff --git a/docs/ref/class-based-views/base.txt b/docs/ref/class-based-views/base.txt index e6c35fffd0..523b39d0e2 100644 --- a/docs/ref/class-based-views/base.txt +++ b/docs/ref/class-based-views/base.txt @@ -222,6 +222,11 @@ RedirectView status code 301. If ``False``, then the redirect will use status code 302. By default, ``permanent`` is ``True``. + .. deprecated:: 1.8 + + The default value of the ``permanent`` attribute will change from + ``True`` to ``False`` in Django 1.9. + .. attribute:: query_string Whether to pass along the GET query string to the new location. If diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index 7064891312..ba0b0cfa2b 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -1097,6 +1097,13 @@ deprecated: you should rename your ``qn`` arguments to ``compiler``, and call ``compiler.quote_name_unless_alias(...)`` where you previously called ``qn(...)``. +Default value of ``RedirectView.permanent`` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The default value of the +:attr:`RedirectView.permanent ` +attribute will change from ``True`` to ``False`` in Django 1.9. + .. removed-features-1.8: Features removed in 1.8 diff --git a/tests/generic_views/test_base.py b/tests/generic_views/test_base.py index 29e225088d..1a1236a084 100644 --- a/tests/generic_views/test_base.py +++ b/tests/generic_views/test_base.py @@ -2,10 +2,14 @@ from __future__ import unicode_literals import time import unittest +import warnings from django.core.exceptions import ImproperlyConfigured from django.http import HttpResponse +from django.utils import six +from django.utils.deprecation import RemovedInDjango19Warning from django.test import TestCase, RequestFactory, override_settings +from django.test.utils import IgnoreDeprecationWarningsMixin from django.views.generic import View, TemplateView, RedirectView from . import views @@ -328,7 +332,7 @@ class TemplateViewTest(TestCase): @override_settings(ROOT_URLCONF='generic_views.urls') -class RedirectViewTest(TestCase): +class RedirectViewTest(IgnoreDeprecationWarningsMixin, TestCase): rf = RequestFactory() @@ -440,6 +444,69 @@ class RedirectViewTest(TestCase): self.assertEqual(response.status_code, 410) +@override_settings(ROOT_URLCONF='generic_views.urls') +class RedirectViewDeprecationTest(TestCase): + + rf = RequestFactory() + + def test_deprecation_warning_init(self): + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always') + + view = RedirectView() + response = view.dispatch(self.rf.head('/python/')) + + self.assertEqual(response.status_code, 410) + self.assertEqual(len(warns), 1) + self.assertIs(warns[0].category, RemovedInDjango19Warning) + self.assertEqual( + six.text_type(warns[0].message), + "Default value of 'RedirectView.permanent' will change " + "from True to False in Django 1.9. Set an explicit value " + "to silence this warning.", + ) + + def test_deprecation_warning_raised_when_permanent_not_passed(self): + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always') + + view_function = RedirectView.as_view(url='/bbb/') + request = self.rf.request(PATH_INFO='/aaa/') + view_function(request) + + self.assertEqual(len(warns), 1) + self.assertIs(warns[0].category, RemovedInDjango19Warning) + self.assertEqual( + six.text_type(warns[0].message), + "Default value of 'RedirectView.permanent' will change " + "from True to False in Django 1.9. Set an explicit value " + "to silence this warning.", + ) + + def test_no_deprecation_warning_when_permanent_passed(self): + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always') + + view_function = RedirectView.as_view(url='/bar/', permanent=False) + request = self.rf.request(PATH_INFO='/foo/') + view_function(request) + + self.assertEqual(len(warns), 0) + + def test_no_deprecation_warning_with_custom_redirectview(self): + class CustomRedirectView(RedirectView): + permanent = False + + with warnings.catch_warnings(record=True) as warns: + warnings.simplefilter('always') + + view_function = CustomRedirectView.as_view(url='/eggs/') + request = self.rf.request(PATH_INFO='/spam/') + view_function(request) + + self.assertEqual(len(warns), 0) + + class GetContextDataTest(unittest.TestCase): def test_get_context_data_super(self): diff --git a/tests/test_client/tests.py b/tests/test_client/tests.py index cf96b89a62..4ed7c6c1f8 100644 --- a/tests/test_client/tests.py +++ b/tests/test_client/tests.py @@ -22,8 +22,11 @@ rather than the HTML rendered to the end-user. """ from __future__ import unicode_literals +import warnings + from django.core import mail from django.http import HttpResponse +from django.utils.deprecation import RemovedInDjango19Warning from django.test import Client, TestCase, RequestFactory from django.test import override_settings @@ -174,14 +177,18 @@ class ClientTest(TestCase): def test_permanent_redirect(self): "GET a URL that redirects permanently elsewhere" - response = self.client.get('/permanent_redirect_view/') - # Check that the response was a 301 (permanent redirect) - self.assertRedirects(response, 'http://testserver/get_view/', status_code=301) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", RemovedInDjango19Warning) + response = self.client.get('/permanent_redirect_view/') + # Check that the response was a 301 (permanent redirect) + self.assertRedirects(response, 'http://testserver/get_view/', status_code=301) - client_providing_host = Client(HTTP_HOST='django.testserver') - response = client_providing_host.get('/permanent_redirect_view/') - # Check that the response was a 301 (permanent redirect) with absolute URI - self.assertRedirects(response, 'http://django.testserver/get_view/', status_code=301) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", RemovedInDjango19Warning) + client_providing_host = Client(HTTP_HOST='django.testserver') + response = client_providing_host.get('/permanent_redirect_view/') + # Check that the response was a 301 (permanent redirect) with absolute URI + self.assertRedirects(response, 'http://django.testserver/get_view/', status_code=301) def test_temporary_redirect(self): "GET a URL that does a non-permanent redirect" @@ -191,26 +198,34 @@ class ClientTest(TestCase): def test_redirect_to_strange_location(self): "GET a URL that redirects to a non-200 page" - response = self.client.get('/double_redirect_view/') + with warnings.catch_warnings(): + warnings.simplefilter("ignore", RemovedInDjango19Warning) + response = self.client.get('/double_redirect_view/') - # Check that the response was a 302, and that - # the attempt to get the redirection location returned 301 when retrieved - self.assertRedirects(response, 'http://testserver/permanent_redirect_view/', target_status_code=301) + # Check that the response was a 302, and that + # the attempt to get the redirection location returned 301 when retrieved + self.assertRedirects(response, 'http://testserver/permanent_redirect_view/', target_status_code=301) def test_follow_redirect(self): "A URL that redirects can be followed to termination." - response = self.client.get('/double_redirect_view/', follow=True) - self.assertRedirects(response, 'http://testserver/get_view/', status_code=302, target_status_code=200) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", RemovedInDjango19Warning) + response = self.client.get('/double_redirect_view/', follow=True) + self.assertRedirects(response, 'http://testserver/get_view/', status_code=302, target_status_code=200) self.assertEqual(len(response.redirect_chain), 2) def test_redirect_http(self): "GET a URL that redirects to an http URI" - response = self.client.get('/http_redirect_view/', follow=True) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", RemovedInDjango19Warning) + response = self.client.get('/http_redirect_view/', follow=True) self.assertFalse(response.test_was_secure_request) def test_redirect_https(self): "GET a URL that redirects to an https URI" - response = self.client.get('/https_redirect_view/', follow=True) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", RemovedInDjango19Warning) + response = self.client.get('/https_redirect_view/', follow=True) self.assertTrue(response.test_was_secure_request) def test_notfound_response(self): diff --git a/tests/urlpatterns_reverse/tests.py b/tests/urlpatterns_reverse/tests.py index 18299fe733..0c4e08369e 100644 --- a/tests/urlpatterns_reverse/tests.py +++ b/tests/urlpatterns_reverse/tests.py @@ -18,7 +18,7 @@ from django.http import HttpRequest, HttpResponseRedirect, HttpResponsePermanent from django.shortcuts import redirect from django.test import TestCase, override_settings from django.utils import six -from django.utils.deprecation import RemovedInDjango20Warning +from django.utils.deprecation import RemovedInDjango19Warning, RemovedInDjango20Warning from admin_scripts.tests import AdminScriptTestCase @@ -309,8 +309,10 @@ class ResolverTests(unittest.TestCase): class ReverseLazyTest(TestCase): def test_redirect_with_lazy_reverse(self): - response = self.client.get('/redirect/') - self.assertRedirects(response, "/redirected_to/", status_code=301) + with warnings.catch_warnings(): + warnings.simplefilter("ignore", RemovedInDjango19Warning) + response = self.client.get('/redirect/') + self.assertRedirects(response, "/redirected_to/", status_code=301) def test_user_permission_with_lazy_reverse(self): User.objects.create_user('alfred', 'alfred@example.com', password='testpw')