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.

Author: jnunemaker

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

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