From 503ce1d01ce6c8eee9818f4e76a9f880bb1a291d Mon Sep 17 00:00:00 2001 From: Yehuda Katz Date: Thu, 30 Jul 2009 21:00:39 -0700 Subject: Update cache_control to be a Hash of options that is used to build the header. * Significantly simplifies setting and modifying cache control in other areas --- .../lib/action_controller/base/conditional_get.rb | 23 ++++----------------- actionpack/lib/action_controller/base/streaming.rb | 2 +- .../lib/action_controller/testing/process.rb | 2 +- actionpack/lib/action_dispatch/http/response.rb | 24 +++++++++++++++++----- actionpack/test/controller/render_test.rb | 2 +- actionpack/test/controller/send_file_test.rb | 2 +- actionpack/test/dispatch/response_test.rb | 4 ++-- 7 files changed, 29 insertions(+), 30 deletions(-) diff --git a/actionpack/lib/action_controller/base/conditional_get.rb b/actionpack/lib/action_controller/base/conditional_get.rb index d287ec4994..6d35137428 100644 --- a/actionpack/lib/action_controller/base/conditional_get.rb +++ b/actionpack/lib/action_controller/base/conditional_get.rb @@ -29,11 +29,7 @@ module ActionController 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(', ') + response.cache_control[:public] = true end if request.fresh?(response) @@ -107,21 +103,10 @@ module ActionController # 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_control = response.headers["Cache-Control"].split(",").map {|k| k.strip } + response.cache_control.merge!(:max_age => seconds, :public => options.delete(:public)) + options.delete(:private) - cache_control << "max-age=#{seconds}" - cache_control.delete("no-cache") - if options[:public] - cache_control.delete("private") - cache_control << "public" - else - cache_control << "private" - 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(', ') + response.cache_control[:extras] = options.map {|k,v| "#{k}=#{v}"} end # Sets a HTTP 1.1 Cache-Control header of "no-cache" so no caching should occur by the browser or diff --git a/actionpack/lib/action_controller/base/streaming.rb b/actionpack/lib/action_controller/base/streaming.rb index f52810ff3a..f0317c6e99 100644 --- a/actionpack/lib/action_controller/base/streaming.rb +++ b/actionpack/lib/action_controller/base/streaming.rb @@ -181,7 +181,7 @@ module ActionController #:nodoc: # after it displays the "open/save" dialog, which means that if you # hit "open" the file isn't there anymore when the application that # is called for handling the download is run, so let's workaround that - headers['Cache-Control'] = 'private' if headers['Cache-Control'] == 'no-cache' + response.cache_control[:public] ||= false end end end diff --git a/actionpack/lib/action_controller/testing/process.rb b/actionpack/lib/action_controller/testing/process.rb index e7c64d0942..d32d5562e8 100644 --- a/actionpack/lib/action_controller/testing/process.rb +++ b/actionpack/lib/action_controller/testing/process.rb @@ -52,7 +52,7 @@ module ActionController #:nodoc: class TestResponse < ActionDispatch::TestResponse def recycle! @status = 200 - @header = Rack::Utils::HeaderHash.new(DEFAULT_HEADERS) + @header = Rack::Utils::HeaderHash.new @writer = lambda { |x| @body << x } @block = nil @length = 0 diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index e58b4b5e19..32cfb5ae44 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -32,8 +32,8 @@ module ActionDispatch # :nodoc: # end # end class Response < Rack::Response - DEFAULT_HEADERS = { "Cache-Control" => "no-cache" } attr_accessor :request + attr_reader :cache_control attr_writer :header alias_method :headers=, :header= @@ -42,7 +42,8 @@ module ActionDispatch # :nodoc: def initialize super - @header = Rack::Utils::HeaderHash.new(DEFAULT_HEADERS) + @cache_control = {} + @header = Rack::Utils::HeaderHash.new end # The response code of the request @@ -196,7 +197,7 @@ module ActionDispatch # :nodoc: private def handle_conditional_get! - if etag? || last_modified? + if etag? || last_modified? || !cache_control.empty? set_conditional_cache_control! elsif nonempty_ok_response? self.etag = body @@ -207,6 +208,8 @@ module ActionDispatch # :nodoc: end set_conditional_cache_control! + else + headers["Cache-Control"] = "no-cache" end end @@ -220,9 +223,20 @@ module ActionDispatch # :nodoc: end def set_conditional_cache_control! - if headers['Cache-Control'] == DEFAULT_HEADERS['Cache-Control'] - headers['Cache-Control'] = 'private, max-age=0, must-revalidate' + if cache_control.empty? + cache_control.merge!(:public => false, :max_age => 0, :must_revalidate => true) end + + public_cache, max_age, must_revalidate, extras = + cache_control.values_at(:public, :max_age, :must_revalidate, :extras) + + options = [] + options << "max-age=#{max_age}" if max_age + options << (public_cache ? "public" : "private") + options << "must-revalidate" if must_revalidate + options.concat(extras) if extras + + headers["Cache-Control"] = options.join(", ") end def convert_content_type! diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index acb0c895e0..d0fa67c945 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -1331,7 +1331,7 @@ class EtagRenderTest < ActionController::TestCase def test_render_200_should_set_etag get :render_hello_world_from_variable assert_equal etag_for("hello david"), @response.headers['ETag'] - assert_equal "private, max-age=0, must-revalidate", @response.headers['Cache-Control'] + assert_equal "max-age=0, private, must-revalidate", @response.headers['Cache-Control'] end def test_render_against_etag_request_should_304_when_match diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index f0c723eaa2..0afebac68c 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -129,7 +129,7 @@ class SendFileTest < ActionController::TestCase # test overriding Cache-Control: no-cache header to fix IE open/save dialog @controller.headers = { 'Cache-Control' => 'no-cache' } @controller.send(:send_file_headers!, options) - h = @controller.headers + @controller.response.prepare! assert_equal 'private', h['Cache-Control'] end diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 2ddc6cb2b5..2b1540c678 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -13,7 +13,7 @@ class ResponseTest < ActiveSupport::TestCase assert_equal 200, status assert_equal({ "Content-Type" => "text/html; charset=utf-8", - "Cache-Control" => "private, max-age=0, must-revalidate", + "Cache-Control" => "max-age=0, private, must-revalidate", "ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"', "Set-Cookie" => "", "Content-Length" => "13" @@ -32,7 +32,7 @@ class ResponseTest < ActiveSupport::TestCase assert_equal 200, status assert_equal({ "Content-Type" => "text/html; charset=utf-8", - "Cache-Control" => "private, max-age=0, must-revalidate", + "Cache-Control" => "max-age=0, private, must-revalidate", "ETag" => '"ebb5e89e8a94e9dd22abf5d915d112b2"', "Set-Cookie" => "", "Content-Length" => "8" -- cgit v1.2.3