From fdbfc98003f0ba2d3a12def63a75560791f3602d Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Sun, 14 Dec 2014 10:17:18 +0100 Subject: [PATCH] Deprecated some arguments of django.shortcuts.render(_to_response). dictionary and context_instance and superseded by context. Refactored tests that relied context_instance with more modern idioms. --- .../auth/tests/test_context_processors.py | 10 ++-- django/contrib/auth/tests/urls.py | 31 +++++------ django/contrib/gis/admin/widgets.py | 8 +-- django/shortcuts.py | 55 +++++++++++-------- docs/internals/deprecation.txt | 2 + docs/ref/contrib/admin/admindocs.txt | 10 ++-- docs/releases/1.8.txt | 2 + docs/topics/http/shortcuts.txt | 43 ++++++++++++--- tests/context_processors/tests.py | 12 +++- tests/context_processors/views.py | 16 ++---- tests/shortcuts/tests.py | 20 +++++-- tests/test_client_regress/tests.py | 20 ++++--- 12 files changed, 137 insertions(+), 92 deletions(-) diff --git a/django/contrib/auth/tests/test_context_processors.py b/django/contrib/auth/tests/test_context_processors.py index e0e75a947e..cb96bf5d6d 100644 --- a/django/contrib/auth/tests/test_context_processors.py +++ b/django/contrib/auth/tests/test_context_processors.py @@ -65,6 +65,10 @@ class PermWrapperTests(TestCase): TEMPLATE_DIRS=( os.path.join(os.path.dirname(upath(__file__)), 'templates'), ), + TEMPLATE_CONTEXT_PROCESSORS=( + 'django.contrib.auth.context_processors.auth', + 'django.contrib.messages.context_processors.messages' + ), ROOT_URLCONF='django.contrib.auth.tests.urls', USE_TZ=False, # required for loading the fixture PASSWORD_HASHERS=('django.contrib.auth.hashers.SHA1PasswordHasher',), @@ -80,9 +84,6 @@ class AuthContextProcessorTests(TestCase): 'django.contrib.sessions.middleware.SessionMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', ), - TEMPLATE_CONTEXT_PROCESSORS=( - 'django.contrib.auth.context_processors.auth', - ), ) def test_session_not_accessed(self): """ @@ -97,9 +98,6 @@ class AuthContextProcessorTests(TestCase): 'django.contrib.sessions.middleware.SessionMiddleware', 'django.contrib.auth.middleware.AuthenticationMiddleware', ), - TEMPLATE_CONTEXT_PROCESSORS=( - 'django.contrib.auth.context_processors.auth', - ), ) def test_session_is_accessed(self): """ diff --git a/django/contrib/auth/tests/urls.py b/django/contrib/auth/tests/urls.py index 8145f2f69c..41e742a93b 100644 --- a/django/contrib/auth/tests/urls.py +++ b/django/contrib/auth/tests/urls.py @@ -1,13 +1,12 @@ from django.conf.urls import url, include from django.contrib import admin -from django.contrib.auth import context_processors from django.contrib.auth.forms import AuthenticationForm from django.contrib.auth.urls import urlpatterns from django.contrib.auth import views from django.contrib.auth.decorators import login_required from django.contrib.messages.api import info from django.http import HttpResponse, HttpRequest -from django.shortcuts import render_to_response +from django.shortcuts import render from django.template import Template, RequestContext from django.views.decorators.cache import never_cache @@ -27,39 +26,35 @@ def remote_user_auth_view(request): def auth_processor_no_attr_access(request): - render_to_response('context_processors/auth_attrs_no_access.html', - context_instance=RequestContext(request, {}, processors=[context_processors.auth])) + render(request, 'context_processors/auth_attrs_no_access.html') # *After* rendering, we check whether the session was accessed - return render_to_response('context_processors/auth_attrs_test_access.html', - {'session_accessed': request.session.accessed}) + return render(request, + 'context_processors/auth_attrs_test_access.html', + {'session_accessed': request.session.accessed}) def auth_processor_attr_access(request): - render_to_response('context_processors/auth_attrs_access.html', - context_instance=RequestContext(request, {}, processors=[context_processors.auth])) - return render_to_response('context_processors/auth_attrs_test_access.html', - {'session_accessed': request.session.accessed}) + render(request, 'context_processors/auth_attrs_access.html') + return render(request, + 'context_processors/auth_attrs_test_access.html', + {'session_accessed': request.session.accessed}) def auth_processor_user(request): - return render_to_response('context_processors/auth_attrs_user.html', - context_instance=RequestContext(request, {}, processors=[context_processors.auth])) + return render(request, 'context_processors/auth_attrs_user.html') def auth_processor_perms(request): - return render_to_response('context_processors/auth_attrs_perms.html', - context_instance=RequestContext(request, {}, processors=[context_processors.auth])) + return render(request, 'context_processors/auth_attrs_perms.html') def auth_processor_perm_in_perms(request): - return render_to_response('context_processors/auth_attrs_perm_in_perms.html', - context_instance=RequestContext(request, {}, processors=[context_processors.auth])) + return render(request, 'context_processors/auth_attrs_perm_in_perms.html') def auth_processor_messages(request): info(request, "Message 1") - return render_to_response('context_processors/auth_attrs_messages.html', - context_instance=RequestContext(request, {}, processors=[context_processors.auth])) + return render(request, 'context_processors/auth_attrs_messages.html') def userpage(request): diff --git a/django/contrib/gis/admin/widgets.py b/django/contrib/gis/admin/widgets.py index 716327cf0a..c85e7a6125 100644 --- a/django/contrib/gis/admin/widgets.py +++ b/django/contrib/gis/admin/widgets.py @@ -1,7 +1,7 @@ import logging from django.forms.widgets import Textarea -from django.template import loader, Context +from django.template import loader from django.utils import six from django.utils import translation @@ -10,7 +10,7 @@ from django.contrib.gis.geos import GEOSGeometry, GEOSException # Creating a template context that contains Django settings # values needed by admin map templates. -geo_context = Context({'LANGUAGE_BIDI': translation.get_language_bidi()}) +geo_context = {'LANGUAGE_BIDI': translation.get_language_bidi()} logger = logging.getLogger('django.contrib.gis') @@ -81,8 +81,8 @@ class OpenLayersWidget(Textarea): # geometry. self.params['wkt'] = wkt - return loader.render_to_string(self.template, self.params, - context_instance=geo_context) + self.params.update(geo_context) + return loader.render_to_string(self.template, self.params) def map_options(self): "Builds the map options hash for the OpenLayers template." diff --git a/django/shortcuts.py b/django/shortcuts.py index f3c07a6972..780f89a6b5 100644 --- a/django/shortcuts.py +++ b/django/shortcuts.py @@ -3,8 +3,6 @@ This module collects helper functions and classes that "span" multiple levels of MVC. In other words, these functions/classes introduce controlled coupling for convenience's sake. """ -import warnings - from django.template import loader, RequestContext from django.template.context import _current_app_undefined from django.template.engine import ( @@ -16,44 +14,57 @@ from django.db.models.manager import Manager from django.db.models.query import QuerySet from django.core import urlresolvers from django.utils import six -from django.utils.deprecation import RemovedInDjango20Warning -def render_to_response(template_name, dictionary=_dictionary_undefined, +def render_to_response(template_name, context=None, context_instance=_context_instance_undefined, - content_type=None, dirs=_dirs_undefined): + content_type=None, status=None, dirs=_dirs_undefined, + dictionary=_dictionary_undefined): """ Returns a HttpResponse whose content is filled with the result of calling django.template.loader.render_to_string() with the passed arguments. """ - # TODO: refactor to avoid the deprecated code path. - with warnings.catch_warnings(): - warnings.filterwarnings("ignore", category=RemovedInDjango20Warning) - content = loader.render_to_string(template_name, dictionary, context_instance, dirs) + if (context_instance is _context_instance_undefined + and dirs is _dirs_undefined + and dictionary is _dictionary_undefined): + # No deprecated arguments were passed - use the new code path + content = loader.get_template(template_name).render(context) - return HttpResponse(content, content_type) + else: + # Some deprecated arguments were passed - use the legacy code path + content = loader.render_to_string( + template_name, context, context_instance, dirs, dictionary) + + return HttpResponse(content, content_type, status) -def render(request, template_name, dictionary=_dictionary_undefined, +def render(request, template_name, context=None, context_instance=_context_instance_undefined, content_type=None, status=None, current_app=_current_app_undefined, - dirs=_dirs_undefined): + dirs=_dirs_undefined, dictionary=_dictionary_undefined): """ Returns a HttpResponse whose content is filled with the result of calling django.template.loader.render_to_string() with the passed arguments. Uses a RequestContext by default. """ - if context_instance is not _context_instance_undefined: - if current_app is not _current_app_undefined: - raise ValueError('If you provide a context_instance you must ' - 'set its current_app before calling render()') - else: - context_instance = RequestContext(request, current_app=current_app) + if (context_instance is _context_instance_undefined + and current_app is _current_app_undefined + and dirs is _dirs_undefined + and dictionary is _dictionary_undefined): + # No deprecated arguments were passed - use the new code path + content = loader.get_template(template_name).render(context, request) - # TODO: refactor to avoid the deprecated code path. - with warnings.catch_warnings(): - warnings.filterwarnings("ignore", category=RemovedInDjango20Warning) - content = loader.render_to_string(template_name, dictionary, context_instance, dirs) + else: + # Some deprecated arguments were passed - use the legacy code path + if context_instance is not _context_instance_undefined: + if current_app is not _current_app_undefined: + raise ValueError('If you provide a context_instance you must ' + 'set its current_app before calling render()') + else: + context_instance = RequestContext(request, current_app=current_app) + + content = loader.render_to_string( + template_name, context, context_instance, dirs, dictionary) return HttpResponse(content, content_type, status) diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index f703b958a0..0adf08ed5e 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -93,6 +93,8 @@ details on these changes. * The ``dictionary`` and ``context_instance`` parameters for the following functions will be removed: + * ``django.shortcuts.render()`` + * ``django.shortcuts.render_to_response()`` * ``django.template.loader.render_to_string()`` * The ``dirs`` parameter for the following functions will be removed: diff --git a/docs/ref/contrib/admin/admindocs.txt b/docs/ref/contrib/admin/admindocs.txt index 6e2ca745cf..040dc25fe0 100644 --- a/docs/ref/contrib/admin/admindocs.txt +++ b/docs/ref/contrib/admin/admindocs.txt @@ -95,6 +95,8 @@ you can document in your view function docstrings include: For example:: + from django.shortcuts import render + from myapp.models import MyModel def my_view(request, slug): @@ -103,8 +105,6 @@ For example:: **Context** - ``RequestContext`` - ``mymodel`` An instance of :model:`myapp.MyModel`. @@ -113,10 +113,8 @@ For example:: :template:`myapp/my_template.html` """ - return render_to_response('myapp/my_template.html', { - 'mymodel': MyModel.objects.get(slug=slug) - }, context_instance=RequestContext(request)) - + context = {'mymodel': MyModel.objects.get(slug=slug)} + return render(request, 'myapp/my_template.html', context) Template tags and filters reference =================================== diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index 3e6a95006e..8f871cb96c 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -1210,6 +1210,8 @@ now optional. The following functions will no longer accept the ``dictionary`` and ``context_instance`` parameters in Django 2.0: +* ``django.shortcuts.render()`` +* ``django.shortcuts.render_to_response()`` * ``django.template.loader.render_to_string()`` Use the ``context`` parameter instead. When ``dictionary`` is passed as a diff --git a/docs/topics/http/shortcuts.txt b/docs/topics/http/shortcuts.txt index 7a92b690e9..5c356c2b3b 100644 --- a/docs/topics/http/shortcuts.txt +++ b/docs/topics/http/shortcuts.txt @@ -15,7 +15,7 @@ introduce controlled coupling for convenience's sake. ``render`` ========== -.. function:: render(request, template_name[, dictionary][, context_instance][, content_type][, status][, current_app][, dirs]) +.. function:: render(request, template_name[, context][, context_instance][, content_type][, status][, current_app][, dirs]) Combines a given template with a given context dictionary and returns an :class:`~django.http.HttpResponse` object with that rendered text. @@ -41,15 +41,24 @@ Required arguments Optional arguments ------------------ -``dictionary`` +``context`` A dictionary of values to add to the template context. By default, this is an empty dictionary. If a value in the dictionary is callable, the view will call it just before rendering the template. + .. versionchanged:: 1.8 + + The ``context`` argument used to be called ``dictionary``. That name + is deprecated in Django 1.8 and will be removed in Django 2.0. + ``context_instance`` The context instance to render the template with. By default, the template will be rendered with a ``RequestContext`` instance (filled with values from - ``request`` and ``dictionary``). + ``request`` and ``context``). + + .. deprecated:: 1.8 + + The ``context_instance`` argument is deprecated. Simply use ``context``. ``content_type`` The MIME type to use for the resulting document. Defaults to the value of @@ -67,7 +76,7 @@ Optional arguments The ``dirs`` parameter was added. -.. versionchanged:: 1.8 +.. deprecated:: 1.8 The ``dirs`` parameter was deprecated. @@ -99,7 +108,7 @@ This example is equivalent to:: ``render_to_response`` ====================== -.. function:: render_to_response(template_name[, dictionary][, context_instance][, content_type][, dirs]) +.. function:: render_to_response(template_name[, context][, context_instance][, content_type][, status][, dirs]) Renders a given template with a given context dictionary and returns an :class:`~django.http.HttpResponse` object with that rendered text. @@ -116,32 +125,48 @@ Required arguments Optional arguments ------------------ -``dictionary`` +``context`` A dictionary of values to add to the template context. By default, this is an empty dictionary. If a value in the dictionary is callable, the view will call it just before rendering the template. + .. versionchanged:: 1.8 + + The ``context`` argument used to be called ``dictionary``. That name + is deprecated in Django 1.8 and will be removed in Django 2.0. + ``context_instance`` The context instance to render the template with. By default, the template will be rendered with a :class:`~django.template.Context` instance (filled - with values from ``dictionary``). If you need to use :ref:`context + with values from ``context``). If you need to use :ref:`context processors `, render the template with a :class:`~django.template.RequestContext` instance instead. Your code might look something like this:: return render_to_response('my_template.html', - my_data_dictionary, + my_context, context_instance=RequestContext(request)) + .. deprecated:: 1.8 + + The ``context_instance`` argument is deprecated. Simply use ``context``. + ``content_type`` The MIME type to use for the resulting document. Defaults to the value of the :setting:`DEFAULT_CONTENT_TYPE` setting. +``status`` + The status code for the response. Defaults to ``200``. + +.. versionchanged:: 1.8 + + The ``status`` parameter was added. + .. versionchanged:: 1.7 The ``dirs`` parameter was added. -.. versionchanged:: 1.8 +.. deprecated:: 1.8 The ``dirs`` parameter was deprecated. diff --git a/tests/context_processors/tests.py b/tests/context_processors/tests.py index 8d58f5c695..7a0d8220a8 100644 --- a/tests/context_processors/tests.py +++ b/tests/context_processors/tests.py @@ -4,7 +4,10 @@ Tests for Django's bundled context processors. from django.test import TestCase, override_settings -@override_settings(ROOT_URLCONF='context_processors.urls') +@override_settings( + ROOT_URLCONF='context_processors.urls', + TEMPLATE_CONTEXT_PROCESSORS=('django.template.context_processors.request',), +) class RequestContextProcessorTests(TestCase): """ Tests for the ``django.template.context_processors.request`` processor. @@ -35,7 +38,12 @@ class RequestContextProcessorTests(TestCase): self.assertContains(response, url) -@override_settings(ROOT_URLCONF='context_processors.urls', DEBUG=True, INTERNAL_IPS=('127.0.0.1',)) +@override_settings( + DEBUG=True, + INTERNAL_IPS=('127.0.0.1',), + ROOT_URLCONF='context_processors.urls', + TEMPLATE_CONTEXT_PROCESSORS=('django.template.context_processors.debug',), +) class DebugContextProcessorTests(TestCase): """ Tests for the ``django.template.context_processors.debug`` processor. diff --git a/tests/context_processors/views.py b/tests/context_processors/views.py index 913877f20a..1a7eda0b16 100644 --- a/tests/context_processors/views.py +++ b/tests/context_processors/views.py @@ -1,20 +1,12 @@ -from django.shortcuts import render_to_response -from django.template import context_processors -from django.template.context import RequestContext +from django.shortcuts import render from .models import DebugObject def request_processor(request): - return render_to_response( - 'context_processors/request_attrs.html', - context_instance=RequestContext(request, {}, processors=[context_processors.request])) + return render(request, 'context_processors/request_attrs.html') def debug_processor(request): - - return render_to_response( - 'context_processors/debug.html', - context_instance=RequestContext(request, { - 'debug_objects': DebugObject.objects, - }, processors=[context_processors.debug])) + context = {'debug_objects': DebugObject.objects} + return render(request, 'context_processors/debug.html', context) diff --git a/tests/shortcuts/tests.py b/tests/shortcuts/tests.py index 029c150644..429738b22f 100644 --- a/tests/shortcuts/tests.py +++ b/tests/shortcuts/tests.py @@ -17,7 +17,9 @@ class ShortcutTests(TestCase): self.assertEqual(response['Content-Type'], 'text/html; charset=utf-8') def test_render_to_response_with_request_context(self): - response = self.client.get('/render_to_response/request_context/') + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=RemovedInDjango20Warning) + response = self.client.get('/render_to_response/request_context/') self.assertEqual(response.status_code, 200) self.assertEqual(response.content, b'FOO.BAR../path/to/static/media/\n') self.assertEqual(response['Content-Type'], 'text/html; charset=utf-8') @@ -42,7 +44,9 @@ class ShortcutTests(TestCase): RequestContext instance in the dictionary argument instead of the context_instance argument. """ - response = self.client.get('/render_to_response/context_instance_misuse/') + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=RemovedInDjango20Warning) + response = self.client.get('/render_to_response/context_instance_misuse/') self.assertContains(response, 'context processor output') def test_render(self): @@ -53,7 +57,9 @@ class ShortcutTests(TestCase): self.assertEqual(response.context.current_app, None) def test_render_with_base_context(self): - response = self.client.get('/render/base_context/') + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=RemovedInDjango20Warning) + response = self.client.get('/render/base_context/') self.assertEqual(response.status_code, 200) self.assertEqual(response.content, b'FOO.BAR..\n') self.assertEqual(response['Content-Type'], 'text/html; charset=utf-8') @@ -70,7 +76,9 @@ class ShortcutTests(TestCase): self.assertEqual(response.content, b'FOO.BAR../path/to/static/media/\n') def test_render_with_current_app(self): - response = self.client.get('/render/current_app/') + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=RemovedInDjango20Warning) + response = self.client.get('/render/current_app/') self.assertEqual(response.context.current_app, "foobar_app") def test_render_with_dirs(self): @@ -83,4 +91,6 @@ class ShortcutTests(TestCase): def test_render_with_current_app_conflict(self): with self.assertRaises(ValueError): - self.client.get('/render/current_app_conflict/') + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=RemovedInDjango20Warning) + self.client.get('/render/current_app_conflict/') diff --git a/tests/test_client_regress/tests.py b/tests/test_client_regress/tests.py index 0d63ee9429..8bd4ad44aa 100644 --- a/tests/test_client_regress/tests.py +++ b/tests/test_client_regress/tests.py @@ -6,6 +6,7 @@ from __future__ import unicode_literals import os import itertools +import warnings from django.core.urlresolvers import reverse, NoReverseMatch from django.template import TemplateSyntaxError, Context, Template @@ -14,6 +15,7 @@ from django.test.client import RedirectCycleError, RequestFactory, encode_file from django.test.utils import ContextList, str_prefix from django.template.response import SimpleTemplateResponse from django.utils._os import upath +from django.utils.deprecation import RemovedInDjango20Warning from django.utils.translation import ugettext_lazy from django.http import HttpResponse from django.contrib.auth.signals import user_logged_out, user_logged_in @@ -999,14 +1001,16 @@ class ContextTests(TestCase): l.keys()) def test_15368(self): - # Need to insert a context processor that assumes certain things about - # the request instance. This triggers a bug caused by some ways of - # copying RequestContext. - with self.settings(TEMPLATE_CONTEXT_PROCESSORS=( - 'test_client_regress.context_processors.special', - )): - response = self.client.get("/request_context_view/") - self.assertContains(response, 'Path: /request_context_view/') + with warnings.catch_warnings(): + warnings.filterwarnings("ignore", category=RemovedInDjango20Warning) + # Need to insert a context processor that assumes certain things about + # the request instance. This triggers a bug caused by some ways of + # copying RequestContext. + with self.settings(TEMPLATE_CONTEXT_PROCESSORS=( + 'test_client_regress.context_processors.special', + )): + response = self.client.get("/request_context_view/") + self.assertContains(response, 'Path: /request_context_view/') def test_nested_requests(self): """