Merge pull request #18948 from kaspth/automatic-collection-caching

Merge multi_fetch_fragments.

17 files changed, 305 insertions, 22 deletions

diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb
index 4760ec1698..2d6607041d 100644
--- a/actionpack/test/controller/caching_test.rb
+++ b/actionpack/test/controller/caching_test.rb
@@ -1,5 +1,6 @@
 require 'fileutils'
 require 'abstract_unit'
+require 'lib/controller/fake_models'
 
 CACHE_DIR = 'test_cache'
 # Don't change '/../temp/' cavalierly or you might hose something you don't want hosed
@@ -349,3 +350,60 @@ class ViewCacheDependencyTest < ActionController::TestCase
     assert_equal %w(trombone flute), HasDependenciesController.new.view_cache_dependencies
   end
 end
+
+class CollectionCacheController < ActionController::Base
+  def index
+    @customers = [Customer.new('david', params[:id] || 1)]
+  end
+
+  def index_ordered
+    @customers = [Customer.new('david', 1), Customer.new('david', 2), Customer.new('david', 3)]
+    render 'index'
+  end
+
+  def index_explicit_render
+    @customers = [Customer.new('david', 1)]
+    render partial: 'customers/customer', collection: @customers
+  end
+
+  def index_with_comment
+    @customers = [Customer.new('david', 1)]
+    render partial: 'customers/commented_customer', collection: @customers, as: :customer
+  end
+end
+
+class AutomaticCollectionCacheTest < ActionController::TestCase
+  def setup
+    super
+    @controller = CollectionCacheController.new
+    @controller.perform_caching = true
+    @controller.cache_store = ActiveSupport::Cache::MemoryStore.new
+  end
+
+  def test_collection_fetches_cached_views
+    get :index
+
+    ActionView::PartialRenderer.expects(:collection_with_template).never
+    get :index
+  end
+
+  def test_preserves_order_when_reading_from_cache_plus_rendering
+    get :index, params: { id: 2 }
+    get :index_ordered
+
+    assert_select ':root', "david, 1\n  david, 2\n  david, 3"
+  end
+
+  def test_explicit_render_call_with_options
+    get :index_explicit_render
+
+    assert_select ':root', "david, 1"
+  end
+
+  def test_caching_works_with_beginning_comment
+    get :index_with_comment
+
+    ActionView::PartialRenderer.expects(:collection_with_template).never
+    get :index_with_comment
+  end
+end
diff --git a/actionpack/test/fixtures/collection_cache/index.html.erb b/actionpack/test/fixtures/collection_cache/index.html.erb
new file mode 100644
index 0000000000..521b1450df
--- /dev/null
+++ b/actionpack/test/fixtures/collection_cache/index.html.erb
@@ -0,0 +1 @@
+<%= render @customers %>
\ No newline at end of file
diff --git a/actionpack/test/fixtures/customers/_commented_customer.html.erb b/actionpack/test/fixtures/customers/_commented_customer.html.erb
new file mode 100644
index 0000000000..d5f6e3b491
--- /dev/null
+++ b/actionpack/test/fixtures/customers/_commented_customer.html.erb
@@ -0,0 +1,4 @@
+<%# I'm a comment %>
+<% cache customer do %>
+  <%= customer.name %>, <%= customer.id %>
+<% end %>
\ No newline at end of file
diff --git a/actionpack/test/fixtures/customers/_customer.html.erb b/actionpack/test/fixtures/customers/_customer.html.erb
new file mode 100644
index 0000000000..67e9f6d411
--- /dev/null
+++ b/actionpack/test/fixtures/customers/_customer.html.erb
@@ -0,0 +1,3 @@
+<% cache customer do %>
+  <%= customer.name %>, <%= customer.id %>
+<% end %>
\ No newline at end of file
diff --git a/actionview/lib/action_view/helpers/cache_helper.rb b/actionview/lib/action_view/helpers/cache_helper.rb
index 9dadfb5ce1..0e2a5f90f4 100644
--- a/actionview/lib/action_view/helpers/cache_helper.rb
+++ b/actionview/lib/action_view/helpers/cache_helper.rb
@@ -110,6 +110,29 @@ module ActionView
       #   <%= some_helper_method(person) %>
       #
       # Now all you'll have to do is change that timestamp when the helper method changes.
+      #
+      # === Automatic Collection Caching
+      #
+      # When rendering collections such as:
+      #
+      #   <%= render @notifications %>
+      #   <%= render partial: 'notifications/notification', collection: @notifications %>
+      #
+      # If the notifications/_notification partial starts with a cache call like so:
+      #
+      #   <% cache notification do %>
+      #     <%= notification.name %>
+      #   <% end %>
+      #
+      # The collection can then automatically use any cached renders for that
+      # template by reading them at once instead of one by one.
+      #
+      # See ActionView::Template::Handlers::ERB.resource_cache_call_pattern for more
+      # information on what cache calls make a template eligible for this collection caching.
+      #
+      # The automatic cache multi read can be turned off like so:
+      #
+      #   <%= render @notifications, cache: false %>
       def cache(name = {}, options = nil, &block)
         if controller.perform_caching
           safe_concat(fragment_for(cache_fragment_name(name, options), options, &block))
@@ -161,6 +184,14 @@ module ActionView
         end
       end
 
+      # Given a key (as described in ActionController::Caching::Fragments.expire_fragment),
+      # returns a key suitable for use in reading, writing, or expiring a
+      # cached fragment. All keys are prefixed with <tt>views/</tt> and uses
+      # ActiveSupport::Cache.expand_cache_key for the expansion.
+      def fragment_cache_key(key)
+        ActiveSupport::Cache.expand_cache_key(key.is_a?(Hash) ? url_for(key).split("://").last : key, :views)
+      end
+
     private
 
       def fragment_name_with_digest(name) #:nodoc:
diff --git a/actionview/lib/action_view/railtie.rb b/actionview/lib/action_view/railtie.rb
index ee3dce24b7..9a26cba574 100644
--- a/actionview/lib/action_view/railtie.rb
+++ b/actionview/lib/action_view/railtie.rb
@@ -36,6 +36,12 @@ module ActionView
       end
     end
 
+    initializer "action_view.collection_caching" do |app|
+      ActiveSupport.on_load(:action_controller) do
+        PartialRenderer.collection_cache = app.config.action_controller.cache_store
+      end
+    end
+
     initializer "action_view.setup_action_pack" do |app|
       ActiveSupport.on_load(:action_controller) do
         ActionView::RoutingUrlFor.include(ActionDispatch::Routing::UrlFor)
diff --git a/actionview/lib/action_view/renderer/partial_renderer.rb b/actionview/lib/action_view/renderer/partial_renderer.rb
index 5ff15411cf..56b8ab1e2d 100644
--- a/actionview/lib/action_view/renderer/partial_renderer.rb
+++ b/actionview/lib/action_view/renderer/partial_renderer.rb
@@ -1,3 +1,4 @@
+require 'action_view/renderer/partial_renderer/collection_caching'
 require 'thread_safe'
 
 module ActionView
@@ -280,6 +281,8 @@ module ActionView
   #     <%- end -%>
   #   <% end %>
   class PartialRenderer < AbstractRenderer
+    include CollectionCaching
+
     PREFIXED_PARTIAL_NAMES = ThreadSafe::Cache.new do |h, k|
       h[k] = ThreadSafe::Cache.new
     end
@@ -321,8 +324,9 @@ module ActionView
         spacer = find_template(@options[:spacer_template], @locals.keys).render(@view, @locals)
       end
 
-      result = @template ? collection_with_template : collection_without_template
-      result.join(spacer).html_safe
+      cache_collection_render do
+        @template ? collection_with_template : collection_without_template
+      end.join(spacer).html_safe
     end
 
     def render_partial
diff --git a/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb
new file mode 100644
index 0000000000..b77c884e66
--- /dev/null
+++ b/actionview/lib/action_view/renderer/partial_renderer/collection_caching.rb
@@ -0,0 +1,70 @@
+require 'active_support/core_ext/object/try'
+
+module ActionView
+  module CollectionCaching # :nodoc:
+    extend ActiveSupport::Concern
+
+    included do
+      # Fallback cache store if Action View is used without Rails.
+      # Otherwise overriden in Railtie to use Rails.cache.
+      mattr_accessor(:collection_cache) { ActiveSupport::Cache::MemoryStore.new }
+    end
+
+    private
+      def cache_collection_render
+        return yield unless cache_collection?
+
+        keyed_collection = collection_by_cache_keys
+        partial_cache = collection_cache.read_multi(*keyed_collection.keys)
+
+        @collection = keyed_collection.reject { |key, _| partial_cache.key?(key) }.values
+        rendered_partials = @collection.any? ? yield.dup : []
+
+        fetch_or_cache_partial(partial_cache, order_by: keyed_collection.each_key) do
+          rendered_partials.shift
+        end
+      end
+
+      def cache_collection?
+        @options.fetch(:cache, automatic_cache_eligible?)
+      end
+
+      def automatic_cache_eligible?
+        single_template_render? && !callable_cache_key? &&
+          @template.eligible_for_collection_caching?(as: @options[:as])
+      end
+
+      def single_template_render?
+        @template # Template is only set when a collection renders one template.
+      end
+
+      def callable_cache_key?
+        @options[:cache].respond_to?(:call)
+      end
+
+      def collection_by_cache_keys
+        seed = callable_cache_key? ? @options[:cache] : ->(i) { i }
+
+        @collection.each_with_object({}) do |item, hash|
+          hash[expanded_cache_key(seed.call(item))] = item
+        end
+      end
+
+      def expanded_cache_key(key)
+        key = @view.fragment_cache_key(@view.cache_fragment_name(key))
+        key.frozen? ? key.dup : key # #read_multi & #write may require mutability, Dalli 2.6.0.
+      end
+
+      def fetch_or_cache_partial(cached_partials, order_by:)
+        cache_options = @options[:cache_options] || @locals[:cache_options] || {}
+
+        order_by.map do |key|
+          cached_partials.fetch(key) do
+            yield.tap do |rendered_partial|
+              collection_cache.write(key, rendered_partial, cache_options)
+            end
+          end
+        end
+      end
+  end
+end
diff --git a/actionview/lib/action_view/template.rb b/actionview/lib/action_view/template.rb
index 305d9c6ee3..377ceb534a 100644
--- a/actionview/lib/action_view/template.rb
+++ b/actionview/lib/action_view/template.rb
@@ -130,6 +130,7 @@ module ActionView
       @source            = source
       @identifier        = identifier
       @handler           = handler
+      @cache_name        = extract_resource_cache_call_name
       @compiled          = false
       @original_encoding = nil
       @locals            = details[:locals] || []
@@ -165,6 +166,10 @@ module ActionView
       @type ||= Types[@formats.first] if @formats.first
     end
 
+    def eligible_for_collection_caching?(as: nil)
+      @cache_name == (as || inferred_cache_name).to_s
+    end
+
     # Receives a view object and return a template similar to self by using @virtual_path.
     #
     # This method is useful if you have a template object but it does not contain its source
@@ -345,5 +350,14 @@ module ActionView
         payload = { virtual_path: @virtual_path, identifier: @identifier }
         ActiveSupport::Notifications.instrument("#{action}.action_view", payload, &block)
       end
+
+      def extract_resource_cache_call_name
+        $1 if @handler.respond_to?(:resource_cache_call_pattern) &&
+          @source =~ @handler.resource_cache_call_pattern
+      end
+
+      def inferred_cache_name
+        @inferred_cache_name ||= @virtual_path.split('/').last.sub('_', '')
+      end
   end
 end
diff --git a/actionview/lib/action_view/template/handlers/erb.rb b/actionview/lib/action_view/template/handlers/erb.rb
index 85a100ed4c..88a8570706 100644
--- a/actionview/lib/action_view/template/handlers/erb.rb
+++ b/actionview/lib/action_view/template/handlers/erb.rb
@@ -123,6 +123,24 @@ module ActionView
           ).src
         end
 
+        # Returns Regexp to extract a cached resource's name from a cache call at the
+        # first line of a template.
+        # The extracted cache name is expected in $1.
+        #
+        #   <% cache notification do %> # => notification
+        #
+        # The pattern should support templates with a beginning comment:
+        #
+        #   <%# Still extractable even though there's a comment %>
+        #   <% cache notification do %> # => notification
+        #
+        # But fail to extract a name if a resource association is cached.
+        #
+        #   <% cache notification.event do %> # => nil
+        def resource_cache_call_pattern
+          /\A(?:<%#.*%>\n?)?<% cache\(?\s*(\w+\.?)/
+        end
+
       private
 
         def valid_encoding(string, encoding)
diff --git a/actionview/test/actionpack/controller/render_test.rb b/actionview/test/actionpack/controller/render_test.rb
index fb826af044..8b47536a18 100644
--- a/actionview/test/actionpack/controller/render_test.rb
+++ b/actionview/test/actionpack/controller/render_test.rb
@@ -31,6 +31,10 @@ class Customer < Struct.new(:name, :id)
   def persisted?
     id.present?
   end
+
+  def cache_key
+    name.to_s
+  end
 end
 
 module Quiz
diff --git a/actionview/test/fixtures/test/_cached_customer.erb b/actionview/test/fixtures/test/_cached_customer.erb
new file mode 100644
index 0000000000..52f35a3497
--- /dev/null
+++ b/actionview/test/fixtures/test/_cached_customer.erb
@@ -0,0 +1,3 @@
+<% cache cached_customer do %>
+  Hello: <%= cached_customer.name %>
+<% end %>
\ No newline at end of file
diff --git a/actionview/test/fixtures/test/_cached_customer_as.erb b/actionview/test/fixtures/test/_cached_customer_as.erb
new file mode 100644
index 0000000000..fca8d19e34
--- /dev/null
+++ b/actionview/test/fixtures/test/_cached_customer_as.erb
@@ -0,0 +1,3 @@
+<% cache buyer do %>
+  <%= greeting %>: <%= customer.name %>
+<% end %>
\ No newline at end of file
diff --git a/actionview/test/template/render_test.rb b/actionview/test/template/render_test.rb
index 94af1dc3ad..22665b6844 100644
--- a/actionview/test/template/render_test.rb
+++ b/actionview/test/template/render_test.rb
@@ -598,3 +598,41 @@ class LazyViewRenderTest < ActiveSupport::TestCase
     silence_warnings { Encoding.default_external = old }
   end
 end
+
+class CachedCollectionViewRenderTest < CachedViewRenderTest
+  class CachedCustomer < Customer; end
+
+  teardown do
+    ActionView::PartialRenderer.collection_cache.clear
+  end
+
+  test "with custom key" do
+    customer = Customer.new("david")
+    key = ActionController::Base.new.fragment_cache_key([customer, 'key'])
+
+    ActionView::PartialRenderer.collection_cache.write(key, 'Hello')
+
+    assert_equal "Hello",
+      @view.render(partial: "test/customer", collection: [customer], cache: ->(item) { [item, 'key'] })
+  end
+
+  test "automatic caching with inferred cache name" do
+    customer = CachedCustomer.new("david")
+    key = ActionController::Base.new.fragment_cache_key(customer)
+
+    ActionView::PartialRenderer.collection_cache.write(key, 'Cached')
+
+    assert_equal "Cached",
+      @view.render(partial: "test/cached_customer", collection: [customer])
+  end
+
+  test "automatic caching with as name" do
+    customer = CachedCustomer.new("david")
+    key = ActionController::Base.new.fragment_cache_key(customer)
+
+    ActionView::PartialRenderer.collection_cache.write(key, 'Cached')
+
+    assert_equal "Cached",
+      @view.render(partial: "test/cached_customer_as", collection: [customer], as: :buyer)
+  end
+end
diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb
index 3a1e1ac3a1..fdef9cbfc0 100644
--- a/activesupport/lib/active_support/cache.rb
+++ b/activesupport/lib/active_support/cache.rb
@@ -325,19 +325,22 @@ module ActiveSupport
       def read_multi(*names)
         options = names.extract_options!
         options = merged_options(options)
-        results = {}
-        names.each do |name|
-          key = namespaced_key(name, options)
-          entry = read_entry(key, options)
-          if entry
-            if entry.expired?
-              delete_entry(key, options)
-            else
-              results[name] = entry.value
+
+        instrument_multi(:read, names, options) do |payload|
+          results = {}
+          names.each do |name|
+            key = namespaced_key(name, options)
+            entry = read_entry(key, options)
+            if entry
+              if entry.expired?
+                delete_entry(key, options)
+              else
+                results[name] = entry.value
+              end
             end
           end
+          results
         end
-        results
       end
 
       # Fetches data from the cache, using the given keys. If there is data in
@@ -527,16 +530,27 @@ module ActiveSupport
         end
 
         def instrument(operation, key, options = nil)
-          log(operation, key, options)
+          log { "Cache #{operation}: #{key}#{options.blank? ? "" : " (#{options.inspect})"}" }
 
           payload = { :key => key }
           payload.merge!(options) if options.is_a?(Hash)
           ActiveSupport::Notifications.instrument("cache_#{operation}.active_support", payload){ yield(payload) }
         end
 
-        def log(operation, key, options = nil)
+        def instrument_multi(operation, keys, options = nil)
+          log do
+            formatted_keys = keys.map { |k| "- #{k}" }.join("\n")
+            "Caches multi #{operation}:\n#{formatted_keys}#{options.blank? ? "" : " (#{options.inspect})"}"
+          end
+
+          payload = { key: keys }
+          payload.merge!(options) if options.is_a?(Hash)
+          ActiveSupport::Notifications.instrument("cache_#{operation}_multi.active_support", payload) { yield(payload) }
+        end
+
+        def log
           return unless logger && logger.debug? && !silence?
-          logger.debug("Cache #{operation}: #{key}#{options.blank? ? "" : " (#{options.inspect})"}")
+          logger.debug(yield)
         end
 
         def find_cached_entry(key, name, options)
diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb
index 61b4f0b8b0..73ae3acea5 100644
--- a/activesupport/lib/active_support/cache/mem_cache_store.rb
+++ b/activesupport/lib/active_support/cache/mem_cache_store.rb
@@ -66,14 +66,17 @@ module ActiveSupport
       def read_multi(*names)
         options = names.extract_options!
         options = merged_options(options)
-        keys_to_names = Hash[names.map{|name| [escape_key(namespaced_key(name, options)), name]}]
-        raw_values = @data.get_multi(keys_to_names.keys, :raw => true)
-        values = {}
-        raw_values.each do |key, value|
-          entry = deserialize_entry(value)
-          values[keys_to_names[key]] = entry.value unless entry.expired?
+
+        instrument_multi(:read, names, options) do
+          keys_to_names = Hash[names.map{|name| [escape_key(namespaced_key(name, options)), name]}]
+          raw_values = @data.get_multi(keys_to_names.keys, :raw => true)
+          values = {}
+          raw_values.each do |key, value|
+            entry = deserialize_entry(value)
+            values[keys_to_names[key]] = entry.value unless entry.expired?
+          end
+          values
         end
-        values
       end
 
       # Increment a cached value. This method uses the memcached incr atomic
diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb
index 98f3ea7a14..095e908907 100644
--- a/activesupport/test/caching_test.rb
+++ b/activesupport/test/caching_test.rb
@@ -1021,6 +1021,15 @@ class CacheStoreLoggerTest < ActiveSupport::TestCase
     @cache.mute { @cache.fetch('foo') { 'bar' } }
     assert @buffer.string.blank?
   end
+
+  def test_multi_read_loggin
+    @cache.write 'hello', 'goodbye'
+    @cache.write 'world', 'earth'
+
+    @cache.read_multi('hello', 'world')
+
+    assert_match "Caches multi read:\n- hello\n- world", @buffer.string.tap { |l| p l }
+  end
 end
 
 class CacheEntryTest < ActiveSupport::TestCase