Fix in-place session data mutations being silently dropped

lib/active_record/session_store/session.rb

@@ -39,11 +39,16 @@ def initialize(*)
 
       # Lazy-deserialize session state.
       def data
-        @data ||= self.class.deserialize(read_attribute(@@data_column_name)) || {}
+        unless @data
+          @data = self.class.deserialize(read_attribute(@@data_column_name)) || {}
+          @data_snapshot = Marshal.load(Marshal.dump(@data))
+        end
+        @data
       end
 
       def data=(data)
-        attribute_will_change!(@@data_column_name) if data != self.data
+        original = @data_snapshot || self.data
+        attribute_will_change!(@@data_column_name) if data != original
         @data = data
       end
 

lib/active_record/session_store/sql_bypass.rb

@@ -94,6 +94,7 @@ def data
         unless @data
           if @serialized_data
             @data, @serialized_data = self.class.deserialize(@serialized_data) || {}, nil
+            @data_snapshot = Marshal.load(Marshal.dump(@data))
           else
             @data = {}
           end
@@ -106,7 +107,8 @@ def loaded?
       end
 
       def data=(data)
-        @data_changed = true if data != self.data
+        original = @data_snapshot || self.data
+        @data_changed = true if data != original
         @data = data
       end
 

test/session_test.rb

@@ -111,6 +111,17 @@ def test_loaded?
         assert !s.loaded?, 'session is not loaded'
       end
 
+      def test_in_place_mutation_is_tracked
+        Session.create_table!
+        session_klass.create!(:data => {"key" => ["a"]}, :session_id => '42')
+        t = session_klass.find_by_session_id('42')
+        assert !t.changed?, 'freshly loaded session is unchanged'
+        data = t.data
+        data["key"] << "b"
+        t.data = data
+        assert t.changed?, 'in-place mutation must be detected as a change'
+      end
+
       def test_data_changes_are_tracked
         Session.create_table!
         session_klass.create!(:data => 'world', :session_id => '10')

test/sql_bypass_test.rb

@@ -28,6 +28,18 @@ def test_persisted?
         assert !s.persisted?, 'this is a new record!'
       end
 
+      def test_in_place_mutation_is_tracked
+        SqlBypass.create_table! unless Session.table_exists?
+        s = SqlBypass.new :data => {"key" => ["a"]}, :session_id => 50
+        s.save
+        t = SqlBypass.find_by_session_id 50
+        assert !t.changed?, 'freshly loaded session is unchanged'
+        data = t.data
+        data["key"] << "b"
+        t.data = data
+        assert t.changed?, 'in-place mutation must be detected as a change'
+      end
+
       def test_changed?
         SqlBypass.create_table! unless Session.table_exists?
         session_id = 20