Skip to content

Commit dc94c09

Browse files
jsjosh
authored andcommitted
The FlashHash and friends causes a lot of needless session storing, when we know for a fact that there's no content in the flash. By not storing the empty hash in the session we save a lot of communication with the various session backends, while still keeping the same interface to the flash. [#2703 state:resolved]
Signed-off-by: Joshua Peek <josh@joshpeek.com>
1 parent 34a1ed0 commit dc94c09

File tree

2 files changed

+17
-4
lines changed

2 files changed

+17
-4
lines changed

actionpack/lib/action_controller/flash.rb

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,11 @@ def sweep #:nodoc:
120120
(@used.keys - keys).each{ |k| @used.delete(k) }
121121
end
122122

123+
def store(session, key = "flash")
124+
return if self.empty?
125+
session[key] = self
126+
end
127+
123128
private
124129
# Used internally by the <tt>keep</tt> and <tt>discard</tt> methods
125130
# use() # marks the entire flash as used
@@ -139,7 +144,10 @@ module InstanceMethods #:nodoc:
139144
protected
140145
def perform_action_with_flash
141146
perform_action_without_flash
142-
remove_instance_variable(:@_flash) if defined? @_flash
147+
if defined? @_flash
148+
@_flash.store(session)
149+
remove_instance_variable(:@_flash)
150+
end
143151
end
144152

145153
def reset_session_with_flash
@@ -151,8 +159,8 @@ def reset_session_with_flash
151159
# read a notice you put there or <tt>flash["notice"] = "hello"</tt>
152160
# to put a new one.
153161
def flash #:doc:
154-
unless defined? @_flash
155-
@_flash = session["flash"] ||= FlashHash.new
162+
if !defined?(@_flash)
163+
@_flash = session["flash"] || FlashHash.new
156164
@_flash.sweep
157165
end
158166

actionpack/test/controller/flash_test.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ def test_update_flash
121121
assert_nil @response.template.assigns["flash_copy"]["that"], "On second flash"
122122
assert_equal "hello again", @response.template.assigns["flash_copy"]["this"], "On second flash"
123123
end
124-
124+
125125
def test_flash_after_reset_session
126126
get :use_flash_after_reset_session
127127
assert_equal "hello", @response.template.assigns["flashy_that"]
@@ -139,4 +139,9 @@ def test_sweep_after_halted_filter_chain
139139
get :std_action
140140
assert_nil @response.template.assigns["flash_copy"]["foo"]
141141
end
142+
143+
def test_does_not_set_the_session_if_the_flash_is_empty
144+
get :std_action
145+
assert_nil session["flash"]
146+
end
142147
end

0 commit comments

Comments
 (0)