Merge pull request #8497 from acapilleri/conditional_cache

Removed :if / :unless conditions to fragment cache in favour of *cache_i...

3 files changed, 55 insertions, 31 deletions

diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index 31822f1eda..30580809e0 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -35,11 +35,23 @@
 
     *DHH*
 
-*   Add :if / :unless conditions to fragment cache:
+*   Add `cache_if` and `cache_unless` for conditional fragment caching:
 
-        <%= cache @model, if: some_condition(@model) do %>
+    Example:
+
+        <%= cache_if condition, project do %>
+          <b>All the topics on this project</b>
+          <%= render project.topics %>
+        <% end %>
 
-    *Stephen Ausman + Fabrizio Regini*
+        #and
+
+        <%= cache_unless condition, project do %>
+          <b>All the topics on this project</b>
+          <%= render project.topics %>
+        <% end %>
+ 
+    *Stephen Ausman + Fabrizio Regini + Angelo Capilleri*
 
 *   Add filter capability to ActionController logs for redirect locations:
 
diff --git a/actionpack/lib/action_view/helpers/cache_helper.rb b/actionpack/lib/action_view/helpers/cache_helper.rb
index 8693f4f0e4..932a5fa65a 100644
--- a/actionpack/lib/action_view/helpers/cache_helper.rb
+++ b/actionpack/lib/action_view/helpers/cache_helper.rb
@@ -110,15 +110,8 @@ module ActionView
       #   <%= 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 && conditions_match?(options)
+        if controller.perform_caching
           safe_concat(fragment_for(cache_fragment_name(name, options), options, &block))
         else
           yield
@@ -127,6 +120,29 @@ module ActionView
         nil
       end
 
+      #Chache fragments of a view if +condition+ is true
+      #
+      #=== Example
+      #   <%= cache_if admin?, project do %>
+      #     <b>All the topics on this project</b>
+      #     <%= render project.topics %>
+      #   <% end %>
+      def cache_if(condition, name = {}, options = nil, &block)
+        if condition
+          cache(name, options, &block)
+        else
+          yield
+        end
+
+        nil
+      end
+
+      #Chache fragments of a view unless +condition+ is true
+      #(see +cache_if+ for a example)
+      def cache_unless(condition, name = {}, options = nil, &block)
+        cache_if !condition, name, options, &block
+      end
+
       # This helper returns the name of a cache key for a given fragment cache
       # call. By supplying skip_digest: true to cache, the digestion of cache
       # fragments can be manually bypassed. This is useful when cache fragments
@@ -144,10 +160,6 @@ module ActionView
 
     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
           [
diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb
index 929545fc10..075347be52 100644
--- a/actionpack/test/controller/log_subscriber_test.rb
+++ b/actionpack/test/controller/log_subscriber_test.rb
@@ -46,20 +46,20 @@ module Another
       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' } %>"
+    def with_fragment_cache_if_with_true_condition
+      render :inline => "<%= cache_if(true, 'foo') { 'bar' } %>"
     end
 
-    def with_fragment_cache_and_if_false_condition
-      render :inline => "<%= cache('foo', :if => false) { 'bar' } %>"
+    def with_fragment_cache_if_with_false_condition
+      render :inline => "<%= cache_if(false, 'foo') { 'bar' } %>"
     end
 
-    def with_fragment_cache_and_unless_false_condition
-      render :inline => "<%= cache('foo', :unless => false) { 'bar' } %>"
+    def with_fragment_cache_unless_with_false_condition
+      render :inline => "<%= cache_unless(false, 'foo') { 'bar' } %>"
     end
 
-    def with_fragment_cache_and_unless_true_condition
-      render :inline => "<%= cache('foo', :unless => true) { 'bar' } %>"
+    def with_fragment_cache_unless_with_true_condition
+      render :inline => "<%= cache_unless(true, 'foo') { 'bar' } %>"
     end
 
     def with_exception
@@ -219,9 +219,9 @@ class ACLogSubscriberTest < ActionController::TestCase
     @controller.config.perform_caching = true
   end
 
-  def test_with_fragment_cache_and_if_true
+  def test_with_fragment_cache_if_with_true
     @controller.config.perform_caching = true
-    get :with_fragment_cache_and_if_true_condition
+    get :with_fragment_cache_if_with_true_condition
     wait
 
     assert_equal 4, logs.size
@@ -231,9 +231,9 @@ class ACLogSubscriberTest < ActionController::TestCase
     @controller.config.perform_caching = true
   end
 
-  def test_with_fragment_cache_and_if_false
+  def test_with_fragment_cache_if_with_false
     @controller.config.perform_caching = true
-    get :with_fragment_cache_and_if_false_condition
+    get :with_fragment_cache_if_with_false_condition
     wait
 
     assert_equal 2, logs.size
@@ -243,9 +243,9 @@ class ACLogSubscriberTest < ActionController::TestCase
     @controller.config.perform_caching = true
   end
 
-  def test_with_fragment_cache_and_unless_true
+  def test_with_fragment_cache_unless_with_true
     @controller.config.perform_caching = true
-    get :with_fragment_cache_and_unless_true_condition
+    get :with_fragment_cache_unless_with_true_condition
     wait
 
     assert_equal 2, logs.size
@@ -255,9 +255,9 @@ class ACLogSubscriberTest < ActionController::TestCase
     @controller.config.perform_caching = true
   end
 
-  def test_with_fragment_cache_and_unless_false
+  def test_with_fragment_cache_unless_with_false
     @controller.config.perform_caching = true
-    get :with_fragment_cache_and_unless_false_condition
+    get :with_fragment_cache_unless_with_false_condition
     wait
 
     assert_equal 4, logs.size