From cf49b06ccc19f86704cae9f4f319032092f3d78a Mon Sep 17 00:00:00 2001 From: Tom Burns Date: Sun, 24 Nov 2013 12:29:15 -0500 Subject: [PATCH 1/4] allow drops to optimize loading a slice of elements --- lib/liquid/tags/for.rb | 7 +++-- test/liquid/tags/for_tag_test.rb | 48 ++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index 5902704..c8887e9 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -75,8 +75,11 @@ module Liquid limit = context[@attributes['limit']] to = limit ? limit.to_i + from : nil - - segment = Utils.slice_collection_using_each(collection, from, to) + segment = if (from != 0 || to != nil) && collection.respond_to?(:load_slice) + collection.load_slice(from, to) + else + Utils.slice_collection_using_each(collection, from, to) + end return render_else(context) if segment.empty? diff --git a/test/liquid/tags/for_tag_test.rb b/test/liquid/tags/for_tag_test.rb index 9186d3f..b98f7f0 100644 --- a/test/liquid/tags/for_tag_test.rb +++ b/test/liquid/tags/for_tag_test.rb @@ -294,4 +294,52 @@ HERE assigns = {'items' => [1,2,3,4,5]} assert_template_result(expected, template, assigns) end + + class LoaderDrop < Liquid::Drop + attr_accessor :each_called, :load_slice_called + + def initialize(data) + @data = data + end + + def each + @each_called = true + @data.each { |el| yield el } + end + + def load_slice(from, to) + @load_slice_called = true + @data[(from..to-1)] + end + end + + def test_iterate_with_each_when_no_limit_applied + loader = LoaderDrop.new([1,2,3,4,5]) + assigns = {'items' => loader} + expected = '12345' + template = '{% for item in items %}{{item}}{% endfor %}' + assert_template_result(expected, template, assigns) + assert loader.each_called + assert !loader.load_slice_called + end + + def test_iterate_with_load_slice_when_limit_applied + loader = LoaderDrop.new([1,2,3,4,5]) + assigns = {'items' => loader} + expected = '1' + template = '{% for item in items limit:1 %}{{item}}{% endfor %}' + assert_template_result(expected, template, assigns) + assert !loader.each_called + assert loader.load_slice_called + end + + def test_iterate_with_load_slice_when_limit_and_offset_applied + loader = LoaderDrop.new([1,2,3,4,5]) + assigns = {'items' => loader} + expected = '34' + template = '{% for item in items offset:2 limit:2 %}{{item}}{% endfor %}' + assert_template_result(expected, template, assigns) + assert !loader.each_called + assert loader.load_slice_called + end end From 2c26a880f0cee54bde80db171a8fceecba251c79 Mon Sep 17 00:00:00 2001 From: Tom Burns Date: Sun, 24 Nov 2013 12:32:32 -0500 Subject: [PATCH 2/4] add another test showing equivalent functionality --- test/liquid/tags/for_tag_test.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/test/liquid/tags/for_tag_test.rb b/test/liquid/tags/for_tag_test.rb index b98f7f0..4dde8a4 100644 --- a/test/liquid/tags/for_tag_test.rb +++ b/test/liquid/tags/for_tag_test.rb @@ -342,4 +342,14 @@ HERE assert !loader.each_called assert loader.load_slice_called end + + def test_iterate_with_load_slice_returns_same_results_as_without + loader = LoaderDrop.new([1,2,3,4,5]) + loader_assigns = {'items' => loader} + array_assigns = {'items' => [1,2,3,4,5]} + expected = '34' + template = '{% for item in items offset:2 limit:2 %}{{item}}{% endfor %}' + assert_template_result(expected, template, loader_assigns) + assert_template_result(expected, template, array_assigns) + end end From e6673526294426404a54b2f137632852828a20e7 Mon Sep 17 00:00:00 2001 From: Tom Burns Date: Sun, 24 Nov 2013 14:00:23 -0500 Subject: [PATCH 3/4] move slice_collection optimization to utils --- lib/liquid/htmltags.rb | 2 +- lib/liquid/tags/for.rb | 6 +----- lib/liquid/utils.rb | 19 +++++++++++++++---- 3 files changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/liquid/htmltags.rb b/lib/liquid/htmltags.rb index 05b0419..8ceab44 100644 --- a/lib/liquid/htmltags.rb +++ b/lib/liquid/htmltags.rb @@ -23,7 +23,7 @@ module Liquid from = @attributes['offset'] ? context[@attributes['offset']].to_i : 0 to = @attributes['limit'] ? from + context[@attributes['limit']].to_i : nil - collection = Utils.slice_collection_using_each(collection, from, to) + collection = Utils.slice_collection(collection, from, to) length = collection.length diff --git a/lib/liquid/tags/for.rb b/lib/liquid/tags/for.rb index c8887e9..9955b5e 100644 --- a/lib/liquid/tags/for.rb +++ b/lib/liquid/tags/for.rb @@ -75,11 +75,7 @@ module Liquid limit = context[@attributes['limit']] to = limit ? limit.to_i + from : nil - segment = if (from != 0 || to != nil) && collection.respond_to?(:load_slice) - collection.load_slice(from, to) - else - Utils.slice_collection_using_each(collection, from, to) - end + segment = Utils.slice_collection(collection, from, to) return render_else(context) if segment.empty? diff --git a/lib/liquid/utils.rb b/lib/liquid/utils.rb index 0bf6df2..fa409e4 100644 --- a/lib/liquid/utils.rb +++ b/lib/liquid/utils.rb @@ -1,5 +1,20 @@ module Liquid module Utils + + def self.slice_collection(collection, from, to) + if (from != 0 || to != nil) && collection.respond_to?(:load_slice) + collection.load_slice(from, to) + else + slice_collection_using_each(collection, from, to) + end + end + + def self.non_blank_string?(collection) + collection.is_a?(String) && collection != '' + end + + private + def self.slice_collection_using_each(collection, from, to) segments = [] index = 0 @@ -22,9 +37,5 @@ module Liquid segments end - - def self.non_blank_string?(collection) - collection.is_a?(String) && collection != '' - end end end From 30e5f06313f0b7b0b37b37dba89af45cfef20ee2 Mon Sep 17 00:00:00 2001 From: Tom Burns Date: Mon, 25 Nov 2013 10:37:10 -0500 Subject: [PATCH 4/4] don't make original slice_collection_using_each private --- lib/liquid/utils.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/liquid/utils.rb b/lib/liquid/utils.rb index fa409e4..cb96524 100644 --- a/lib/liquid/utils.rb +++ b/lib/liquid/utils.rb @@ -13,8 +13,6 @@ module Liquid collection.is_a?(String) && collection != '' end - private - def self.slice_collection_using_each(collection, from, to) segments = [] index = 0