From 686a1174149971d59e3f2aa4fa474e640752e790 Mon Sep 17 00:00:00 2001 From: Stephen Crosby Date: Fri, 27 Sep 2024 19:14:43 -0700 Subject: [PATCH 1/3] Use reject(&:nil?) in enabled? check Rejecting nil has the advantage of allowing use of null object patterns using objects that implement nil? but are not strictly nil. This also works well with the 'actor cannot be nil' guard clause which uses `nil?` to check nilness. --- lib/flipper/feature.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/flipper/feature.rb b/lib/flipper/feature.rb index f446f921e..e734e32f9 100644 --- a/lib/flipper/feature.rb +++ b/lib/flipper/feature.rb @@ -100,7 +100,7 @@ def clear # # Returns true if enabled, false if not. def enabled?(*actors) - actors = actors.flatten.compact.map { |actor| Types::Actor.wrap(actor) } + actors = actors.flatten.reject(&:nil?).map { |actor| Types::Actor.wrap(actor) } actors = nil if actors.empty? # thing is left for backwards compatibility From e29a800b9bbf36c63c7b8cf425790fad7ae685c5 Mon Sep 17 00:00:00 2001 From: John Nunemaker Date: Thu, 7 Nov 2024 12:53:50 -0500 Subject: [PATCH 2/3] Add comment about null object pattern --- lib/flipper/feature.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/flipper/feature.rb b/lib/flipper/feature.rb index e734e32f9..2a63f4311 100644 --- a/lib/flipper/feature.rb +++ b/lib/flipper/feature.rb @@ -100,6 +100,8 @@ def clear # # Returns true if enabled, false if not. def enabled?(*actors) + # Allows null object pattern. See PR for more. + # https://github.com/flippercloud/flipper/pull/887 actors = actors.flatten.reject(&:nil?).map { |actor| Types::Actor.wrap(actor) } actors = nil if actors.empty? From d91f5f357fb1061aeb567df6b1ce0d57f928be06 Mon Sep 17 00:00:00 2001 From: Stephen Crosby Date: Thu, 7 Nov 2024 13:53:09 -0800 Subject: [PATCH 3/3] Add test for actor implementing .nil? --- spec/flipper/feature_spec.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/spec/flipper/feature_spec.rb b/spec/flipper/feature_spec.rb index d583af0e9..84c7fb75e 100644 --- a/spec/flipper/feature_spec.rb +++ b/spec/flipper/feature_spec.rb @@ -76,6 +76,26 @@ expect(subject.enabled?(actors)).to be(false) end end + + context "for an object that implements .nil? == true" do + let(:actor) { Flipper::Actor.new("User;1") } + + before do + def actor.nil? + true + end + end + + it 'returns true if feature is enabled' do + subject.enable + expect(subject.enabled?(actor)).to be(true) + end + + it 'returns false if feature is disabled' do + subject.disable + expect(subject.enabled?(actor)).to be(false) + end + end end describe '#to_s' do