Merge pull request #795 from flippercloud/poll-cold-start-fix

Poll cold start fix

Author: jnunemaker

lib/flipper/adapters/http.rb

@@ -59,8 +59,8 @@ def get_all
         response = @client.get("/features?exclude_gate_names=true")
         raise Error, response unless response.is_a?(Net::HTTPOK)
 
-        parsed_response = Typecast.from_json(response.body)
-        parsed_features = parsed_response.fetch('features')
+        parsed_response = response.body.empty? ? {} : Typecast.from_json(response.body)
+        parsed_features = parsed_response['features'] || []
         gates_by_key = parsed_features.each_with_object({}) do |parsed_feature, hash|
           hash[parsed_feature['key']] = parsed_feature['gates']
           hash

lib/flipper/adapters/poll.rb

@@ -18,6 +18,21 @@ def initialize(poller, adapter)
         @adapter = adapter
         @poller = poller
         @last_synced_at = 0
+
+        # If the adapter is empty, we need to sync before starting the poller.
+        # Yes, this will block the main thread, but that's better than thinking
+        # nothing is enabled.
+        if adapter.features.empty?
+          begin
+            @poller.sync
+          rescue => error
+            # TODO: Warn here that it's possible that no data has been synced
+            # and flags are being evaluated without flag data being present
+            # until a sync completes. We rescue to avoid flipper being down
+            # causing your processes to crash.
+          end
+        end
+
         @poller.start
       end
 

spec/flipper/adapters/poll_spec.rb

@@ -0,0 +1,41 @@
+require 'flipper/adapters/poll'
+
+RSpec.describe Flipper::Adapters::Poll do
+  let(:remote_adapter) {
+    adapter = Flipper::Adapters::Memory.new(threadsafe: true)
+    flipper = Flipper.new(adapter)
+    flipper.enable(:search)
+    flipper.enable(:analytics)
+    adapter
+  }
+  let(:local_adapter) { Flipper::Adapters::Memory.new(threadsafe: true) }
+  let(:poller) {
+    Flipper::Poller.get("for_spec", {
+      start_automatically: false,
+      remote_adapter: remote_adapter,
+    })
+  }
+
+  it "syncs in main thread if local adapter is empty" do
+    instance = described_class.new(poller, local_adapter)
+    instance.features # call something to force sync
+    expect(local_adapter.features).to eq(remote_adapter.features)
+  end
+
+  it "does not sync in main thread if local adapter is not empty" do
+    # make local not empty by importing remote
+    flipper = Flipper.new(local_adapter)
+    flipper.import(remote_adapter)
+
+    # make a fake poller to verify calls
+    poller = double("Poller", last_synced_at: Concurrent::AtomicFixnum.new(0))
+    expect(poller).to receive(:start).twice
+    expect(poller).not_to receive(:sync)
+
+    # create new instance and call something to force sync
+    instance = described_class.new(poller, local_adapter)
+    instance.features # call something to force sync
+
+    expect(local_adapter.features).to eq(remote_adapter.features)
+  end
+end

spec/flipper/cloud_spec.rb

@@ -43,6 +43,7 @@
 
   context 'initialize with token and options' do
     it 'sets correct url' do
+      stub_request(:any, %r{fakeflipper.com}).to_return(status: 200)
       instance = described_class.new(token: 'asdf', url: 'https://www.fakeflipper.com/sadpanda')
       # pardon the nesting...
       memoized = instance.adapter
@@ -78,27 +79,31 @@
   end
 
   it 'can set debug_output' do
+    instance = Flipper::Adapters::Http::Client.new(token: 'asdf', url: 'https://www.flippercloud.io/adapter')
     expect(Flipper::Adapters::Http::Client).to receive(:new)
-      .with(hash_including(debug_output: STDOUT)).at_least(:once)
+      .with(hash_including(debug_output: STDOUT)).at_least(:once).and_return(instance)
     described_class.new(token: 'asdf', debug_output: STDOUT)
   end
 
   it 'can set read_timeout' do
+    instance = Flipper::Adapters::Http::Client.new(token: 'asdf', url: 'https://www.flippercloud.io/adapter')
     expect(Flipper::Adapters::Http::Client).to receive(:new)
-      .with(hash_including(read_timeout: 1)).at_least(:once)
+      .with(hash_including(read_timeout: 1)).at_least(:once).and_return(instance)
     described_class.new(token: 'asdf', read_timeout: 1)
   end
 
   it 'can set open_timeout' do
+    instance = Flipper::Adapters::Http::Client.new(token: 'asdf', url: 'https://www.flippercloud.io/adapter')
     expect(Flipper::Adapters::Http::Client).to receive(:new)
-      .with(hash_including(open_timeout: 1)).at_least(:once)
+      .with(hash_including(open_timeout: 1)).at_least(:once).and_return(instance)
     described_class.new(token: 'asdf', open_timeout: 1)
   end
 
   if RUBY_VERSION >= '2.6.0'
     it 'can set write_timeout' do
+      instance = Flipper::Adapters::Http::Client.new(token: 'asdf', url: 'https://www.flippercloud.io/adapter')
       expect(Flipper::Adapters::Http::Client).to receive(:new)
-        .with(hash_including(open_timeout: 1)).at_least(:once)
+        .with(hash_including(open_timeout: 1)).at_least(:once).and_return(instance)
       described_class.new(token: 'asdf', open_timeout: 1)
     end
   end

spec/flipper/engine_spec.rb

@@ -12,6 +12,7 @@
   end
 
   before do
+    stub_request(:get, /flippercloud\.io/).to_return(status: 200, body: "{}")
     Rails.application = nil
     ActiveSupport::Dependencies.autoload_paths = ActiveSupport::Dependencies.autoload_paths.dup
     ActiveSupport::Dependencies.autoload_once_paths = ActiveSupport::Dependencies.autoload_once_paths.dup