Decouple the merge/normalization and conditional cache control logic

The prior logic was trying to do too many things at once. For all responses,
we want to perform two distinct steps:

* Merge/normalize the `Cache-Control` values found in HTTP headers and those
  found in the `@cache_control` hash.
* Conditionally set a default `Cache-Control` header value when we have an ETag

This change separates these concerns since the merge/normalize step should
occur for all responses, but the second should only occur when we have already
set an ETag/last modified value. Normally ETag middleware will set a default
`Cache-Control`, but only if an existing ETag is not already set. So, in the
cases where an ETag is set, we need to set the default `Cache-Control` value
ourselves.

2 files changed, 13 insertions, 5 deletions

diff --git a/actionpack/lib/action_dispatch/http/cache.rb b/actionpack/lib/action_dispatch/http/cache.rb
index ec5704ff9e..2107b4d224 100644
--- a/actionpack/lib/action_dispatch/http/cache.rb
+++ b/actionpack/lib/action_dispatch/http/cache.rb
@@ -166,16 +166,23 @@ module ActionDispatch
           @cache_control = cache_control_headers
         end
 
-        def handle_conditional_get!
-          set_conditional_cache_control!(@cache_control)
-        end
-
+        DEFAULT_CACHE_CONTROL = "max-age=0, private, must-revalidate".freeze
         NO_CACHE              = "no-cache".freeze
         PUBLIC                = "public".freeze
         PRIVATE               = "private".freeze
         MUST_REVALIDATE       = "must-revalidate".freeze
 
-        def set_conditional_cache_control!(cache_control)
+        def handle_conditional_get!
+          # Normally default cache control setting is handled by ETag
+          # middleware. But, if an etag is already set, the middleware
+          # defaults to `no-cache` unless a default `Cache-Control` value is
+          # previously set. So, set a default one here.
+          if (etag? || last_modified?) && !self._cache_control
+            self._cache_control = DEFAULT_CACHE_CONTROL
+          end
+        end
+
+        def merge_and_normalize_cache_control!(cache_control)
           control = {}
           cc_headers = cache_control_headers
           if extras = cc_headers.delete(:extras)
diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb
index b314dbecfe..0c7b153420 100644
--- a/actionpack/lib/action_dispatch/http/response.rb
+++ b/actionpack/lib/action_dispatch/http/response.rb
@@ -433,6 +433,7 @@ module ActionDispatch # :nodoc:
     def before_committed
       return if committed?
       assign_default_content_type_and_charset!
+      merge_and_normalize_cache_control!(@cache_control)
       handle_conditional_get!
       handle_no_content!
     end