Browse Source

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.
Aymeric Augustin 10 năm trước cách đây
mục cha
commit
a3e783fe11

+ 3 - 3
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)

+ 4 - 7
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

+ 3 - 3
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(

+ 30 - 2
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:

+ 5 - 0
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
 

+ 8 - 7
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(
             '<h1>Not Found</h1>'
             '<p>The requested URL {{ request_path }} was not found on this server.</p>')
+        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('<h1>Server Error (500)</h1>', 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('<h1>Bad Request (400)</h1>', 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('<h1>403 Forbidden</h1>', content_type='text/html')
-    return http.HttpResponseForbidden(template.render(RequestContext(request)))
+    return http.HttpResponseForbidden(template.render(request=request))

+ 6 - 0
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:
 

+ 2 - 0
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`
 ------------------------------------------------------------------------------------------------
 

+ 17 - 0
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
+<get_template-upgrade-django-18>` shows how to adapt affected code.
+
+All this also applies to :func:`~django.template.loader.select_template()`.
+
 ``current_app`` argument of template-related APIs
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

+ 20 - 1
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)

+ 4 - 4
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')

+ 6 - 6
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):
         """

+ 12 - 3
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"