From a60a20bb7faae4d0758676b865f625275c500e4e Mon Sep 17 00:00:00 2001 From: bogdanvlviv Date: Mon, 22 Aug 2016 21:16:08 +0300 Subject: [PATCH] Added ability update locking_column value --- activerecord/CHANGELOG.md | 5 ++ .../lib/active_record/locking/optimistic.rb | 14 ++-- activerecord/test/cases/dirty_test.rb | 2 +- activerecord/test/cases/locking_test.rb | 76 +++++++++++++++++++ 4 files changed, 90 insertions(+), 7 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index db9c394a65..f847c71420 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,8 @@ +* Optimistic locking: Added ability update locking_column value. + Ignore optimistic locking if update with new locking_column value. + + *bogdanvlviv* + * Fixed: Optimistic locking does not work well with null in the database. Fixes #26024 diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index fa9e789b24..d39b7181f4 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -78,17 +78,19 @@ def _create_record(attribute_names = self.attribute_names, *) # :nodoc: def _update_record(attribute_names = self.attribute_names) #:nodoc: return super unless locking_enabled? - return 0 if attribute_names.empty? lock_col = self.class.locking_column - previous_lock_value = read_attribute_before_type_cast(lock_col) - - increment_lock - attribute_names += [lock_col] - attribute_names.uniq! + return super if attribute_names.include?(lock_col) + return 0 if attribute_names.empty? begin + previous_lock_value = read_attribute_before_type_cast(lock_col) + + increment_lock + + attribute_names.push(lock_col) + relation = self.class.unscoped affected_rows = relation.where( diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index 09bd00291d..3bd8475bb0 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -341,7 +341,7 @@ def test_partial_update def test_partial_update_with_optimistic_locking person = Person.new(first_name: "foo") - old_lock_version = 1 + old_lock_version = person.lock_version with_partial_writes Person, false do assert_queries(2) { 2.times { person.save! } } diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 8b6a969c9e..0579df0a07 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -247,6 +247,44 @@ def test_lock_without_default_should_work_with_null_in_the_database assert_equal "new title2", t2.title end + def test_lock_without_default_should_update_with_lock_col + t1 = LockWithoutDefault.create(title: "title1", lock_version: 6) + + assert_equal 6, t1.lock_version + + t1.update(lock_version: 0) + t1.reload + + assert_equal 0, t1.lock_version + end + + def test_lock_without_default_queries_count + t1 = LockWithoutDefault.create(title: "title1") + + assert_equal "title1", t1.title + assert_equal 0, t1.lock_version + + assert_queries(1) { t1.update(title: "title2") } + + t1.reload + assert_equal "title2", t1.title + assert_equal 1, t1.lock_version + + assert_queries(1) { t1.update(title: "title3", lock_version: 6) } + + t1.reload + assert_equal "title3", t1.title + assert_equal 6, t1.lock_version + + t2 = LockWithoutDefault.new(title: "title1") + + assert_queries(1) { t2.save! } + + t2.reload + assert_equal "title1", t2.title + assert_equal 0, t2.lock_version + end + def test_lock_with_custom_column_without_default_sets_version_to_zero t1 = LockWithCustomColumnWithoutDefault.new @@ -283,6 +321,44 @@ def test_lock_with_custom_column_without_default_should_work_with_null_in_the_da assert_equal "new title2", t2.title end + def test_lock_with_custom_column_without_default_should_update_with_lock_col + t1 = LockWithCustomColumnWithoutDefault.create(title: "title1", custom_lock_version: 6) + + assert_equal 6, t1.custom_lock_version + + t1.update(custom_lock_version: 0) + t1.reload + + assert_equal 0, t1.custom_lock_version + end + + def test_lock_with_custom_column_without_default_queries_count + t1 = LockWithCustomColumnWithoutDefault.create(title: "title1") + + assert_equal "title1", t1.title + assert_equal 0, t1.custom_lock_version + + assert_queries(1) { t1.update(title: "title2") } + + t1.reload + assert_equal "title2", t1.title + assert_equal 1, t1.custom_lock_version + + assert_queries(1) { t1.update(title: "title3", custom_lock_version: 6) } + + t1.reload + assert_equal "title3", t1.title + assert_equal 6, t1.custom_lock_version + + t2 = LockWithCustomColumnWithoutDefault.new(title: "title1") + + assert_queries(1) { t2.save! } + + t2.reload + assert_equal "title1", t2.title + assert_equal 0, t2.custom_lock_version + end + def test_readonly_attributes assert_equal Set.new([ "name" ]), ReadonlyNameShip.readonly_attributes -- GitLab