From 58d848ab5a41462f3f5989641f8fcfd81e0eea65 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Sat, 6 Dec 2025 20:45:26 -0500 Subject: [PATCH 01/11] Handle sync causing write on read request This was reported by someone using cloud. If they were using database selector middleware and a read request hit when cloud wanted to sync it would error. This is ugly. There has to be a better way. I'll ask around. --- lib/flipper/adapters/active_record.rb | 44 +++++++-- spec/flipper/adapters/active_record_spec.rb | 98 +++++++++++++++++++++ 2 files changed, 135 insertions(+), 7 deletions(-) diff --git a/lib/flipper/adapters/active_record.rb b/lib/flipper/adapters/active_record.rb index 1509cd146..cec23f467 100644 --- a/lib/flipper/adapters/active_record.rb +++ b/lib/flipper/adapters/active_record.rb @@ -41,7 +41,7 @@ def features # Public: Adds a feature to the set of known features. def add(feature) - with_connection(@feature_class) do + with_write_connection(@feature_class) do @feature_class.transaction(requires_new: true) do begin # race condition, but add is only used by enable/disable which happen @@ -60,7 +60,7 @@ def add(feature) # Public: Removes a feature from the set of known features. def remove(feature) - with_connection(@feature_class) do + with_write_connection(@feature_class) do @feature_class.transaction do @feature_class.where(key: feature.key).destroy_all clear(feature) @@ -71,7 +71,7 @@ def remove(feature) # Public: Clears the gate values for a feature. def clear(feature) - with_connection(@gate_class) { @gate_class.where(feature_key: feature.key).destroy_all } + with_write_connection(@gate_class) { @gate_class.where(feature_key: feature.key).destroy_all } true end @@ -165,9 +165,11 @@ def disable(feature, gate, thing) when :integer set(feature, gate, thing) when :json - delete(feature, gate) + with_write_connection(@gate_class) do + delete(feature, gate) + end when :set - with_connection(@gate_class) do + with_write_connection(@gate_class) do @gate_class.where(feature_key: feature.key, key: gate.key, value: thing.value).destroy_all end else @@ -190,7 +192,7 @@ def set(feature, gate, thing, options = {}) raise VALUE_TO_TEXT_WARNING if json_feature && value_not_text? - with_connection(@gate_class) do + with_write_connection(@gate_class) do @gate_class.transaction(requires_new: true) do clear(feature) if clear_feature delete(feature, gate) @@ -215,7 +217,7 @@ def delete(feature, gate) end def enable_multi(feature, gate, thing) - with_connection(@gate_class) do |connection| + with_write_connection(@gate_class) do |connection| begin connection.transaction(requires_new: true) do @gate_class.create! do |g| @@ -271,6 +273,34 @@ def with_connection(model = @feature_class, &block) model.connection_pool.with_connection(&block) end + def with_write_connection(model = @feature_class, &block) + warn VALUE_TO_TEXT_WARNING if !warned_about_value_not_text? && value_not_text? + + # Use connected_to for role switching if available (Rails 6.1+) + # This ensures writes go to primary/write database when using read/write roles + if model.respond_to?(:connected_to) + begin + # Find the abstract class that manages the connection + # Walk up the inheritance chain to find it + connection_class = model + until connection_class.abstract_class? || connection_class == ::ActiveRecord::Base + connection_class = connection_class.superclass + break if connection_class == ::ActiveRecord::Base || !connection_class.respond_to?(:abstract_class?) + end + + connection_class.connected_to(role: :writing) do + model.connection_pool.with_connection(&block) + end + rescue NotImplementedError + # connected_to not available or not configured for roles, fall back + model.connection_pool.with_connection(&block) + end + else + # Fall back to regular connection for single database or older Rails + model.connection_pool.with_connection(&block) + end + end + def warned_about_value_not_text? return @warned_about_value_not_text if defined?(@warned_about_value_not_text) @warned_about_value_not_text = true diff --git a/spec/flipper/adapters/active_record_spec.rb b/spec/flipper/adapters/active_record_spec.rb index a1d112551..6221734b2 100644 --- a/spec/flipper/adapters/active_record_spec.rb +++ b/spec/flipper/adapters/active_record_spec.rb @@ -214,6 +214,104 @@ end end + context 'with read/write roles' do + # Skip for older Rails versions that don't support connected_to with roles + next unless ActiveRecord::Base.respond_to?(:connected_to) && ActiveRecord.version >= Gem::Version.new('6.1') + + # Skip for SQLite as it doesn't handle role-based connections well with :memory: databases + next if config["adapter"] == "sqlite3" + + let(:abstract_class) do + # Create a named abstract class (Rails requires names for connects_to) + klass = Class.new(ActiveRecord::Base) do + self.abstract_class = true + end + stub_const('TestApplicationRecord', klass) + + # Now configure connects_to with the same database for both roles + # In production, these would be different (primary/replica) + klass.connects_to database: { + writing: config, + reading: config + } + + klass + end + + after do + # Disconnect role-based connections to avoid interfering with database cleanup + ActiveRecord::Base.connection_handler.clear_all_connections! + end + + let(:feature_class) do + klass = Class.new(abstract_class) do + self.table_name = 'flipper_features' + validates :key, presence: true + end + stub_const('TestFeature', klass) + klass + end + + let(:gate_class) do + klass = Class.new(abstract_class) do + self.table_name = 'flipper_gates' + end + stub_const('TestGate', klass) + klass + end + + let(:adapter_with_roles) do + described_class.new( + feature_class: feature_class, + gate_class: gate_class + ) + end + + it 'can perform write operations when forced to reading role' do + abstract_class.connected_to(role: :reading) do + flipper = Flipper.new(adapter_with_roles) + + feature = flipper[:test_feature] + expect { feature.enable }.not_to raise_error + expect(feature.enabled?).to be(true) + expect { feature.disable }.not_to raise_error + expect(feature.enabled?).to be(false) + + feature = flipper[:actor_test] + actor = Struct.new(:flipper_id).new(123) + expect { feature.enable_actor(actor) }.not_to raise_error + expect(feature.enabled?(actor)).to be(true) + expect { feature.disable_actor(actor) }.not_to raise_error + expect(feature.enabled?(actor)).to be(false) + + feature = flipper[:gate_test] + expect { feature.enable_percentage_of_time(50) }.not_to raise_error + expect { feature.disable_percentage_of_time }.not_to raise_error + feature.enable + expect { feature.remove }.not_to raise_error + + feature = flipper[:expression_test] + expression = Flipper.property(:plan).eq("premium") + expect { feature.enable_expression(expression) }.not_to raise_error + expect(feature.expression).to eq(expression) + expect { feature.disable_expression }.not_to raise_error + expect(feature.expression).to be_nil + end + end + + it 'does not hold onto connections during write operations' do + ActiveRecord::Base.connection_handler.clear_active_connections! + + abstract_class.connected_to(role: :reading) do + flipper = Flipper.new(adapter_with_roles) + feature = flipper[:connection_test] + + feature.enable + expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + end + end + end + context 'requiring "flipper-active_record"' do before do Flipper.configuration = nil From 4112c5a4c013cf21a37f1130f53ee3b93b9f9877 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Sun, 7 Dec 2025 09:26:16 -0500 Subject: [PATCH 02/11] Just run ci on push so we dont get duplicate actions --- .github/workflows/ci.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c20c695cc..d41563152 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,9 +1,8 @@ name: CI -on: [push, pull_request] +on: [push] jobs: test: - if: github.event_name == 'push' || (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository) name: Test on ruby ${{ matrix.ruby }} and rails ${{ matrix.rails }} runs-on: ubuntu-latest services: From fe729a695fbb778748db0d01a6f1a279aec5a1cd Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Sun, 7 Dec 2025 09:27:03 -0500 Subject: [PATCH 03/11] Only push for examples too --- .github/workflows/examples.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/examples.yml b/.github/workflows/examples.yml index 82319441a..97bf5b460 100644 --- a/.github/workflows/examples.yml +++ b/.github/workflows/examples.yml @@ -1,5 +1,5 @@ name: Examples -on: [push, pull_request] +on: [push] jobs: test: if: github.repository_owner == 'flippercloud' From bdce7c635954ced360a1b5115749c0826dff7c6c Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Mon, 8 Dec 2025 09:07:06 -0500 Subject: [PATCH 04/11] Read the rails source, AI is crazy sometimes Much easier to just use the methods they provide. --- lib/flipper/adapters/active_record.rb | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/lib/flipper/adapters/active_record.rb b/lib/flipper/adapters/active_record.rb index cec23f467..f0113ca10 100644 --- a/lib/flipper/adapters/active_record.rb +++ b/lib/flipper/adapters/active_record.rb @@ -276,27 +276,21 @@ def with_connection(model = @feature_class, &block) def with_write_connection(model = @feature_class, &block) warn VALUE_TO_TEXT_WARNING if !warned_about_value_not_text? && value_not_text? - # Use connected_to for role switching if available (Rails 6.1+) - # This ensures writes go to primary/write database when using read/write roles - if model.respond_to?(:connected_to) - begin - # Find the abstract class that manages the connection - # Walk up the inheritance chain to find it - connection_class = model - until connection_class.abstract_class? || connection_class == ::ActiveRecord::Base - connection_class = connection_class.superclass - break if connection_class == ::ActiveRecord::Base || !connection_class.respond_to?(:abstract_class?) - end - + # Use Rails' built-in method to find the class that controls the connection + # This walks up the inheritance chain to find which class called connects_to + if model.respond_to?(:connection_class_for_self) + connection_class = model.connection_class_for_self + + # Only use connected_to if this class actually has connects_to configured + # connection_class? returns true when connects_to was called on the class + if connection_class.respond_to?(:connection_class?) && connection_class.connection_class? connection_class.connected_to(role: :writing) do model.connection_pool.with_connection(&block) end - rescue NotImplementedError - # connected_to not available or not configured for roles, fall back + else model.connection_pool.with_connection(&block) end else - # Fall back to regular connection for single database or older Rails model.connection_pool.with_connection(&block) end end From 5f9c69a7d187e0385f5233d97e64cfa0f399df64 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Mon, 8 Dec 2025 09:11:44 -0500 Subject: [PATCH 05/11] Use rails shorthand method Cleans it up a bit. --- lib/flipper/adapters/active_record.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/flipper/adapters/active_record.rb b/lib/flipper/adapters/active_record.rb index f0113ca10..553aace48 100644 --- a/lib/flipper/adapters/active_record.rb +++ b/lib/flipper/adapters/active_record.rb @@ -270,12 +270,10 @@ def value_not_text? def with_connection(model = @feature_class, &block) warn VALUE_TO_TEXT_WARNING if !warned_about_value_not_text? && value_not_text? - model.connection_pool.with_connection(&block) + model.with_connection(&block) end def with_write_connection(model = @feature_class, &block) - warn VALUE_TO_TEXT_WARNING if !warned_about_value_not_text? && value_not_text? - # Use Rails' built-in method to find the class that controls the connection # This walks up the inheritance chain to find which class called connects_to if model.respond_to?(:connection_class_for_self) @@ -285,13 +283,13 @@ def with_write_connection(model = @feature_class, &block) # connection_class? returns true when connects_to was called on the class if connection_class.respond_to?(:connection_class?) && connection_class.connection_class? connection_class.connected_to(role: :writing) do - model.connection_pool.with_connection(&block) + with_connection(model, &block) end else - model.connection_pool.with_connection(&block) + with_connection(model, &block) end else - model.connection_pool.with_connection(&block) + with_connection(model, &block) end end From 80e05e3a7a8def5098245d7d4f5c9cfc0a3e001d Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Mon, 8 Dec 2025 09:17:45 -0500 Subject: [PATCH 06/11] seems we need this until we drop Rails 7 and below --- lib/flipper/adapters/active_record.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/flipper/adapters/active_record.rb b/lib/flipper/adapters/active_record.rb index 553aace48..b0aab44fc 100644 --- a/lib/flipper/adapters/active_record.rb +++ b/lib/flipper/adapters/active_record.rb @@ -270,7 +270,7 @@ def value_not_text? def with_connection(model = @feature_class, &block) warn VALUE_TO_TEXT_WARNING if !warned_about_value_not_text? && value_not_text? - model.with_connection(&block) + model.connection_pool.with_connection(&block) end def with_write_connection(model = @feature_class, &block) From c2b941d6b566e96f0a8d103c50166a4d421e0810 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Mon, 8 Dec 2025 09:32:33 -0500 Subject: [PATCH 07/11] Only available in >= 7.1 --- spec/flipper/adapters/active_record_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/flipper/adapters/active_record_spec.rb b/spec/flipper/adapters/active_record_spec.rb index 6221734b2..c8ecb2242 100644 --- a/spec/flipper/adapters/active_record_spec.rb +++ b/spec/flipper/adapters/active_record_spec.rb @@ -216,7 +216,7 @@ context 'with read/write roles' do # Skip for older Rails versions that don't support connected_to with roles - next unless ActiveRecord::Base.respond_to?(:connected_to) && ActiveRecord.version >= Gem::Version.new('6.1') + next unless ActiveRecord::Base.respond_to?(:connected_to) && ActiveRecord.version >= Gem::Version.new('7.1') # Skip for SQLite as it doesn't handle role-based connections well with :memory: databases next if config["adapter"] == "sqlite3" From d9199e3f125d5f1d799d5a2fe60e481ea2258a3f Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Mon, 8 Dec 2025 09:47:10 -0500 Subject: [PATCH 08/11] Fix deprecation warning --- spec/flipper/adapters/active_record_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/flipper/adapters/active_record_spec.rb b/spec/flipper/adapters/active_record_spec.rb index c8ecb2242..16a8d1eee 100644 --- a/spec/flipper/adapters/active_record_spec.rb +++ b/spec/flipper/adapters/active_record_spec.rb @@ -94,7 +94,7 @@ context "ActiveRecord connection_pool" do before do - ActiveRecord::Base.connection_handler.clear_active_connections! + ActiveRecord::Base.connection_handler.clear_active_connections!(:all) end context "#features" do @@ -300,7 +300,7 @@ end it 'does not hold onto connections during write operations' do - ActiveRecord::Base.connection_handler.clear_active_connections! + ActiveRecord::Base.connection_handler.clear_active_connections!(:all) abstract_class.connected_to(role: :reading) do flipper = Flipper.new(adapter_with_roles) From a15832bf2f15b8a05dfdd81dffb9e3f5caaf825d Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Mon, 8 Dec 2025 10:26:46 -0500 Subject: [PATCH 09/11] Refactor ActiveRecord connection handling in tests In some versions I was getting deprecation message. In others I was getting no arguments allowed. In still others it just worked. Hopefully this gets all versions working. Updated tests in `active_record_spec.rb` to use helper methods for checking active connections and clearing them. This improves readability and maintainability of the test code while ensuring that ActiveRecord connections are properly managed during feature operations. --- spec/flipper/adapters/active_record_spec.rb | 80 ++++++++++++--------- 1 file changed, 45 insertions(+), 35 deletions(-) diff --git a/spec/flipper/adapters/active_record_spec.rb b/spec/flipper/adapters/active_record_spec.rb index 16a8d1eee..a949d5cb8 100644 --- a/spec/flipper/adapters/active_record_spec.rb +++ b/spec/flipper/adapters/active_record_spec.rb @@ -94,36 +94,36 @@ context "ActiveRecord connection_pool" do before do - ActiveRecord::Base.connection_handler.clear_active_connections!(:all) + clear_active_connections! end context "#features" do it "does not hold onto connections" do - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) subject.features - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) end it "does not release previously held connection" do ActiveRecord::Base.connection # establish a new connection - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) subject.features - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) end end context "#get_all" do it "does not hold onto connections" do - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) subject.get_all - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) end it "does not release previously held connection" do ActiveRecord::Base.connection # establish a new connection - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) subject.get_all - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) end end @@ -131,24 +131,24 @@ let(:feature) { Flipper::Feature.new(:search, subject) } it "does not hold onto connections" do - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) subject.add(feature) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) subject.remove(feature) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) subject.clear(feature) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) end it "does not release previously held connection" do ActiveRecord::Base.connection # establish a new connection - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) subject.add(feature) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) subject.remove(feature) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) subject.clear(feature) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) end end @@ -156,16 +156,16 @@ let(:feature) { Flipper::Feature.new(:search, subject) } it "does not hold onto connections" do - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) subject.get_multi([feature]) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) end it "does not release previously held connection" do ActiveRecord::Base.connection # establish a new connection - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) subject.get_multi([feature]) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) end end @@ -174,20 +174,20 @@ let(:gate) { feature.gate(:boolean)} it "does not hold onto connections" do - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) subject.enable(feature, gate, gate.wrap(true)) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) subject.disable(feature, gate, gate.wrap(false)) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) end it "does not release previously held connection" do ActiveRecord::Base.connection # establish a new connection - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) subject.enable(feature, gate, gate.wrap(true)) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) subject.disable(feature, gate, gate.wrap(false)) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) end end @@ -196,20 +196,20 @@ let(:gate) { feature.gate(:group) } it "does not hold onto connections" do - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) subject.enable(feature, gate, gate.wrap(:admin)) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) subject.disable(feature, gate, gate.wrap(:admin)) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) end it "does not release previously held connection" do ActiveRecord::Base.connection # establish a new connection - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) subject.enable(feature, gate, gate.wrap(:admin)) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) subject.disable(feature, gate, gate.wrap(:admin)) - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(true) + expect(active_connections?).to be(true) end end end @@ -300,14 +300,14 @@ end it 'does not hold onto connections during write operations' do - ActiveRecord::Base.connection_handler.clear_active_connections!(:all) + clear_active_connections! abstract_class.connected_to(role: :reading) do flipper = Flipper.new(adapter_with_roles) feature = flipper[:connection_test] feature.enable - expect(ActiveRecord::Base.connection_handler.active_connections?).to be(false) + expect(active_connections?).to be(false) end end end @@ -363,4 +363,14 @@ end end end + + def active_connections? + method = ActiveRecord::Base.connection_handler.method(:active_connections?) + method.arity == 0 ? method.call : method.call(:all) + end + + def clear_active_connections! + method = ActiveRecord::Base.connection_handler.method(:clear_active_connections!) + method.arity == 0 ? method.call : method.call(:all) + end end From b1d04fbde972fbfa3e47d07d1b5bf1e2ffdb4415 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Mon, 8 Dec 2025 10:57:39 -0500 Subject: [PATCH 10/11] One more to fix --- spec/flipper/adapters/active_record_spec.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/spec/flipper/adapters/active_record_spec.rb b/spec/flipper/adapters/active_record_spec.rb index a949d5cb8..df865c168 100644 --- a/spec/flipper/adapters/active_record_spec.rb +++ b/spec/flipper/adapters/active_record_spec.rb @@ -240,7 +240,7 @@ after do # Disconnect role-based connections to avoid interfering with database cleanup - ActiveRecord::Base.connection_handler.clear_all_connections! + clear_all_connections! end let(:feature_class) do @@ -373,4 +373,9 @@ def clear_active_connections! method = ActiveRecord::Base.connection_handler.method(:clear_active_connections!) method.arity == 0 ? method.call : method.call(:all) end + + def clear_all_connections! + method = ActiveRecord::Base.connection_handler.method(:clear_all_connections!) + method.arity == 0 ? method.call : method.call(:all) + end end From 323dc8530afd4654e873d8f2d6f6f33a479e6a96 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Mon, 8 Dec 2025 14:20:05 -0500 Subject: [PATCH 11/11] use if instead of unless to avoid possibly skipping later specs --- spec/flipper/adapters/active_record_spec.rb | 160 ++++++++++---------- 1 file changed, 81 insertions(+), 79 deletions(-) diff --git a/spec/flipper/adapters/active_record_spec.rb b/spec/flipper/adapters/active_record_spec.rb index df865c168..c5be3c724 100644 --- a/spec/flipper/adapters/active_record_spec.rb +++ b/spec/flipper/adapters/active_record_spec.rb @@ -214,100 +214,102 @@ end end - context 'with read/write roles' do - # Skip for older Rails versions that don't support connected_to with roles - next unless ActiveRecord::Base.respond_to?(:connected_to) && ActiveRecord.version >= Gem::Version.new('7.1') + if ActiveRecord::Base.respond_to?(:connected_to) && ActiveRecord.version >= Gem::Version.new('7.1') + context 'with read/write roles' do + # Skip for older Rails versions that don't support connected_to with roles - # Skip for SQLite as it doesn't handle role-based connections well with :memory: databases - next if config["adapter"] == "sqlite3" - let(:abstract_class) do - # Create a named abstract class (Rails requires names for connects_to) - klass = Class.new(ActiveRecord::Base) do - self.abstract_class = true - end - stub_const('TestApplicationRecord', klass) + # Skip for SQLite as it doesn't handle role-based connections well with :memory: databases + next if config["adapter"] == "sqlite3" - # Now configure connects_to with the same database for both roles - # In production, these would be different (primary/replica) - klass.connects_to database: { - writing: config, - reading: config - } + let(:abstract_class) do + # Create a named abstract class (Rails requires names for connects_to) + klass = Class.new(ActiveRecord::Base) do + self.abstract_class = true + end + stub_const('TestApplicationRecord', klass) - klass - end + # Now configure connects_to with the same database for both roles + # In production, these would be different (primary/replica) + klass.connects_to database: { + writing: config, + reading: config + } - after do - # Disconnect role-based connections to avoid interfering with database cleanup - clear_all_connections! - end + klass + end - let(:feature_class) do - klass = Class.new(abstract_class) do - self.table_name = 'flipper_features' - validates :key, presence: true + after do + # Disconnect role-based connections to avoid interfering with database cleanup + clear_all_connections! end - stub_const('TestFeature', klass) - klass - end - let(:gate_class) do - klass = Class.new(abstract_class) do - self.table_name = 'flipper_gates' + let(:feature_class) do + klass = Class.new(abstract_class) do + self.table_name = 'flipper_features' + validates :key, presence: true + end + stub_const('TestFeature', klass) + klass end - stub_const('TestGate', klass) - klass - end - let(:adapter_with_roles) do - described_class.new( - feature_class: feature_class, - gate_class: gate_class - ) - end + let(:gate_class) do + klass = Class.new(abstract_class) do + self.table_name = 'flipper_gates' + end + stub_const('TestGate', klass) + klass + end - it 'can perform write operations when forced to reading role' do - abstract_class.connected_to(role: :reading) do - flipper = Flipper.new(adapter_with_roles) - - feature = flipper[:test_feature] - expect { feature.enable }.not_to raise_error - expect(feature.enabled?).to be(true) - expect { feature.disable }.not_to raise_error - expect(feature.enabled?).to be(false) - - feature = flipper[:actor_test] - actor = Struct.new(:flipper_id).new(123) - expect { feature.enable_actor(actor) }.not_to raise_error - expect(feature.enabled?(actor)).to be(true) - expect { feature.disable_actor(actor) }.not_to raise_error - expect(feature.enabled?(actor)).to be(false) - - feature = flipper[:gate_test] - expect { feature.enable_percentage_of_time(50) }.not_to raise_error - expect { feature.disable_percentage_of_time }.not_to raise_error - feature.enable - expect { feature.remove }.not_to raise_error - - feature = flipper[:expression_test] - expression = Flipper.property(:plan).eq("premium") - expect { feature.enable_expression(expression) }.not_to raise_error - expect(feature.expression).to eq(expression) - expect { feature.disable_expression }.not_to raise_error - expect(feature.expression).to be_nil + let(:adapter_with_roles) do + described_class.new( + feature_class: feature_class, + gate_class: gate_class + ) end - end - it 'does not hold onto connections during write operations' do - clear_active_connections! + it 'can perform write operations when forced to reading role' do + abstract_class.connected_to(role: :reading) do + flipper = Flipper.new(adapter_with_roles) + + feature = flipper[:test_feature] + expect { feature.enable }.not_to raise_error + expect(feature.enabled?).to be(true) + expect { feature.disable }.not_to raise_error + expect(feature.enabled?).to be(false) + + feature = flipper[:actor_test] + actor = Struct.new(:flipper_id).new(123) + expect { feature.enable_actor(actor) }.not_to raise_error + expect(feature.enabled?(actor)).to be(true) + expect { feature.disable_actor(actor) }.not_to raise_error + expect(feature.enabled?(actor)).to be(false) + + feature = flipper[:gate_test] + expect { feature.enable_percentage_of_time(50) }.not_to raise_error + expect { feature.disable_percentage_of_time }.not_to raise_error + feature.enable + expect { feature.remove }.not_to raise_error + + feature = flipper[:expression_test] + expression = Flipper.property(:plan).eq("premium") + expect { feature.enable_expression(expression) }.not_to raise_error + expect(feature.expression).to eq(expression) + expect { feature.disable_expression }.not_to raise_error + expect(feature.expression).to be_nil + end + end - abstract_class.connected_to(role: :reading) do - flipper = Flipper.new(adapter_with_roles) - feature = flipper[:connection_test] + it 'does not hold onto connections during write operations' do + clear_active_connections! - feature.enable - expect(active_connections?).to be(false) + abstract_class.connected_to(role: :reading) do + flipper = Flipper.new(adapter_with_roles) + feature = flipper[:connection_test] + + feature.enable + expect(active_connections?).to be(false) + end end end end