From c8a2b4993c7d496d4eb5842a9a325e20f06b5bac Mon Sep 17 00:00:00 2001
From: =?utf-8?q?Johan=20S=C3=B8rensen?= <johan@johansorensen.com>
Date: Wed, 27 May 2009 15:18:59 +0200
Subject: [PATCH 2/2] Only save the session if we're actually writing to it

While the session is lazily loaded, there's no point in actually setting the
session if the only access was through the getter.
---
 .../action_controller/session/abstract_store.rb    |   11 +++++++++-
 .../test/activerecord/active_record_store_test.rb  |   22 ++++++++++++++++++++
 .../controller/session/mem_cache_store_test.rb     |    8 +++++++
 3 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/actionpack/lib/action_controller/session/abstract_store.rb b/actionpack/lib/action_controller/session/abstract_store.rb
index f6369ab..16a9127 100644
--- a/actionpack/lib/action_controller/session/abstract_store.rb
+++ b/actionpack/lib/action_controller/session/abstract_store.rb
@@ -15,6 +15,7 @@ module ActionController
           @by = by
           @env = env
           @loaded = false
+          @updated = false
         end
 
         def session_id
@@ -32,6 +33,7 @@ module ActionController
         def []=(key, value)
           load! unless @loaded
           super
+          @updated = true
         end
 
         def to_hash
@@ -57,6 +59,10 @@ module ActionController
             @loaded
           end
 
+          def updated?
+            @updated
+          end
+
           def load!
             stale_session_check! do
               id, session = @by.send(:load_session, @env)
@@ -125,7 +131,10 @@ module ActionController
         options = env[ENV_SESSION_OPTIONS_KEY]
 
         if !session_data.is_a?(AbstractStore::SessionHash) || session_data.send(:loaded?) || options[:expire_after]
-          session_data.send(:load!) if session_data.is_a?(AbstractStore::SessionHash) && !session_data.send(:loaded?)
+          if session_data.is_a?(AbstractStore::SessionHash)
+            session_data.send(:load!) if !session_data.send(:loaded?)
+            return response if !session_data.send(:updated?)
+          end
 
           sid = options[:id] || generate_sid
 
diff --git a/actionpack/test/activerecord/active_record_store_test.rb b/actionpack/test/activerecord/active_record_store_test.rb
index bde36eb..14c6fc1 100644
--- a/actionpack/test/activerecord/active_record_store_test.rb
+++ b/actionpack/test/activerecord/active_record_store_test.rb
@@ -21,6 +21,11 @@ class ActiveRecordStoreTest < ActionController::IntegrationTest
       render :text => "foo: #{session[:foo].inspect}"
     end
 
+    def set_cookie_and_get_session_value
+      cookies["kittens"] = { :value => "fluffy" }
+      render :text => "foo: #{session[:foo].inspect}"
+    end
+
     def get_session_id
       session[:foo]
       render :text => "#{request.session_options[:id]}"
@@ -73,6 +78,23 @@ class ActiveRecordStoreTest < ActionController::IntegrationTest
     end
   end
 
+  def test_getting_session_value_does_not_set_cookie
+    with_test_route_set do
+      get '/get_session_value'
+      assert_response :success
+      assert_equal "", headers["Set-Cookie"]
+    end
+  end
+
+  def test_getting_session_value_and_setting_a_cookie_doesnt_delete_all_cookies
+    with_test_route_set do
+      get '/set_cookie_and_get_session_value'
+      assert_response :success
+      assert_equal 'foo: nil', response.body
+      assert_equal({"kittens" => "fluffy"}, response.cookies)
+    end
+  end
+
   def test_setting_session_value_after_session_reset
     with_test_route_set do
       get '/set_session_value'
diff --git a/actionpack/test/controller/session/mem_cache_store_test.rb b/actionpack/test/controller/session/mem_cache_store_test.rb
index 2f80a3c..c7988f5 100644
--- a/actionpack/test/controller/session/mem_cache_store_test.rb
+++ b/actionpack/test/controller/session/mem_cache_store_test.rb
@@ -61,6 +61,14 @@ class MemCacheStoreTest < ActionController::IntegrationTest
       end
     end
 
+    def test_getting_session_value_does_not_set_cookie
+      with_test_route_set do
+        get '/get_session_value'
+        assert_response :success
+        assert_equal "", headers["Set-Cookie"]
+      end
+    end
+
     def test_setting_session_value_after_session_reset
       with_test_route_set do
         get '/set_session_value'
-- 
1.6.1