etag! and last_modified! conditional GET helpers

4 files changed, 150 insertions, 61 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index da9fdbfd9d..f6432fe0be 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,9 @@
 *Edge*
 
+* Conditional GET utility methods.  [Jeremy Kemper]
+  * etag!([:admin, post, current_user]) sets the ETag response header and returns head(:not_modified) if it matches the If-None-Match request header.
+  * last_modified!(post.updated_at) sets Last-Modified and returns head(:not_modified) if it's no later than If-Modified-Since.
+
 * All 2xx requests are considered successful [Josh Peek]
 
 * Fixed that AssetTagHelper#compute_public_path shouldn't cache the asset_host along with the source or per-request proc's won't run [DHH]
diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index c56812c2d9..50727c67c4 100755
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -519,6 +519,8 @@ module ActionController #:nodoc:
     public
       # Extracts the action_name from the request parameters and performs that action.
       def process(request, response, method = :perform_action, *arguments) #:nodoc:
+        response.request = request
+
         initialize_template_class(response)
         assign_shortcuts(request, response)
         initialize_current_url
@@ -529,8 +531,6 @@ module ActionController #:nodoc:
         send(method, *arguments)
 
         assign_default_content_type_and_charset
-
-        response.request = request
         response.prepare! unless component_request?
         response
       ensure
@@ -968,6 +968,17 @@ 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)
+        head(:not_modified) if response.last_modified!(utc_time)
+      end
+
+      # Sets the ETag response header. Returns 304 Not Modified if the
+      # If-None-Match request header matches.
+      def etag!(etag)
+        head(:not_modified) if response.etag!(etag)
+      end
 
       # Clears the rendered results, allowing for another render to be performed.
       def erase_render_results #:nodoc:
diff --git a/actionpack/lib/action_controller/response.rb b/actionpack/lib/action_controller/response.rb
index 1d9f6676ba..9955532844 100755
--- a/actionpack/lib/action_controller/response.rb
+++ b/actionpack/lib/action_controller/response.rb
@@ -41,20 +41,48 @@ module ActionController
       set_content_length!
     end
 
+    # Sets the Last-Modified response header. Returns whether it's older than
+    # the If-Modified-Since request header.
+    def last_modified!(utc_time)
+      headers['Last-Modified'] ||= utc_time.httpdate
+      if request && since = request.headers['HTTP_IF_MODIFIED_SINCE']
+        utc_time <= Time.rfc2822(since)
+      end
+    end
+
+    # Sets the ETag response header. Returns whether it matches the
+    # If-None-Match request header.
+    def etag!(tag)
+      headers['ETag'] ||= %("#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(tag))}")
+      if request && request.headers['HTTP_IF_NONE_MATCH'] == headers['ETag']
+        true
+      end
+    end
 
     private
       def handle_conditional_get!
-        if body.is_a?(String) && (headers['Status'] ? headers['Status'][0..2] == '200' : true)  && !body.empty?
-          self.headers['ETag'] ||= %("#{Digest::MD5.hexdigest(body)}")
-          self.headers['Cache-Control'] = 'private, max-age=0, must-revalidate' if headers['Cache-Control'] == DEFAULT_HEADERS['Cache-Control']
+        if nonempty_ok_response?
+          set_conditional_cache_control!
 
-          if request.headers['HTTP_IF_NONE_MATCH'] == headers['ETag']
-            self.headers['Status'] = '304 Not Modified'
+          if etag!(body)
+            headers['Status'] = '304 Not Modified'
             self.body = ''
           end
         end
       end
 
+      def nonempty_ok_response?
+        status = headers['Status']
+        ok = !status || status[0..2] == '200'
+        ok && body.is_a?(String) && !body.empty?
+      end
+
+      def set_conditional_cache_control!
+        if headers['Cache-Control'] == DEFAULT_HEADERS['Cache-Control']
+          headers['Cache-Control'] = 'private, max-age=0, must-revalidate'
+        end
+      end
+
       def convert_content_type!
         if content_type = headers.delete("Content-Type")
           self.headers["type"] = content_type
@@ -73,4 +101,4 @@ module ActionController
         self.headers["Content-Length"] = body.size unless body.respond_to?(:call)
       end
   end
-end
\ No newline at end of file
+end
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index 9a94db4b00..041c54c7fd 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -8,14 +8,18 @@ module Fun
   end
 end
 
-
-# FIXME: crashes Ruby 1.9
 class TestController < ActionController::Base
   layout :determine_layout
 
   def hello_world
   end
 
+  def conditional_hello
+    etag! [:foo, 123]
+    last_modified! Time.now.utc.beginning_of_day
+    render :action => 'hello_world' unless performed?
+  end
+
   def render_hello_world
     render :template => "test/hello_world"
   end
@@ -408,6 +412,72 @@ class RenderTest < Test::Unit::TestCase
     assert_equal "Goodbye, Local David", @response.body
   end
 
+  def test_should_render_formatted_template
+    get :formatted_html_erb
+    assert_equal 'formatted html erb', @response.body
+  end
+
+  def test_should_render_formatted_xml_erb_template
+    get :formatted_xml_erb, :format => :xml
+    assert_equal '<test>passed formatted xml erb</test>', @response.body
+  end
+
+  def test_should_render_formatted_html_erb_template
+    get :formatted_xml_erb
+    assert_equal '<test>passed formatted html erb</test>', @response.body
+  end
+
+  def test_should_render_formatted_html_erb_template_with_faulty_accepts_header
+    @request.env["HTTP_ACCEPT"] = "image/gif, image/x-xbitmap, image/jpeg, image/pjpeg, appliction/x-shockwave-flash, */*"
+    get :formatted_xml_erb
+    assert_equal '<test>passed formatted html erb</test>', @response.body
+  end
+
+  def test_should_render_html_formatted_partial
+    get :partial
+    assert_equal 'partial html', @response.body
+  end
+
+  def test_should_render_html_partial_with_dot
+    get :partial_dot_html
+    assert_equal 'partial html', @response.body
+  end
+
+  def test_should_render_html_formatted_partial_with_rjs
+    xhr :get, :partial_as_rjs
+    assert_equal %(Element.replace("foo", "partial html");), @response.body
+  end
+
+  def test_should_render_html_formatted_partial_with_rjs_and_js_format
+    xhr :get, :respond_to_partial_as_rjs
+    assert_equal %(Element.replace("foo", "partial html");), @response.body
+  end
+
+  def test_should_render_js_partial
+    xhr :get, :partial, :format => 'js'
+    assert_equal 'partial js', @response.body
+  end
+
+  def test_should_render_with_alternate_default_render
+    xhr :get, :render_alternate_default
+    assert_equal %(Element.replace("foo", "partial html");), @response.body
+  end
+
+  def test_should_render_xml_but_keep_custom_content_type
+    get :render_xml_with_custom_content_type
+    assert_equal "application/atomsvc+xml", @response.content_type
+  end
+end
+
+class EtagRenderTest < Test::Unit::TestCase
+  def setup
+    @request    = ActionController::TestRequest.new
+    @response   = ActionController::TestResponse.new
+    @controller = TestController.new
+
+    @request.host = "www.nextangle.com"
+  end
+
   def test_render_200_should_set_etag
     get :render_hello_world_from_variable
     assert_equal etag_for("hello david"), @response.headers['ETag']
@@ -460,64 +530,40 @@ class RenderTest < Test::Unit::TestCase
     assert_equal etag_for("<wrapper>\n<html>\n  <p>Hello </p>\n<p>This is grand!</p>\n</html>\n</wrapper>\n"), @response.headers['ETag']
   end
 
-  def test_should_render_formatted_template
-    get :formatted_html_erb
-    assert_equal 'formatted html erb', @response.body
-  end
-
-  def test_should_render_formatted_xml_erb_template
-    get :formatted_xml_erb, :format => :xml
-    assert_equal '<test>passed formatted xml erb</test>', @response.body
-  end
-
-  def test_should_render_formatted_html_erb_template
-    get :formatted_xml_erb
-    assert_equal '<test>passed formatted html erb</test>', @response.body
-  end
-
-  def test_should_render_formatted_html_erb_template_with_faulty_accepts_header
-    @request.env["HTTP_ACCEPT"] = "image/gif, image/x-xbitmap, image/jpeg, image/pjpeg, appliction/x-shockwave-flash, */*"
-    get :formatted_xml_erb
-    assert_equal '<test>passed formatted html erb</test>', @response.body
-  end
-
-  def test_should_render_html_formatted_partial
-    get :partial
-    assert_equal 'partial html', @response.body
-  end
-
-  def test_should_render_html_partial_with_dot
-    get :partial_dot_html
-    assert_equal 'partial html', @response.body
-  end
+  protected
+    def etag_for(text)
+      %("#{Digest::MD5.hexdigest(text)}")
+    end
+end
 
-  def test_should_render_html_formatted_partial_with_rjs
-    xhr :get, :partial_as_rjs
-    assert_equal %(Element.replace("foo", "partial html");), @response.body
-  end
+class LastModifiedRenderTest < Test::Unit::TestCase
+  def setup
+    @request    = ActionController::TestRequest.new
+    @response   = ActionController::TestResponse.new
+    @controller = TestController.new
 
-  def test_should_render_html_formatted_partial_with_rjs_and_js_format
-    xhr :get, :respond_to_partial_as_rjs
-    assert_equal %(Element.replace("foo", "partial html");), @response.body
+    @request.host = "www.nextangle.com"
+    @last_modified = Time.now.utc.beginning_of_day.httpdate
   end
 
-  def test_should_render_js_partial
-    xhr :get, :partial, :format => 'js'
-    assert_equal 'partial js', @response.body
+  def test_responds_with_last_modified
+    get :conditional_hello
+    assert_equal @last_modified, @response.headers['Last-Modified']
   end
 
-  def test_should_render_with_alternate_default_render
-    xhr :get, :render_alternate_default
-    assert_equal %(Element.replace("foo", "partial html");), @response.body
+  def test_request_not_modified
+    @request.headers["HTTP_IF_MODIFIED_SINCE"] = @last_modified
+    get :conditional_hello
+    assert_equal "304 Not Modified", @response.headers['Status']
+    assert @response.body.blank?, @response.body
+    assert_equal @last_modified, @response.headers['Last-Modified']
   end
 
-  def test_should_render_xml_but_keep_custom_content_type
-    get :render_xml_with_custom_content_type
-    assert_equal "application/atomsvc+xml", @response.content_type
+  def test_request_modified
+    @request.headers["HTTP_IF_MODIFIED_SINCE"] = 'Thu, 16 Jul 2008 00:00:00 GMT'
+    get :conditional_hello
+    assert_equal "200 OK", @response.headers['Status']
+    assert !@response.body.blank?
+    assert_equal @last_modified, @response.headers['Last-Modified']
   end
-
-  protected
-    def etag_for(text)
-      %("#{Digest::MD5.hexdigest(text)}")
-    end
 end