Added stale?/fresh? and fresh_when methods to provide a layer of abstraction above request.fresh? and friends [DHH]

4 files changed, 113 insertions, 30 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index c68bfc753c..2fd37484cb 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,39 @@
 *Edge*
 
+* Added stale?/fresh? and fresh_when methods to provide a layer of abstraction above request.fresh? and friends [DHH]. Example:
+
+    class ArticlesController < ApplicationController
+      def show_with_respond_to_block
+        @article = Article.find(params[:id])
+
+
+        # If the request sends headers that differs from the options provided to stale?, then
+        # the request is indeed stale and the respond_to block is triggered (and the options
+        # to the stale? call is set on the response).
+        #
+        # If the request headers match, then the request is fresh and the respond_to block is
+        # not triggered. Instead the default render will occur, which will check the last-modified
+        # and etag headers and conclude that it only needs to send a "304 Not Modified" instead
+        # of rendering the template.
+        if stale?(:last_modified => @article.published_at.utc, :etag => @article)
+          respond_to do |wants|
+            # normal response processing
+          end
+        end
+      end
+
+      def show_with_implied_render
+        @article = Article.find(params[:id])
+
+        # Sets the response headers and checks them against the request, if the request is stale
+        # (i.e. no match of either etag or last-modified), then the default render of the template happens.
+        # If the request is fresh, then the default render will return a "304 Not Modified"
+        # instead of rendering the template.
+        fresh_when(:last_modified => @article.published_at.utc, :etag => @article)
+      end
+    end
+
+
 * Added inline builder yield to atom_feed_helper tags where appropriate [Sam Ruby]. Example:
 
     entry.summary :type => 'xhtml' do |xhtml|
diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index 3ede681253..4c5c5ac597 100644
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -965,22 +965,6 @@ module ActionController #:nodoc:
         render :nothing => true, :status => status
       end
 
-      # Sets the Last-Modified response header. Returns 304 Not Modified if the
-      # If-Modified-Since request header is <= last modified.
-      def last_modified!(utc_time)
-        response.last_modified= utc_time
-        if request.if_modified_since && request.if_modified_since <= utc_time
-          head(:not_modified)
-        end
-      end
-
-      # Sets the ETag response header. Returns 304 Not Modified if the
-      # If-None-Match request header matches.
-      def etag!(etag)
-        response.etag = etag
-        head(:not_modified) if response.etag == request.if_none_match
-      end
-
       # Clears the rendered results, allowing for another render to be performed.
       def erase_render_results #:nodoc:
         response.body = nil
@@ -1090,6 +1074,54 @@ module ActionController #:nodoc:
         @performed_redirect = true
       end
 
+      # Sets the etag and/or last_modified on the response and checks it against
+      # the client request. If the request doesn't match the options provided, the
+      # request is considered stale and should be generated from scratch. Otherwise,
+      # it's fresh and we don't need to generate anything and can rely on the default
+      # reply of "304 Not Modified".
+      #
+      # Example:
+      #
+      #   def show
+      #     @article = Article.find(params[:id])
+      #
+      #     if stale?(:etag => @article, :last_modified => @article.created_at.utc)
+      #       @statistics = @article.really_expensive_call
+      #       respond_to do |format|
+      #         # all the supported formats
+      #       end
+      #     end
+      #   end
+      def stale?(options)
+        fresh_when(options)
+        !request.fresh?(response)
+      end
+
+      # The opposite of stale? provided for parity when that feels more natural.
+      def fresh?(options)
+        !stale?(options)
+      end
+
+      # Sets the etag, last_modified, or both such that the request can be short-circuited
+      # with a "304 Not Modified" response instead of rendering a template when the request
+      # is already fresh. 
+      #
+      # Example:
+      #
+      #   def show
+      #     @article = Article.find(params[:id])
+      #     fresh_when(:etag => @article, :last_modified => @article.created_at.utc)
+      #   end
+      # 
+      # This will render the show template if the request isn't sending a matching etag or 
+      # If-Modified-Since header and just a "304 Not Modified" response if there's a match.
+      def fresh_when(options)
+        options.assert_valid_keys(:etag, :last_modified)
+
+        response.etag          = options[:etag]          if options[:etag]
+        response.last_modified = options[:last_modified] if options[:last_modified]
+      end
+
       # Sets a HTTP 1.1 Cache-Control header. Defaults to issuing a "private" instruction, so that
       # intermediate caches shouldn't cache the response.
       #
@@ -1176,7 +1208,11 @@ module ActionController #:nodoc:
       end
 
       def default_render #:nodoc:
-        render
+        if request.fresh?(response)
+          head :not_modified
+        else
+          render
+        end
       end
 
       def perform_action
diff --git a/actionpack/lib/action_controller/request.rb b/actionpack/lib/action_controller/request.rb
index 5e492e3ee1..9f33cbc55f 100755
--- a/actionpack/lib/action_controller/request.rb
+++ b/actionpack/lib/action_controller/request.rb
@@ -120,9 +120,19 @@ module ActionController
     end
 
     # Check response freshness (Last-Modified and ETag) against request
-    # If-Modified-Since and If-None-Match conditions.
+    # If-Modified-Since and If-None-Match conditions. If both headers are
+    # supplied, both must match, or the request is not considered fresh.
     def fresh?(response)
-      not_modified?(response.last_modified) || etag_matches?(response.etag)
+      case
+      when if_modified_since && if_none_match 
+        not_modified?(response.last_modified) && etag_matches?(response.etag) 
+      when if_modified_since 
+        not_modified?(response.last_modified) 
+      when if_none_match 
+        etag_matches?(response.etag) 
+      else 
+        false 
+      end 
     end
 
     # Returns the Mime type for the \format used in the request.
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index 7b8bb6856b..98d4cb3ffe 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -30,24 +30,20 @@ class TestController < ActionController::Base
   end
 
   def conditional_hello
-    response.last_modified = Time.now.utc.beginning_of_day
-    response.etag = [:foo, 123]
-
-    if request.fresh?(response)
-      head :not_modified
-    else
+    if stale?(:last_modified => Time.now.utc.beginning_of_day, :etag => [:foo, 123])
       render :action => 'hello_world'
     end
   end
-  
+
   def conditional_hello_with_bangs
     render :action => 'hello_world'
   end
   before_filter :handle_last_modified_and_etags, :only=>:conditional_hello_with_bangs
   
   def handle_last_modified_and_etags
-    last_modified! Time.now.utc.beginning_of_day
-    etag! [:foo, 123]
+    if fresh?(:last_modified => Time.now.utc.beginning_of_day, :etag => [ :foo, 123 ])
+      head :not_modified
+    end
   end
 
   def render_hello_world
@@ -248,7 +244,7 @@ class TestController < ActionController::Base
     if @alternate_default_render
       @alternate_default_render.call
     else
-      render
+      super
     end
   end
 
@@ -1422,6 +1418,13 @@ class LastModifiedRenderTest < Test::Unit::TestCase
     assert_equal @last_modified, @response.headers['Last-Modified']
   end
 
+  def test_request_not_modified_but_etag_differs
+    @request.if_modified_since = @last_modified
+    @request.if_none_match = "234"
+    get :conditional_hello
+    assert_response :success
+  end
+
   def test_request_modified
     @request.if_modified_since = 'Thu, 16 Jul 2008 00:00:00 GMT'
     get :conditional_hello
@@ -1445,7 +1448,7 @@ class LastModifiedRenderTest < Test::Unit::TestCase
   def test_last_modified_works_with_less_than_too
     @request.if_modified_since = 5.years.ago.httpdate
     get :conditional_hello_with_bangs
-    assert_response :not_modified
+    assert_response :success
   end
 end