Allow fresh_when/stale? to take a record instead of an options hash [DHH]

actionpack/CHANGELOG.md

@@ -1,5 +1,7 @@
 ## Rails 3.2.0 (unreleased) ##
 
+*   Allow fresh_when/stale? to take a record instead of an options hash *DHH*
+
 *   Assets should use the request protocol by default or default to relative if no request is available *Jonathan del Strother*
 
 *   Log "Filter chain halted as CALLBACKNAME rendered or redirected" every time a before callback halts *José Valim*

actionpack/lib/action_controller/metal/conditional_get.rb

@@ -23,8 +23,27 @@ module ActionController
     # This will render the show template if the request isn't sending a matching etag or
     # If-Modified-Since header and just a <tt>304 Not Modified</tt> response if there's a match.
     #
-    def fresh_when(options)
-      options.assert_valid_keys(:etag, :last_modified, :public)
+    # You can also just pass a record where last_modified will be set by calling updated_at and the etag by passing the object itself. Example:
+    #
+    #   def show
+    #     @article = Article.find(params[:id])
+    #     fresh_when(@article)
+    #   end
+    #
+    # When passing a record, you can still set whether the public header:
+    #
+    #   def show
+    #     @article = Article.find(params[:id])
+    #     fresh_when(@article, :public => true)
+    #   end
+    def fresh_when(record_or_options, additional_options = {})
+      if record_or_options.is_a? Hash
+        options = record_or_options
+        options.assert_valid_keys(:etag, :last_modified, :public)
+      else
+        record  = record_or_options
+        options = { :etag => record, :last_modified => record.try(:updated_at) }.merge(additional_options)
+      end
 
       response.etag          = options[:etag]          if options[:etag]
       response.last_modified = options[:last_modified] if options[:last_modified]
@@ -55,8 +74,34 @@ module ActionController
     #       end
     #     end
     #   end
-    def stale?(options)
-      fresh_when(options)
+    #
+    # You can also just pass a record where last_modified will be set by calling updated_at and the etag by passing the object itself. Example:
+    #
+    #   def show
+    #     @article = Article.find(params[:id])
+    #
+    #     if stale?(@article)
+    #       @statistics = @article.really_expensive_call
+    #       respond_to do |format|
+    #         # all the supported formats
+    #       end
+    #     end
+    #   end
+    #
+    # When passing a record, you can still set whether the public header:
+    #
+    #   def show
+    #     @article = Article.find(params[:id])
+    #
+    #     if stale?(@article, :public => true)
+    #       @statistics = @article.really_expensive_call
+    #       respond_to do |format|
+    #         # all the supported formats
+    #       end
+    #     end
+    #   end
+    def stale?(record_or_options, additional_options = {})
+      fresh_when(record_or_options, additional_options)
       !request.fresh?(response)
     end
 

actionpack/test/controller/render_test.rb

@@ -50,12 +50,28 @@ class TestController < ActionController::Base
     end
   end
 
+  def conditional_hello_with_record
+    record = Struct.new(:updated_at, :cache_key).new(Time.now.utc.beginning_of_day, "foo/123")
+    
+    if stale?(record)
+      render :action => 'hello_world'
+    end
+  end
+
   def conditional_hello_with_public_header
     if stale?(:last_modified => Time.now.utc.beginning_of_day, :etag => [:foo, 123], :public => true)
       render :action => 'hello_world'
     end
   end
 
+  def conditional_hello_with_public_header_with_record
+    record = Struct.new(:updated_at, :cache_key).new(Time.now.utc.beginning_of_day, "foo/123")
+
+    if stale?(record, :public => true)
+      render :action => 'hello_world'
+    end
+  end
+
   def conditional_hello_with_public_header_and_expires_at
     expires_in 1.minute
     if stale?(:last_modified => Time.now.utc.beginning_of_day, :etag => [:foo, 123], :public => true)
@@ -1440,6 +1456,36 @@ class LastModifiedRenderTest < ActionController::TestCase
     assert_equal @last_modified, @response.headers['Last-Modified']
   end
 
+
+  def test_responds_with_last_modified_with_record
+    get :conditional_hello_with_record
+    assert_equal @last_modified, @response.headers['Last-Modified']
+  end
+
+  def test_request_not_modified_with_record
+    @request.if_modified_since = @last_modified
+    get :conditional_hello_with_record
+    assert_equal 304, @response.status.to_i
+    assert_blank @response.body
+    assert_equal @last_modified, @response.headers['Last-Modified']
+  end
+
+  def test_request_not_modified_but_etag_differs_with_record
+    @request.if_modified_since = @last_modified
+    @request.if_none_match = "234"
+    get :conditional_hello_with_record
+    assert_response :success
+  end
+
+  def test_request_modified_with_record
+    @request.if_modified_since = 'Thu, 16 Jul 2008 00:00:00 GMT'
+    get :conditional_hello_with_record
+    assert_equal 200, @response.status.to_i
+    assert_present @response.body
+    assert_equal @last_modified, @response.headers['Last-Modified']
+  end
+
+
   def test_request_with_bang_gets_last_modified
     get :conditional_hello_with_bangs
     assert_equal @last_modified, @response.headers['Last-Modified']