Update Session to utilize indiffernt access

8 files changed, 127 insertions, 16 deletions

diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb
index b43bb9dc17..4702f22810 100644
--- a/actionpack/lib/action_controller/test_case.rb
+++ b/actionpack/lib/action_controller/test_case.rb
@@ -165,7 +165,7 @@ module ActionController
     def initialize(session = {})
       super(nil, nil)
       @id = SecureRandom.hex(16)
-      @data = stringify_keys(session)
+      @data = session.with_indifferent_access
       @loaded = true
     end
 
diff --git a/actionpack/lib/action_dispatch/request/session.rb b/actionpack/lib/action_dispatch/request/session.rb
index f6428af636..38d0da3e67 100644
--- a/actionpack/lib/action_dispatch/request/session.rb
+++ b/actionpack/lib/action_dispatch/request/session.rb
@@ -88,13 +88,13 @@ module ActionDispatch
       # nil if the given key is not found in the session.
       def [](key)
         load_for_read!
-        @delegate[key.to_s]
+        @delegate[key]
       end
 
       # Returns true if the session has the given key or false.
       def has_key?(key)
         load_for_read!
-        @delegate.key?(key.to_s)
+        @delegate.key?(key)
       end
       alias :key? :has_key?
       alias :include? :has_key?
@@ -112,7 +112,7 @@ module ActionDispatch
       # Writes given value to given key of the session.
       def []=(key, value)
         load_for_write!
-        @delegate[key.to_s] = value
+        @delegate[key] = value
       end
 
       # Clears the session.
@@ -139,13 +139,13 @@ module ActionDispatch
       #   # => {"session_id"=>"e29b9ea315edf98aad94cc78c34cc9b2", "foo" => "bar"}
       def update(hash)
         load_for_write!
-        @delegate.update stringify_keys(hash)
+        @delegate.update hash
       end
 
       # Deletes given key from the session.
       def delete(key)
         load_for_write!
-        @delegate.delete key.to_s
+        @delegate.delete key
       end
 
       # Returns value of the given key from the session, or raises +KeyError+
@@ -165,9 +165,9 @@ module ActionDispatch
       def fetch(key, default=Unspecified, &block)
         load_for_read!
         if default == Unspecified
-          @delegate.fetch(key.to_s, &block)
+          @delegate.fetch(key, &block)
         else
-          @delegate.fetch(key.to_s, default, &block)
+          @delegate.fetch(key, default, &block)
         end
       end
 
@@ -211,15 +211,9 @@ module ActionDispatch
       def load!
         id, session = @by.load_session @req
         options[:id] = id
-        @delegate.replace(stringify_keys(session))
+        @delegate.replace(session)
         @loaded = true
       end
-
-      def stringify_keys(other)
-        other.each_with_object({}) { |(key, value), hash|
-          hash[key.to_s] = value
-        }
-      end
     end
   end
 end
diff --git a/actionpack/test/dispatch/request/session_test.rb b/actionpack/test/dispatch/request/session_test.rb
index 3fc4ffd71c..3433d82791 100644
--- a/actionpack/test/dispatch/request/session_test.rb
+++ b/actionpack/test/dispatch/request/session_test.rb
@@ -105,7 +105,7 @@ module ActionDispatch
         end
       end
 
-      def test_with_indifferent_access
+      def test_indifferent_access
         s = Session.create(store, req, {})
 
         s[:one] = { test: "deep" }
diff --git a/actionpack/test/dispatch/session/abstract_store_test.rb b/actionpack/test/dispatch/session/abstract_store_test.rb
index d38d1bbce6..c9ce5cad42 100644
--- a/actionpack/test/dispatch/session/abstract_store_test.rb
+++ b/actionpack/test/dispatch/session/abstract_store_test.rb
@@ -46,6 +46,22 @@ module ActionDispatch
         assert_equal session.to_hash, session1.to_hash
       end
 
+      def test_previous_session_has_indifferent_access
+        env = {}
+        as = MemoryStore.new app
+        as.call(env)
+
+        assert @env
+        session = Request::Session.find ActionDispatch::Request.new @env
+        session[:foo] = { bar: "baz" }
+
+        as.call(@env)
+        session = Request::Session.find ActionDispatch::Request.new @env
+
+        assert_equal session[:foo][:bar], "baz"
+        assert_equal session[:foo]["bar"], "baz"
+      end
+
       private
       def app(&block)
         @env = nil
diff --git a/actionpack/test/dispatch/session/cache_store_test.rb b/actionpack/test/dispatch/session/cache_store_test.rb
index dbb996973d..b911392cf1 100644
--- a/actionpack/test/dispatch/session/cache_store_test.rb
+++ b/actionpack/test/dispatch/session/cache_store_test.rb
@@ -12,6 +12,11 @@ class CacheStoreTest < ActionDispatch::IntegrationTest
       head :ok
     end
 
+    def set_deep_session_value
+      session[:foo] = { bar: "baz" }
+      head :ok
+    end
+
     def set_serialized_session_value
       session[:foo] = SessionAutoloadTest::Foo.new
       head :ok
@@ -21,6 +26,14 @@ class CacheStoreTest < ActionDispatch::IntegrationTest
       render plain: "foo: #{session[:foo].inspect}"
     end
 
+    def get_deep_session_value_with_symbol
+      render plain: "foo: { bar: #{session[:foo][:bar].inspect} }"
+    end
+
+    def get_deep_session_value_with_string
+      render plain: "foo: { \"bar\" => #{session[:foo]["bar"].inspect} }"
+    end
+
     def get_session_id
       render plain: "#{request.session.id}"
     end
@@ -160,6 +173,22 @@ class CacheStoreTest < ActionDispatch::IntegrationTest
     end
   end
 
+  def test_previous_session_has_indifferent_access
+    with_test_route_set do
+      get '/set_deep_session_value'
+      assert_response :success
+      assert cookies['_session_id']
+
+      get '/get_deep_session_value_with_symbol'
+      assert_response :success
+      assert_equal 'foo: { bar: "baz" }', response.body
+
+      get '/get_deep_session_value_with_string'
+      assert_response :success
+      assert_equal 'foo: { "bar" => "baz" }', response.body
+    end
+  end
+
   private
     def with_test_route_set
       with_routing do |set|
diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb
index f07e215e3a..71402b021a 100644
--- a/actionpack/test/dispatch/session/cookie_store_test.rb
+++ b/actionpack/test/dispatch/session/cookie_store_test.rb
@@ -24,10 +24,23 @@ class CookieStoreTest < ActionDispatch::IntegrationTest
       render plain: Rack::Utils.escape(Verifier.generate(session.to_hash))
     end
 
+    def set_deep_session_value
+      session[:foo] = { bar: "baz" }
+      render plain: Rack::Utils.escape(Verifier.generate(session.to_hash))
+    end
+
     def get_session_value
       render plain: "foo: #{session[:foo].inspect}"
     end
 
+    def get_deep_session_value_with_symbol
+      render plain: "foo: { bar: #{session[:foo][:bar].inspect} }"
+    end
+
+    def get_deep_session_value_with_string
+      render plain: "foo: { \"bar\" => #{session[:foo]["bar"].inspect} }"
+    end
+
     def get_session_id
       render plain: "id: #{request.session.id}"
     end
@@ -81,6 +94,15 @@ class CookieStoreTest < ActionDispatch::IntegrationTest
     end
   end
 
+  def test_session_indifferent_access
+    with_test_route_set do
+      cookies[SessionKey] = SignedBar
+      get '/get_session_value'
+      assert_response :success
+      assert_equal 'foo: "bar"', response.body
+    end
+  end
+
   def test_getting_session_id
     with_test_route_set do
       cookies[SessionKey] = SignedBar
@@ -332,6 +354,18 @@ class CookieStoreTest < ActionDispatch::IntegrationTest
     end
   end
 
+  def test_previous_session_has_indifferent_access
+    with_test_route_set do
+      get '/set_deep_session_value'
+
+      get '/get_deep_session_value_with_symbol'
+      assert_equal 'foo: { bar: "baz" }', response.body
+
+      get '/get_deep_session_value_with_string'
+      assert_equal 'foo: { "bar" => "baz" }', response.body
+    end
+  end
+
   private
 
     # Overwrite get to send SessionSecret in env hash
diff --git a/actionpack/test/dispatch/session/mem_cache_store_test.rb b/actionpack/test/dispatch/session/mem_cache_store_test.rb
index 3fed9bad4f..2e6b42856f 100644
--- a/actionpack/test/dispatch/session/mem_cache_store_test.rb
+++ b/actionpack/test/dispatch/session/mem_cache_store_test.rb
@@ -13,6 +13,11 @@ class MemCacheStoreTest < ActionDispatch::IntegrationTest
       head :ok
     end
 
+    def set_deep_session_value
+      session[:foo] = { bar: "baz" }
+      head :ok
+    end
+
     def set_serialized_session_value
       session[:foo] = SessionAutoloadTest::Foo.new
       head :ok
@@ -22,6 +27,14 @@ class MemCacheStoreTest < ActionDispatch::IntegrationTest
       render plain: "foo: #{session[:foo].inspect}"
     end
 
+    def get_deep_session_value_with_symbol
+      render plain: "foo: { bar: #{session[:foo][:bar].inspect} }"
+    end
+
+    def get_deep_session_value_with_string
+      render plain: "foo: { \"bar\" => #{session[:foo]["bar"].inspect} }"
+    end
+
     def get_session_id
       render plain: "#{request.session.id}"
     end
@@ -179,6 +192,24 @@ class MemCacheStoreTest < ActionDispatch::IntegrationTest
     rescue Dalli::RingError => ex
       skip ex.message, ex.backtrace
     end
+
+    def test_previous_session_has_indifferent_access
+      with_test_route_set do
+        get '/set_deep_session_value'
+        assert_response :success
+        assert cookies['_session_id']
+
+        get '/get_deep_session_value_with_symbol'
+        assert_response :success
+        assert_equal 'foo: { bar: "baz" }', response.body
+
+        get '/get_deep_session_value_with_string'
+        assert_response :success
+        assert_equal 'foo: { "bar" => "baz" }', response.body
+      end
+    rescue Dalli::RingError => ex
+      skip ex.message, ex.backtrace
+    end
   rescue LoadError, RuntimeError, Dalli::DalliError
     $stderr.puts "Skipping MemCacheStoreTest tests. Start memcached and try again."
   end
diff --git a/actionpack/test/dispatch/session/test_session_test.rb b/actionpack/test/dispatch/session/test_session_test.rb
index 3e61d123e3..332c2ae3c8 100644
--- a/actionpack/test/dispatch/session/test_session_test.rb
+++ b/actionpack/test/dispatch/session/test_session_test.rb
@@ -60,4 +60,11 @@ class ActionController::TestSessionTest < ActiveSupport::TestCase
     session = ActionController::TestSession.new(one: '1')
     assert_equal(2, session.fetch('2') { |key| key.to_i })
   end
+
+  def test_fetch_returns_indifferent_access
+    session = ActionController::TestSession.new(three: { two: '1' })
+    three = session.fetch(:three)
+    assert_equal('1', three[:two])
+    assert_equal('1', three["two"])
+  end
 end