-
-
Notifications
You must be signed in to change notification settings - Fork 432
Poll Memoizer #704
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Poll Memoizer #704
Changes from all commits
db480ca
da51be8
9ccc5e8
729f06b
61aeebc
97ce4d8
2cc069f
901d308
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,8 +21,21 @@ 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', | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If multiple
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't the key just be unique per adapter?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or rather a better way to phrase that, is there a way to make the key unique per instance? We have the name. That would help with different adapters. Perhaps there is another way to add uniqueness per instance too... hmm |
||
| interval: 5, | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FIXME: make configurable |
||
| instrumenter: @instrumenter | ||
| ).tap do |poll| | ||
| # Force poller to sync in current thread now | ||
| poll.poller.sync | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd be tempted to make a method for this to avoid reaching into poller. |
||
| end | ||
| elsif memoize | ||
| Adapters::Memoizable.new(adapter) | ||
| else | ||
| adapter | ||
| end | ||
|
|
||
| @memoized_features = {} | ||
| end | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
Uh oh!
There was an error while loading. Please reload this page.