Skip to content
Browse files

Added ability to pass in :public => true to fresh_when, stale?, and e…

…xpires_in to make the request proxy cachable [#2095 state:committed]

Signed-off-by: David Heinemeier Hansson <david@loudthinking.com>
  • Loading branch information...
1 parent 7058c13 commit f2a32bd0dedf11021027e36cc2c99f97434cae17 @Gregg Gregg committed with dhh
Showing with 109 additions and 7 deletions.
  1. +2 −0 actionpack/CHANGELOG
  2. +35 −7 actionpack/lib/action_controller/base.rb
  3. +72 −0 actionpack/test/controller/render_test.rb
View
2 actionpack/CHANGELOG
@@ -1,5 +1,7 @@
*2.3.1 [RC2] (February ?, 2009)*
+* Added ability to pass in :public => true to fresh_when, stale?, and expires_in to make the request proxy cachable #2095 [Gregg Pollack]
+
* Fixed that passing a custom form builder would be forwarded to nested fields_for calls #2023 [Eloy Duran/Nate Wiger]
* Added partial scoping to TranslationHelper#translate, so if you call translate(".foo") from the people/index.html.erb template, you'll actually be calling I18n.translate("people.index.foo") [DHH]
View
42 actionpack/lib/action_controller/base.rb
@@ -1133,6 +1133,11 @@ def redirect_to_full_url(url, status)
# request is considered stale and should be generated from scratch. Otherwise,
# it's fresh and we don't need to generate anything and a reply of "304 Not Modified" is sent.
#
+ # Parameters:
+ # * <tt>:etag</tt>
+ # * <tt>:last_modified</tt>
+ # * <tt>:public</tt> By default the Cache-Control header is private, set this to true if you want your application to be cachable by other devices (proxy caches).
+ #
# Example:
#
# def show
@@ -1153,20 +1158,34 @@ def stale?(options)
# Sets the etag, last_modified, or both on the response and renders a
# "304 Not Modified" response if the request is already fresh.
#
+ # Parameters:
+ # * <tt>:etag</tt>
+ # * <tt>:last_modified</tt>
+ # * <tt>:public</tt> By default the Cache-Control header is private, set this to true if you want your application to be cachable by other devices (proxy caches).
+ #
# Example:
#
# def show
# @article = Article.find(params[:id])
- # fresh_when(:etag => @article, :last_modified => @article.created_at.utc)
+ # fresh_when(:etag => @article, :last_modified => @article.created_at.utc, :public => true)
# 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)
+ options.assert_valid_keys(:etag, :last_modified, :public)
response.etag = options[:etag] if options[:etag]
response.last_modified = options[:last_modified] if options[:last_modified]
+
+ if options[:public]
+ cache_control = response.headers["Cache-Control"].split(",").map {|k| k.strip }
+ cache_control.delete("private")
+ cache_control.delete("no-cache")
+ cache_control << "public"
+ response.headers["Cache-Control"] = cache_control.join(', ')
+ end
if request.fresh?(response)
head :not_modified
@@ -1178,15 +1197,24 @@ def fresh_when(options)
#
# Examples:
# expires_in 20.minutes
- # expires_in 3.hours, :private => false
- # expires in 3.hours, 'max-stale' => 5.hours, :private => nil, :public => true
+ # expires_in 3.hours, :public => true
+ # expires in 3.hours, 'max-stale' => 5.hours, :public => true
#
# This method will overwrite an existing Cache-Control header.
# See http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html for more possibilities.
def expires_in(seconds, options = {}) #:doc:
- cache_options = { 'max-age' => seconds, 'private' => true }.symbolize_keys.merge!(options.symbolize_keys)
- cache_options.delete_if { |k,v| v.nil? or v == false }
- cache_control = cache_options.map{ |k,v| v == true ? k.to_s : "#{k.to_s}=#{v.to_s}"}
+ cache_control = response.headers["Cache-Control"].split(",").map {|k| k.strip }
+
+ cache_control << "max-age=#{seconds}"
+ if options[:public]
+ cache_control.delete("private")
+ cache_control.delete("no-cache")
+ cache_control << "public"
+ end
+
+ # This allows for additional headers to be passed through like 'max-stale' => 5.hours
+ cache_control += options.symbolize_keys.reject{|k,v| k == :public || k == :private }.map{ |k,v| v == true ? k.to_s : "#{k.to_s}=#{v.to_s}"}
+
response.headers["Cache-Control"] = cache_control.join(', ')
end
View
72 actionpack/test/controller/render_test.rb
@@ -36,6 +36,39 @@ def conditional_hello
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_and_expires_at
+ expires_in 1.minute
+ 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_expires_in
+ expires_in 1.minute
+ render :action => 'hello_world'
+ end
+
+ def conditional_hello_with_expires_in_with_public
+ expires_in 1.minute, :public => true
+ render :action => 'hello_world'
+ end
+
+ def conditional_hello_with_expires_in_with_public_with_more_keys
+ expires_in 1.minute, :public => true, 'max-stale' => 5.hours
+ render :action => 'hello_world'
+ end
+
+ def conditional_hello_with_expires_in_with_public_with_more_keys_old_syntax
+ expires_in 1.minute, :public => true, :private => nil, 'max-stale' => 5.hours
+ render :action => 'hello_world'
+ end
def conditional_hello_with_bangs
render :action => 'hello_world'
@@ -1464,6 +1497,35 @@ def test_render_call_to_partial_with_layout_in_main_layout_and_within_content_fo
end
end
+class ExpiresInRenderTest < ActionController::TestCase
+ tests TestController
+
+ def setup
+ @request.host = "www.nextangle.com"
+ end
+
+ def test_expires_in_header
+ get :conditional_hello_with_expires_in
+ assert_equal "max-age=60, private", @response.headers["Cache-Control"]
+ end
+
+ def test_expires_in_header
+ get :conditional_hello_with_expires_in_with_public
+ assert_equal "max-age=60, public", @response.headers["Cache-Control"]
+ end
+
+ def test_expires_in_header_with_additional_headers
+ get :conditional_hello_with_expires_in_with_public_with_more_keys
+ assert_equal "max-age=60, public, max-stale=18000", @response.headers["Cache-Control"]
+ end
+
+ def test_expires_in_old_syntax
+ get :conditional_hello_with_expires_in_with_public_with_more_keys_old_syntax
+ assert_equal "max-age=60, public, max-stale=18000", @response.headers["Cache-Control"]
+ end
+end
+
+
class EtagRenderTest < ActionController::TestCase
tests TestController
@@ -1553,6 +1615,16 @@ def test_etag_with_bang_should_obey_if_none_match
get :conditional_hello_with_bangs
assert_response :not_modified
end
+
+ def test_etag_with_public_true_should_set_header
+ get :conditional_hello_with_public_header
+ assert_equal "public", @response.headers['Cache-Control']
+ end
+
+ def test_etag_with_public_true_should_set_header_and_retain_other_headers
+ get :conditional_hello_with_public_header_and_expires_at
+ assert_equal "max-age=60, public", @response.headers['Cache-Control']
+ end
protected
def etag_for(text)

2 comments on commit f2a32bd

@spencermiles

This is really terrible - changing :private to :public without any deprecation warning? Not cool.

@jeremy
Ruby on Rails member

It's a mistake, Spencer. Please raise the issue on Lighthouse ticket #2095.

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