From d24dc88dad0577f86efbd91cb9295f957f150b2d Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Fri, 8 Jan 2021 18:06:05 +0900 Subject: [PATCH] Revert "Fix update with dirty locking column to not match latest object accidentally" --- activemodel/lib/active_model/attribute.rb | 14 +++++----- .../lib/active_record/locking/optimistic.rb | 2 +- activerecord/test/cases/locking_test.rb | 27 ++----------------- 3 files changed, 10 insertions(+), 33 deletions(-) diff --git a/activemodel/lib/active_model/attribute.rb b/activemodel/lib/active_model/attribute.rb index 613d188c37..7facc6cc75 100644 --- a/activemodel/lib/active_model/attribute.rb +++ b/activemodel/lib/active_model/attribute.rb @@ -133,13 +133,14 @@ module ActiveModel coder["value"] = value if defined?(@value) end - def original_value_for_database - if assigned? - original_attribute.original_value_for_database - else - _original_value_for_database + protected + def original_value_for_database + if assigned? + original_attribute.original_value_for_database + else + _original_value_for_database + end end - end private attr_reader :original_attribute @@ -167,7 +168,6 @@ module ActiveModel def _original_value_for_database value_before_type_cast end - private :_original_value_for_database end class FromUser < Attribute # :nodoc: diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 48eb3222f8..015562d55c 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -98,7 +98,7 @@ module ActiveRecord affected_rows = self.class._update_record( attributes_with_values(attribute_names), @primary_key => id_in_database, - locking_column => @attributes[locking_column].original_value_for_database + locking_column => previous_lock_value ) if affected_rows != 1 diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index ee55d5200b..164dfb5a74 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -238,37 +238,14 @@ 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 = person2.lock_version + person.lock_version = 42 - changes = { - "first_name" => ["Michael", "Douglas Adams"], - "lock_version" => [0, 43], - } - assert_equal changes, person.changes + assert_predicate person, :lock_version_changed? person.save end