Move ETag and ConditionalGet logic from AD::Response to the middleware stack.

2 files changed, 46 insertions, 22 deletions

diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb
index 4d04184b20..075e3c5692 100644
--- a/railties/lib/rails/application.rb
+++ b/railties/lib/rails/application.rb
@@ -145,8 +145,8 @@ module Rails
         rack_cache = config.action_controller.perform_caching && config.action_dispatch.rack_cache
 
         require "action_dispatch/http/rack_cache" if rack_cache
+        middleware.use ::Rack::Cache, rack_cache  if rack_cache
 
-        middleware.use ::Rack::Cache, rack_cache if rack_cache
         middleware.use ::ActionDispatch::Static, config.static_asset_paths if config.serve_static_assets
         middleware.use ::Rack::Lock if !config.allow_concurrency
         middleware.use ::Rack::Runtime
@@ -165,6 +165,8 @@ module Rails
         middleware.use ::ActionDispatch::ParamsParser
         middleware.use ::Rack::MethodOverride
         middleware.use ::ActionDispatch::Head
+        middleware.use ::Rack::ConditionalGet
+        middleware.use ::Rack::ETag, "no-cache"
         middleware.use ::ActionDispatch::BestStandardsSupport, config.action_dispatch.best_standards_support if config.action_dispatch.best_standards_support
       end
     end
diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb
index 2372ad85b8..173ac40b12 100644
--- a/railties/test/application/middleware_test.rb
+++ b/railties/test/application/middleware_test.rb
@@ -36,6 +36,8 @@ module ApplicationTests
         "ActionDispatch::ParamsParser",
         "Rack::MethodOverride",
         "ActionDispatch::Head",
+        "Rack::ConditionalGet",
+        "Rack::ETag",
         "ActionDispatch::BestStandardsSupport"
       ], middleware
     end
@@ -45,27 +47,7 @@ module ApplicationTests
 
       boot!
 
-      assert_equal [
-        "Rack::Cache",
-        "ActionDispatch::Static",
-        "Rack::Lock",
-        "ActiveSupport::Cache::Strategy::LocalCache",
-        "Rack::Runtime",
-        "Rails::Rack::Logger",
-        "ActionDispatch::ShowExceptions",
-        "ActionDispatch::RemoteIp",
-        "Rack::Sendfile",
-        "ActionDispatch::Callbacks",
-        "ActiveRecord::ConnectionAdapters::ConnectionManagement",
-        "ActiveRecord::QueryCache",
-        "ActionDispatch::Cookies",
-        "ActionDispatch::Session::CookieStore",
-        "ActionDispatch::Flash",
-        "ActionDispatch::ParamsParser",
-        "Rack::MethodOverride",
-        "ActionDispatch::Head",
-        "ActionDispatch::BestStandardsSupport"
-      ], middleware
+      assert_equal "Rack::Cache", middleware.first
     end
 
     test "removing Active Record omits its middleware" do
@@ -129,6 +111,46 @@ module ApplicationTests
       assert_equal "Rack::Config", middleware.first
     end
 
+    # ConditionalGet + Etag
+    test "conditional get + etag middlewares handle http caching based on body" do
+      make_basic_app
+
+      class ::OmgController < ActionController::Base
+        def index
+          if params[:nothing]
+            render :text => ""
+          else
+            render :text => "OMG"
+          end
+        end
+      end
+
+      etag = "5af83e3196bf99f440f31f2e1a6c9afe".inspect
+
+      get "/"
+      assert_equal 200, last_response.status
+      assert_equal "OMG", last_response.body
+      assert_equal "text/html; charset=utf-8", last_response.headers["Content-Type"]
+      assert_equal "max-age=0, private, must-revalidate", last_response.headers["Cache-Control"]
+      assert_equal etag, last_response.headers["Etag"]
+
+      get "/", {}, "HTTP_IF_NONE_MATCH" => etag
+      assert_equal 304, last_response.status
+      assert_equal "", last_response.body
+      assert_equal nil, last_response.headers["Content-Type"]
+      assert_equal "max-age=0, private, must-revalidate", last_response.headers["Cache-Control"]
+      assert_equal etag, last_response.headers["Etag"]
+
+      get "/?nothing=true"
+      puts last_response.body
+      assert_equal 200, last_response.status
+      assert_equal "", last_response.body
+      assert_equal "text/html; charset=utf-8", last_response.headers["Content-Type"]
+      assert_equal "no-cache", last_response.headers["Cache-Control"]
+      assert_equal nil, last_response.headers["Etag"]
+    end
+
+    # Show exceptions middleware
     test "show exceptions middleware filter backtrace before logging" do
       my_middleware = Struct.new(:app) do
         def call(env)