Merge pull request #282 from Shopify/load_slice

allow drops to optimize loading a slice of elements

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
 

lib/liquid/tags/for.rb

@@ -75,8 +75,7 @@ module Liquid
       limit = context[@attributes['limit']]
       to    = limit ? limit.to_i + from : nil
 
-
+      segment = Utils.slice_collection(collection, from, to)
-      segment = Utils.slice_collection_using_each(collection, from, to)
 
       return render_else(context) if segment.empty?
 

lib/liquid/utils.rb

@@ -1,5 +1,18 @@
 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
+
     def self.slice_collection_using_each(collection, from, to)
       segments = []
       index = 0
@@ -22,9 +35,5 @@ module Liquid
 
       segments
     end
-
-    def self.non_blank_string?(collection)
-      collection.is_a?(String) && collection != ''
-    end
   end
 end

test/liquid/tags/for_tag_test.rb

@@ -294,4 +294,62 @@ 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
+
+  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