From a3e783fe11dd25bbf84bfb6201186566ed473506 Mon Sep 17 00:00:00 2001 From: Aymeric Augustin Date: Thu, 8 Jan 2015 15:03:43 +0100 Subject: [PATCH] Deprecated passing a Context to a generic Template.render. A deprecation path is required because the return type of django.template.loader.get_template changed during the multiple template engines refactor. test_csrf_token_in_404 was incorrect: it tested the case when the hardcoded template was rendered, and that template doesn't depend on the CSRF token. This commit makes it test the case when a custom template is rendered. --- .../contrib/admin/templatetags/admin_list.py | 6 ++-- django/contrib/flatpages/views.py | 11 +++---- django/contrib/syndication/views.py | 6 ++-- django/template/backends/django.py | 32 +++++++++++++++++-- django/template/response.py | 5 +++ django/views/defaults.py | 15 +++++---- docs/internals/deprecation.txt | 6 ++++ docs/ref/templates/upgrading.txt | 2 ++ docs/releases/1.8.txt | 17 ++++++++++ tests/template_backends/test_django.py | 21 +++++++++++- tests/template_tests/test_loaders.py | 8 ++--- tests/template_tests/tests.py | 12 +++---- tests/view_tests/tests/test_defaults.py | 15 +++++++-- 13 files changed, 120 insertions(+), 36 deletions(-) diff --git a/django/contrib/admin/templatetags/admin_list.py b/django/contrib/admin/templatetags/admin_list.py index 266b880d99..6f7c530aa7 100644 --- a/django/contrib/admin/templatetags/admin_list.py +++ b/django/contrib/admin/templatetags/admin_list.py @@ -19,7 +19,7 @@ from django.utils.translation import ugettext as _ from django.utils.encoding import force_text from django.template import Library from django.template.loader import get_template -from django.template.context import Context + register = Library() @@ -412,11 +412,11 @@ def search_form(cl): @register.simple_tag def admin_list_filter(cl, spec): tpl = get_template(spec.template) - return tpl.render(Context({ + return tpl.render({ 'title': spec.title, 'choices': list(spec.choices(cl)), 'spec': spec, - })) + }) @register.inclusion_tag('admin/actions.html', takes_context=True) diff --git a/django/contrib/flatpages/views.py b/django/contrib/flatpages/views.py index 81d04c33b8..b8f65e4e55 100644 --- a/django/contrib/flatpages/views.py +++ b/django/contrib/flatpages/views.py @@ -3,7 +3,7 @@ from django.contrib.flatpages.models import FlatPage from django.contrib.sites.shortcuts import get_current_site from django.http import Http404, HttpResponse, HttpResponsePermanentRedirect from django.shortcuts import get_object_or_404 -from django.template import loader, RequestContext +from django.template import loader from django.utils.safestring import mark_safe from django.views.decorators.csrf import csrf_protect @@ -58,9 +58,9 @@ def render_flatpage(request, f): from django.contrib.auth.views import redirect_to_login return redirect_to_login(request.path) if f.template_name: - t = loader.select_template((f.template_name, DEFAULT_TEMPLATE)) + template = loader.select_template((f.template_name, DEFAULT_TEMPLATE)) else: - t = loader.get_template(DEFAULT_TEMPLATE) + template = loader.get_template(DEFAULT_TEMPLATE) # To avoid having to always use the "|safe" filter in flatpage templates, # mark the title and content as already safe (since they are raw HTML @@ -68,8 +68,5 @@ def render_flatpage(request, f): f.title = mark_safe(f.title) f.content = mark_safe(f.content) - c = RequestContext(request, { - 'flatpage': f, - }) - response = HttpResponse(t.render(c)) + response = HttpResponse(template.render({'flatpage': f}, request)) return response diff --git a/django/contrib/syndication/views.py b/django/contrib/syndication/views.py index ea3777153b..da38323b01 100644 --- a/django/contrib/syndication/views.py +++ b/django/contrib/syndication/views.py @@ -6,7 +6,7 @@ from django.conf import settings from django.contrib.sites.shortcuts import get_current_site from django.core.exceptions import ImproperlyConfigured, ObjectDoesNotExist from django.http import HttpResponse, Http404 -from django.template import loader, TemplateDoesNotExist, RequestContext +from django.template import loader, TemplateDoesNotExist from django.utils import feedgenerator from django.utils.encoding import force_text, iri_to_uri, smart_text from django.utils.html import escape @@ -162,11 +162,11 @@ class Feed(object): context = self.get_context_data(item=item, site=current_site, obj=obj, request=request) if title_tmp is not None: - title = title_tmp.render(RequestContext(request, context)) + title = title_tmp.render(context, request) else: title = self.__get_dynamic_attr('item_title', item) if description_tmp is not None: - description = description_tmp.render(RequestContext(request, context)) + description = description_tmp.render(context, request) else: description = self.__get_dynamic_attr('item_description', item) link = add_domain( diff --git a/django/template/backends/django.py b/django/template/backends/django.py index 63747f7c7f..2fe3227237 100644 --- a/django/template/backends/django.py +++ b/django/template/backends/django.py @@ -1,9 +1,12 @@ # Since this package contains a "django" module, this is required on Python 2. from __future__ import absolute_import +import warnings + from django.conf import settings from django.template.context import Context, RequestContext from django.template.engine import _dirs_undefined, Engine +from django.utils.deprecation import RemovedInDjango20Warning from .base import BaseEngine @@ -40,8 +43,33 @@ class Template(object): return self.template.origin def render(self, context=None, request=None): - # TODO: require context to be a dict -- through a deprecation path? - if not isinstance(context, Context): + # A deprecation path is required here to cover the following usage: + # >>> from django.template import Context + # >>> from django.template.loader import get_template + # >>> template = get_template('hello.html') + # >>> template.render(Context({'name': 'world'})) + # In Django 1.7 get_template() returned a django.template.Template. + # In Django 1.8 it returns a django.template.backends.django.Template. + # In Django 2.0 the isinstance checks should be removed. If passing a + # Context or a RequestContext works by accident, it won't be an issue + # per se, but it won't be officially supported either. + if isinstance(context, RequestContext): + if request is not None and request is not context.request: + raise ValueError( + "render() was called with a RequestContext and a request " + "argument which refer to different requests. Make sure " + "that the context argument is a dict or at least that " + "the two arguments refer to the same request.") + warnings.warn( + "render() must be called with a dict, not a RequestContext.", + RemovedInDjango20Warning, stacklevel=2) + + elif isinstance(context, Context): + warnings.warn( + "render() must be called with a dict, not a Context.", + RemovedInDjango20Warning, stacklevel=2) + + else: if request is None: context = Context(context) else: diff --git a/django/template/response.py b/django/template/response.py index 6f23f66b7d..8f175b14e6 100644 --- a/django/template/response.py +++ b/django/template/response.py @@ -82,6 +82,11 @@ class SimpleTemplateResponse(HttpResponse): """ template = self.resolve_template(self.template_name) context = self.resolve_context(self.context_data) + # TODO - remove this hack - makes the tests pass until the next commit + try: + template = template.template + except AttributeError: + pass content = template.render(context) return content diff --git a/django/views/defaults.py b/django/views/defaults.py index c33284e756..e71756d1dd 100644 --- a/django/views/defaults.py +++ b/django/views/defaults.py @@ -1,6 +1,5 @@ from django import http -from django.template import (Context, RequestContext, - loader, Template, TemplateDoesNotExist) +from django.template import loader, Context, Engine, TemplateDoesNotExist from django.views.decorators.csrf import requires_csrf_token @@ -17,15 +16,17 @@ def page_not_found(request, template_name='404.html'): request_path The path of the requested URL (e.g., '/app/pages/bad_page/') """ + context = {'request_path': request.path} try: template = loader.get_template(template_name) + body = template.render(context, request) content_type = None # Django will use DEFAULT_CONTENT_TYPE except TemplateDoesNotExist: - template = Template( + template = Engine().from_string( '

Not Found

' '

The requested URL {{ request_path }} was not found on this server.

') + body = template.render(Context(context)) content_type = 'text/html' - body = template.render(RequestContext(request, {'request_path': request.path})) return http.HttpResponseNotFound(body, content_type=content_type) @@ -41,7 +42,7 @@ def server_error(request, template_name='500.html'): template = loader.get_template(template_name) except TemplateDoesNotExist: return http.HttpResponseServerError('

Server Error (500)

', content_type='text/html') - return http.HttpResponseServerError(template.render(Context({}))) + return http.HttpResponseServerError(template.render()) @requires_csrf_token @@ -56,7 +57,7 @@ def bad_request(request, template_name='400.html'): template = loader.get_template(template_name) except TemplateDoesNotExist: return http.HttpResponseBadRequest('

Bad Request (400)

', content_type='text/html') - return http.HttpResponseBadRequest(template.render(Context({}))) + return http.HttpResponseBadRequest(template.render()) # This can be called when CsrfViewMiddleware.process_view has not run, @@ -77,4 +78,4 @@ def permission_denied(request, template_name='403.html'): template = loader.get_template(template_name) except TemplateDoesNotExist: return http.HttpResponseForbidden('

403 Forbidden

', content_type='text/html') - return http.HttpResponseForbidden(template.render(RequestContext(request))) + return http.HttpResponseForbidden(template.render(request=request)) diff --git a/docs/internals/deprecation.txt b/docs/internals/deprecation.txt index fe392cff97..e9161e7254 100644 --- a/docs/internals/deprecation.txt +++ b/docs/internals/deprecation.txt @@ -108,6 +108,12 @@ details on these changes. * The backwards compatibility alias ``django.template.loader.BaseLoader`` will be removed. +* Django template objects returned by + :func:`~django.template.loader.get_template` and + :func:`~django.template.loader.select_template` won't accept a + :class:`~django.template.Context` in their + :meth:`~django.template.backends.base.Template.render()` method anymore. + * The ``current_app`` parameter for the following function and classes will be removed: diff --git a/docs/ref/templates/upgrading.txt b/docs/ref/templates/upgrading.txt index ca336e016d..c7b07464b0 100644 --- a/docs/ref/templates/upgrading.txt +++ b/docs/ref/templates/upgrading.txt @@ -95,6 +95,8 @@ entire :setting:`TEMPLATES` setting instead. :mod:`django.template.loader` ============================= +.. _get_template-upgrade-django-18: + :func:`~django.template.loader.get_template` and :func:`~django.template.loader.select_template` ------------------------------------------------------------------------------------------------ diff --git a/docs/releases/1.8.txt b/docs/releases/1.8.txt index 122064971c..4c3b398cd1 100644 --- a/docs/releases/1.8.txt +++ b/docs/releases/1.8.txt @@ -1380,6 +1380,23 @@ to construct the "view on site" URL. This URL is now accessible using the sure to provide a default value for the ``form_class`` argument since it's now optional. +Rendering templates loaded by :func:`~django.template.loader.get_template()` with a :class:`~django.template.Context` +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +The return type of :func:`~django.template.loader.get_template()` has changed +in Django 1.8: instead of a :class:`django.template.Template`, it returns a +``Template`` instance whose exact type depends on which backend loaded it. + +Both classes provide a ``render()`` method, however, the former takes a +:class:`django.template.Context` as an argument while the latter expects a +:class:`dict`. This change is enforced through a deprecation path for Django +templates. + +Since it's easier to understand with examples, the :ref:`upgrade guide +` shows how to adapt affected code. + +All this also applies to :func:`~django.template.loader.select_template()`. + ``current_app`` argument of template-related APIs ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/tests/template_backends/test_django.py b/tests/template_backends/test_django.py index f332abbf24..2aaf6b57d8 100644 --- a/tests/template_backends/test_django.py +++ b/tests/template_backends/test_django.py @@ -1,5 +1,7 @@ +from django.template import RequestContext from django.template.backends.django import DjangoTemplates -from django.test import RequestFactory +from django.test import ignore_warnings, RequestFactory +from django.utils.deprecation import RemovedInDjango20Warning from template_tests.test_response import test_processor_name @@ -32,3 +34,20 @@ class DjangoTemplatesTests(TemplateStringsTests): # Check that context overrides context processors content = template.render({'processors': 'no'}, request) self.assertEqual(content, 'no') + + @ignore_warnings(category=RemovedInDjango20Warning) + def test_request_context_conflicts_with_request(self): + template = self.engine.from_string('hello') + + request = RequestFactory().get('/') + request_context = RequestContext(request) + # This doesn't raise an exception. + template.render(request_context, request) + + other_request = RequestFactory().get('/') + msg = ("render() was called with a RequestContext and a request " + "argument which refer to different requests. Make sure " + "that the context argument is a dict or at least that " + "the two arguments refer to the same request.") + with self.assertRaisesMessage(ValueError, msg): + template.render(request_context, other_request) diff --git a/tests/template_tests/test_loaders.py b/tests/template_tests/test_loaders.py index 073e57bf67..735dbac87a 100644 --- a/tests/template_tests/test_loaders.py +++ b/tests/template_tests/test_loaders.py @@ -210,12 +210,12 @@ class TemplateDirsOverrideTest(SimpleTestCase): def test_get_template(self): for dirs in self.dirs_iter: template = loader.get_template('test_dirs.html', dirs=dirs) - self.assertEqual(template.render(Context({})), 'spam eggs\n') + self.assertEqual(template.render(), 'spam eggs\n') def test_select_template(self): for dirs in self.dirs_iter: template = loader.select_template(['test_dirs.html'], dirs=dirs) - self.assertEqual(template.render(Context({})), 'spam eggs\n') + self.assertEqual(template.render(), 'spam eggs\n') @override_settings(TEMPLATES=[{ @@ -236,7 +236,7 @@ class PriorityCacheLoader(SimpleTestCase): Check that the order of template loader works. Refs #21460. """ t1 = loader.get_template('priority/foo.html') - self.assertEqual(t1.render(Context({})), 'priority\n') + self.assertEqual(t1.render(), 'priority\n') @override_settings(TEMPLATES=[{ @@ -255,4 +255,4 @@ class PriorityLoader(SimpleTestCase): Check that the order of template loader works. Refs #21460. """ t1 = loader.get_template('priority/foo.html') - self.assertEqual(t1.render(Context({})), 'priority\n') + self.assertEqual(t1.render(), 'priority\n') diff --git a/tests/template_tests/tests.py b/tests/template_tests/tests.py index b404a83098..8b93039dca 100644 --- a/tests/template_tests/tests.py +++ b/tests/template_tests/tests.py @@ -156,7 +156,7 @@ class TemplateLoaderTests(SimpleTestCase): r = None try: tmpl = loader.select_template([load_name]) - r = tmpl.render(template.Context({})) + r = tmpl.render() except template.TemplateDoesNotExist as e: self.assertEqual(e.args[0], 'missing.html') self.assertEqual(r, None, 'Template rendering unexpectedly succeeded, produced: ->%r<-' % r) @@ -182,7 +182,7 @@ class TemplateLoaderTests(SimpleTestCase): tmpl = loader.get_template(load_name) r = None try: - r = tmpl.render(template.Context({})) + r = tmpl.render() except template.TemplateDoesNotExist as e: self.assertEqual(e.args[0], 'missing.html') self.assertEqual(r, None, 'Template rendering unexpectedly succeeded, produced: ->%r<-' % r) @@ -207,7 +207,7 @@ class TemplateLoaderTests(SimpleTestCase): tmpl = loader.get_template(load_name) r = None try: - r = tmpl.render(template.Context({})) + r = tmpl.render() except template.TemplateDoesNotExist as e: self.assertEqual(e.args[0], 'missing.html') self.assertEqual(r, None, 'Template rendering unexpectedly succeeded, produced: ->%r<-' % r) @@ -216,7 +216,7 @@ class TemplateLoaderTests(SimpleTestCase): # result that behaves incorrectly on subsequent attempts. tmpl = loader.get_template(load_name) try: - tmpl.render(template.Context({})) + tmpl.render() except template.TemplateDoesNotExist as e: self.assertEqual(e.args[0], 'missing.html') self.assertEqual(r, None, 'Template rendering unexpectedly succeeded, produced: ->%r<-' % r) @@ -262,7 +262,7 @@ class TemplateLoaderTests(SimpleTestCase): t = loader.get_template('recursive_include.html') self.assertEqual( "Recursion! A1 Recursion! B1 B2 B3 Recursion! C1", - t.render(Context({'comments': comments})).replace(' ', '').replace('\n', ' ').strip(), + t.render({'comments': comments}).replace(' ', '').replace('\n', ' ').strip(), ) @@ -400,7 +400,7 @@ class TemplateRegressionTests(SimpleTestCase): """ t = loader.get_template('included_content.html') with self.assertRaises(urlresolvers.NoReverseMatch): - t.render(Context({})) + t.render() def test_debug_tag_non_ascii(self): """ diff --git a/tests/view_tests/tests/test_defaults.py b/tests/view_tests/tests/test_defaults.py index 7b95211a5a..491e1bd562 100644 --- a/tests/view_tests/tests/test_defaults.py +++ b/tests/view_tests/tests/test_defaults.py @@ -19,6 +19,16 @@ class DefaultsTests(TestCase): response = self.client.get(url) self.assertEqual(response.status_code, 404) + @override_settings(TEMPLATES=[{ + 'BACKEND': 'django.template.backends.django.DjangoTemplates', + 'OPTIONS': { + 'loaders': [ + ('django.template.loaders.locmem.Loader', { + '404.html': '{{ csrf_token }}', + }), + ], + }, + }]) def test_csrf_token_in_404(self): """ The 404 page should have the csrf_token available in the context @@ -26,9 +36,8 @@ class DefaultsTests(TestCase): # See ticket #14565 for url in self.non_existing_urls: response = self.client.get(url) - csrf_token = response.context['csrf_token'] - self.assertNotEqual(str(csrf_token), 'NOTPROVIDED') - self.assertNotEqual(str(csrf_token), '') + self.assertNotEqual(response.content, 'NOTPROVIDED') + self.assertNotEqual(response.content, '') def test_server_error(self): "The server_error view raises a 500 status"