Fix that after commit callbacks on update does not triggered when optimistic locking is enabled
This issue is caused by `@_trigger_update_callback` won't be updated due to `_update_record` in `Locking::Optimistic` doesn't call `super` when optimistic locking is enabled. Now optimistic locking concern when updating is supported by `_update_row` low level API, therefore overriding `_update_record` is no longer necessary. Removing the method just fix the issue. Closes #29096. Closes #29321. Closes #30823.
This commit is contained in:
parent
ccac681122
commit
263f01d93d
@ -61,13 +61,6 @@ def locking_enabled? #:nodoc:
|
||||
end
|
||||
|
||||
private
|
||||
|
||||
def increment_lock
|
||||
lock_col = self.class.locking_column
|
||||
previous_lock_value = send(lock_col)
|
||||
send("#{lock_col}=", previous_lock_value + 1)
|
||||
end
|
||||
|
||||
def _create_record(attribute_names = self.attribute_names, *)
|
||||
if locking_enabled?
|
||||
# We always want to persist the locking version, even if we don't detect
|
||||
@ -77,41 +70,13 @@ def _create_record(attribute_names = self.attribute_names, *)
|
||||
super
|
||||
end
|
||||
|
||||
def _update_record(attribute_names = self.attribute_names)
|
||||
attribute_names &= self.class.column_names
|
||||
return super unless locking_enabled?
|
||||
return 0 if attribute_names.empty?
|
||||
|
||||
begin
|
||||
lock_col = self.class.locking_column
|
||||
|
||||
previous_lock_value = read_attribute_before_type_cast(lock_col)
|
||||
|
||||
increment_lock
|
||||
|
||||
attribute_names.push(lock_col)
|
||||
|
||||
affected_rows = self.class._update_record(
|
||||
attributes_with_values_for_update(attribute_names),
|
||||
self.class.primary_key => id_in_database,
|
||||
lock_col => previous_lock_value
|
||||
)
|
||||
|
||||
unless affected_rows == 1
|
||||
raise ActiveRecord::StaleObjectError.new(self, "update")
|
||||
end
|
||||
|
||||
affected_rows
|
||||
|
||||
# If something went wrong, revert the locking_column value.
|
||||
rescue Exception
|
||||
send("#{lock_col}=", previous_lock_value.to_i)
|
||||
|
||||
raise
|
||||
end
|
||||
def _touch_row(attribute_names, time)
|
||||
super
|
||||
ensure
|
||||
clear_attribute_change(self.class.locking_column) if locking_enabled?
|
||||
end
|
||||
|
||||
def _update_row(attribute_names, attempted_action)
|
||||
def _update_row(attribute_names, attempted_action = "update")
|
||||
return super unless locking_enabled?
|
||||
|
||||
begin
|
||||
@ -135,10 +100,8 @@ def _update_row(attribute_names, attempted_action)
|
||||
|
||||
# If something went wrong, revert the locking_column value.
|
||||
rescue Exception
|
||||
self[locking_column] = previous_lock_value
|
||||
self[locking_column] = previous_lock_value.to_i
|
||||
raise
|
||||
ensure
|
||||
clear_attribute_change(locking_column)
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -656,18 +656,11 @@ def touch(*names, time: nil)
|
||||
MSG
|
||||
end
|
||||
|
||||
time ||= current_time_from_proper_timezone
|
||||
attribute_names = timestamp_attributes_for_update_in_model
|
||||
attribute_names.concat(names)
|
||||
|
||||
unless attribute_names.empty?
|
||||
attribute_names.each do |attr_name|
|
||||
write_attribute(attr_name, time)
|
||||
clear_attribute_change(attr_name)
|
||||
end
|
||||
|
||||
affected_rows = _update_row(attribute_names, "touch")
|
||||
|
||||
affected_rows = _touch_row(attribute_names, time)
|
||||
@_trigger_update_callback = affected_rows == 1
|
||||
else
|
||||
true
|
||||
@ -688,7 +681,18 @@ def _delete_row
|
||||
self.class._delete_record(self.class.primary_key => id_in_database)
|
||||
end
|
||||
|
||||
def _update_row(attribute_names, attempted_action)
|
||||
def _touch_row(attribute_names, time)
|
||||
time ||= current_time_from_proper_timezone
|
||||
|
||||
attribute_names.each do |attr_name|
|
||||
write_attribute(attr_name, time)
|
||||
clear_attribute_change(attr_name)
|
||||
end
|
||||
|
||||
_update_row(attribute_names, "touch")
|
||||
end
|
||||
|
||||
def _update_row(attribute_names, attempted_action = "update")
|
||||
self.class._update_record(
|
||||
attributes_with_values(attribute_names),
|
||||
self.class.primary_key => id_in_database
|
||||
@ -712,7 +716,7 @@ def _update_record(attribute_names = self.attribute_names)
|
||||
affected_rows = 0
|
||||
@_trigger_update_callback = true
|
||||
else
|
||||
affected_rows = _update_row(attribute_names, "update")
|
||||
affected_rows = _update_row(attribute_names)
|
||||
@_trigger_update_callback = affected_rows == 1
|
||||
end
|
||||
|
||||
|
@ -394,6 +394,28 @@ def add_transaction_execution_blocks(record)
|
||||
end
|
||||
end
|
||||
|
||||
class TransactionAfterCommitCallbacksWithOptimisticLockingTest < ActiveRecord::TestCase
|
||||
class PersonWithCallbacks < ActiveRecord::Base
|
||||
self.table_name = :people
|
||||
|
||||
after_create_commit { |record| record.history << :commit_on_create }
|
||||
after_update_commit { |record| record.history << :commit_on_update }
|
||||
after_destroy_commit { |record| record.history << :commit_on_destroy }
|
||||
|
||||
def history
|
||||
@history ||= []
|
||||
end
|
||||
end
|
||||
|
||||
def test_after_commit_callbacks_with_optimistic_locking
|
||||
person = PersonWithCallbacks.create!(first_name: "first name")
|
||||
person.update!(first_name: "another name")
|
||||
person.destroy
|
||||
|
||||
assert_equal [:commit_on_create, :commit_on_update, :commit_on_destroy], person.history
|
||||
end
|
||||
end
|
||||
|
||||
class CallbacksOnMultipleActionsTest < ActiveRecord::TestCase
|
||||
self.use_transactional_tests = false
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user