Merge pull request #39321 from kamipo/fix_update_with_dirty_locking_column

Fix update with dirty locking column to not match latest object accidentally

activemodel/lib/active_model/attribute.rb

@@ -132,14 +132,13 @@ module ActiveModel
       coder["value"] = value if defined?(@value)
     end
 
-    protected
+    def original_value_for_database
-      def original_value_for_database
+      if assigned?
-        if assigned?
+        original_attribute.original_value_for_database
-          original_attribute.original_value_for_database
+      else
-        else
+        _original_value_for_database
-          _original_value_for_database
-        end
       end
+    end
 
     private
       attr_reader :original_attribute
@@ -167,6 +166,7 @@ module ActiveModel
         def _original_value_for_database
           value_before_type_cast
         end
+        private :_original_value_for_database
       end
 
       class FromUser < Attribute # :nodoc:

activerecord/lib/active_record/locking/optimistic.rb

@@ -88,7 +88,7 @@ module ActiveRecord
             affected_rows = self.class._update_record(
               attributes_with_values(attribute_names),
               @primary_key => id_in_database,
-              locking_column => previous_lock_value
+              locking_column => @attributes[locking_column].original_value_for_database
             )
 
             if affected_rows != 1

activerecord/test/cases/locking_test.rb

@@ -238,14 +238,37 @@ class OptimisticLockingTest < ActiveRecord::TestCase
     end
   end
 
+  def test_update_with_dirty_locking_column
+    person = Person.find(1)
+    person.first_name = "Douglas Adams"
+    person.lock_version = 42
+
+    changes = {
+      "first_name" => ["Michael", "Douglas Adams"],
+      "lock_version" => [0, 42],
+    }
+    assert_equal changes, person.changes
+
+    assert person.save!
+    assert_empty person.changes
+  end
+
   def test_explicit_update_lock_column_raise_error
     person = Person.find(1)
 
+    person2 = Person.find(1)
+    person2.lock_version = 42
+    person2.save!
+
     assert_raises(ActiveRecord::StaleObjectError) do
       person.first_name = "Douglas Adams"
-      person.lock_version = 42
+      person.lock_version = person2.lock_version
 
-      assert_predicate person, :lock_version_changed?
+      changes = {
+        "first_name" => ["Michael", "Douglas Adams"],
+        "lock_version" => [0, 43],
+      }
+      assert_equal changes, person.changes
 
       person.save
     end