Skip to content

Commit

Permalink
only call callbacks after transaction is committed
Browse files Browse the repository at this point in the history
  • Loading branch information
bdurand committed Dec 5, 2024
1 parent 191385b commit b77fdaa
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 10 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## 2.2.0

### Changed

- After save callbacks are now called only after the transaction is committed and settings have been persisted to the data store. When updating multiple records the callbacks will be called after all changes have been persisted rather than immediatly after calling `save!` on each record.

## 2.1.2

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2.1.2
2.2.0
46 changes: 37 additions & 9 deletions lib/super_settings/setting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def bulk_update(params, changed_by = nil)
all_valid, settings = update_settings(params, changed_by)
if all_valid
storage.with_connection do
storage.transaction do
transaction do |_changes|
settings.each do |setting|
setting.save!
end
Expand Down Expand Up @@ -202,6 +202,38 @@ def clear_last_updated_cache
cache&.delete(Setting::LAST_UPDATED_CACHE_KEY)
end

# Wrap a block of code in a transaction.
#
# @api private
def transaction(&block)
changes = Thread.current[:super_settings_transaction]
return yield if changes

changes = []
Thread.current[:super_settings_transaction] = changes

begin
@storage.transaction(&block)

clear_last_updated_cache

changes.each do |setting|
setting.send(:call_after_save_callbacks)
setting.send(:clear_changes)
end
ensure
Thread.current[:super_settings_transaction] = nil
end
end

# Add a record to the current transaction.
#
# @api private
def add_record_to_transaction(record)
changes = Thread.current[:super_settings_transaction]
changes << record if changes
end

private

# Updates settings in memory from an array of parameters.
Expand Down Expand Up @@ -454,17 +486,13 @@ def save!
self.created_at ||= timestamp
self.updated_at = timestamp if updated_at.nil? || !changed?(:updated_at)

return if @changes.empty?

self.class.storage.with_connection do
self.class.storage.transaction do
self.class.transaction do
record_value_change
@record.save!
end

begin
self.class.clear_last_updated_cache
call_after_save_callbacks
ensure
clear_changes
self.class.add_record_to_transaction(self)
end
end
nil
Expand Down
8 changes: 8 additions & 0 deletions spec/super_settings/setting_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,14 @@
setting.save!
expect(FakeLogger.instance.messages).to include({key: [nil, "foo"], value: [nil, "bar"]})
end

it "should not call the after_save hooks if the record could not be saved" do
FakeLogger.instance.messages.clear
setting = SuperSettings::Setting.new(key: "foo", value: "bar")
setting.key = nil
expect { setting.save! }.to raise_error(SuperSettings::Setting::InvalidRecordError)
expect(FakeLogger.instance.messages).to eq []
end
end

describe "changes" do
Expand Down

0 comments on commit b77fdaa

Please sign in to comment.