From d338ccb9a631a4f021de91272e3972d586061584 Mon Sep 17 00:00:00 2001 From: Samuel Date: Mon, 22 Jul 2019 11:38:23 -0400 Subject: [PATCH] Add isolated subcontexts An isolated subcontext inherits the environment, filters, and static registers of its supercontext, but with a fresh (isolated) scope. This will pave the way for adding the `render` tag, which renders templates in such a subcontext. --- lib/liquid/context.rb | 22 +- lib/liquid/tags/render.rb | 89 ++++++++ test/integration/tags/render_rag_test.rb | 255 +++++++++++++++++++++++ test/integration/tags/render_tag_test.rb | 230 ++++++++++++++++++++ test/unit/context_unit_test.rb | 58 ++++++ 5 files changed, 652 insertions(+), 2 deletions(-) create mode 100644 lib/liquid/tags/render.rb create mode 100644 test/integration/tags/render_rag_test.rb create mode 100644 test/integration/tags/render_tag_test.rb diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index 2dcc6af..a05cdaa 100644 --- a/lib/liquid/context.rb +++ b/lib/liquid/context.rb @@ -12,13 +12,18 @@ module Liquid # # context['bob'] #=> nil class Context class Context - attr_reader :scopes, :errors, :registers, :environments, :resource_limits + attr_reader :scopes, :errors, :registers, :environments, :resource_limits, :static_registers attr_accessor :exception_renderer, :template_name, :partial, :global_filter, :strict_variables, :strict_filters - def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = nil) + def self.build(environments: {}, outer_scope: {}, registers: {}, rethrow_errors: false, resource_limits: nil, static_registers: {}) + new(environments, outer_scope, registers, rethrow_errors, resource_limits, static_registers) + end + + def initialize(environments = {}, outer_scope = {}, registers = {}, rethrow_errors = false, resource_limits = nil, static_registers = {}) @environments = [environments].flatten @scopes = [(outer_scope || {})] @registers = registers + @static_registers = static_registers.tap(&:freeze) @errors = [] @partial = false @strict_variables = false @@ -126,6 +131,19 @@ module Liquid @this_stack_used = old_stack_used end + # Creates a new context inheriting resource limits, filters, environment etc., + # but with an isolated scope. + def new_isolated_subcontext + Context.build( + environments: environments, + resource_limits: resource_limits, + static_registers: static_registers + ).tap do |subcontext| + subcontext.exception_renderer = exception_renderer + subcontext.add_filters(@filters) + end + end + def clear_instance_assigns @scopes[0] = {} end diff --git a/lib/liquid/tags/render.rb b/lib/liquid/tags/render.rb new file mode 100644 index 0000000..f0f6f10 --- /dev/null +++ b/lib/liquid/tags/render.rb @@ -0,0 +1,89 @@ +module Liquid + # + # TODO: docs + # + class Render < Tag + Syntax = /(#{QuotedFragment}+)/o + + attr_reader :template_name_expr, :attributes + + def initialize(tag_name, markup, options) + super + + if markup =~ Syntax + template_name = $1 + + @template_name_expr = Expression.parse(template_name) + + @attributes = {} + markup.scan(TagAttributes) do |key, value| + @attributes[key] = Expression.parse(value) + end + + else + raise SyntaxError.new(options[:locale].t("errors.syntax.include".freeze)) + end + end + + def parse(_tokens) + end + + def render_to_output_buffer(context, output) + template_name = context.evaluate(@template_name_expr) + raise ArgumentError.new(options[:locale].t("errors.argument.include")) unless template_name + + partial = load_cached_partial(template_name, context) + + inner_context = Context.new + inner_context.template_name = template_name + inner_context.partial = true + @attributes.each do |key, value| + inner_context[key] = context.evaluate(value) + end + partial.render_to_output_buffer(inner_context, output) + + # TODO: Put into a new #isolated_stack method in Context? + inner_context.errors.each { |e| context.errors << e } + + output + end + + private + + alias_method :parse_context, :options + private :parse_context + + def load_cached_partial(template_name, context) + cached_partials = context.registers[:cached_partials] || {} + + if cached = cached_partials[template_name] + return cached + end + source = read_template_from_file_system(context) + begin + parse_context.partial = true + partial = Liquid::Template.parse(source, parse_context) + ensure + parse_context.partial = false + end + cached_partials[template_name] = partial + context.registers[:cached_partials] = cached_partials + partial + end + + def read_template_from_file_system(context) + file_system = context.registers[:file_system] || Liquid::Template.file_system + file_system.read_template_file(context.evaluate(@template_name_expr)) + end + + class ParseTreeVisitor < Liquid::ParseTreeVisitor + def children + [ + @node.template_name_expr, + ] + @node.attributes.values + end + end + end + + Template.register_tag('render'.freeze, Render) +end diff --git a/test/integration/tags/render_rag_test.rb b/test/integration/tags/render_rag_test.rb new file mode 100644 index 0000000..2352bc1 --- /dev/null +++ b/test/integration/tags/render_rag_test.rb @@ -0,0 +1,255 @@ +require 'test_helper' + +class TestFileSystem + def read_template_file(template_path) + case template_path + when "product" + "Product: {{ product.title }} " + + when "locale_variables" + "Locale: {{echo1}} {{echo2}}" + + when "variant" + "Variant: {{ variant.title }}" + + when "nested_template" + "{% include 'header' %} {% include 'body' %} {% include 'footer' %}" + + when "body" + "body {% include 'body_detail' %}" + + when "nested_product_template" + "Product: {{ nested_product_template.title }} {%include 'details'%} " + + when "recursively_nested_template" + "-{% include 'recursively_nested_template' %}" + + when "pick_a_source" + "from TestFileSystem" + + when 'assignments' + "{% assign foo = 'bar' %}" + + when 'break' + "{% break %}" + + else + template_path + end + end +end + +class OtherFileSystem + def read_template_file(template_path) + 'from OtherFileSystem' + end +end + +class CountingFileSystem + attr_reader :count + def read_template_file(template_path) + @count ||= 0 + @count += 1 + 'from CountingFileSystem' + end +end + +class CustomInclude < Liquid::Tag + Syntax = /(#{Liquid::QuotedFragment}+)(\s+(?:with|for)\s+(#{Liquid::QuotedFragment}+))?/o + + def initialize(tag_name, markup, tokens) + markup =~ Syntax + @template_name = $1 + super + end + + def parse(tokens) + end + + def render_to_output_buffer(context, output) + output << @template_name[1..-2] + output + end +end + +class IncludeTagTest < Minitest::Test + include Liquid + + def setup + Liquid::Template.file_system = TestFileSystem.new + end + + def test_include_tag_looks_for_file_system_in_registers_first + assert_equal 'from OtherFileSystem', + Template.parse("{% include 'pick_a_source' %}").render!({}, registers: { file_system: OtherFileSystem.new }) + end + + def test_include_tag_with + assert_template_result "Product: Draft 151cm ", + "{% include 'product' with products[0] %}", "products" => [ { 'title' => 'Draft 151cm' }, { 'title' => 'Element 155cm' } ] + end + + def test_include_tag_with_default_name + assert_template_result "Product: Draft 151cm ", + "{% include 'product' %}", "product" => { 'title' => 'Draft 151cm' } + end + + def test_include_tag_for + assert_template_result "Product: Draft 151cm Product: Element 155cm ", + "{% include 'product' for products %}", "products" => [ { 'title' => 'Draft 151cm' }, { 'title' => 'Element 155cm' } ] + end + + def test_include_tag_with_local_variables + assert_template_result "Locale: test123 ", "{% include 'locale_variables' echo1: 'test123' %}" + end + + def test_include_tag_with_multiple_local_variables + assert_template_result "Locale: test123 test321", + "{% include 'locale_variables' echo1: 'test123', echo2: 'test321' %}" + end + + def test_include_tag_with_multiple_local_variables_from_context + assert_template_result "Locale: test123 test321", + "{% include 'locale_variables' echo1: echo1, echo2: more_echos.echo2 %}", + 'echo1' => 'test123', 'more_echos' => { "echo2" => 'test321' } + end + + def test_included_templates_assigns_variables + assert_template_result "bar", "{% include 'assignments' %}{{ foo }}" + end + + def test_nested_include_tag + assert_template_result "body body_detail", "{% include 'body' %}" + + assert_template_result "header body body_detail footer", "{% include 'nested_template' %}" + end + + def test_nested_include_with_variable + assert_template_result "Product: Draft 151cm details ", + "{% include 'nested_product_template' with product %}", "product" => { "title" => 'Draft 151cm' } + + assert_template_result "Product: Draft 151cm details Product: Element 155cm details ", + "{% include 'nested_product_template' for products %}", "products" => [{ "title" => 'Draft 151cm' }, { "title" => 'Element 155cm' }] + end + + def test_recursively_included_template_does_not_produce_endless_loop + infinite_file_system = Class.new do + def read_template_file(template_path) + "-{% include 'loop' %}" + end + end + + Liquid::Template.file_system = infinite_file_system.new + + assert_raises(Liquid::StackLevelError) do + Template.parse("{% include 'loop' %}").render! + end + end + + def test_dynamically_choosen_template + assert_template_result "Test123", "{% include template %}", "template" => 'Test123' + assert_template_result "Test321", "{% include template %}", "template" => 'Test321' + + assert_template_result "Product: Draft 151cm ", "{% include template for product %}", + "template" => 'product', 'product' => { 'title' => 'Draft 151cm' } + end + + def test_include_tag_caches_second_read_of_same_partial + file_system = CountingFileSystem.new + assert_equal 'from CountingFileSystemfrom CountingFileSystem', + Template.parse("{% include 'pick_a_source' %}{% include 'pick_a_source' %}").render!({}, registers: { file_system: file_system }) + assert_equal 1, file_system.count + end + + def test_include_tag_doesnt_cache_partials_across_renders + file_system = CountingFileSystem.new + assert_equal 'from CountingFileSystem', + Template.parse("{% include 'pick_a_source' %}").render!({}, registers: { file_system: file_system }) + assert_equal 1, file_system.count + + assert_equal 'from CountingFileSystem', + Template.parse("{% include 'pick_a_source' %}").render!({}, registers: { file_system: file_system }) + assert_equal 2, file_system.count + end + + def test_include_tag_within_if_statement + assert_template_result "foo_if_true", "{% if true %}{% include 'foo_if_true' %}{% endif %}" + end + + def test_custom_include_tag + original_tag = Liquid::Template.tags['include'] + Liquid::Template.tags['include'] = CustomInclude + begin + assert_equal "custom_foo", + Template.parse("{% include 'custom_foo' %}").render! + ensure + Liquid::Template.tags['include'] = original_tag + end + end + + def test_custom_include_tag_within_if_statement + original_tag = Liquid::Template.tags['include'] + Liquid::Template.tags['include'] = CustomInclude + begin + assert_equal "custom_foo_if_true", + Template.parse("{% if true %}{% include 'custom_foo_if_true' %}{% endif %}").render! + ensure + Liquid::Template.tags['include'] = original_tag + end + end + + def test_does_not_add_error_in_strict_mode_for_missing_variable + Liquid::Template.file_system = TestFileSystem.new + + a = Liquid::Template.parse(' {% include "nested_template" %}') + a.render! + assert_empty a.errors + end + + def test_passing_options_to_included_templates + assert_raises(Liquid::SyntaxError) do + Template.parse("{% include template %}", error_mode: :strict).render!("template" => '{{ "X" || downcase }}') + end + with_error_mode(:lax) do + assert_equal 'x', Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: true).render!("template" => '{{ "X" || downcase }}') + end + assert_raises(Liquid::SyntaxError) do + Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: [:locale]).render!("template" => '{{ "X" || downcase }}') + end + with_error_mode(:lax) do + assert_equal 'x', Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: [:error_mode]).render!("template" => '{{ "X" || downcase }}') + end + end + + def test_render_raise_argument_error_when_template_is_undefined + assert_raises(Liquid::ArgumentError) do + template = Liquid::Template.parse('{% include undefined_variable %}') + template.render! + end + assert_raises(Liquid::ArgumentError) do + template = Liquid::Template.parse('{% include nil %}') + template.render! + end + end + + def test_including_via_variable_value + assert_template_result "from TestFileSystem", "{% assign page = 'pick_a_source' %}{% include page %}" + + assert_template_result "Product: Draft 151cm ", "{% assign page = 'product' %}{% include page %}", "product" => { 'title' => 'Draft 151cm' } + + assert_template_result "Product: Draft 151cm ", "{% assign page = 'product' %}{% include page for foo %}", "foo" => { 'title' => 'Draft 151cm' } + end + + def test_including_with_strict_variables + template = Liquid::Template.parse("{% include 'simple' %}", error_mode: :warn) + template.render(nil, strict_variables: true) + + assert_equal [], template.errors + end + + def test_break_through_include + assert_template_result "1", "{% for i in (1..3) %}{{ i }}{% break %}{{ i }}{% endfor %}" + assert_template_result "1", "{% for i in (1..3) %}{{ i }}{% include 'break' %}{{ i }}{% endfor %}" + end +end # IncludeTagTest + diff --git a/test/integration/tags/render_tag_test.rb b/test/integration/tags/render_tag_test.rb new file mode 100644 index 0000000..928dc38 --- /dev/null +++ b/test/integration/tags/render_tag_test.rb @@ -0,0 +1,230 @@ +require 'test_helper' + +class StubFileSystem + def initialize(values) + @values = values + end + + def read_template_file(template_path) + @values.fetch(template_path) + end +end + +class RenderTagTest < Minitest::Test + include Liquid + + def test_render_with_no_arguments + Liquid::Template.file_system = StubFileSystem.new('source' => 'rendered content') + assert_template_result 'rendered content', "{% render 'source' %}" + end + + def test_render_tag_looks_for_file_system_in_registers_first + file_system = StubFileSystem.new('pick_a_source' => 'from register file system') + assert_equal 'from register file system', + Template.parse("{% render 'pick_a_source' %}").render!({}, registers: { file_system: file_system }) + end + + def test_render_passes_named_arguments_into_inner_scope + Liquid::Template.file_system = StubFileSystem.new('product' => '{{ inner_product.title }}') + assert_template_result 'My Product', "{% render 'product', inner_product: outer_product %}", + 'outer_product' => { 'title' => 'My Product' } + end + + def test_render_accepts_literals_as_arguments + Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ price }}') + assert_template_result '123', "{% render 'snippet', price: 123 %}" + end + + def test_render_accepts_multiple_named_arguments + Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ one }} {{ two }}') + assert_template_result '1 2', "{% render 'snippet', one: 1, two: 2 %}" + end + + def test_render_does_inherit_parent_scope_variables + Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ outer_variable }}') + assert_template_result '', "{% render 'snippet' %}", 'outer_variable' => 'should not be visible' + end + + def test_render_does_not_inherit_variable_with_same_name_as_snippet + Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ snippet }}') + assert_template_result '', "{% render 'snippet' %}", 'snippet' => 'should not be visible' + end + + def test_render_sets_the_correct_template_name_for_errors + Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ unsafe }}') + Liquid::Template.taint_mode = :error + + template = Liquid::Template.parse("{% render 'snippet', unsafe: unsafe %}") + template.render('unsafe' => String.new('unsafe').tap(&:taint)) + refute_empty template.errors + + assert_equal 'snippet', template.errors.first.template_name + end + + def test_render_sets_the_correct_template_name_for_warnings + Liquid::Template.file_system = StubFileSystem.new('snippet' => '{{ unsafe }}') + Liquid::Template.taint_mode = :warn + + template = Liquid::Template.parse("{% render 'snippet', unsafe: unsafe %}") + template.render('unsafe' => String.new('unsafe').tap(&:taint)) + refute_empty template.warnings + + assert_equal 'snippet', template.errors.first.template_name + end + + def test_render_does_not_mutate_parent_scope + Liquid::Template.file_system = StubFileSystem.new('snippet' => '{% assign inner = 1 %}') + assert_template_result '', "{% render 'snippet' %}{{ inner }}" + end + + def test_nested_render_tag + Liquid::Template.file_system = StubFileSystem.new( + 'one' => "one {{ render 'two' }}", + 'two' => 'two' + ) + assert_template_result 'one two', "{% include 'one' %}" + end + + def test_nested_include_with_variable + skip 'To be implemented' + assert_template_result "Product: Draft 151cm details ", + "{% include 'nested_product_template' with product %}", "product" => { "title" => 'Draft 151cm' } + + assert_template_result "Product: Draft 151cm details Product: Element 155cm details ", + "{% include 'nested_product_template' for products %}", "products" => [{ "title" => 'Draft 151cm' }, { "title" => 'Element 155cm' }] + end + + def test_recursively_included_template_does_not_produce_endless_loop + skip 'To be implemented' + infinite_file_system = Class.new do + def read_template_file(template_path) + "-{% include 'loop' %}" + end + end + + Liquid::Template.file_system = infinite_file_system.new + + assert_raises(Liquid::StackLevelError) do + Template.parse("{% include 'loop' %}").render! + end + end + + def test_dynamically_choosen_template + skip 'To be implemented' + assert_template_result "Test123", "{% include template %}", "template" => 'Test123' + assert_template_result "Test321", "{% include template %}", "template" => 'Test321' + + assert_template_result "Product: Draft 151cm ", "{% include template for product %}", + "template" => 'product', 'product' => { 'title' => 'Draft 151cm' } + end + + def test_include_tag_caches_second_read_of_same_partial + skip 'To be implemented' + file_system = CountingFileSystem.new + assert_equal 'from CountingFileSystemfrom CountingFileSystem', + Template.parse("{% include 'pick_a_source' %}{% include 'pick_a_source' %}").render!({}, registers: { file_system: file_system }) + assert_equal 1, file_system.count + end + + def test_include_tag_doesnt_cache_partials_across_renders + skip 'To be implemented' + file_system = CountingFileSystem.new + assert_equal 'from CountingFileSystem', + Template.parse("{% include 'pick_a_source' %}").render!({}, registers: { file_system: file_system }) + assert_equal 1, file_system.count + + assert_equal 'from CountingFileSystem', + Template.parse("{% include 'pick_a_source' %}").render!({}, registers: { file_system: file_system }) + assert_equal 2, file_system.count + end + + def test_include_tag_within_if_statement + skip 'To be implemented' + assert_template_result "foo_if_true", "{% if true %}{% include 'foo_if_true' %}{% endif %}" + end + + def test_custom_include_tag + skip 'To be implemented' + original_tag = Liquid::Template.tags['include'] + Liquid::Template.tags['include'] = CustomInclude + begin + assert_equal "custom_foo", + Template.parse("{% include 'custom_foo' %}").render! + ensure + Liquid::Template.tags['include'] = original_tag + end + end + + def test_custom_include_tag_within_if_statement + skip 'To be implemented' + original_tag = Liquid::Template.tags['include'] + Liquid::Template.tags['include'] = CustomInclude + begin + assert_equal "custom_foo_if_true", + Template.parse("{% if true %}{% include 'custom_foo_if_true' %}{% endif %}").render! + ensure + Liquid::Template.tags['include'] = original_tag + end + end + + def test_does_not_add_error_in_strict_mode_for_missing_variable + skip 'To be implemented' + Liquid::Template.file_system = TestFileSystem.new + + a = Liquid::Template.parse(' {% include "nested_template" %}') + a.render! + assert_empty a.errors + end + + def test_passing_options_to_included_templates + skip 'To be implemented' + assert_raises(Liquid::SyntaxError) do + Template.parse("{% include template %}", error_mode: :strict).render!("template" => '{{ "X" || downcase }}') + end + with_error_mode(:lax) do + assert_equal 'x', Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: true).render!("template" => '{{ "X" || downcase }}') + end + assert_raises(Liquid::SyntaxError) do + Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: [:locale]).render!("template" => '{{ "X" || downcase }}') + end + with_error_mode(:lax) do + assert_equal 'x', Template.parse("{% include template %}", error_mode: :strict, include_options_blacklist: [:error_mode]).render!("template" => '{{ "X" || downcase }}') + end + end + + def test_render_raise_argument_error_when_template_is_undefined + skip 'To be implemented' + assert_raises(Liquid::ArgumentError) do + template = Liquid::Template.parse('{% include undefined_variable %}') + template.render! + end + assert_raises(Liquid::ArgumentError) do + template = Liquid::Template.parse('{% include nil %}') + template.render! + end + end + + def test_including_via_variable_value + skip 'To be implemented' + assert_template_result "from TestFileSystem", "{% assign page = 'pick_a_source' %}{% include page %}" + + assert_template_result "Product: Draft 151cm ", "{% assign page = 'product' %}{% include page %}", "product" => { 'title' => 'Draft 151cm' } + + assert_template_result "Product: Draft 151cm ", "{% assign page = 'product' %}{% include page for foo %}", "foo" => { 'title' => 'Draft 151cm' } + end + + def test_including_with_strict_variables + skip 'To be implemented' + template = Liquid::Template.parse("{% include 'simple' %}", error_mode: :warn) + template.render(nil, strict_variables: true) + + assert_equal [], template.errors + end + + def test_break_through_include + skip 'To be implemented' + assert_template_result "1", "{% for i in (1..3) %}{{ i }}{% break %}{{ i }}{% endfor %}" + assert_template_result "1", "{% for i in (1..3) %}{{ i }}{% include 'break' %}{{ i }}{% endfor %}" + end +end # IncludeTagTest + diff --git a/test/unit/context_unit_test.rb b/test/unit/context_unit_test.rb index fab19b8..1545763 100644 --- a/test/unit/context_unit_test.rb +++ b/test/unit/context_unit_test.rb @@ -473,6 +473,64 @@ class ContextUnitTest < Minitest::Test assert_equal 'hi', context.apply_global_filter('hi') end + def test_new_isolated_subcontext_does_not_inherit_variables + super_context = Context.new + super_context['my_variable'] = 'some value' + subcontext = super_context.new_isolated_subcontext + + assert_nil subcontext['my_variable'] + end + + def test_new_isolated_subcontext_inherits_environment + super_context = Context.new('my_environment_value' => 'my value') + subcontext = super_context.new_isolated_subcontext + + assert_equal 'my value',subcontext['my_environment_value'] + end + + def test_new_isolated_subcontext_inherits_resource_limits + resource_limits = ResourceLimits.new({}) + super_context = Context.new({}, {}, {}, false, resource_limits) + subcontext = super_context.new_isolated_subcontext + assert_equal resource_limits, subcontext.resource_limits + end + + def test_new_isolated_subcontext_inherits_exception_renderer + super_context = Context.new + super_context.exception_renderer = -> (_e) { 'my exception message' } + subcontext = super_context.new_isolated_subcontext + assert_equal 'my exception message', subcontext.handle_error(Liquid::Error.new) + end + + def test_new_isolated_subcontext_does_not_inherit_non_static_registers + registers = { + my_register: :my_value + } + super_context = Context.new({}, {}, registers) + subcontext = super_context.new_isolated_subcontext + assert_nil subcontext.registers[:my_register] + end + + def test_new_isolated_subcontext_inherits_static_registers + super_context = Context.build(static_registers: { my_register: :my_value }) + subcontext = super_context.new_isolated_subcontext + assert_equal :my_value, subcontext.static_registers[:my_register] + end + + def test_new_isolated_subcontext_inherits_filters + my_filter = Module.new do + def my_filter(*) + 'my filter result' + end + end + + super_context = Context.new + super_context.add_filters([my_filter]) + subcontext = super_context.new_isolated_subcontext + template = Template.parse('{{ 123 | my_filter }}') + assert_equal 'my filter result', template.render(subcontext) + end + private def assert_no_object_allocations