From db480cafe0d9b19c17520ee65a0632c2b1e3ef06 Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Sun, 12 Mar 2023 17:32:33 -0400 Subject: [PATCH 1/7] Poll < DualWrite and instantiate Poller --- lib/flipper.rb | 1 + lib/flipper/adapters/poll.rb | 67 +++++++++++++++--------- lib/flipper/adapters/poll/poller.rb | 1 + lib/flipper/cloud/configuration.rb | 13 ++--- lib/flipper/poller.rb | 24 ++++----- spec/flipper/cloud/configuration_spec.rb | 2 +- spec/flipper/cloud_spec.rb | 6 +-- spec/flipper/poller_spec.rb | 2 +- 8 files changed, 64 insertions(+), 52 deletions(-) diff --git a/lib/flipper.rb b/lib/flipper.rb index db81b00ce..cbbf09852 100644 --- a/lib/flipper.rb +++ b/lib/flipper.rb @@ -145,6 +145,7 @@ def groups_registry=(registry) require 'flipper/adapters/memoizable' require 'flipper/adapters/memory' require 'flipper/adapters/instrumented' +require 'flipper/adapters/poll' require 'flipper/configuration' require 'flipper/dsl' require 'flipper/errors' diff --git a/lib/flipper/adapters/poll.rb b/lib/flipper/adapters/poll.rb index aa194dec7..38a02aad5 100644 --- a/lib/flipper/adapters/poll.rb +++ b/lib/flipper/adapters/poll.rb @@ -1,38 +1,57 @@ -require 'flipper/adapters/sync/synchronizer' +require 'flipper/adapters/dual_write' require 'flipper/poller' module Flipper module Adapters - class Poll - extend Forwardable - include ::Flipper::Adapter + # An adapter that keeps a local and remote adapter in sync via a background poller. + # + # Synchronization is performed when the adapter is accessed if the background + # poller has synced. + class Poll < DualWrite + # Public: The Poller used to sync in a background thread. + attr_reader :poller - # Deprecated - Poller = ::Flipper::Poller - - # Public: The name of the adapter. - attr_reader :name, :adapter, :poller - - def_delegators :synced_adapter, :features, :get, :get_multi, :get_all, :add, :remove, :clear, :enable, :disable - - def initialize(poller, adapter) + # Instantiate a new Poll adapter. + # + # local = Flipper::Adapters::ActiveRecord.new + # remote = Flipper::Adapters::Http.new(url: 'http://example.com/flipper') + # adapter = Flipper::Adapters::Poll.new(local, remote, key: 'unique_poller_name', interval: 5) + # + # local - Local adapter that will be used for reads and gets synchronized on an interval. + # remote - Remote adapter that will be polled on an interval. + # key: The key used to identify the poller. + # **options: Options to pass to the poller. See Flipper::Poller for options. + def initialize(local, remote, key:, **options) + super(local, remote) @name = :poll - @adapter = adapter - @poller = poller + @poller = Flipper::Poller.get(key, remote, options).tap(&:start) @last_synced_at = 0 - @poller.start + @sync_automatically = true end - private + # Public: Synchronizes the local adapter with the current state of the remote adapter. + def sync + if @sync_automatically + poller_last_synced_at = @poller.last_synced_at.value + if poller_last_synced_at > @last_synced_at + @local.import(@poller.adapter) + @last_synced_at = poller_last_synced_at + end + end + if block_given? + begin + sync_automatically_was, @sync_automatically = @sync_automatically, false + yield + ensure + @sync_automatically = sync_automatically_was + end + end + end - def synced_adapter - @poller.start - poller_last_synced_at = @poller.last_synced_at.value - if poller_last_synced_at > @last_synced_at - Flipper::Adapters::Sync::Synchronizer.new(@adapter, @poller.adapter).call - @last_synced_at = poller_last_synced_at + %i[features get get_multi get_all add remove clear enable disable].each do |method| + define_method(method) do |*args| + sync { super(*args) } end - @adapter end end end diff --git a/lib/flipper/adapters/poll/poller.rb b/lib/flipper/adapters/poll/poller.rb index 156859cde..384b158b1 100644 --- a/lib/flipper/adapters/poll/poller.rb +++ b/lib/flipper/adapters/poll/poller.rb @@ -1,2 +1,3 @@ warn "DEPRECATION WARNING: Flipper::Adapters::Poll::Poller is deprecated. Use Flipper::Poller instead." require 'flipper/adapters/poll' +Flipper::Adapters::Poll::Poller = ::Flipper::Poller diff --git a/lib/flipper/cloud/configuration.rb b/lib/flipper/cloud/configuration.rb index 23a1fc8e4..d20ac40b8 100644 --- a/lib/flipper/cloud/configuration.rb +++ b/lib/flipper/cloud/configuration.rb @@ -1,7 +1,6 @@ require "socket" require "flipper/adapters/http" require "flipper/adapters/poll" -require "flipper/poller" require "flipper/adapters/memory" require "flipper/adapters/dual_write" require "flipper/adapters/sync/synchronizer" @@ -163,16 +162,12 @@ def dual_write_adapter Flipper::Adapters::DualWrite.new(local_adapter, http_adapter) end - def poller - Flipper::Poller.get(@url + @token, { + def poll_adapter + Flipper::Adapters::Poll.new(local_adapter, http_adapter, { + key: @url + @token, interval: sync_interval, - remote_adapter: http_adapter, instrumenter: instrumenter, - }).tap(&:start) - end - - def poll_adapter - Flipper::Adapters::Poll.new(poller, dual_write_adapter) + }) end def http_adapter diff --git a/lib/flipper/poller.rb b/lib/flipper/poller.rb index 95640318a..a6f1935bd 100644 --- a/lib/flipper/poller.rb +++ b/lib/flipper/poller.rb @@ -12,34 +12,32 @@ def self.instances end private_class_method :instances - def self.get(key, options = {}) - instances.compute_if_absent(key) { new(options) } + def self.get(key, *args) + instances.compute_if_absent(key) { new(*args) } end def self.reset instances.each {|_,poller| poller.stop }.clear end - def initialize(options = {}) - @thread = nil - @pid = Process.pid - @mutex = Mutex.new + def initialize(remote_adapter, options = {}) + @remote_adapter = remote_adapter @instrumenter = options.fetch(:instrumenter, Instrumenters::Noop) - @remote_adapter = options.fetch(:remote_adapter) @interval = options.fetch(:interval, 10).to_f - @last_synced_at = Concurrent::AtomicFixnum.new(0) - @adapter = Adapters::Memory.new if @interval < 1 warn "Flipper::Cloud poll interval must be greater than or equal to 1 but was #{@interval}. Setting @interval to 1." @interval = 1 end - @start_automatically = options.fetch(:start_automatically, true) + @thread = nil + @pid = Process.pid + @mutex = Mutex.new + @last_synced_at = Concurrent::AtomicFixnum.new(0) + @adapter = Adapters::Memory.new - if options.fetch(:shutdown_automatically, true) - at_exit { stop } - end + start if options.fetch(:start_automatically, true) + at_exit { stop } if options.fetch(:shutdown_automatically, true) end def start diff --git a/spec/flipper/cloud/configuration_spec.rb b/spec/flipper/cloud/configuration_spec.rb index 432285c81..f35f6321c 100644 --- a/spec/flipper/cloud/configuration_spec.rb +++ b/spec/flipper/cloud/configuration_spec.rb @@ -72,7 +72,7 @@ stub_request(:get, /flippercloud\.io/).to_return(status: 200, body: "{}") instance = described_class.new(required_options.merge(sync_interval: 1)) - poller = instance.send(:poller) + poller = instance.send(:poll_adapter).poller expect(poller.interval).to eq(1) end diff --git a/spec/flipper/cloud_spec.rb b/spec/flipper/cloud_spec.rb index 8fcfad46e..1881da403 100644 --- a/spec/flipper/cloud_spec.rb +++ b/spec/flipper/cloud_spec.rb @@ -26,12 +26,10 @@ # pardon the nesting... memoized_adapter = @instance.adapter poll_adapter = memoized_adapter.adapter - dual_write_adapter = poll_adapter.adapter expect(poll_adapter).to be_instance_of(Flipper::Adapters::Poll) - expect(dual_write_adapter).to be_instance_of(Flipper::Adapters::DualWrite) - http_adapter = dual_write_adapter.remote + http_adapter = poll_adapter.remote client = http_adapter.client expect(client.uri.scheme).to eq('https') expect(client.uri.host).to eq('www.flippercloud.io') @@ -44,7 +42,7 @@ context 'initialize with token and options' do it 'sets correct url' do @instance = described_class.new(token: 'asdf', url: 'https://www.fakeflipper.com/sadpanda') - uri = @instance.adapter.adapter.adapter.remote.client.uri + uri = @instance.adapter.adapter.remote.client.uri expect(uri.scheme).to eq('https') expect(uri.host).to eq('www.fakeflipper.com') expect(uri.path).to eq('/sadpanda') diff --git a/spec/flipper/poller_spec.rb b/spec/flipper/poller_spec.rb index 7fd5d442b..c733571ca 100644 --- a/spec/flipper/poller_spec.rb +++ b/spec/flipper/poller_spec.rb @@ -7,7 +7,7 @@ subject do described_class.new( - remote_adapter: remote_adapter, + remote_adapter, start_automatically: false, interval: Float::INFINITY ) From da51be8d8b1ae4bd48270036f440bbe0e2c1a90a Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Mon, 13 Mar 2023 09:30:47 -0400 Subject: [PATCH 2/7] Use `Poll` adapter instead of `Memoizable` when `memoize: :poll` --- lib/flipper/dsl.rb | 13 +++++++++++-- lib/flipper/railtie.rb | 9 +++++++-- spec/flipper/dsl_spec.rb | 11 +++++++++++ 3 files changed, 29 insertions(+), 4 deletions(-) diff --git a/lib/flipper/dsl.rb b/lib/flipper/dsl.rb index 88b82b045..1d5484694 100644 --- a/lib/flipper/dsl.rb +++ b/lib/flipper/dsl.rb @@ -21,8 +21,17 @@ class DSL def initialize(adapter, options = {}) @instrumenter = options.fetch(:instrumenter, Instrumenters::Noop) memoize = options.fetch(:memoize, true) - adapter = Adapters::Memoizable.new(adapter) if memoize - @adapter = adapter + @adapter = if memoize == :poll + Adapters::Poll.new(Adapters::Memory.new, adapter, key: 'memoizer', interval: 5).tap do |poll| + # Force poller to sync in current thread now + poll.poller.sync + end + elsif memoize + Adapters::Memoizable.new(adapter) + else + adapter + end + @memoized_features = {} end diff --git a/lib/flipper/railtie.rb b/lib/flipper/railtie.rb index a5d4f5c2c..f68f098e6 100644 --- a/lib/flipper/railtie.rb +++ b/lib/flipper/railtie.rb @@ -19,7 +19,10 @@ class Railtie < Rails::Railtie initializer "flipper.default", before: :load_config_initializers do |app| Flipper.configure do |config| config.default do - Flipper.new(config.adapter, instrumenter: app.config.flipper.instrumenter) + Flipper.new(config.adapter, { + instrumenter: app.config.flipper.instrumenter, + memoize: app.config.flipper.memoize, + }) end end end @@ -35,7 +38,9 @@ class Railtie < Rails::Railtie initializer "flipper.memoizer", after: :load_config_initializers do |app| flipper = app.config.flipper - if flipper.memoize + if flipper.memoize == :poll + app.middleware.use Flipper::Middleware::Sync + elsif flipper.memoize app.middleware.use Flipper::Middleware::Memoizer, { env_key: flipper.env_key, preload: flipper.preload, diff --git a/spec/flipper/dsl_spec.rb b/spec/flipper/dsl_spec.rb index 071e2fbed..aa257ccde 100644 --- a/spec/flipper/dsl_spec.rb +++ b/spec/flipper/dsl_spec.rb @@ -14,6 +14,17 @@ end end + context 'when using the :poll memoize strategy' do + it 'wraps the given adapter with Flipper::Adapters::Poll' do + dsl = described_class.new(adapter, memoize: :poll) + expect(dsl.adapter).to be_a(Flipper::Adapters::Poll) + expect(dsl.adapter.local).to be_a(Flipper::Adapters::Memory) + expect(dsl.adapter.remote).to be(adapter) + + expect(Flipper::Poller.get('memoizer')).to be(dsl.adapter.poller) + end + end + context 'when disabling memoization' do it 'uses the given adapter directly' do dsl = described_class.new(adapter, memoize: false) From 9ccc5e838cca87e2b02e45d9e0a469ced129ae8b Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Mon, 13 Mar 2023 10:35:54 -0400 Subject: [PATCH 3/7] Add Middleware::Sync to sync Poll adapter around requests --- lib/flipper.rb | 1 + lib/flipper/adapters/poll.rb | 7 ++++ lib/flipper/cloud/configuration.rb | 4 +- lib/flipper/dsl.rb | 6 ++- lib/flipper/middleware/sync.rb | 19 +++++++++ spec/flipper/middleware/sync_spec.rb | 62 ++++++++++++++++++++++++++++ 6 files changed, 96 insertions(+), 3 deletions(-) create mode 100644 lib/flipper/middleware/sync.rb create mode 100644 spec/flipper/middleware/sync_spec.rb diff --git a/lib/flipper.rb b/lib/flipper.rb index cbbf09852..d046c6129 100644 --- a/lib/flipper.rb +++ b/lib/flipper.rb @@ -156,6 +156,7 @@ def groups_registry=(registry) require 'flipper/identifier' require 'flipper/middleware/memoizer' require 'flipper/middleware/setup_env' +require 'flipper/middleware/sync' require 'flipper/poller' require 'flipper/registry' require 'flipper/type' diff --git a/lib/flipper/adapters/poll.rb b/lib/flipper/adapters/poll.rb index 38a02aad5..a96c80e10 100644 --- a/lib/flipper/adapters/poll.rb +++ b/lib/flipper/adapters/poll.rb @@ -30,6 +30,13 @@ def initialize(local, remote, key:, **options) end # Public: Synchronizes the local adapter with the current state of the remote adapter. + # If given a block, the adapter will be synced once and then not synced again for the + # duration of the block. + # + # poll = Flipper::Adapters::Poll.new(local, remote) + # poll.sync do + # # Long running operation that doesn't need to be synced + # end def sync if @sync_automatically poller_last_synced_at = @poller.last_synced_at.value diff --git a/lib/flipper/cloud/configuration.rb b/lib/flipper/cloud/configuration.rb index d20ac40b8..810274296 100644 --- a/lib/flipper/cloud/configuration.rb +++ b/lib/flipper/cloud/configuration.rb @@ -163,11 +163,11 @@ def dual_write_adapter end def poll_adapter - Flipper::Adapters::Poll.new(local_adapter, http_adapter, { + Flipper::Adapters::Poll.new(local_adapter, http_adapter, key: @url + @token, interval: sync_interval, instrumenter: instrumenter, - }) + ) end def http_adapter diff --git a/lib/flipper/dsl.rb b/lib/flipper/dsl.rb index 1d5484694..780a70282 100644 --- a/lib/flipper/dsl.rb +++ b/lib/flipper/dsl.rb @@ -22,7 +22,11 @@ def initialize(adapter, options = {}) @instrumenter = options.fetch(:instrumenter, Instrumenters::Noop) memoize = options.fetch(:memoize, true) @adapter = if memoize == :poll - Adapters::Poll.new(Adapters::Memory.new, adapter, key: 'memoizer', interval: 5).tap do |poll| + Adapters::Poll.new(Adapters::Memory.new, adapter, + key: 'memoizer', + interval: 5, + instrumenter: @instrumenter + ).tap do |poll| # Force poller to sync in current thread now poll.poller.sync end diff --git a/lib/flipper/middleware/sync.rb b/lib/flipper/middleware/sync.rb new file mode 100644 index 000000000..6a81fd99d --- /dev/null +++ b/lib/flipper/middleware/sync.rb @@ -0,0 +1,19 @@ +module Flipper + module Middleware + class Sync + def initialize(app, options = {}) + @app = app + @env_key = options.fetch(:env_key, 'flipper') + end + + def call(env) + flipper = env.fetch(@env_key) { Flipper } + if flipper.adapter.respond_to?(:sync) + flipper.adapter.sync { @app.call(env) } + else + @app.call(env) + end + end + end + end +end diff --git a/spec/flipper/middleware/sync_spec.rb b/spec/flipper/middleware/sync_spec.rb new file mode 100644 index 000000000..d53b6d344 --- /dev/null +++ b/spec/flipper/middleware/sync_spec.rb @@ -0,0 +1,62 @@ +require 'rack/test' +require 'active_support/cache' +require 'flipper/adapters/active_support_cache_store' +require 'flipper/adapters/operation_logger' + +RSpec.describe Flipper::Middleware::Sync do + include Rack::Test::Methods + + let(:memory_adapter) { Flipper::Adapters::Memory.new } + let(:adapter) do + Flipper::Adapters::OperationLogger.new(memory_adapter) + end + let(:env) { {} } + let(:app) { lambda { |_env| [200, {}, nil] } } + + subject do + described_class.new(app) + end + + RSpec.shared_examples_for 'sync middleware' do + it 'delegates to the app' do + expect(app).to receive(:call).and_call_original + subject.call(env) + end + + it 'calls #sync around the request' do + expect(flipper.adapter).to be_a(Flipper::Adapters::Poll) + expect(flipper.adapter).to receive(:sync).and_yield + subject.call(env) + end + end + + context 'when memoize: :poll' do + let(:flipper) { Flipper.new(adapter, memoize: :poll) } + + context 'with Flipper setup in env' do + let(:env) { { 'flipper' => flipper } } + + it_behaves_like 'sync middleware' + end + + context 'defaults to Flipper' do + before do + Flipper.configure do |config| + config.default { flipper } + end + end + + it_behaves_like 'sync middleware' + end + end + + context 'when memoize: false' do + let(:flipper) { Flipper.new(adapter, memoize: false) } + let(:env) { { 'flipper' => flipper } } + + it 'delegates to the app' do + expect(app).to receive(:call).and_call_original + subject.call(env) + end + end +end From 729f06be1210395e6ee2c5f58fea845ca5e4800d Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Mon, 13 Mar 2023 10:46:34 -0400 Subject: [PATCH 4/7] Specs for Poll adapter --- spec/flipper/adapters/poll_spec.rb | 66 ++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) create mode 100644 spec/flipper/adapters/poll_spec.rb diff --git a/spec/flipper/adapters/poll_spec.rb b/spec/flipper/adapters/poll_spec.rb new file mode 100644 index 000000000..fd794b48a --- /dev/null +++ b/spec/flipper/adapters/poll_spec.rb @@ -0,0 +1,66 @@ +require 'flipper/adapters/poll' +require 'flipper/adapters/operation_logger' +require 'active_support/notifications' + +RSpec.describe Flipper::Adapters::Poll do + let(:remote_adapter) do + Flipper::Adapters::OperationLogger.new Flipper::Adapters::Memory.new + end + let(:local_adapter) do + Flipper::Adapters::OperationLogger.new Flipper::Adapters::Memory.new + end + let(:local) { Flipper.new(local_adapter) } + let(:remote) { Flipper.new(remote_adapter) } + let(:poll) { Flipper.new(subject) } + + subject do + described_class.new(local_adapter, remote_adapter, key: 'test', start_automatically: false) + end + + it_should_behave_like 'a flipper adapter' + + it 'syncs features when poller has been synced' do + remote.enable(:search) + + subject.poller.sync # sync poller from remote + + expect(subject.poller.adapter).to receive(:get_all).and_call_original + expect(poll[:search].boolean_value).to be(true) + expect(subject.features.sort).to eq(%w(search)) + end + + it 'writes to both local and remote' do + poll.enable(:search) + + expect(local[:search].boolean_value).to be(true) + expect(remote[:search].boolean_value).to be(true) + end + + it 'does not sync features with poller has not been synced' do + # Perform initial sync + subject.poller.sync + subject.features + + # Remote feature enabled, but poller has not synced yet + remote.enable(:search) + expect(subject.poller.adapter).to_not receive(:get_all) + + expect(subject.features.sort).to eq(%w()) + end + + describe '#sync' do + it "performs initial sync and then does not sync during block" do + remote.enable(:search) + subject.poller.sync # Sync poller + + subject.sync do + expect(poll[:search].boolean_value).to be(true) + + remote.enable(:stats) + subject.poller.sync # Sync poller + + expect(poll[:stats].boolean_value).to be(false) + end + end + end +end From 61aeebc427cc819e245c135549c4413f3e11d26e Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Mon, 13 Mar 2023 12:17:40 -0400 Subject: [PATCH 5/7] Test that Sync middleware is used when memoize = :poll --- spec/flipper/railtie_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/flipper/railtie_spec.rb b/spec/flipper/railtie_spec.rb index ee1d6cf34..f7ebb2c0c 100644 --- a/spec/flipper/railtie_spec.rb +++ b/spec/flipper/railtie_spec.rb @@ -71,6 +71,11 @@ expect(subject.middleware).to include(Flipper::Middleware::Memoizer) end + it 'uses Sync middleware if config.memoize = :poll' do + initializer { config.memoize = :poll } + expect(subject.middleware).to include(Flipper::Middleware::Sync) + end + it 'does not use Memoizer middleware if config.memoize = false' do initializer { config.memoize = false } expect(subject.middleware).not_to include(Flipper::Middleware::Memoizer) From 97ce4d842fa07dd468c35244149c95e34f2da4c4 Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Mon, 13 Mar 2023 12:21:37 -0400 Subject: [PATCH 6/7] Use `memoize` option when using Cloud --- lib/flipper/cloud/configuration.rb | 4 ++++ lib/flipper/cloud/dsl.rb | 5 ++++- lib/flipper/cloud/engine.rb | 3 ++- spec/flipper/cloud/dsl_spec.rb | 23 +++++++++++++++++++++++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/lib/flipper/cloud/configuration.rb b/lib/flipper/cloud/configuration.rb index 810274296..d4d4052b9 100644 --- a/lib/flipper/cloud/configuration.rb +++ b/lib/flipper/cloud/configuration.rb @@ -40,6 +40,9 @@ def self.brow_instances # Public: net/http write timeout for all http requests (default: 5). attr_accessor :write_timeout + # Public: Memoize adapter operations. Defaults to true. + attr_accessor :memoize + # Public: IO stream to send debug output too. Off by default. # # # for example, this would send all http request information to STDOUT @@ -85,6 +88,7 @@ def initialize(options = {}) @sync_interval = options.fetch(:sync_interval) { ENV.fetch("FLIPPER_CLOUD_SYNC_INTERVAL", 10).to_f } @sync_secret = options.fetch(:sync_secret) { ENV["FLIPPER_CLOUD_SYNC_SECRET"] } @local_adapter = options.fetch(:local_adapter) { Adapters::Memory.new } + @memoize = options.fetch(:memoize, true) @debug_output = options[:debug_output] @adapter_block = ->(adapter) { adapter } self.url = options.fetch(:url) { ENV.fetch("FLIPPER_CLOUD_URL", DEFAULT_URL) } diff --git a/lib/flipper/cloud/dsl.rb b/lib/flipper/cloud/dsl.rb index c9ceb2c1c..3f165a8a7 100644 --- a/lib/flipper/cloud/dsl.rb +++ b/lib/flipper/cloud/dsl.rb @@ -7,7 +7,10 @@ class DSL < SimpleDelegator def initialize(cloud_configuration) @cloud_configuration = cloud_configuration - super Flipper.new(@cloud_configuration.adapter, instrumenter: @cloud_configuration.instrumenter) + super Flipper.new(@cloud_configuration.adapter, + instrumenter: @cloud_configuration.instrumenter, + memoize: @cloud_configuration.memoize + ) end def sync diff --git a/lib/flipper/cloud/engine.rb b/lib/flipper/cloud/engine.rb index 60f2636a8..cf4730ab0 100644 --- a/lib/flipper/cloud/engine.rb +++ b/lib/flipper/cloud/engine.rb @@ -15,7 +15,8 @@ class Engine < Rails::Engine if ENV["FLIPPER_CLOUD_TOKEN"] Flipper::Cloud.new( local_adapter: config.adapter, - instrumenter: app.config.flipper.instrumenter + instrumenter: app.config.flipper.instrumenter, + memoize: app.config.flipper.memoize ) else warn "Missing FLIPPER_CLOUD_TOKEN environment variable. Disabling Flipper::Cloud." diff --git a/spec/flipper/cloud/dsl_spec.rb b/spec/flipper/cloud/dsl_spec.rb index 9dec2ac27..ad829e0c9 100644 --- a/spec/flipper/cloud/dsl_spec.rb +++ b/spec/flipper/cloud/dsl_spec.rb @@ -79,4 +79,27 @@ expect(enable_stub).to have_been_requested end end + + context "when memoize = :poll" do + let(:local_adapter) do + Flipper::Adapters::OperationLogger.new Flipper::Adapters::Memory.new + end + + let(:cloud_configuration) do + cloud_configuration = Flipper::Cloud::Configuration.new({ + token: "asdf", + sync_secret: "tasty", + local_adapter: local_adapter, + memoize: :poll + }) + end + + subject do + described_class.new(cloud_configuration) + end + + it "uses a poll adaptor" do + expect(subject.adapter).to be_a(Flipper::Adapters::Poll) + end + end end From 2cc069f01208765253b4072d1822ed220afaccce Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Mon, 13 Mar 2023 18:29:37 -0400 Subject: [PATCH 7/7] Skip Memoizer middleware if adapter is not Memoizable --- lib/flipper/middleware/memoizer.rb | 6 +++++- spec/flipper/middleware/memoizer_spec.rb | 12 ++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/flipper/middleware/memoizer.rb b/lib/flipper/middleware/memoizer.rb index 2f01a43f9..7dcda4290 100644 --- a/lib/flipper/middleware/memoizer.rb +++ b/lib/flipper/middleware/memoizer.rb @@ -43,7 +43,11 @@ def call(env) private def memoize?(request) - if @opts[:if] + flipper = request.env.fetch(@env_key) { Flipper } + + if !flipper.adapter.is_a?(Flipper::Adapters::Memoizable) + return false + elsif @opts[:if] @opts[:if].call(request) elsif @opts[:unless] !@opts[:unless].call(request) diff --git a/spec/flipper/middleware/memoizer_spec.rb b/spec/flipper/middleware/memoizer_spec.rb index 2b317700e..8314ce5a6 100644 --- a/spec/flipper/middleware/memoizer_spec.rb +++ b/spec/flipper/middleware/memoizer_spec.rb @@ -1,5 +1,6 @@ require 'rack/test' require 'active_support/cache' +require 'active_support/notifications' require 'flipper/adapters/active_support_cache_store' require 'flipper/adapters/operation_logger' @@ -415,4 +416,15 @@ def get(uri, params = {}, env = {}, &block) expect(logged_memory.count(:get_all)).to be(1) end end + + context 'with memoize:false' do + let(:flipper) { Flipper.new(adapter, memoize: false) } + + it 'delegates to the app' do + app = lambda { |_env| [200, {}, nil] } + expect(app).to receive(:call).once.and_call_original + middleware = described_class.new(app) + middleware.call(env) + end + end end