From f9454d8cf386fcbe63a8a0e08d6067420010f269 Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Fri, 31 Jan 2025 13:53:17 +0100 Subject: [PATCH] Fix array filters to not support nested properties --- History.md | 4 + lib/liquid/standardfilters.rb | 41 ++------ lib/liquid/version.rb | 2 +- test/integration/standard_filter_test.rb | 117 ----------------------- 4 files changed, 13 insertions(+), 151 deletions(-) diff --git a/History.md b/History.md index 87635f6a3..aeee0e0d7 100644 --- a/History.md +++ b/History.md @@ -2,6 +2,10 @@ ## 5.8.0 (unreleased) +## 5.7.2 2025-01-31 + +* Fix array filters to not support nested properties + ## 5.7.1 2025-01-24 * Fix the `find` and `find_index`filters to return `nil` when filtering empty arrays diff --git a/lib/liquid/standardfilters.rb b/lib/liquid/standardfilters.rb index aee011847..4291d86e8 100644 --- a/lib/liquid/standardfilters.rb +++ b/lib/liquid/standardfilters.rb @@ -387,7 +387,7 @@ def sort(input, property = nil) end elsif ary.all? { |el| el.respond_to?(:[]) } begin - ary.sort { |a, b| nil_safe_compare(fetch_property(a, property), fetch_property(b, property)) } + ary.sort { |a, b| nil_safe_compare(a[property], b[property]) } rescue TypeError raise_property_error(property) end @@ -416,7 +416,7 @@ def sort_natural(input, property = nil) end elsif ary.all? { |el| el.respond_to?(:[]) } begin - ary.sort { |a, b| nil_safe_casecmp(fetch_property(a, property), fetch_property(b, property)) } + ary.sort { |a, b| nil_safe_casecmp(a[property], b[property]) } rescue TypeError raise_property_error(property) end @@ -504,7 +504,7 @@ def uniq(input, property = nil) [] else ary.uniq do |item| - fetch_property(item, property) + item[property] rescue TypeError raise_property_error(property) rescue NoMethodError @@ -540,7 +540,7 @@ def map(input, property) if property == "to_liquid" e elsif e.respond_to?(:[]) - r = fetch_property(e, property) + r = e[property] r.is_a?(Proc) ? r.call : r end end @@ -564,7 +564,7 @@ def compact(input, property = nil) [] else ary.reject do |item| - fetch_property(item, property).nil? + item[property].nil? rescue TypeError raise_property_error(property) rescue NoMethodError @@ -950,7 +950,7 @@ def sum(input, property = nil) if property.nil? item elsif item.respond_to?(:[]) - fetch_property(item, property) + item[property] else 0 end @@ -976,9 +976,9 @@ def filter_array(input, property, target_value, default_value = [], &block) block.call(ary) do |item| if target_value.nil? - fetch_property(item, property) + item[property] else - fetch_property(item, property) == target_value + item[property] == target_value end rescue TypeError raise_property_error(property) @@ -988,31 +988,6 @@ def filter_array(input, property, target_value, default_value = [], &block) end end - def fetch_property(drop, property_or_keys) - ## - # This keeps backward compatibility by supporting properties containing - # dots. This is valid in Liquid syntax and used in some runtimes, such as - # Shopify with metafields. - # - # Using this approach, properties like 'price.value' can be accessed in - # both of the following examples: - # - # ``` - # [ - # { 'name' => 'Item 1', 'price.price' => 40000 }, - # { 'name' => 'Item 2', 'price' => { 'value' => 39900 } } - # ] - # ``` - value = drop[property_or_keys] - - return value if !value.nil? || !property_or_keys.is_a?(String) - - keys = property_or_keys.split('.') - keys.reduce(drop) do |drop, key| - drop.respond_to?(:[]) ? drop[key] : drop - end - end - def raise_property_error(property) raise Liquid::ArgumentError, "cannot select the property '#{property}'" end diff --git a/lib/liquid/version.rb b/lib/liquid/version.rb index 8cfd6e80f..205d20800 100644 --- a/lib/liquid/version.rb +++ b/lib/liquid/version.rb @@ -2,5 +2,5 @@ # frozen_string_literal: true module Liquid - VERSION = "5.7.1" + VERSION = "5.7.2" end diff --git a/test/integration/standard_filter_test.rb b/test/integration/standard_filter_test.rb index 0ef8e2bc6..ef1278322 100644 --- a/test/integration/standard_filter_test.rb +++ b/test/integration/standard_filter_test.rb @@ -54,30 +54,6 @@ def each(&block) end end -class TestDeepEnumerable < Liquid::Drop - include Enumerable - - class Product < Liquid::Drop - attr_reader :title, :price, :premium - - def initialize(title:, price:, premium: nil) - @title = { "content" => title, "language" => "en" } - @price = { "value" => price, "unit" => "USD" } - @premium = { "category" => premium } if premium - end - end - - def each(&block) - [ - Product.new(title: "Pro goggles", price: 1299), - Product.new(title: "Thermal gloves", price: 1299), - Product.new(title: "Alpine jacket", price: 3999, premium: 'Basic'), - Product.new(title: "Mountain boots", price: 3899, premium: 'Pro'), - Product.new(title: "Safety helmet", price: 1999) - ].each(&block) - end -end - class NumberLikeThing < Liquid::Drop def initialize(amount) @amount = amount @@ -438,15 +414,6 @@ def test_sort_natural_invalid_property end end - def test_sort_natural_with_deep_enumerables - template = <<~LIQUID - {{- products | sort_natural: 'title.content' | map: 'title.content' | join: ', ' -}} - LIQUID - expected_output = "Alpine jacket, Mountain boots, Pro goggles, Safety helmet, Thermal gloves" - - assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new }) - end - def test_legacy_sort_hash assert_equal([{ a: 1, b: 2 }], @filters.sort(a: 1, b: 2)) end @@ -483,15 +450,6 @@ def test_uniq_invalid_property end end - def test_uniq_with_deep_enumerables - template = <<~LIQUID - {{- products | uniq: 'price.value' | map: "title.content" | join: ', ' -}} - LIQUID - expected_output = "Pro goggles, Alpine jacket, Mountain boots, Safety helmet" - - assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new }) - end - def test_compact_empty_array assert_equal([], @filters.compact([], "a")) end @@ -508,15 +466,6 @@ def test_compact_invalid_property end end - def test_compact_with_deep_enumerables - template = <<~LIQUID - {{- products | compact: 'premium.category' | map: 'title.content' | join: ', ' -}} - LIQUID - expected_output = "Alpine jacket, Mountain boots" - - assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new }) - end - def test_reverse assert_equal([4, 3, 2, 1], @filters.reverse([1, 2, 3, 4])) end @@ -626,15 +575,6 @@ def test_sort_works_on_enumerables assert_template_result("213", '{{ foo | sort: "bar" | map: "foo" }}', { "foo" => TestEnumerable.new }) end - def test_sort_with_deep_enumerables - template = <<~LIQUID - {{- products | sort: 'price.value' | map: 'title.content' | join: ', ' -}} - LIQUID - expected_output = "Pro goggles, Thermal gloves, Safety helmet, Mountain boots, Alpine jacket" - - assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new }) - end - def test_first_and_last_call_to_liquid assert_template_result('foobar', '{{ foo | first }}', { 'foo' => [ThingWithToLiquid.new] }) assert_template_result('foobar', '{{ foo | last }}', { 'foo' => [ThingWithToLiquid.new] }) @@ -951,15 +891,6 @@ def test_reject_with_false_value assert_template_result(expected_output, template, { "array" => array }) end - def test_reject_with_deep_enumerables - template = <<~LIQUID - {{- products | reject: 'title.content', 'Pro goggles' | map: 'price.value' | join: ', ' -}} - LIQUID - expected_output = "1299, 3999, 3899, 1999" - - assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new }) - end - def test_has array = [ { "handle" => "alpha", "ok" => true }, @@ -1028,16 +959,6 @@ def test_has_with_false_value_when_does_not_have_it assert_template_result(expected_output, template, { "array" => array }) end - def test_has_with_deep_enumerables - template = <<~LIQUID - {{- products | has: 'title.content', 'Pro goggles' -}}, - {{- products | has: 'title.content', 'foo' -}} - LIQUID - expected_output = "true,false" - - assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new }) - end - def test_find_with_value products = [ { "title" => "Pro goggles", "price" => 1299 }, @@ -1056,16 +977,6 @@ def test_find_with_value assert_template_result(expected_output, template, { "products" => products }) end - def test_find_with_deep_enumerables - template = <<~LIQUID - {%- assign product = products | find: 'title.content', 'Pro goggles' -%} - {{- product.title.content -}} - LIQUID - expected_output = "Pro goggles" - - assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new }) - end - def test_find_with_empty_arrays template = <<~LIQUID {%- assign product = products | find: 'title.content', 'Not found' -%} @@ -1096,16 +1007,6 @@ def test_find_index_with_value assert_template_result(expected_output, template, { "products" => products }) end - def test_find_index_with_deep_enumerables - template = <<~LIQUID - {%- assign index = products | find_index: 'title.content', 'Alpine jacket' -%} - {{- index -}} - LIQUID - expected_output = "2" - - assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new }) - end - def test_find_index_with_empty_arrays template = <<~LIQUID {%- assign index = products | find_index: 'title.content', 'Not found' -%} @@ -1216,15 +1117,6 @@ def test_where_array_of_only_unindexable_values assert_nil(@filters.where([nil], "ok")) end - def test_where_with_deep_enumerables - template = <<~LIQUID - {{- products | where: 'title.content', 'Pro goggles' | map: 'price.value' -}} - LIQUID - expected_output = "1299" - - assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new }) - end - def test_all_filters_never_raise_non_liquid_exception test_drop = TestDrop.new(value: "test") test_drop.context = Context.new @@ -1376,15 +1268,6 @@ def test_sum_with_floats_and_indexable_map_values assert_template_result("0", "{{ input | sum: 'subtotal' }}", { "input" => input }) end - def test_sum_with_deep_enumerables - template = <<~LIQUID - {{- products | sum: 'price.value' -}} - LIQUID - expected_output = "12495" - - assert_template_result(expected_output, template, { "products" => TestDeepEnumerable.new }) - end - private def with_timezone(tz)