From 6f8f0f6a99379ff599a692e79a4fa5f76a52278a Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Sat, 16 Sep 2023 16:50:12 -0400 Subject: [PATCH 1/6] Add Strict adapter to ensure feature exists on get --- lib/flipper.rb | 1 + lib/flipper/adapters/strict.rb | 44 ++++++++++++++++++++++++++++ spec/flipper/adapters/strict_spec.rb | 28 ++++++++++++++++++ 3 files changed, 73 insertions(+) create mode 100644 lib/flipper/adapters/strict.rb create mode 100644 spec/flipper/adapters/strict_spec.rb diff --git a/lib/flipper.rb b/lib/flipper.rb index 8ede71fff..958617144 100644 --- a/lib/flipper.rb +++ b/lib/flipper.rb @@ -176,6 +176,7 @@ def groups_registry=(registry) require 'flipper/adapter' require 'flipper/adapters/memoizable' require 'flipper/adapters/memory' +require 'flipper/adapters/strict' require 'flipper/adapters/instrumented' require 'flipper/configuration' require 'flipper/dsl' diff --git a/lib/flipper/adapters/strict.rb b/lib/flipper/adapters/strict.rb new file mode 100644 index 000000000..65b03a04b --- /dev/null +++ b/lib/flipper/adapters/strict.rb @@ -0,0 +1,44 @@ +module Flipper + module Adapters + # An adapter that ensures a feature exists before checking it. + class Strict + extend Forwardable + include ::Flipper::Adapter + attr_reader :name, :adapter + + class NotFound < ::Flipper::Error + def initialize(name) + super "Could not find feature #{name.inspect}. Call `Flipper.add(#{name.inspect})` to create it." + end + end + + HANDLERS = { + raise: ->(feature) { raise NotFound.new(feature.key) }, + warn: ->(feature) { warn NotFound.new(feature.key).message }, + noop: ->(_) { }, + } + + DEFAULT_NOT_FOUND = + + def_delegators :@adapter, :features, :get, :get_multi, :get_all, :add, :remove, :clear, :enable, :disable + + def initialize(adapter, name: :strict, handler: :raise) + @name = name + @adapter = adapter + @handler = handler.is_a?(Symbol) ? HANDLERS.fetch(handler) : handler + end + + def get(feature) + assert_feature_exists(feature) + @adapter.get(feature) + end + + private + + def assert_feature_exists(feature) + @handler.call(feature) unless @adapter.features.include?(feature.key) + end + + end + end +end diff --git a/spec/flipper/adapters/strict_spec.rb b/spec/flipper/adapters/strict_spec.rb new file mode 100644 index 000000000..d5e5a4acc --- /dev/null +++ b/spec/flipper/adapters/strict_spec.rb @@ -0,0 +1,28 @@ +RSpec.describe Flipper::Adapters::Strict do + let(:flipper) { Flipper.new(subject) } + let(:feature) { flipper[:unknown] } + + it_should_behave_like 'a flipper adapter' do + subject { described_class.new(Flipper::Adapters::Memory.new, handler: :noop) } + end + + context "handler: :raise" do + subject { described_class.new(Flipper::Adapters::Memory.new, handler: :raise) } + + context "#get" do + it "raises an error for unknown feature" do + expect { subject.get(feature) }.to raise_error(Flipper::Adapters::Strict::NotFound) + end + end + end + + context "handler: :warn" do + subject { described_class.new(Flipper::Adapters::Memory.new, handler: :warn) } + + context "#get" do + it "raises an error for unknown feature" do + expect(silence { subject.get(feature) }).to match(/Could not find feature "unknown"/) + end + end + end +end From 486f97bafbfff1d47aac08cd23e245e2b0913dee Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Sat, 16 Sep 2023 19:38:59 -0400 Subject: [PATCH 2/6] Add get_multi, remove name param --- lib/flipper/adapters/strict.rb | 13 ++++++++----- spec/flipper/adapters/strict_spec.rb | 12 ++++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/flipper/adapters/strict.rb b/lib/flipper/adapters/strict.rb index 65b03a04b..e196ddffe 100644 --- a/lib/flipper/adapters/strict.rb +++ b/lib/flipper/adapters/strict.rb @@ -18,12 +18,10 @@ def initialize(name) noop: ->(_) { }, } - DEFAULT_NOT_FOUND = + def_delegators :@adapter, :features, :get_all, :add, :remove, :clear, :enable, :disable - def_delegators :@adapter, :features, :get, :get_multi, :get_all, :add, :remove, :clear, :enable, :disable - - def initialize(adapter, name: :strict, handler: :raise) - @name = name + def initialize(adapter, handler: :raise) + @name = :strict @adapter = adapter @handler = handler.is_a?(Symbol) ? HANDLERS.fetch(handler) : handler end @@ -33,6 +31,11 @@ def get(feature) @adapter.get(feature) end + def get_multi(features) + features.each { |feature| assert_feature_exists(feature) } + @adapter.get_multi(features) + end + private def assert_feature_exists(feature) diff --git a/spec/flipper/adapters/strict_spec.rb b/spec/flipper/adapters/strict_spec.rb index d5e5a4acc..e68496fc9 100644 --- a/spec/flipper/adapters/strict_spec.rb +++ b/spec/flipper/adapters/strict_spec.rb @@ -14,6 +14,12 @@ expect { subject.get(feature) }.to raise_error(Flipper::Adapters::Strict::NotFound) end end + + context "#get_multi" do + it "raises an error for unknown feature" do + expect { subject.get_multi([feature]) }.to raise_error(Flipper::Adapters::Strict::NotFound) + end + end end context "handler: :warn" do @@ -24,5 +30,11 @@ expect(silence { subject.get(feature) }).to match(/Could not find feature "unknown"/) end end + + context "#get_multi" do + it "raises an error for unknown feature" do + expect(silence { subject.get_multi([feature]) }).to match(/Could not find feature "unknown"/) + end + end end end From 23ee22b2e64eee3600051424a951169aa7eb6d69 Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Sat, 16 Sep 2023 21:56:16 -0400 Subject: [PATCH 3/6] Spec for proc handler --- spec/flipper/adapters/strict_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/spec/flipper/adapters/strict_spec.rb b/spec/flipper/adapters/strict_spec.rb index e68496fc9..6bd0b152d 100644 --- a/spec/flipper/adapters/strict_spec.rb +++ b/spec/flipper/adapters/strict_spec.rb @@ -37,4 +37,26 @@ end end end + + context "handler: Proc" do + let(:unknown_features) { [] } + subject do + described_class.new(Flipper::Adapters::Memory.new, handler: ->(feature) { unknown_features << feature.key}) + end + + + context "#get" do + it "raises an error 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 + subject.get_multi([flipper[:foo], flipper[:bar]]) + expect(unknown_features).to eq(["foo", "bar"]) + end + end + end end From f05194b5f05c41d20cea36be9a475078c7a8473e Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Sat, 16 Sep 2023 22:00:39 -0400 Subject: [PATCH 4/6] Example for strict adapter --- examples/strict.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 examples/strict.rb diff --git a/examples/strict.rb b/examples/strict.rb new file mode 100644 index 000000000..cbabdb2b5 --- /dev/null +++ b/examples/strict.rb @@ -0,0 +1,18 @@ +require 'bundler/setup' +require 'flipper' + +adapter = Flipper::Adapters::Strict.new(Flipper::Adapters::Memory.new) +flipper = Flipper.new(adapter) + +begin + puts "Checking :unknown_feature, which should raise an error." + flipper.enabled?(:unknown_feature) + warn "An error was not raised, but should have been" + exit 1 +rescue Flipper::Adapters::Strict::NotFound => exception + puts "Ok, the exepcted error was raised: #{exception.message}" +end + +puts "Flipper.add(:new_feature)" +flipper.add(:new_feature) +puts "Flipper.enabled?(:new_feature) => #{flipper.enabled?(:new_feature)}" From 0f92e8a8ed79266efcc23a79b6c4dd08be8744c4 Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Sat, 16 Sep 2023 22:11:30 -0400 Subject: [PATCH 5/6] Explose handler --- lib/flipper/adapters/strict.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/flipper/adapters/strict.rb b/lib/flipper/adapters/strict.rb index e196ddffe..6c9131068 100644 --- a/lib/flipper/adapters/strict.rb +++ b/lib/flipper/adapters/strict.rb @@ -4,7 +4,7 @@ module Adapters class Strict extend Forwardable include ::Flipper::Adapter - attr_reader :name, :adapter + attr_reader :name, :adapter, :handler class NotFound < ::Flipper::Error def initialize(name) From 944d1749745ea7245353e5004284e3f0d7f4a676 Mon Sep 17 00:00:00 2001 From: Brandon Keepers Date: Wed, 20 Sep 2023 11:59:14 -0400 Subject: [PATCH 6/6] Remove kwarg for Strict handler, use arg or block --- lib/flipper/adapters/strict.rb | 4 ++-- spec/flipper/adapters/strict_spec.rb | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/flipper/adapters/strict.rb b/lib/flipper/adapters/strict.rb index 6c9131068..bfdf048bc 100644 --- a/lib/flipper/adapters/strict.rb +++ b/lib/flipper/adapters/strict.rb @@ -20,10 +20,10 @@ def initialize(name) def_delegators :@adapter, :features, :get_all, :add, :remove, :clear, :enable, :disable - def initialize(adapter, handler: :raise) + def initialize(adapter, handler = nil, &block) @name = :strict @adapter = adapter - @handler = handler.is_a?(Symbol) ? HANDLERS.fetch(handler) : handler + @handler = block || HANDLERS.fetch(handler) end def get(feature) diff --git a/spec/flipper/adapters/strict_spec.rb b/spec/flipper/adapters/strict_spec.rb index 6bd0b152d..c17056d3a 100644 --- a/spec/flipper/adapters/strict_spec.rb +++ b/spec/flipper/adapters/strict_spec.rb @@ -3,11 +3,11 @@ let(:feature) { flipper[:unknown] } it_should_behave_like 'a flipper adapter' do - subject { described_class.new(Flipper::Adapters::Memory.new, handler: :noop) } + subject { described_class.new(Flipper::Adapters::Memory.new, :noop) } end - context "handler: :raise" do - subject { described_class.new(Flipper::Adapters::Memory.new, handler: :raise) } + context "handler = :raise" do + subject { described_class.new(Flipper::Adapters::Memory.new, :raise) } context "#get" do it "raises an error for unknown feature" do @@ -22,8 +22,8 @@ end end - context "handler: :warn" do - subject { described_class.new(Flipper::Adapters::Memory.new, handler: :warn) } + context "handler = :warn" do + subject { described_class.new(Flipper::Adapters::Memory.new, :warn) } context "#get" do it "raises an error for unknown feature" do @@ -38,10 +38,10 @@ end end - context "handler: Proc" do + context "handler = Block" do let(:unknown_features) { [] } subject do - described_class.new(Flipper::Adapters::Memory.new, handler: ->(feature) { unknown_features << feature.key}) + described_class.new(Flipper::Adapters::Memory.new) { |feature| unknown_features << feature.key} end