Skip to content
Browse files

Merge pull request #8371 from freegenie/5396-conditional-fragment-cac…

…hing

Allow fragment cache to accept :if and :unless options.

Closes #5396
  • Loading branch information...
2 parents 1b32c06 + 6b014a4 commit 6ed4ad133486f235fcba136d3a660d89f769beb9 @rafaelfranca rafaelfranca committed
View
6 actionpack/CHANGELOG.md
@@ -1,4 +1,10 @@
## Rails 4.0.0 (unreleased) ##
+* Add :if / :unless conditions to fragment cache:
+
+ <%= cache @model, if: some_condition(@model) do %>
+
+ *Stephen Ausman + Fabrizio Regini*
+
* Add filter capability to ActionController logs for redirect locations:
config.filter_redirect << 'http://please.hide.it/'
View
14 actionpack/lib/action_view/helpers/cache_helper.rb
@@ -110,8 +110,15 @@ module CacheHelper
# <%= some_helper_method(person) %>
#
# Now all you'll have to do is change that timestamp when the helper method changes.
+ #
+ # ==== Conditional caching
+ #
+ # You can pass :if and :unless options, to conditionally perform or skip the cache.
+ #
+ # <%= cache @model, if: some_condition(@model) do %>
+ #
def cache(name = {}, options = nil, &block)
- if controller.perform_caching
+ if controller.perform_caching && conditions_match?(options)
safe_concat(fragment_for(cache_fragment_name(name, options), options, &block))
else
yield
@@ -136,6 +143,11 @@ def cache_fragment_name(name = {}, options = nil)
end
private
+
+ def conditions_match?(options)
+ !(options && (!options.fetch(:if, true) || options.fetch(:unless, false)))
+ end
+
def fragment_name_with_digest(name) #:nodoc:
if @virtual_path
[
View
64 actionpack/test/controller/log_subscriber_test.rb
@@ -46,6 +46,22 @@ def with_fragment_cache_and_percent_in_key
render :inline => "<%= cache('foo%bar'){ 'Contains % sign in key' } %>"
end
+ def with_fragment_cache_and_if_true_condition
+ render :inline => "<%= cache('foo', :if => true) { 'bar' } %>"
+ end
+
+ def with_fragment_cache_and_if_false_condition
+ render :inline => "<%= cache('foo', :if => false) { 'bar' } %>"
+ end
+
+ def with_fragment_cache_and_unless_false_condition
+ render :inline => "<%= cache('foo', :unless => false) { 'bar' } %>"
+ end
+
+ def with_fragment_cache_and_unless_true_condition
+ render :inline => "<%= cache('foo', :unless => true) { 'bar' } %>"
+ end
+
def with_exception
raise Exception
end
@@ -203,6 +219,54 @@ def test_with_fragment_cache
@controller.config.perform_caching = true
end
+ def test_with_fragment_cache_and_if_true
+ @controller.config.perform_caching = true
+ get :with_fragment_cache_and_if_true_condition
+ wait
+
+ assert_equal 4, logs.size
+ assert_match(/Read fragment views\/foo/, logs[1])
+ assert_match(/Write fragment views\/foo/, logs[2])
+ ensure
+ @controller.config.perform_caching = true
+ end
+
+ def test_with_fragment_cache_and_if_false
+ @controller.config.perform_caching = true
+ get :with_fragment_cache_and_if_false_condition
+ wait
+
+ assert_equal 2, logs.size
+ assert_no_match(/Read fragment views\/foo/, logs[1])
+ assert_no_match(/Write fragment views\/foo/, logs[2])
+ ensure
+ @controller.config.perform_caching = true
+ end
+
+ def test_with_fragment_cache_and_unless_true
+ @controller.config.perform_caching = true
+ get :with_fragment_cache_and_unless_true_condition
+ wait
+
+ assert_equal 2, logs.size
+ assert_no_match(/Read fragment views\/foo/, logs[1])
+ assert_no_match(/Write fragment views\/foo/, logs[2])
+ ensure
+ @controller.config.perform_caching = true
+ end
+
+ def test_with_fragment_cache_and_unless_false
+ @controller.config.perform_caching = true
+ get :with_fragment_cache_and_unless_false_condition
+ wait
+
+ assert_equal 4, logs.size
+ assert_match(/Read fragment views\/foo/, logs[1])
+ assert_match(/Write fragment views\/foo/, logs[2])
+ ensure
+ @controller.config.perform_caching = true
+ end
+
def test_with_fragment_cache_and_percent_in_key
@controller.config.perform_caching = true
get :with_fragment_cache_and_percent_in_key

3 comments on commit 6ed4ad1

@acapilleri

I think that something like cache_unless(condition, option, &block)and `cache_if(condition, option, &block) could be much concise and near to rails standard of view helpers
But I'm not sure if add the helpers above or change this PR
what do you think? Thanks in advance
@rafaelfranca @carlosantoniodasilva

@rafaelfranca
Ruby on Rails member

@acapilleri I think it is good. If we are going to change we have to remove the :if and :unless options

@carlosantoniodasilva
Ruby on Rails member

Hm yeah, since we have similar methods like link_to_if/unless, might be a good fit.

Please sign in to comment.
Something went wrong with that request. Please try again.