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.
This commit is contained in:
Preston Timmons 2015-03-31 21:57:26 -05:00 committed by Tim Graham
parent 4ea1909d3c
commit 0808ccce38
6 changed files with 46 additions and 4 deletions

View File

@ -1301,10 +1301,16 @@ class Library(object):
class InclusionNode(TagHelperNode): class InclusionNode(TagHelperNode):
def render(self, context): 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) resolved_args, resolved_kwargs = self.get_resolved_arguments(context)
_dict = func(*resolved_args, **resolved_kwargs) _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): if isinstance(file_name, Template):
t = file_name t = file_name
elif isinstance(getattr(file_name, 'template', None), Template): elif isinstance(getattr(file_name, 'template', None), Template):
@ -1313,7 +1319,7 @@ class Library(object):
t = context.template.engine.select_template(file_name) t = context.template.engine.select_template(file_name)
else: else:
t = context.template.engine.get_template(file_name) t = context.template.engine.get_template(file_name)
self.nodelist = t.nodelist context.render_context[self] = t
new_context = context.new(_dict) new_context = context.new(_dict)
# Copy across the CSRF token, if present, because # Copy across the CSRF token, if present, because
# inclusion tags are often used for forms, and we need # inclusion tags are often used for forms, and we need
@ -1322,7 +1328,7 @@ class Library(object):
csrf_token = context.get('csrf_token', None) csrf_token = context.get('csrf_token', None)
if csrf_token is not None: if csrf_token is not None:
new_context['csrf_token'] = csrf_token new_context['csrf_token'] = csrf_token
return self.nodelist.render(new_context) return t.render(new_context)
function_name = (name or function_name = (name or
getattr(func, '_decorated_function', func).__name__) getattr(func, '_decorated_function', func).__name__)

View File

@ -0,0 +1 @@
{% block content %}base{% endblock %}

View File

@ -0,0 +1,2 @@
{% extends 'inclusion_base.html' %}
{% block content %}one{% endblock %}

View File

@ -0,0 +1,2 @@
{% extends 'inclusion_base.html' %}
{% block content %}two{% endblock %}

View File

@ -164,3 +164,13 @@ def inclusion_tag_without_context_parameter(arg):
"""Expected inclusion_tag_without_context_parameter __doc__""" """Expected inclusion_tag_without_context_parameter __doc__"""
return {} return {}
inclusion_tag_without_context_parameter.anything = "Expected inclusion_tag_without_context_parameter __dict__" 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 {}

View File

@ -2,7 +2,8 @@ from __future__ import unicode_literals
import os 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 import SimpleTestCase, ignore_warnings
from django.test.utils import extend_sys_path from django.test.utils import extend_sys_path
from django.utils.deprecation import RemovedInDjango20Warning from django.utils.deprecation import RemovedInDjango20Warning
@ -259,6 +260,26 @@ class InclusionTagTests(TagTestCase):
c.use_l10n = True c.use_l10n = True
self.assertEqual(t.render(c).strip(), '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): class AssignmentTagTests(TagTestCase):