aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDavid Heinemeier Hansson <david@loudthinking.com>2016-08-22 13:34:35 -0700
committerGitHub <noreply@github.com>2016-08-22 13:34:35 -0700
commitdebd774d632ae7e1e6c0a7d0306979159df39b63 (patch)
tree743d98e205e426284db8b823ad4cd133188c96e0
parent3bfd35245278f410bb2165e859f1cfeb352e0d77 (diff)
downloadrails-debd774d632ae7e1e6c0a7d0306979159df39b63.tar.gz
rails-debd774d632ae7e1e6c0a7d0306979159df39b63.tar.bz2
rails-debd774d632ae7e1e6c0a7d0306979159df39b63.zip
Include the content of the flash in the auto-generated etag (#26250)
Include the content of the flash in the auto-generated etag
-rw-r--r--actionpack/CHANGELOG.md12
-rw-r--r--actionpack/lib/action_controller.rb1
-rw-r--r--actionpack/lib/action_controller/base.rb1
-rw-r--r--actionpack/lib/action_controller/metal/etag_with_flash.rb16
-rw-r--r--actionpack/test/controller/flash_test.rb29
5 files changed, 59 insertions, 0 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md
index d1a2b9b827..a66a1e8af3 100644
--- a/actionpack/CHANGELOG.md
+++ b/actionpack/CHANGELOG.md
@@ -1,3 +1,15 @@
+* Include the content of the flash in the auto-generated etag. This solves the following problem:
+
+ 1. POST /messages
+ 2. redirect_to messages_url, notice: 'Message was created'
+ 3. GET /messages/1
+ 4. GET /messages
+
+ Step 4 would before still include the flash message, even though it's no longer relevant,
+ because the etag cache was recorded with the flash in place and didn't change when it was gone.
+
+ *DHH*
+
* SSL: Changes redirect behavior for all non-GET and non-HEAD requests
(like POST/PUT/PATCH etc) to `http://` resources to redirect to `https://`
with a [307 status code](http://tools.ietf.org/html/rfc7231#section-6.4.7) instead of [301 status code](http://tools.ietf.org/html/rfc7231#section-6.4.2).
diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb
index fc86a907b3..50f20aa789 100644
--- a/actionpack/lib/action_controller.rb
+++ b/actionpack/lib/action_controller.rb
@@ -23,6 +23,7 @@ module ActionController
autoload :Cookies
autoload :DataStreaming
autoload :EtagWithTemplateDigest
+ autoload :EtagWithFlash
autoload :Flash
autoload :ForceSSL
autoload :Head
diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index 68a526eccb..ca8066cd82 100644
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -213,6 +213,7 @@ module ActionController
Renderers::All,
ConditionalGet,
EtagWithTemplateDigest,
+ EtagWithFlash,
Caching,
MimeResponds,
ImplicitRender,
diff --git a/actionpack/lib/action_controller/metal/etag_with_flash.rb b/actionpack/lib/action_controller/metal/etag_with_flash.rb
new file mode 100644
index 0000000000..474d75f02e
--- /dev/null
+++ b/actionpack/lib/action_controller/metal/etag_with_flash.rb
@@ -0,0 +1,16 @@
+module ActionController
+ # When you're using the flash, it's generally used as a conditional on the view.
+ # This means the content of the view depends on the flash. Which in turn means
+ # that the etag for a response should be computed with the content of the flash
+ # in mind. This does that by including the content of the flash as a component
+ # in the etag that's generated for a response.
+ module EtagWithFlash
+ extend ActiveSupport::Concern
+
+ include ActionController::ConditionalGet
+
+ included do
+ etag { flash unless flash.empty? }
+ end
+ end
+end
diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb
index cabbe2d608..e5f24f1a3a 100644
--- a/actionpack/test/controller/flash_test.rb
+++ b/actionpack/test/controller/flash_test.rb
@@ -263,6 +263,13 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest
flash[:bar] = "for great justice"
head :ok
end
+
+ def set_flash_optionally
+ flash.now.notice = params[:flash]
+ if stale? etag: "abe"
+ render inline: "maybe flash"
+ end
+ end
end
def test_flash
@@ -310,6 +317,28 @@ class FlashIntegrationTest < ActionDispatch::IntegrationTest
end
end
+ def test_flash_factored_into_etag
+ with_test_route_set do
+ get "/set_flash_optionally"
+ no_flash_etag = response.etag
+
+ get "/set_flash_optionally", params: { flash: "hello!" }
+ hello_flash_etag = response.etag
+
+ assert_not_equal no_flash_etag, hello_flash_etag
+
+ get "/set_flash_optionally", params: { flash: "hello!" }
+ another_hello_flash_etag = response.etag
+
+ assert_equal another_hello_flash_etag, hello_flash_etag
+
+ get "/set_flash_optionally", params: { flash: "goodbye!" }
+ goodbye_flash_etag = response.etag
+
+ assert_not_equal another_hello_flash_etag, goodbye_flash_etag
+ end
+ end
+
private
# Overwrite get to send SessionSecret in env hash