diff --git a/activemodel/lib/active_model/attribute.rb b/activemodel/lib/active_model/attribute.rb index 7facc6cc75656..5bb5e747efdb2 100644 --- a/activemodel/lib/active_model/attribute.rb +++ b/activemodel/lib/active_model/attribute.rb @@ -133,14 +133,13 @@ def encode_with(coder) coder["value"] = value if defined?(@value) end - protected - def original_value_for_database - if assigned? - original_attribute.original_value_for_database - else - _original_value_for_database - end + def original_value_for_database + if assigned? + original_attribute.original_value_for_database + else + _original_value_for_database end + end private attr_reader :original_attribute @@ -165,9 +164,10 @@ def type_cast(value) type.deserialize(value) end - def _original_value_for_database - value_before_type_cast - end + private + def _original_value_for_database + value_before_type_cast + end end class FromUser < Attribute # :nodoc: diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 015562d55c8df..2611b8d72347e 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -89,7 +89,9 @@ def _update_row(attribute_names, attempted_action = "update") begin locking_column = self.class.locking_column - previous_lock_value = attribute_before_type_cast(locking_column) + lock_attribute_was = @attributes[locking_column] + lock_value_for_database = _lock_value_for_database(locking_column) + attribute_names = attribute_names.dup if attribute_names.frozen? attribute_names << locking_column @@ -98,7 +100,7 @@ def _update_row(attribute_names, attempted_action = "update") affected_rows = self.class._update_record( attributes_with_values(attribute_names), @primary_key => id_in_database, - locking_column => previous_lock_value + locking_column => lock_value_for_database ) if affected_rows != 1 @@ -109,7 +111,7 @@ def _update_row(attribute_names, attempted_action = "update") # If something went wrong, revert the locking_column value. rescue Exception - self[locking_column] = previous_lock_value.to_i + @attributes[locking_column] = lock_attribute_was raise end end @@ -121,7 +123,7 @@ def destroy_row affected_rows = self.class._delete_record( @primary_key => id_in_database, - locking_column => attribute_before_type_cast(locking_column) + locking_column => _lock_value_for_database(locking_column) ) if affected_rows != 1 @@ -131,6 +133,14 @@ def destroy_row affected_rows end + def _lock_value_for_database(locking_column) + if will_save_change_to_attribute?(locking_column) + @attributes[locking_column].value_for_database + else + @attributes[locking_column].original_value_for_database + end + end + module ClassMethods DEFAULT_LOCKING_COLUMN = "lock_version" diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 164dfb5a740f1..5de4eb3b952cf 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -340,6 +340,39 @@ def test_lock_without_default_should_work_with_null_in_the_database assert_equal "new title2", t2.title end + def test_update_with_lock_version_without_default_should_work_on_dirty_value_before_type_cast + ActiveRecord::Base.connection.execute("INSERT INTO lock_without_defaults(title) VALUES('title1')") + t1 = LockWithoutDefault.last + + assert_equal 0, t1.lock_version + assert_nil t1.lock_version_before_type_cast + + t1.lock_version = t1.lock_version + + assert_equal 0, t1.lock_version + assert_equal 0, t1.lock_version_before_type_cast + + assert_nothing_raised { t1.update!(title: "new title1") } + assert_equal 1, t1.lock_version + assert_equal "new title1", t1.title + end + + def test_destroy_with_lock_version_without_default_should_work_on_dirty_value_before_type_cast + ActiveRecord::Base.connection.execute("INSERT INTO lock_without_defaults(title) VALUES('title1')") + t1 = LockWithoutDefault.last + + assert_equal 0, t1.lock_version + assert_nil t1.lock_version_before_type_cast + + t1.lock_version = t1.lock_version + + assert_equal 0, t1.lock_version + assert_equal 0, t1.lock_version_before_type_cast + + assert_nothing_raised { t1.destroy! } + assert_predicate t1, :destroyed? + end + def test_lock_without_default_queries_count t1 = LockWithoutDefault.create(title: "title1")