Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bring back support for callable cache key when rendering collection #25616

Merged
merged 1 commit into from Jul 21, 2016
Merged
Changes from all commits
Commits
File filter
Filter file types
Jump to
Jump to file
Failed to load files.

Always

Just for now

@@ -393,9 +393,14 @@ def index_with_comment
@customers = [Customer.new('david', 1)]
render partial: 'customers/commented_customer', collection: @customers, as: :customer, cached: true
end

def index_with_callable_cache_key
@customers = [Customer.new('david', 1)]
render partial: 'customers/customer', collection: @customers, cached: -> customer { 'cached_david' }
end
end

class AutomaticCollectionCacheTest < ActionController::TestCase
class CollectionCacheTest < ActionController::TestCase

This comment has been minimized.

@kaspth

kaspth Jul 20, 2016
Member

wups! 😄

def setup
super
@controller = CollectionCacheController.new
@@ -438,6 +443,11 @@ def test_caching_works_with_beginning_comment
assert_equal 1, @controller.partial_rendered_times
end

def test_caching_with_callable_cache_key
get :index_with_callable_cache_key
assert_customer_cached 'cached_david', 'david, 1'
end

private
def assert_customer_cached(key, content)
assert_match content,
@@ -130,9 +130,10 @@ module CacheHelper
#
# When rendering a collection of objects that each use the same partial, a `cached`
# option can be passed.
#
# For collections rendered such:
#
# <%= render partial: 'notifications/notification', collection: @notifications, cached: true %>
# <%= render partial: 'projects/project', collection: @projects, cached: true %>
#
# The `cached: true` will make Action View's rendering read several templates
# from cache at once instead of one call per template.
@@ -142,13 +143,21 @@ module CacheHelper
# Works great alongside individual template fragment caching.
# For instance if the template the collection renders is cached like:
#
# # notifications/_notification.html.erb
# <% cache notification do %>
# # projects/_project.html.erb
# <% cache project do %>
# <%# ... %>
# <% end %>
#
# Any collection renders will find those cached templates when attempting
# to read multiple templates at once.
#
# If your collection cache depends on multiple sources (try to avoid this to keep things simple),
# you can name all these dependencies as part of a block that returns an array:
#
# <%= render partial: 'projects/project', collection: @projects, cached: -> project { [ project, current_user ] } %>
#
# This will include both records as part of the cache key and updating either of them will
# expire the cache.
def cache(name = {}, options = {}, &block)
if controller.respond_to?(:perform_caching) && controller.perform_caching
name_options = options.slice(:skip_digest, :virtual_path)
@@ -25,9 +25,15 @@ def cache_collection_render(instrumentation_payload)
end
end

def callable_cache_key?
@options[:cached].respond_to?(:call)
end

This comment has been minimized.

@kaspth

kaspth Jul 5, 2016
Member

Let's inline this helper. I know I did write it like this originally, but I since came to view it as an overabstraction 😁

This comment has been minimized.

@kaspth

kaspth Jul 5, 2016
Member

Actually, forget that. Let's keep it 😄

This comment has been minimized.

@ignatiusreza

ignatiusreza Jul 6, 2016
Author Contributor

forgetting it 🙈


def collection_by_cache_keys
seed = callable_cache_key? ? @options[:cached] : ->(i) { i }

@collection.each_with_object({}) do |item, hash|
hash[expanded_cache_key(item)] = item
hash[expanded_cache_key(seed.call(item))] = item
end
end

ProTip! Use n and p to navigate between commits in a pull request.