From 4d1264cb5afb93dba198dc559c22a88521fc45c4 Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Fri, 28 Apr 2017 00:23:50 +0300 Subject: [PATCH] Fix ActiveRecord::Persistence#touch with locking `ActiveRecord::Persistence#touch` does not work well when optimistic locking enabled and `locking_column`, without default value, is null in the database. --- activerecord/CHANGELOG.md | 5 ++++ activerecord/lib/active_record/persistence.rb | 2 +- activerecord/test/cases/locking_test.rb | 27 +++++++++++++++++-- activerecord/test/schema/schema.rb | 2 ++ 4 files changed, 33 insertions(+), 3 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index f361e44fcd..c4ee75c9a2 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,8 @@ +* `ActiveRecord::Persistence#touch` does not work well when optimistic locking enabled and + `locking_column`, without default value, is null in the database. + + *bogdanvlviv* + * Fix destroying existing object does not work well when optimistic locking enabled and `locking column` is null in the database. diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index f652c7c3a1..b2dba5516e 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -526,7 +526,7 @@ module ActiveRecord if locking_enabled? locking_column = self.class.locking_column - scope = scope.where(locking_column => _read_attribute(locking_column)) + scope = scope.where(locking_column => read_attribute_before_type_cast(locking_column)) changes[locking_column] = increment_lock end diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 703487db35..2fc52393f2 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -244,10 +244,33 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_equal 0, t1.lock_version_before_type_cast end + def test_touch_existing_lock_without_default_should_work_with_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.touch + + assert_equal 1, t1.lock_version + end + + def test_touch_stale_object_with_lock_without_default + t1 = LockWithoutDefault.create!(title: "title1") + stale_object = LockWithoutDefault.find(t1.id) + + t1.update!(title: "title2") + + assert_raises(ActiveRecord::StaleObjectError) do + stale_object.touch + end + end + def test_lock_without_default_should_work_with_null_in_the_database ActiveRecord::Base.connection.execute("INSERT INTO lock_without_defaults(title) VALUES('title1')") t1 = LockWithoutDefault.last - t2 = LockWithoutDefault.last + t2 = LockWithoutDefault.find(t1.id) assert_equal 0, t1.lock_version assert_nil t1.lock_version_before_type_cast @@ -304,7 +327,7 @@ class OptimisticLockingTest < ActiveRecord::TestCase ActiveRecord::Base.connection.execute("INSERT INTO lock_without_defaults_cust(title) VALUES('title1')") t1 = LockWithCustomColumnWithoutDefault.last - t2 = LockWithCustomColumnWithoutDefault.last + t2 = LockWithCustomColumnWithoutDefault.find(t1.id) assert_equal 0, t1.custom_lock_version assert_nil t1.custom_lock_version_before_type_cast diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index dc8bdb98ff..6da0ef26f6 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -453,11 +453,13 @@ ActiveRecord::Schema.define do create_table :lock_without_defaults, force: true do |t| t.column :title, :string t.column :lock_version, :integer + t.timestamps null: true end create_table :lock_without_defaults_cust, force: true do |t| t.column :title, :string t.column :custom_lock_version, :integer + t.timestamps null: true end create_table :magazines, force: true do |t|