Skip to content

Commit

Permalink
require new transaction for add/set as well
Browse files Browse the repository at this point in the history
  • Loading branch information
jnunemaker committed Sep 3, 2024
1 parent 1c9e0a2 commit 0a38a46
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 9 deletions.
17 changes: 9 additions & 8 deletions lib/flipper/adapters/active_record.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,15 @@ def features
# Public: Adds a feature to the set of known features.
def add(feature)
with_connection(@feature_class) do
# race condition, but add is only used by enable/disable which happen
# super rarely, so it shouldn't matter in practice
@feature_class.transaction do
unless @feature_class.where(key: feature.key).exists?
begin
@feature_class.transaction(requires_new: true) do
begin
# race condition, but add is only used by enable/disable which happen
# super rarely, so it shouldn't matter in practice
unless @feature_class.where(key: feature.key).exists?
@feature_class.create!(key: feature.key)
rescue ::ActiveRecord::RecordNotUnique
end
rescue ::ActiveRecord::RecordNotUnique
# already added
end
end
end
Expand Down Expand Up @@ -221,7 +222,7 @@ def set(feature, gate, thing, options = {})
raise VALUE_TO_TEXT_WARNING if json_feature && value_not_text?

with_connection(@gate_class) do
@gate_class.transaction do
@gate_class.transaction(requires_new: true) do
clear(feature) if clear_feature
delete(feature, gate)
begin
Expand Down Expand Up @@ -255,7 +256,7 @@ def enable_multi(feature, gate, thing)
end
end
rescue ::ActiveRecord::RecordNotUnique
# already added so no need move on with life
# already added so move on with life
end
end

Expand Down
4 changes: 3 additions & 1 deletion spec/flipper/adapters/active_record_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,9 @@

ActiveRecord::Base.transaction do
flipper.enable_actor(:foo, actor)
expect(Flipper::Adapters::ActiveRecord::Gate.count).to eq 1
# any read on the next line is fine, just need to ensure that
# poisoned transaction isn't raised
expect(Flipper::Adapters::ActiveRecord::Gate.count).to eq(1)
end
end

Expand Down

0 comments on commit 0a38a46

Please sign in to comment.