diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index db9c394a652f97c08c304dcc6f3dff895ba24bd1..f847c714204beb888c6da2cddf4b03591f603674 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 fa9e789b2421b81922d197de584a3437d6760fe7..d39b7181f41dd56d3da84f1f0144a2db0f3c00bc 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 09bd00291d0ab1fef9332cca807905be3e4891fb..3bd8475bb039e656144a936249a930a22d5ee397 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 8b6a969c9e12d736ce95dc4003ddd94b152791b0..0579df0a07d9285ef705784326c7e75b42aaf0d4 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