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>

2 files changed, 43 insertions, 34 deletions

diff --git a/actionpack/lib/action_controller/base/chained/flash.rb b/actionpack/lib/action_controller/base/chained/flash.rb
index 7a8dd2dcf9..2d084ba1ab 100644
--- a/actionpack/lib/action_controller/base/chained/flash.rb
+++ b/actionpack/lib/action_controller/base/chained/flash.rb
@@ -30,7 +30,7 @@ module ActionController #:nodoc:
 
     # TODO : Remove the defined? check when new base is the main base
     depends_on Session if defined?(ActionController::Http)
-    
+
     included do
       # TODO : Remove the defined? check when new base is the main base
       if defined?(ActionController::Http)
@@ -129,6 +129,11 @@ module ActionController #:nodoc:
         (@used.keys - keys).each{ |k| @used.delete(k) }
       end
 
+      def store(session, key = "flash")
+        return if self.empty?
+        session[key] = self
+      end
+
       private
         # Used internally by the <tt>keep</tt> and <tt>discard</tt> methods
         #     use()               # marks the entire flash as used
@@ -145,48 +150,47 @@ module ActionController #:nodoc:
 
     module InstanceMethodsForBase #:nodoc:
       protected
+        def perform_action_with_flash
+          perform_action_without_flash
+          if defined? @_flash
+            @_flash.store(session)
+            remove_instance_variable(:@_flash)
+          end
+        end
 
-      def perform_action_with_flash
-        perform_action_without_flash
-        remove_instance_variable(:@_flash) if defined?(@_flash)
-      end
-
-      def reset_session_with_flash
-        reset_session_without_flash
-        remove_instance_variable(:@_flash) if defined?(@_flash)
-      end
+        def reset_session_with_flash
+          reset_session_without_flash
+          remove_instance_variable(:@_flash) if defined?(@_flash)
+        end
     end
 
     module InstanceMethodsForNewBase #:nodoc:
       protected
+        def process_action(method_name)
+          super
+          if defined? @_flash
+            @_flash.store(session)
+            remove_instance_variable(:@_flash)
+          end
+        end
 
-      def reset_session
-        super
-        remove_flash_instance_variable
-      end
-
-      def process_action(method_name)
-        super
-        remove_flash_instance_variable
-      end
-      
-      def remove_flash_instance_variable
-        remove_instance_variable(:@_flash) if defined?(@_flash)
-      end
+        def reset_session
+          super
+          remove_instance_variable(:@_flash) if defined?(@_flash)
+        end
     end
 
     protected
+      # Access the contents of the flash. Use <tt>flash["notice"]</tt> to
+      # read a notice you put there or <tt>flash["notice"] = "hello"</tt>
+      # to put a new one.
+      def flash #:doc:
+        if !defined?(@_flash)
+          @_flash = session["flash"] || FlashHash.new
+          @_flash.sweep
+        end
 
-    # Access the contents of the flash. Use <tt>flash["notice"]</tt> to
-    # read a notice you put there or <tt>flash["notice"] = "hello"</tt>
-    # to put a new one.
-    def flash #:doc:
-      unless defined?(@_flash)
-        @_flash = session["flash"] ||= FlashHash.new
-        @_flash.sweep
+        @_flash
       end
-
-      @_flash
-    end
   end
 end
diff --git a/actionpack/test/controller/flash_test.rb b/actionpack/test/controller/flash_test.rb
index ee215bb19e..c448f36cb3 100644
--- a/actionpack/test/controller/flash_test.rb
+++ b/actionpack/test/controller/flash_test.rb
@@ -122,7 +122,7 @@ class FlashTest < ActionController::TestCase
     assert_nil                  assigns["flash_copy"]["that"], "On second flash"
     assert_equal "hello again", assigns["flash_copy"]["this"], "On second flash"
   end
-  
+
   def test_flash_after_reset_session
     get :use_flash_after_reset_session
     assert_equal "hello",    assigns["flashy_that"]
@@ -130,6 +130,11 @@ class FlashTest < ActionController::TestCase
     assert_nil   assigns["flashy_that_reset"]
   end 
 
+  def test_does_not_set_the_session_if_the_flash_is_empty
+    get :std_action
+    assert_nil session["flash"]
+  end
+
   def test_sweep_after_halted_filter_chain
     get :std_action
     assert_nil assigns["flash_copy"]["foo"]