Fix adding implicitly rendered template digests to ETags

Signed-off-by: Jeremy Daer <jeremydaer@gmail.com>

3 files changed, 43 insertions, 11 deletions

diff --git a/actionpack/lib/action_controller/metal/etag_with_template_digest.rb b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb
index 669cf55bca..75ac996793 100644
--- a/actionpack/lib/action_controller/metal/etag_with_template_digest.rb
+++ b/actionpack/lib/action_controller/metal/etag_with_template_digest.rb
@@ -39,8 +39,14 @@ module ActionController
       end
     end
 
+    # Pick the template digest to include in the ETag. If the +:template+ option
+    # is present, use the named template. If +:template+ is nil or absent, use
+    # the default controller/action template. If +:template+ is false, omit the
+    # template digest from the ETag.
     def pick_template_for_etag(options)
-      options.fetch(:template) { "#{controller_name}/#{action_name}" }
+      unless options[:template] == false
+        options[:template] || "#{controller_name}/#{action_name}"
+      end
     end
 
     def lookup_and_digest_template(template)
diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb
index 8d3134630d..652c06af19 100644
--- a/actionpack/test/controller/render_test.rb
+++ b/actionpack/test/controller/render_test.rb
@@ -2,6 +2,9 @@ require 'abstract_unit'
 require 'controller/fake_models'
 
 class TestControllerWithExtraEtags < ActionController::Base
+  def self.controller_name; 'test'; end
+  def self.controller_path; 'test'; end
+
   etag { nil  }
   etag { 'ab' }
   etag { :cde }
@@ -17,7 +20,7 @@ class TestControllerWithExtraEtags < ActionController::Base
   end
 
   def strong
-    render plain: "stale" if stale?(strong_etag: 'strong')
+    render plain: "stale" if stale?(strong_etag: 'strong', template: false)
   end
 
   def with_template
@@ -25,6 +28,10 @@ class TestControllerWithExtraEtags < ActionController::Base
       render plain: 'stale'
     end
   end
+
+  def with_implicit_template
+    fresh_when(etag: '123')
+  end
 end
 
 class ImplicitRenderTestController < ActionController::Base
@@ -528,20 +535,28 @@ class EtagRenderTest < ActionController::TestCase
     get :with_template
     assert_response :not_modified
 
-    # Modify the template digest
-    path = File.expand_path('../../fixtures/test/hello_world.erb', __FILE__)
-    old = File.read(path)
+    modify_template(:hello_world) do
+      request.if_none_match = etag
+      get :with_template
+      assert_response :ok
+      assert_not_equal etag, @response.etag
+    end
+  end
 
-    begin
-      File.write path, 'foo'
-      ActionView::LookupContext::DetailsKey.clear
+  def test_etag_reflects_implicit_template_digest
+    get :with_implicit_template
+    assert_response :ok
+    assert_not_nil etag = @response.etag
 
+    request.if_none_match = etag
+    get :with_implicit_template
+    assert_response :not_modified
+
+    modify_template(:with_implicit_template) do
       request.if_none_match = etag
-      get :with_template
+      get :with_implicit_template
       assert_response :ok
       assert_not_equal etag, @response.etag
-    ensure
-      File.write path, old
     end
   end
 
@@ -553,6 +568,16 @@ class EtagRenderTest < ActionController::TestCase
     def strong_etag(record)
       %("#{Digest::MD5.hexdigest(ActiveSupport::Cache.expand_cache_key(record))}")
     end
+
+    def modify_template(name)
+      path = File.expand_path("../../fixtures/test/#{name}.erb", __FILE__)
+      original = File.read(path)
+      File.write(path, "#{original} Modified!")
+      ActionView::LookupContext::DetailsKey.clear
+      yield
+    ensure
+      File.write(path, original)
+    end
 end
 
 class MetalRenderTest < ActionController::TestCase
diff --git a/actionpack/test/fixtures/test/with_implicit_template.erb b/actionpack/test/fixtures/test/with_implicit_template.erb
new file mode 100644
index 0000000000..474488cd13
--- /dev/null
+++ b/actionpack/test/fixtures/test/with_implicit_template.erb
@@ -0,0 +1 @@
+Hello explicitly!