From f08bc757ebe108df46d76d6fd0029546539f817f Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Sat, 29 Apr 2017 08:21:05 +0000 Subject: [PATCH] Fix destroy with locking_column value null Fix destroying existing object does not work well when optimistic locking enabled and `locking column` is null in the database. Follow 22a822e5813ef7ea9ab6dbbb670a363899a083af, #28914 --- activerecord/CHANGELOG.md | 5 ++++ .../lib/active_record/locking/optimistic.rb | 2 +- activerecord/test/cases/locking_test.rb | 25 +++++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c65deffa8b..f361e44fcd 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,8 @@ +* Fix destroying existing object does not work well when optimistic locking enabled and + `locking column` is null in the database. + + *bogdanvlviv* + * Use bulk INSERT to insert fixtures for better performance. *Kir Shatrov* diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index f8fcb7ccaa..522da6a571 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -128,7 +128,7 @@ module ActiveRecord if locking_enabled? locking_column = self.class.locking_column - relation = relation.where(locking_column => _read_attribute(locking_column)) + relation = relation.where(locking_column => read_attribute_before_type_cast(locking_column)) end relation diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 2b139a5da4..703487db35 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -453,6 +453,31 @@ class OptimisticLockingWithSchemaChangeTest < ActiveRecord::TestCase PersonalLegacyThing.reset_column_information end + def test_destroy_existing_object_with_locking_column_value_null_in_the_database + 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.destroy + + assert t1.destroyed? + end + + def test_destroy_stale_object + t1 = LockWithoutDefault.create!(title: "title1") + stale_object = LockWithoutDefault.find(t1.id) + + t1.update!(title: "title2") + + assert_raises(ActiveRecord::StaleObjectError) do + stale_object.destroy! + end + + refute stale_object.destroyed? + end + private def add_counter_column_to(model, col = "test_count")