diff --git a/examples/group_dynamic_lookup.rb b/examples/group_dynamic_lookup.rb new file mode 100644 index 000000000..c31d857e8 --- /dev/null +++ b/examples/group_dynamic_lookup.rb @@ -0,0 +1,90 @@ +require File.expand_path('../example_setup', __FILE__) + +require 'flipper' +require 'flipper/adapters/memory' + +adapter = Flipper::Adapters::Memory.new +flipper = Flipper.new(adapter) +stats = flipper[:stats] + +# Register group +Flipper.register(:enabled_team_member) do |actor, context| + combos = context.actors_value.map { |flipper_id| flipper_id.split(":", 2) } + team_names = combos.select { |class_name, id| class_name == "Team" }.map { |class_name, id| id } + teams = team_names.map { |name| Team.find(name) } + teams.any? { |team| team.member?(actor) } +end + +# Some class that represents actor that will be trying to do something +class User + attr_reader :id + + def initialize(id) + @id = id + end + + def flipper_id + "User:#{@id}" + end +end + +class Team + attr_reader :name + + def self.all + @all ||= {} + end + + def self.find(name) + all.fetch(name.to_s) + end + + def initialize(name, members) + @name = name.to_s + @members = members + self.class.all[@name] = self + end + + def id + @name + end + + def member?(actor) + @members.map(&:id).include?(actor.id) + end + + def flipper_id + "Team:#{@name}" + end +end + +jnunemaker = User.new("jnunemaker") +jbarnette = User.new("jbarnette") +aroben = User.new("aroben") + +core_app = Team.new(:core_app, [jbarnette, jnunemaker]) +feature_flags = Team.new(:feature_flags, [aroben, jnunemaker]) + +stats.enable_actor jbarnette + +actors = [jbarnette, jnunemaker, aroben] + +actors.each do |actor| + if stats.enabled?(actor) + puts "stats are enabled for #{actor.id}" + else + puts "stats are NOT enabled for #{actor.id}" + end +end + +puts "enabling team_actor group" +stats.enable_actor core_app +stats.enable_group :enabled_team_member + +actors.each do |actor| + if stats.enabled?(actor) + puts "stats are enabled for #{actor.id}" + else + puts "stats are NOT enabled for #{actor.id}" + end +end diff --git a/lib/flipper/feature.rb b/lib/flipper/feature.rb index bc82afe7a..7170209e0 100644 --- a/lib/flipper/feature.rb +++ b/lib/flipper/feature.rb @@ -1,6 +1,7 @@ require 'flipper/errors' require 'flipper/type' require 'flipper/gate' +require 'flipper/feature_check_context' require 'flipper/gate_values' require 'flipper/instrumenters/noop' @@ -85,18 +86,19 @@ def remove def enabled?(thing = nil) instrument(:enabled?) { |payload| values = gate_values - - payload[:thing] = gate(:actor).wrap(thing) unless thing.nil? - - open_gate = gates.detect { |gate| - gate.open?(thing, values[gate.key], feature_name: @name) - } - - if open_gate.nil? - false - else + thing = gate(:actor).wrap(thing) unless thing.nil? + payload[:thing] = thing + context = FeatureCheckContext.new( + feature_name: @name, + values: values, + thing: thing, + ) + + if open_gate = gates.detect { |gate| gate.open?(context) } payload[:gate_name] = open_gate.name true + else + false end } end diff --git a/lib/flipper/feature_check_context.rb b/lib/flipper/feature_check_context.rb new file mode 100644 index 000000000..ed2f660a1 --- /dev/null +++ b/lib/flipper/feature_check_context.rb @@ -0,0 +1,44 @@ +module Flipper + class FeatureCheckContext + # Public: The name of the feature. + attr_reader :feature_name + + # Public: The GateValues instance that keeps track of the values for the + # gates for the feature. + attr_reader :values + + # Public: The thing we want to know if a feature is enabled for. + attr_reader :thing + + def initialize(options = {}) + @feature_name = options.fetch(:feature_name) + @values = options.fetch(:values) + @thing = options.fetch(:thing) + end + + # Public: Convenience method for groups value like Feature has. + def groups_value + values.groups + end + + # Public: Convenience method for actors value value like Feature has. + def actors_value + values.actors + end + + # Public: Convenience method for boolean value value like Feature has. + def boolean_value + values.boolean + end + + # Public: Convenience method for percentage of actors value like Feature has. + def percentage_of_actors_value + values.percentage_of_actors + end + + # Public: Convenience method for percentage of time value like Feature has. + def percentage_of_time_value + values.percentage_of_time + end + end +end diff --git a/lib/flipper/gates/actor.rb b/lib/flipper/gates/actor.rb index 9024e35e1..a85985ee8 100644 --- a/lib/flipper/gates/actor.rb +++ b/lib/flipper/gates/actor.rb @@ -22,12 +22,13 @@ def enabled?(value) # Internal: Checks if the gate is open for a thing. # # Returns true if gate open for thing, false if not. - def open?(thing, value, options = {}) - if thing.nil? + def open?(context) + value = context.values[key] + if context.thing.nil? false else - if protects?(thing) - actor = wrap(thing) + if protects?(context.thing) + actor = wrap(context.thing) enabled_actor_ids = value enabled_actor_ids.include?(actor.value) else diff --git a/lib/flipper/gates/boolean.rb b/lib/flipper/gates/boolean.rb index ead724b94..7aa2a1dc0 100644 --- a/lib/flipper/gates/boolean.rb +++ b/lib/flipper/gates/boolean.rb @@ -23,8 +23,8 @@ def enabled?(value) # # Returns true if explicitly set to true, false if explicitly set to false # or nil if not explicitly set. - def open?(thing, value, options = {}) - value + def open?(context) + context.values[key] end def wrap(thing) diff --git a/lib/flipper/gates/group.rb b/lib/flipper/gates/group.rb index 5575188d4..30f5c6930 100644 --- a/lib/flipper/gates/group.rb +++ b/lib/flipper/gates/group.rb @@ -22,14 +22,15 @@ def enabled?(value) # Internal: Checks if the gate is open for a thing. # # Returns true if gate open for thing, false if not. - def open?(thing, value, options = {}) - if thing.nil? + def open?(context) + value = context.values[key] + if context.thing.nil? false else value.any? { |name| begin group = Flipper.group(name) - group.match?(thing) + group.match?(context.thing, context) rescue GroupNotRegistered false end diff --git a/lib/flipper/gates/percentage_of_actors.rb b/lib/flipper/gates/percentage_of_actors.rb index a094e3a0a..7c07b8f76 100644 --- a/lib/flipper/gates/percentage_of_actors.rb +++ b/lib/flipper/gates/percentage_of_actors.rb @@ -24,12 +24,13 @@ def enabled?(value) # Internal: Checks if the gate is open for a thing. # # Returns true if gate open for thing, false if not. - def open?(thing, value, options = {}) - if Types::Actor.wrappable?(thing) - actor = Types::Actor.wrap(thing) - feature_name = options.fetch(:feature_name) - key = "#{feature_name}#{actor.value}" - Zlib.crc32(key) % 100 < value + def open?(context) + percentage = context.values[key] + + if Types::Actor.wrappable?(context.thing) + actor = Types::Actor.wrap(context.thing) + key = "#{context.feature_name}#{actor.value}" + Zlib.crc32(key) % 100 < percentage else false end diff --git a/lib/flipper/gates/percentage_of_time.rb b/lib/flipper/gates/percentage_of_time.rb index 8587f36cc..47de10aba 100644 --- a/lib/flipper/gates/percentage_of_time.rb +++ b/lib/flipper/gates/percentage_of_time.rb @@ -22,7 +22,8 @@ def enabled?(value) # Internal: Checks if the gate is open for a thing. # # Returns true if gate open for thing, false if not. - def open?(thing, value, options = {}) + def open?(context) + value = context.values[key] rand < (value / 100.0) end diff --git a/lib/flipper/types/group.rb b/lib/flipper/types/group.rb index 665065975..955ad6d83 100644 --- a/lib/flipper/types/group.rb +++ b/lib/flipper/types/group.rb @@ -11,11 +11,22 @@ def self.wrap(group_or_name) def initialize(name, &block) @name = name.to_sym @value = @name - @block = block + + if block_given? + @block = block + @single_argument = @block.arity == 1 + else + @block = lambda { |thing, context| false } + @single_argument = false + end end - def match?(*args) - @block.call(*args) + def match?(thing, context) + if @single_argument + @block.call(thing) + else + @block.call(thing, context) + end end end end diff --git a/spec/flipper/feature_check_context_spec.rb b/spec/flipper/feature_check_context_spec.rb new file mode 100644 index 000000000..6dc9d343b --- /dev/null +++ b/spec/flipper/feature_check_context_spec.rb @@ -0,0 +1,67 @@ +require 'helper' + +RSpec.describe Flipper::FeatureCheckContext do + let(:feature_name) { :new_profiles } + let(:values) { Flipper::GateValues.new({}) } + let(:thing) { Struct.new(:flipper_id).new("5") } + let(:options) { + { + feature_name: feature_name, + values: values, + thing: thing, + } + } + + it "initializes just fine" do + instance = described_class.new(options) + expect(instance.feature_name).to eq(feature_name) + expect(instance.values).to eq(values) + expect(instance.thing).to eq(thing) + end + + it "requires feature_name" do + options.delete(:feature_name) + expect { + described_class.new(options) + }.to raise_error(KeyError) + end + + it "requires values" do + options.delete(:values) + expect { + described_class.new(options) + }.to raise_error(KeyError) + end + + it "requires thing" do + options.delete(:thing) + expect { + described_class.new(options) + }.to raise_error(KeyError) + end + + it "knows actors_value" do + instance = described_class.new(options.merge(values: Flipper::GateValues.new({actors: Set["User:1"]}))) + expect(instance.actors_value).to eq(Set["User:1"]) + end + + it "knows groups_value" do + instance = described_class.new(options.merge(values: Flipper::GateValues.new({groups: Set["admins"]}))) + expect(instance.groups_value).to eq(Set["admins"]) + end + + it "knows boolean_value" do + instance = described_class.new(options.merge(values: Flipper::GateValues.new({boolean: true}))) + expect(instance.boolean_value).to eq(true) + end + + it "knows percentage_of_actors_value" do + instance = described_class.new(options.merge(values: Flipper::GateValues.new({percentage_of_actors: 14}))) + expect(instance.percentage_of_actors_value).to eq(14) + end + + it "knows percentage_of_time_value" do + instance = described_class.new(options.merge(values: Flipper::GateValues.new({percentage_of_time: 41}))) + expect(instance.percentage_of_time_value).to eq(41) + end +end diff --git a/spec/flipper/feature_spec.rb b/spec/flipper/feature_spec.rb index 56402ca65..d9d2671cf 100644 --- a/spec/flipper/feature_spec.rb +++ b/spec/flipper/feature_spec.rb @@ -606,7 +606,7 @@ context "with actor instance" do it "updates the gate values to include the actor" do actor = Struct.new(:flipper_id).new(5) - instance = Flipper::Types::Actor.wrap(actor) + instance = Flipper::Types::Actor.new(actor) expect(subject.gate_values.actors).to be_empty subject.enable_actor(instance) expect(subject.gate_values.actors).to eq(Set["5"]) diff --git a/spec/flipper/gates/boolean_spec.rb b/spec/flipper/gates/boolean_spec.rb index 3165a87b1..ffa032dd4 100644 --- a/spec/flipper/gates/boolean_spec.rb +++ b/spec/flipper/gates/boolean_spec.rb @@ -7,6 +7,14 @@ described_class.new } + def context(bool) + Flipper::FeatureCheckContext.new( + feature_name: feature_name, + values: Flipper::GateValues.new({boolean: bool}), + thing: Flipper::Types::Actor.new(Struct.new(:flipper_id).new(1)), + ) + end + describe "#enabled?" do context "for true value" do it "returns true" do @@ -24,13 +32,13 @@ describe "#open?" do context "for true value" do it "returns true" do - expect(subject.open?(Object.new, true, feature_name: feature_name)).to eq(true) + expect(subject.open?(context(true))).to be(true) end end context "for false value" do it "returns false" do - expect(subject.open?(Object.new, false, feature_name: feature_name)).to eq(false) + expect(subject.open?(context(false))).to be(false) end end end diff --git a/spec/flipper/gates/group_spec.rb b/spec/flipper/gates/group_spec.rb index 88f52c2b1..173cbda89 100644 --- a/spec/flipper/gates/group_spec.rb +++ b/spec/flipper/gates/group_spec.rb @@ -7,6 +7,14 @@ described_class.new } + def context(set) + Flipper::FeatureCheckContext.new( + feature_name: feature_name, + values: Flipper::GateValues.new({groups: set}), + thing: Flipper::Types::Actor.new(Struct.new(:flipper_id).new("5")), + ) + end + describe "#open?" do context "with a group in adapter, but not registered" do before do @@ -15,7 +23,7 @@ it "ignores group" do thing = Struct.new(:flipper_id).new('5') - expect(subject.open?(thing, Set[:newbs, :staff], feature_name: feature_name)).to eq(true) + expect(subject.open?(context(Set[:newbs, :staff]))).to be(true) end end @@ -26,7 +34,7 @@ it "raises error" do expect { - subject.open?(Object.new, Set[:stinkers], feature_name: feature_name) + subject.open?(context(Set[:stinkers])) }.to raise_error(NoMethodError) end end diff --git a/spec/flipper/gates/percentage_of_actors_spec.rb b/spec/flipper/gates/percentage_of_actors_spec.rb index 452e4e7c1..9f6600ad3 100644 --- a/spec/flipper/gates/percentage_of_actors_spec.rb +++ b/spec/flipper/gates/percentage_of_actors_spec.rb @@ -7,6 +7,14 @@ described_class.new } + def context(integer, feature = feature_name, thing = nil) + Flipper::FeatureCheckContext.new( + feature_name: feature, + values: Flipper::GateValues.new({percentage_of_actors: integer}), + thing: thing || Flipper::Types::Actor.new(Struct.new(:flipper_id).new(1)), + ) + end + describe "#open?" do context "when compared against two features" do let(:percentage) { 0.05 } @@ -19,12 +27,12 @@ let(:feature_one_enabled_actors) do gate = described_class.new - actors.select { |actor| gate.open? actor, percentage_as_integer, feature_name: :name_one } + actors.select { |actor| gate.open? context(percentage_as_integer, :name_one, actor) } end let(:feature_two_enabled_actors) do gate = described_class.new - actors.select { |actor| gate.open? actor, percentage_as_integer, feature_name: :name_two } + actors.select { |actor| gate.open? context(percentage_as_integer, :name_two, actor) } end it "does not enable both features for same set of actors" do diff --git a/spec/flipper/types/group_spec.rb b/spec/flipper/types/group_spec.rb index 20044e606..e6036972b 100644 --- a/spec/flipper/types/group_spec.rb +++ b/spec/flipper/types/group_spec.rb @@ -2,6 +2,8 @@ require 'flipper/types/group' RSpec.describe Flipper::Types::Group do + let(:fake_context) { double("FeatureCheckContext") } + subject do Flipper.register(:admins) { |actor| actor.admin? } end @@ -42,25 +44,48 @@ let(:non_admin_actor) { double('Actor', :admin? => false) } it "returns true if block matches" do - expect(subject.match?(admin_actor)).to eq(true) + expect(subject.match?(admin_actor, fake_context)).to eq(true) end it "returns false if block does not match" do - expect(subject.match?(non_admin_actor)).to eq(false) + expect(subject.match?(non_admin_actor, fake_context)).to eq(false) end it "returns true for truthy block results" do group = Flipper::Types::Group.new(:examples) do |actor| actor.email =~ /@example\.com/ end - expect(group.match?(double('Actor', :email => "foo@example.com"))).to be_truthy + expect(group.match?(double('Actor', :email => "foo@example.com"), fake_context)).to be_truthy end it "returns false for falsey block results" do group = Flipper::Types::Group.new(:examples) do |actor| nil end - expect(group.match?(double('Actor'))).to be_falsey + expect(group.match?(double('Actor'), fake_context)).to be_falsey + end + + it "can yield without context as block argument" do + context = Flipper::FeatureCheckContext.new( + feature_name: :my_feature, + values: Flipper::GateValues.new({}), + thing: Flipper::Types::Actor.new(Struct.new(:flipper_id).new(1)), + ) + group = Flipper.register(:group_with_context) { |actor| actor } + yielded_actor = group.match?(admin_actor, context) + expect(yielded_actor).to be(admin_actor) + end + + it "can yield with context as block argument" do + context = Flipper::FeatureCheckContext.new( + feature_name: :my_feature, + values: Flipper::GateValues.new({}), + thing: Flipper::Types::Actor.new(Struct.new(:flipper_id).new(1)), + ) + group = Flipper.register(:group_with_context) { |actor, context| [actor, context] } + yielded_actor, yielded_context = group.match?(admin_actor, context) + expect(yielded_actor).to be(admin_actor) + expect(yielded_context).to be(context) end end end