From 1bf77d7798053767b77a77960250ad43b8cf3833 Mon Sep 17 00:00:00 2001 From: Thierry Joyal Date: Mon, 7 Mar 2022 10:11:12 -0500 Subject: [PATCH] Consolidate contextualization of inputs via the context --- History.md | 1 + lib/liquid/context.rb | 25 +++++++++----- lib/liquid/standardfilters.rb | 3 +- lib/liquid/variable_lookup.rb | 10 ++---- test/integration/standard_filter_test.rb | 38 ++++++++++++++++++++-- test/integration/tags/standard_tag_test.rb | 34 +++++++++++++++---- 6 files changed, 85 insertions(+), 26 deletions(-) diff --git a/History.md b/History.md index cca6692f9..5cf7a9ec9 100644 --- a/History.md +++ b/History.md @@ -4,6 +4,7 @@ ### Fixes * StandardFilter: Fix missing @context on iterations (#1525) [Thierry Joyal] +* Consolidate contextualization of inputs via the context (#XXX) [Thierry Joyal] ### Deprecation * Condition#evaluate to require mandatory context argument in Liquid 6.0.0 (#1527) [Thierry Joyal] diff --git a/lib/liquid/context.rb b/lib/liquid/context.rb index 3d6ffc96b..ac1789299 100644 --- a/lib/liquid/context.rb +++ b/lib/liquid/context.rb @@ -187,16 +187,11 @@ def find_variable(key, raise_on_not_found: true) # path and find_index() is optimized in MRI to reduce object allocation index = @scopes.find_index { |s| s.key?(key) } - variable = if index + if index lookup_and_evaluate(@scopes[index], key, raise_on_not_found: raise_on_not_found) else try_variable_find_in_environments(key, raise_on_not_found: raise_on_not_found) end - - variable = variable.to_liquid - variable.context = self if variable.respond_to?(:context=) - - variable end def lookup_and_evaluate(obj, key, raise_on_not_found: true) @@ -207,9 +202,9 @@ def lookup_and_evaluate(obj, key, raise_on_not_found: true) value = obj[key] if value.is_a?(Proc) && obj.respond_to?(:[]=) - obj[key] = value.arity == 0 ? value.call : value.call(self) + obj[key] = contextualize(value) else - value + contextualize(value) end end @@ -228,6 +223,20 @@ def tag_disabled?(tag_name) @disabled_tags.fetch(tag_name, 0) > 0 end + # Convert input objects into liquid aware representations + # Procs will be resolved + # Assigns the context (self) through context= + def contextualize(object) + if object.is_a?(Proc) + object = object.arity == 0 ? object.call : object.call(self) + end + + object = object.to_liquid + object.context = self if object.respond_to?(:context=) + + object + end + protected attr_writer :base_scope_depth, :warnings, :errors, :strainer, :filters, :disabled_tags diff --git a/lib/liquid/standardfilters.rb b/lib/liquid/standardfilters.rb index fade736c2..8c95caf9e 100644 --- a/lib/liquid/standardfilters.rb +++ b/lib/liquid/standardfilters.rb @@ -582,8 +582,7 @@ def empty? def each @input.each do |e| - e = e.respond_to?(:to_liquid) ? e.to_liquid : e - e.context = @context if e.respond_to?(:context=) + e = @context.contextualize(e) yield(e) end end diff --git a/lib/liquid/variable_lookup.rb b/lib/liquid/variable_lookup.rb index 9d5dba681..f2a65e6d7 100644 --- a/lib/liquid/variable_lookup.rb +++ b/lib/liquid/variable_lookup.rb @@ -49,15 +49,14 @@ def evaluate(context) ((object.respond_to?(:key?) && object.key?(key)) || (object.respond_to?(:fetch) && key.is_a?(Integer))) - # if its a proc we will replace the entry with the proc - res = context.lookup_and_evaluate(object, key) - object = res.to_liquid + object = context.lookup_and_evaluate(object, key) # Some special cases. If the part wasn't in square brackets and # no key with the same name was found we interpret following calls # as commands and call them on the current object elsif @command_flags & (1 << i) != 0 && object.respond_to?(key) - object = object.send(key).to_liquid + object = object.send(key) + object = context.contextualize(object) # No key was present with the desired value and it wasn't one of the directly supported # keywords either. The only thing we got left is to return nil or @@ -66,9 +65,6 @@ def evaluate(context) return nil unless context.strict_variables raise Liquid::UndefinedVariable, "undefined variable #{key}" end - - # If we are dealing with a drop here we have to - object.context = context if object.respond_to?(:context=) end object diff --git a/test/integration/standard_filter_test.rb b/test/integration/standard_filter_test.rb index 0342742a5..f97010f99 100644 --- a/test/integration/standard_filter_test.rb +++ b/test/integration/standard_filter_test.rb @@ -34,6 +34,10 @@ def initialize(value:) def registers @context.registers end + + def to_s + "TestDrop(value:#{@value})" + end end class TestModel @@ -467,10 +471,38 @@ def test_sort_calls_to_liquid end def test_map_over_proc - drop = TestDrop.new(value: "testfoo") + drop = TestDrop.new(value: "123") p = proc { drop } templ = '{{ procs | map: "value" }}' - assert_template_result("testfoo", templ, "procs" => [p]) + assert_template_result("123", templ, "procs" => [p]) + end + + def test_join_over_proc + drop = TestDrop.new(value: "123") + p = proc { drop } + templ = '{{ procs | join }}' + assert_template_result('TestDrop(value:123)', templ, "procs" => [p]) + end + + def test_sort_over_proc + drop = TestDrop.new(value: "123") + p = proc { drop } + templ = '{{ procs | sort: "value" }}' + assert_template_result("TestDrop(value:123)", templ, "procs" => [p]) + end + + def test_where_over_proc + drop = TestDrop.new(value: "123") + p = proc { drop } + templ = '{{ procs | where: "value", "123" }}' + assert_template_result("TestDrop(value:123)", templ, "procs" => [p]) + end + + def test_uniq_over_proc + drop = TestDrop.new(value: "123") + p = proc { drop } + templ = '{{ procs | uniq }}' + assert_template_result("TestDrop(value:123)", templ, "procs" => [p]) end def test_map_over_drops_returning_procs @@ -888,7 +920,7 @@ def test_all_filters_never_raise_non_liquid_exception { foo: "bar" }, [{ "foo" => "bar" }, { "foo" => 123 }, { "foo" => nil }, { "foo" => true }, { "foo" => ["foo", "bar"] }], { 1 => "bar" }, - ["foo", 123, nil, true, false, Drop, ["foo"], { foo: "bar" }], + ["foo", 123, nil, true, false, ["foo"], { foo: "bar" }], ] StandardFilters.public_instance_methods(false).each do |method| arg_count = @filters.method(method).arity diff --git a/test/integration/tags/standard_tag_test.rb b/test/integration/tags/standard_tag_test.rb index ea7dca158..fa271120a 100644 --- a/test/integration/tags/standard_tag_test.rb +++ b/test/integration/tags/standard_tag_test.rb @@ -272,14 +272,36 @@ def test_multiple_named_cycles_with_names_from_context '{%cycle var1: "one", "two" %} {%cycle var2: "one", "two" %} {%cycle var1: "one", "two" %} {%cycle var2: "one", "two" %} {%cycle var1: "one", "two" %} {%cycle var2: "one", "two" %}', assigns) end - def test_size_of_array - assigns = { "array" => [1, 2, 3, 4] } - assert_template_result('array has 4 elements', "array has {{ array.size }} elements", assigns) + def test_command_methods_of_array + assigns = { "array" => [11, 22, 33] } + + assert_template_result("3", "{{ array.size }}", assigns) + + assert_template_result("11", "{{ array.first }}", assigns) + + assert_template_result("33", "{{ array.last }}", assigns) + end + + def test_command_methods_of_hash + assigns = { "hash" => { a: 11, b: 22, c: 33 } } + + assert_template_result("3", "{{ hash.size }}", assigns) + + assert_template_result("a11", "{{ hash.first }}", assigns) + assert_template_result("a", "{{ hash.first.first }}", assigns) + assert_template_result("11", "{{ hash.first.last }}", assigns) + + assert_template_result("", "{{ hash.last }}", assigns) end - def test_size_of_hash - assigns = { "hash" => { a: 1, b: 2, c: 3, d: 4 } } - assert_template_result('hash has 4 elements', "hash has {{ hash.size }} elements", assigns) + def test_command_methods_with_proc + skip("Liquid-C does not properly resolve Procs in with command methods") if ENV['LIQUID_C'] == '1' + + assigns = { "array" => [proc { "test" }] } + assert_template_result("test", "{{ array.first }}", assigns) + + assigns = { "hash" => { a: proc { "test" } } } + assert_template_result("test", "{{ hash.first.last }}", assigns) end def test_illegal_symbols