Skip to content
This repository
Browse code

Update cache_control to be a Hash of options that is used to build th…

…e header.

  * Significantly simplifies setting and modifying cache control in other areas
  • Loading branch information...
commit 503ce1d01ce6c8eee9818f4e76a9f880bb1a291d 1 parent f2a3572
Yehuda Katz authored July 30, 2009
23  actionpack/lib/action_controller/base/conditional_get.rb
@@ -29,11 +29,7 @@ def fresh_when(options)
29 29
       response.last_modified = options[:last_modified] if options[:last_modified]
30 30
 
31 31
       if options[:public]
32  
-        cache_control = response.headers["Cache-Control"].split(",").map {|k| k.strip }
33  
-        cache_control.delete("private")
34  
-        cache_control.delete("no-cache")
35  
-        cache_control << "public"
36  
-        response.headers["Cache-Control"] = cache_control.join(', ')
  32
+        response.cache_control[:public] = true
37 33
       end
38 34
 
39 35
       if request.fresh?(response)
@@ -107,21 +103,10 @@ def stale?(options)
107 103
     # This method will overwrite an existing Cache-Control header.
108 104
     # See http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html for more possibilities.
109 105
     def expires_in(seconds, options = {}) #:doc:
110  
-      cache_control = response.headers["Cache-Control"].split(",").map {|k| k.strip }
  106
+      response.cache_control.merge!(:max_age => seconds, :public => options.delete(:public))
  107
+      options.delete(:private)
111 108
 
112  
-      cache_control << "max-age=#{seconds}"
113  
-      cache_control.delete("no-cache")
114  
-      if options[:public]
115  
-        cache_control.delete("private")
116  
-        cache_control << "public"
117  
-      else
118  
-        cache_control << "private"
119  
-      end
120  
-
121  
-      # This allows for additional headers to be passed through like 'max-stale' => 5.hours
122  
-      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}"}
123  
-
124  
-      response.headers["Cache-Control"] = cache_control.join(', ')
  109
+      response.cache_control[:extras] = options.map {|k,v| "#{k}=#{v}"}
125 110
     end
126 111
 
127 112
     # Sets a HTTP 1.1 Cache-Control header of "no-cache" so no caching should occur by the browser or
2  actionpack/lib/action_controller/base/streaming.rb
@@ -181,7 +181,7 @@ def send_file_headers!(options)
181 181
         # after it displays the "open/save" dialog, which means that if you
182 182
         # hit "open" the file isn't there anymore when the application that
183 183
         # is called for handling the download is run, so let's workaround that
184  
-        headers['Cache-Control'] = 'private' if headers['Cache-Control'] == 'no-cache'
  184
+        response.cache_control[:public] ||= false
185 185
       end
186 186
   end
187 187
 end
2  actionpack/lib/action_controller/testing/process.rb
@@ -52,7 +52,7 @@ def recycle!
52 52
   class TestResponse < ActionDispatch::TestResponse
53 53
     def recycle!
54 54
       @status = 200
55  
-      @header = Rack::Utils::HeaderHash.new(DEFAULT_HEADERS)
  55
+      @header = Rack::Utils::HeaderHash.new
56 56
       @writer = lambda { |x| @body << x }
57 57
       @block = nil
58 58
       @length = 0
24  actionpack/lib/action_dispatch/http/response.rb
@@ -32,8 +32,8 @@ module ActionDispatch # :nodoc:
32 32
   #    end
33 33
   #  end
34 34
   class Response < Rack::Response
35  
-    DEFAULT_HEADERS = { "Cache-Control" => "no-cache" }
36 35
     attr_accessor :request
  36
+    attr_reader :cache_control
37 37
 
38 38
     attr_writer :header
39 39
     alias_method :headers=, :header=
@@ -42,7 +42,8 @@ class Response < Rack::Response
42 42
 
43 43
     def initialize
44 44
       super
45  
-      @header = Rack::Utils::HeaderHash.new(DEFAULT_HEADERS)
  45
+      @cache_control = {}
  46
+      @header = Rack::Utils::HeaderHash.new
46 47
     end
47 48
 
48 49
     # The response code of the request
@@ -196,7 +197,7 @@ def cookies
196 197
 
197 198
     private
198 199
       def handle_conditional_get!
199  
-        if etag? || last_modified?
  200
+        if etag? || last_modified? || !cache_control.empty?
200 201
           set_conditional_cache_control!
201 202
         elsif nonempty_ok_response?
202 203
           self.etag = body
@@ -207,6 +208,8 @@ def handle_conditional_get!
207 208
           end
208 209
 
209 210
           set_conditional_cache_control!
  211
+        else
  212
+          headers["Cache-Control"] = "no-cache"
210 213
         end
211 214
       end
212 215
 
@@ -220,9 +223,20 @@ def string_body?
220 223
       end
221 224
 
222 225
       def set_conditional_cache_control!
223  
-        if headers['Cache-Control'] == DEFAULT_HEADERS['Cache-Control']
224  
-          headers['Cache-Control'] = 'private, max-age=0, must-revalidate'
  226
+        if cache_control.empty?
  227
+          cache_control.merge!(:public => false, :max_age => 0, :must_revalidate => true)
225 228
         end
  229
+
  230
+        public_cache, max_age, must_revalidate, extras =
  231
+          cache_control.values_at(:public, :max_age, :must_revalidate, :extras)
  232
+
  233
+        options = []
  234
+        options << "max-age=#{max_age}" if max_age
  235
+        options << (public_cache ? "public" : "private")
  236
+        options << "must-revalidate" if must_revalidate
  237
+        options.concat(extras) if extras
  238
+
  239
+        headers["Cache-Control"] = options.join(", ")
226 240
       end
227 241
 
228 242
       def convert_content_type!
2  actionpack/test/controller/render_test.rb
@@ -1331,7 +1331,7 @@ def test_render_blank_body_shouldnt_set_etag
1331 1331
   def test_render_200_should_set_etag
1332 1332
     get :render_hello_world_from_variable
1333 1333
     assert_equal etag_for("hello david"), @response.headers['ETag']
1334  
-    assert_equal "private, max-age=0, must-revalidate", @response.headers['Cache-Control']
  1334
+    assert_equal "max-age=0, private, must-revalidate", @response.headers['Cache-Control']
1335 1335
   end
1336 1336
 
1337 1337
   def test_render_against_etag_request_should_304_when_match
2  actionpack/test/controller/send_file_test.rb
@@ -129,7 +129,7 @@ def test_send_file_headers!
129 129
     # test overriding Cache-Control: no-cache header to fix IE open/save dialog
130 130
     @controller.headers = { 'Cache-Control' => 'no-cache' }
131 131
     @controller.send(:send_file_headers!, options)
132  
-    h = @controller.headers
  132
+    @controller.response.prepare!
133 133
     assert_equal 'private', h['Cache-Control']
134 134
   end
135 135
 
4  actionpack/test/dispatch/response_test.rb
@@ -13,7 +13,7 @@ def setup
13 13
     assert_equal 200, status
14 14
     assert_equal({
15 15
       "Content-Type" => "text/html; charset=utf-8",
16  
-      "Cache-Control" => "private, max-age=0, must-revalidate",
  16
+      "Cache-Control" => "max-age=0, private, must-revalidate",
17 17
       "ETag" => '"65a8e27d8879283831b664bd8b7f0ad4"',
18 18
       "Set-Cookie" => "",
19 19
       "Content-Length" => "13"
@@ -32,7 +32,7 @@ def setup
32 32
     assert_equal 200, status
33 33
     assert_equal({
34 34
       "Content-Type" => "text/html; charset=utf-8",
35  
-      "Cache-Control" => "private, max-age=0, must-revalidate",
  35
+      "Cache-Control" => "max-age=0, private, must-revalidate",
36 36
       "ETag" => '"ebb5e89e8a94e9dd22abf5d915d112b2"',
37 37
       "Set-Cookie" => "",
38 38
       "Content-Length" => "8"

0 notes on commit 503ce1d

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