Fixes expires_now and cleans things up a bit

4 files changed, 22 insertions, 13 deletions

diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb
index 52f5a9727e..5156fbc1d5 100644
--- a/actionpack/lib/action_controller/metal/conditional_get.rb
+++ b/actionpack/lib/action_controller/metal/conditional_get.rb
@@ -80,7 +80,7 @@ module ActionController
     # Sets a HTTP 1.1 Cache-Control header of "no-cache" so no caching should occur by the browser or
     # intermediate caches (like caching proxy servers).
     def expires_now #:doc:
-      response.headers["Cache-Control"] = "no-cache"
+      response.cache_control.replace(:no_cache => true)
     end
   end
 end
diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb
index bff030f0e4..1e366520c9 100755
--- a/actionpack/lib/action_dispatch/http/request.rb
+++ b/actionpack/lib/action_dispatch/http/request.rb
@@ -136,19 +136,16 @@ module ActionDispatch
     # If-Modified-Since and If-None-Match conditions. If both headers are
     # supplied, both must match, or the request is not considered fresh.
     def fresh?(response)
-      case
-      when if_modified_since && if_none_match
-        not_modified?(response.last_modified) && etag_matches?(response.etag)
-      when if_modified_since
-        not_modified?(response.last_modified)
-      when if_none_match
-        etag_matches?(response.etag)
-      else
-        false
-      end
-    end
+      last_modified = if_modified_since
+      etag          = if_none_match
 
-    ONLY_ALL = [Mime::ALL].freeze
+      return false unless last_modified || etag
+
+      success = true
+      success &&= not_modified?(response.last_modified) if last_modified
+      success &&= etag_matches?(response.etag) if etag
+      success
+    end
 
     # Returns the Mime type for the \format used in the request.
     #
diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb
index 3e3b473178..b3ed7c9d1a 100644
--- a/actionpack/lib/action_dispatch/http/response.rb
+++ b/actionpack/lib/action_dispatch/http/response.rb
@@ -270,6 +270,8 @@ module ActionDispatch # :nodoc:
 
         if control.empty?
           headers["Cache-Control"] = DEFAULT_CACHE_CONTROL
+        elsif @cache_control[:no_cache]
+          headers["Cache-Control"] = "no-cache"
         else
           extras  = control[:extras]
           max_age = control[:max_age]
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index 2db524ca4b..ac8dad7c42 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -73,6 +73,11 @@ class TestController < ActionController::Base
     render :action => 'hello_world'
   end
 
+  def conditional_hello_with_expires_now
+    expires_now
+    render :action => 'hello_world'
+  end
+
   def conditional_hello_with_bangs
     render :action => 'hello_world'
   end
@@ -1321,6 +1326,11 @@ class ExpiresInRenderTest < ActionController::TestCase
     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
+
+  def test_expires_now
+    get :conditional_hello_with_expires_now
+    assert_equal "no-cache", @response.headers["Cache-Control"]
+  end
 end