diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 4760ec16984..2d6607041d6 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 00000000000..521b1450df2 --- /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 00000000000..d5f6e3b4911 --- /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 00000000000..67e9f6d4116 --- /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 9dadfb5ce1a..0e2a5f90f48 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 views/ 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 ee3dce24b78..9a26cba5747 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 5ff15411cf1..56b8ab1e2dd 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 00000000000..b77c884e662 --- /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 305d9c6ee3b..377ceb534ae 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 85a100ed4c9..88a85707067 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 fb826af044a..8b47536a186 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 00000000000..52f35a3497e --- /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 00000000000..fca8d19e34b --- /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 94af1dc3ad3..22665b68448 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 3a1e1ac3a11..fdef9cbfc0c 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 61b4f0b8b0b..73ae3acea5e 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 98f3ea7a141..095e9089075 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