From 0a38a46fe0634d0b16a0ca467dc84a1dfcb0d02c Mon Sep 17 00:00:00 2001
From: John Nunemaker <nunemaker@gmail.com>
Date: Tue, 3 Sep 2024 14:36:30 -0400
Subject: [PATCH] require new transaction for add/set as well

---
 lib/flipper/adapters/active_record.rb       | 17 +++++++++--------
 spec/flipper/adapters/active_record_spec.rb |  4 +++-
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/lib/flipper/adapters/active_record.rb b/lib/flipper/adapters/active_record.rb
index 6f7f4a8a..04cc6f3a 100644
--- a/lib/flipper/adapters/active_record.rb
+++ b/lib/flipper/adapters/active_record.rb
@@ -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
@@ -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
@@ -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
 
diff --git a/spec/flipper/adapters/active_record_spec.rb b/spec/flipper/adapters/active_record_spec.rb
index a4313356..90573945 100644
--- a/spec/flipper/adapters/active_record_spec.rb
+++ b/spec/flipper/adapters/active_record_spec.rb
@@ -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