Merge pull request #982 from flippercloud/strict-adapter-sync-mode

Add sync mode bypass to Strict adapter

Author: jnunemaker

lib/flipper/adapters/strict.rb

@@ -10,11 +10,40 @@ def initialize(name)
         end
       end
 
+      class << self
+        # Returns whether sync mode is enabled for the current thread.
+        # When sync mode is enabled, strict checks are not enforced,
+        # allowing sync operations to add features and bring local state
+        # in line with remote state.
+        def sync_mode
+          Thread.current[:flipper_strict_sync_mode]
+        end
+
+        def sync_mode=(value)
+          Thread.current[:flipper_strict_sync_mode] = value
+        end
+
+        # Executes a block with sync mode enabled. Strict checks will
+        # not be enforced within the block.
+        def with_sync_mode
+          old_value = sync_mode
+          self.sync_mode = true
+          yield
+        ensure
+          self.sync_mode = old_value
+        end
+      end
+
       def initialize(adapter, handler = nil, &block)
         super(adapter)
         @handler = block || handler
       end
 
+      def add(feature)
+        assert_feature_exists(feature) unless self.class.sync_mode
+        super
+      end
+
       def get(feature)
         assert_feature_exists(feature)
         super
@@ -28,6 +57,7 @@ def get_multi(features)
       private
 
       def assert_feature_exists(feature)
+        return if self.class.sync_mode
         return if @adapter.features.include?(feature.key)
 
         case handler

lib/flipper/adapters/sync/synchronizer.rb

@@ -1,6 +1,7 @@
 require "flipper/feature"
 require "flipper/gate_values"
 require "flipper/adapters/actor_limit"
+require "flipper/adapters/strict"
 require "flipper/adapters/sync/feature_synchronizer"
 
 module Flipper
@@ -29,7 +30,9 @@ def initialize(local, remote, options = {})
         # Public: Forces a sync.
         def call
           @instrumenter.instrument("synchronizer_call.flipper") do
-            Flipper::Adapters::ActorLimit.with_sync_mode { sync }
+            Flipper::Adapters::Strict.with_sync_mode do
+              Flipper::Adapters::ActorLimit.with_sync_mode { sync }
+            end
           end
         end
 

spec/flipper/adapters/strict_spec.rb

@@ -21,23 +21,35 @@
           expect { subject.get_multi([feature]) }.to raise_error(Flipper::Adapters::Strict::NotFound)
         end
       end
+
+      context "#add" do
+        it "raises an error for unknown feature" do
+          expect { subject.add(feature) }.to raise_error(Flipper::Adapters::Strict::NotFound)
+        end
+      end
     end
   end
 
   context "handler = :warn" do
     subject { described_class.new(Flipper::Adapters::Memory.new, :warn) }
 
     context "#get" do
-      it "raises an error for unknown feature" do
+      it "warns for unknown feature" do
         expect(capture_output { subject.get(feature) }).to match(/Could not find feature "unknown"/)
       end
     end
 
     context "#get_multi" do
-      it "raises an error for unknown feature" do
+      it "warns for unknown feature" do
         expect(capture_output { subject.get_multi([feature]) }).to match(/Could not find feature "unknown"/)
       end
     end
+
+    context "#add" do
+      it "warns for unknown feature" do
+        expect(capture_output { subject.add(feature) }).to match(/Could not find feature "unknown"/)
+      end
+    end
   end
 
   context "handler = Block" do
@@ -48,17 +60,63 @@
 
 
     context "#get" do
-      it "raises an error for unknown feature" do
+      it "calls block for unknown feature" do
         subject.get(feature)
         expect(unknown_features).to eq(["unknown"])
       end
     end
 
     context "#get_multi" do
-      it "raises an error for unknown feature" do
+      it "calls block for unknown feature" do
         subject.get_multi([flipper[:foo], flipper[:bar]])
         expect(unknown_features).to eq(["foo", "bar"])
       end
     end
+
+    context "#add" do
+      it "calls block for unknown feature" do
+        subject.add(feature)
+        expect(unknown_features).to eq(["unknown"])
+      end
+    end
+  end
+
+  describe ".with_sync_mode" do
+    subject { described_class.new(Flipper::Adapters::Memory.new, :raise) }
+
+    it "bypasses strict checks for add" do
+      described_class.with_sync_mode do
+        expect { subject.add(feature) }.not_to raise_error
+      end
+    end
+
+    it "bypasses strict checks for get" do
+      described_class.with_sync_mode do
+        expect { subject.get(feature) }.not_to raise_error
+      end
+    end
+
+    it "bypasses strict checks for get_multi" do
+      described_class.with_sync_mode do
+        expect { subject.get_multi([feature]) }.not_to raise_error
+      end
+    end
+
+    it "restores previous sync mode after block" do
+      described_class.with_sync_mode do
+        # inside sync mode
+      end
+      expect { subject.add(feature) }.to raise_error(Flipper::Adapters::Strict::NotFound)
+    end
+
+    it "restores previous sync mode even on error" do
+      begin
+        described_class.with_sync_mode do
+          raise "boom"
+        end
+      rescue RuntimeError
+      end
+      expect { subject.add(feature) }.to raise_error(Flipper::Adapters::Strict::NotFound)
+    end
   end
 end