From fef93abf5a682459454f9eaf48d9e08fa08e91fa Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 25 Mar 2013 04:10:25 +0100 Subject: [PATCH 1/6] Clean up Railtie. --- lib/multi_fetch_fragments.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/multi_fetch_fragments.rb b/lib/multi_fetch_fragments.rb index 42bfd92..1a69efc 100644 --- a/lib/multi_fetch_fragments.rb +++ b/lib/multi_fetch_fragments.rb @@ -82,8 +82,8 @@ def cache_collection? class Railtie < Rails::Railtie initializer "multi_fetch_fragments.initialize" do |app| - ActionView::PartialRenderer.class_eval do - include MultiFetchFragments + ActiveSupport.on_load(:action_view) do + ActionView::PartialRenderer.send(:include, MultiFetchFragments) end end end From daa504049ab230f8032fdd8473e391d62c264742 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 25 Mar 2013 04:10:45 +0100 Subject: [PATCH 2/6] Check caching being enabled on the view's controller. --- lib/multi_fetch_fragments.rb | 2 +- spec/multi_fetch_fragments_spec.rb | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/multi_fetch_fragments.rb b/lib/multi_fetch_fragments.rb index 1a69efc..60b06d4 100644 --- a/lib/multi_fetch_fragments.rb +++ b/lib/multi_fetch_fragments.rb @@ -77,7 +77,7 @@ def render_collection_with_multi_fetch_cache def cache_collection? cache_option = @options[:cache].presence || @locals[:cache].presence - ActionController::Base.perform_caching && cache_option + @view.controller.perform_caching && @view.controller.cache_store && cache_option end class Railtie < Rails::Railtie diff --git a/spec/multi_fetch_fragments_spec.rb b/spec/multi_fetch_fragments_spec.rb index db11379..b0adae2 100644 --- a/spec/multi_fetch_fragments_spec.rb +++ b/spec/multi_fetch_fragments_spec.rb @@ -4,7 +4,9 @@ it "doesn't smoke" do MultiFetchFragments::Railtie.run_initializers - view = ActionView::Base.new([File.dirname(__FILE__)], {}) + controller = ActionController::Base.new + view = ActionView::Base.new([File.dirname(__FILE__)], {}, controller) + view.render(:partial => "views/customer", :collection => [ Customer.new("david"), Customer.new("mary") ]).should == "Hello: david\nHello: mary\n" end @@ -14,13 +16,14 @@ MultiFetchFragments::Railtie.run_initializers controller = ActionController::Base.new + controller.cache_store = cache_mock view = ActionView::Base.new([File.dirname(__FILE__)], {}, controller) customer = Customer.new("david") key = controller.fragment_cache_key([customer, 'key']) - cache_mock.should_receive(:read_multi).with([key]).and_return({key => 'Hello'}) + cache_mock.should_receive(:read_multi).with([key], {}).and_return({key => 'Hello'}) - view.render(:partial => "views/customer", :collection => [ customer ], :cache => Proc.new{ |item| [item, 'key']}).should == "Hello" + view.render(:partial => "views/customer", :collection => [ customer ], :cache => Proc.new{ |item| [item, 'key']}).should == "Hello: david\n" end end From db156eee529687722d81b74594bb478dd24c4812 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 25 Mar 2013 04:12:53 +0100 Subject: [PATCH 3/6] Wrap cache read/write calls in notification instrumentation for fragment read/write logging and use correct template when templates are derived using item.to_partial_path @collection_data is an array containing the partial path for every item. Items in @collection need to have a @collection_data counterpart at the same index. --- lib/multi_fetch_fragments.rb | 78 +++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 33 deletions(-) diff --git a/lib/multi_fetch_fragments.rb b/lib/multi_fetch_fragments.rb index 60b06d4..a8df833 100644 --- a/lib/multi_fetch_fragments.rb +++ b/lib/multi_fetch_fragments.rb @@ -7,7 +7,6 @@ module MultiFetchFragments private def render_collection_with_multi_fetch_cache - return nil if @collection.blank? if @options.key?(:spacer_template) @@ -17,57 +16,70 @@ def render_collection_with_multi_fetch_cache results = [] if cache_collection? + cache_options = @options.fetch(:cache_options, {}) - additional_cache_options = @options.fetch(:cache_options, {}) - keys_to_collection_map = {} - - @collection.each do |item| + keys_to_item_info_map = {} + @collection.each_with_index do |item, index| key = @options[:cache].respond_to?(:call) ? @options[:cache].call(item) : item - key_with_optional_digest = nil - if defined?(@view.fragment_name_with_digest) - key_with_optional_digest = @view.fragment_name_with_digest(key) - else - key_with_optional_digest = key - end + key = @view.fragment_name_with_digest(key) if @view.respond_to?(:fragment_name_with_digest) - expanded_key = @view.controller.fragment_cache_key(key_with_optional_digest) + expanded_key = @view.controller.fragment_cache_key(key) - keys_to_collection_map[expanded_key] = item + keys_to_item_info_map[expanded_key] = item, index end - # cache.read_multi & cache.write interfaces may require mutable keys, ie. dalli 2.6.0 - mutable_keys = keys_to_collection_map.keys.collect { |key| key.dup } + # cache.read_multi and cache.write interfaces may require mutable keys, ie. dalli 2.6.0 + keys = keys_to_item_info_map.keys.map(&:dup) + + cached_results = @view.controller.instrument_fragment_cache(:read_fragment, keys.join("; ")) do + cached_results = @view.controller.cache_store.read_multi(keys, cache_options) + + cached_results.each do |key, result| + cached_results[key] = result.html_safe if result.respond_to?(:html_safe) + end - result_hash = Rails.cache.read_multi(mutable_keys) + cached_results + end # if we had a cached value, we don't need to render that object from the collection. # if it wasn't cached, we need to render those objects as before - @collection = (keys_to_collection_map.keys - result_hash.keys).map do |key| - keys_to_collection_map[key] - end + uncached_keys = keys - cached_results.keys + + # @collection_data is only used if no @path could be found that covers all items + use_collection_data = @path.nil? && @collection_data - non_cached_results = [] + uncached_collection = [] + uncached_collection_data = [] if use_collection_data + uncached_keys.each do |key| + item, index = keys_to_item_info_map[key] - # sequentially render any non-cached objects remaining - if @collection.any? - non_cached_results = @template ? collection_with_template : collection_without_template + uncached_collection << item + uncached_collection_data << @collection_data[index] if use_collection_data end - # sort the result according to the keys that were fed in, cache the non-cached results - mutable_keys.each do |key| + @collection = uncached_collection + @collection_data = uncached_collection_data if use_collection_data - cached_value = result_hash[key] - if cached_value - results << cached_value - else - non_cached_result = non_cached_results.shift - Rails.cache.write(key, non_cached_result, additional_cache_options) + # sequentially render any uncached objects remaining + uncached_results = [] + unless @collection.empty? + uncached_results = @template ? collection_with_template : collection_without_template + end + + # sort the result according to the keys that were fed in, cache the uncached results + keys.each do |key| + result = cached_results[key] + if result.nil? + result = uncached_results.shift - results << non_cached_result + @view.controller.instrument_fragment_cache(:write_fragment, key) do + @view.controller.cache_store.write(key, result.to_str, cache_options) + end end + + results << result end - else results = @template ? collection_with_template : collection_without_template end From 07c6313a6558446102ad2f82ea1e2337ccbffd78 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 24 Jun 2013 16:57:47 +0200 Subject: [PATCH 4/6] Pass a list instead of an Array to #read_multi. --- lib/multi_fetch_fragments.rb | 2 +- spec/multi_fetch_fragments_spec.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/multi_fetch_fragments.rb b/lib/multi_fetch_fragments.rb index a8df833..f40ffb8 100644 --- a/lib/multi_fetch_fragments.rb +++ b/lib/multi_fetch_fragments.rb @@ -33,7 +33,7 @@ def render_collection_with_multi_fetch_cache keys = keys_to_item_info_map.keys.map(&:dup) cached_results = @view.controller.instrument_fragment_cache(:read_fragment, keys.join("; ")) do - cached_results = @view.controller.cache_store.read_multi(keys, cache_options) + cached_results = @view.controller.cache_store.read_multi(*keys, cache_options) cached_results.each do |key, result| cached_results[key] = result.html_safe if result.respond_to?(:html_safe) diff --git a/spec/multi_fetch_fragments_spec.rb b/spec/multi_fetch_fragments_spec.rb index b0adae2..b9fb8a2 100644 --- a/spec/multi_fetch_fragments_spec.rb +++ b/spec/multi_fetch_fragments_spec.rb @@ -22,8 +22,8 @@ customer = Customer.new("david") key = controller.fragment_cache_key([customer, 'key']) - cache_mock.should_receive(:read_multi).with([key], {}).and_return({key => 'Hello'}) + cache_mock.should_receive(:read_multi).with(key, {}).and_return({key => 'Hello'}) - view.render(:partial => "views/customer", :collection => [ customer ], :cache => Proc.new{ |item| [item, 'key']}).should == "Hello: david\n" + view.render(:partial => "views/customer", :collection => [ customer ], :cache => Proc.new{ |item| [item, 'key']}).should == "Hello" end end From 1d5fabad828f9f0874226d0286b92f83e64f5e13 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 26 Aug 2014 08:14:11 +0200 Subject: [PATCH 5/6] Don't call #to_str on a nil result. --- lib/multi_fetch_fragments.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/multi_fetch_fragments.rb b/lib/multi_fetch_fragments.rb index f40ffb8..506cf02 100644 --- a/lib/multi_fetch_fragments.rb +++ b/lib/multi_fetch_fragments.rb @@ -74,7 +74,7 @@ def render_collection_with_multi_fetch_cache result = uncached_results.shift @view.controller.instrument_fragment_cache(:write_fragment, key) do - @view.controller.cache_store.write(key, result.to_str, cache_options) + @view.controller.cache_store.write(key, result.try(:to_str), cache_options) end end From aef16d8034c2ef5f6a4599d2c5e5b60fd2de6d0d Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Wed, 15 Oct 2014 02:17:16 +0200 Subject: [PATCH 6/6] Don't assume every rendered view has a controller. --- lib/multi_fetch_fragments.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/multi_fetch_fragments.rb b/lib/multi_fetch_fragments.rb index 506cf02..ecad080 100644 --- a/lib/multi_fetch_fragments.rb +++ b/lib/multi_fetch_fragments.rb @@ -89,7 +89,7 @@ def render_collection_with_multi_fetch_cache def cache_collection? cache_option = @options[:cache].presence || @locals[:cache].presence - @view.controller.perform_caching && @view.controller.cache_store && cache_option + @view.controller && @view.controller.perform_caching && @view.controller.cache_store && cache_option end class Railtie < Rails::Railtie