From 0808ccce3808235c5b5a56e3f689cec0d4bc0ebf Mon Sep 17 00:00:00 2001 From: Preston Timmons Date: Tue, 31 Mar 2015 21:57:26 -0500 Subject: [PATCH] Fixed #23441, #24555 -- Improved the behavior of InclusionNode. This change: * Makes the InclusionNode cache-safe by removing render-time side effects to its nodelist. * Ensures the render_context stack is properly scoped and reset by updating the render call to use Template.render rather than Nodelist.render. --- django/template/base.py | 12 +++++++--- .../templates/inclusion_base.html | 1 + .../templates/inclusion_extends1.html | 2 ++ .../templates/inclusion_extends2.html | 2 ++ .../template_tests/templatetags/inclusion.py | 10 ++++++++ tests/template_tests/test_custom.py | 23 ++++++++++++++++++- 6 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 tests/template_tests/templates/inclusion_base.html create mode 100644 tests/template_tests/templates/inclusion_extends1.html create mode 100644 tests/template_tests/templates/inclusion_extends2.html diff --git a/django/template/base.py b/django/template/base.py index a29f1332a8..b9a305591c 100644 --- a/django/template/base.py +++ b/django/template/base.py @@ -1301,10 +1301,16 @@ class Library(object): class InclusionNode(TagHelperNode): def render(self, context): + """ + Renders the specified template and context. Caches the + template object in render_context to avoid reparsing and + loading when used in a for loop. + """ resolved_args, resolved_kwargs = self.get_resolved_arguments(context) _dict = func(*resolved_args, **resolved_kwargs) - if not getattr(self, 'nodelist', False): + t = context.render_context.get(self) + if t is None: if isinstance(file_name, Template): t = file_name elif isinstance(getattr(file_name, 'template', None), Template): @@ -1313,7 +1319,7 @@ class Library(object): t = context.template.engine.select_template(file_name) else: t = context.template.engine.get_template(file_name) - self.nodelist = t.nodelist + context.render_context[self] = t new_context = context.new(_dict) # Copy across the CSRF token, if present, because # inclusion tags are often used for forms, and we need @@ -1322,7 +1328,7 @@ class Library(object): csrf_token = context.get('csrf_token', None) if csrf_token is not None: new_context['csrf_token'] = csrf_token - return self.nodelist.render(new_context) + return t.render(new_context) function_name = (name or getattr(func, '_decorated_function', func).__name__) diff --git a/tests/template_tests/templates/inclusion_base.html b/tests/template_tests/templates/inclusion_base.html new file mode 100644 index 0000000000..795c4dad32 --- /dev/null +++ b/tests/template_tests/templates/inclusion_base.html @@ -0,0 +1 @@ +{% block content %}base{% endblock %} diff --git a/tests/template_tests/templates/inclusion_extends1.html b/tests/template_tests/templates/inclusion_extends1.html new file mode 100644 index 0000000000..4827f42d3c --- /dev/null +++ b/tests/template_tests/templates/inclusion_extends1.html @@ -0,0 +1,2 @@ +{% extends 'inclusion_base.html' %} +{% block content %}one{% endblock %} diff --git a/tests/template_tests/templates/inclusion_extends2.html b/tests/template_tests/templates/inclusion_extends2.html new file mode 100644 index 0000000000..0ec4d64c04 --- /dev/null +++ b/tests/template_tests/templates/inclusion_extends2.html @@ -0,0 +1,2 @@ +{% extends 'inclusion_base.html' %} +{% block content %}two{% endblock %} diff --git a/tests/template_tests/templatetags/inclusion.py b/tests/template_tests/templatetags/inclusion.py index 77c9abeb5c..1631ce92b9 100644 --- a/tests/template_tests/templatetags/inclusion.py +++ b/tests/template_tests/templatetags/inclusion.py @@ -164,3 +164,13 @@ def inclusion_tag_without_context_parameter(arg): """Expected inclusion_tag_without_context_parameter __doc__""" return {} inclusion_tag_without_context_parameter.anything = "Expected inclusion_tag_without_context_parameter __dict__" + + +@register.inclusion_tag('inclusion_extends1.html') +def inclusion_extends1(): + return {} + + +@register.inclusion_tag('inclusion_extends2.html') +def inclusion_extends2(): + return {} diff --git a/tests/template_tests/test_custom.py b/tests/template_tests/test_custom.py index 2b70380825..fae8fc4e0b 100644 --- a/tests/template_tests/test_custom.py +++ b/tests/template_tests/test_custom.py @@ -2,7 +2,8 @@ from __future__ import unicode_literals import os -from django.template import Context, Template, TemplateSyntaxError +from django.template import Context, Engine, Template, TemplateSyntaxError +from django.template.base import Node from django.test import SimpleTestCase, ignore_warnings from django.test.utils import extend_sys_path from django.utils.deprecation import RemovedInDjango20Warning @@ -259,6 +260,26 @@ class InclusionTagTests(TagTestCase): c.use_l10n = True self.assertEqual(t.render(c).strip(), 'True') + def test_no_render_side_effect(self): + """ + #23441 -- InclusionNode shouldn't modify its nodelist at render time. + """ + engine = Engine(app_dirs=True) + template = engine.from_string('{% load inclusion %}{% inclusion_no_params %}') + count = template.nodelist.get_nodes_by_type(Node) + template.render(Context({})) + self.assertEqual(template.nodelist.get_nodes_by_type(Node), count) + + def test_render_context_is_cleared(self): + """ + #24555 -- InclusionNode should push and pop the render_context stack + when rendering. Otherwise, leftover values such as blocks from + extending can interfere with subsequent rendering. + """ + engine = Engine(app_dirs=True) + template = engine.from_string('{% load inclusion %}{% inclusion_extends1 %}{% inclusion_extends2 %}') + self.assertEqual(template.render(Context({})).strip(), 'one\ntwo') + class AssignmentTagTests(TagTestCase):