diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index eae02292..ebbe8952 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -50,7 +50,7 @@ jobs: - name: Install dependencies run: bundle install - name: Download test cases - run: git clone --depth 5 --branch v4.2.2 https://github.com/Unleash/client-specification.git client-specification + run: git clone --depth 5 --branch v4.3.1 https://github.com/Unleash/client-specification.git client-specification - name: Run tests run: bundle exec rake env: diff --git a/lib/unleash/activation_strategy.rb b/lib/unleash/activation_strategy.rb index 758611e2..80230b90 100644 --- a/lib/unleash/activation_strategy.rb +++ b/lib/unleash/activation_strategy.rb @@ -15,7 +15,7 @@ def initialize(name, params, constraints = [], variants = []) self.params = {} end - if constraints.is_a?(Array) && constraints.each { |c| c.is_a?(Constraint) } + if constraints.is_a?(Array) && constraints.each{ |c| c.is_a?(Constraint) } self.constraints = constraints else Unleash.logger.warn "Invalid constraints provided for ActivationStrategy (contraints: #{constraints})" @@ -23,23 +23,27 @@ def initialize(name, params, constraints = [], variants = []) self.constraints = [] end - if variants.is_a?(Array) - self.variants = variants - .select { |v| v.is_a?(Hash) && v.has_key?("name") } - .map { |v| - VariantDefinition.new( - v.fetch("name", ""), - v.fetch("weight", 0), - v.fetch("payload", nil), - v.fetch("stickiness", nil), - v.fetch("overrides", []) - ) - } - end + self.variants = initialize_variant_definitions(variants) if variants.is_a?(Array) end def matches_context?(context) - self.constraints.any? { |c| c.matches_context? context } + self.constraints.any?{ |c| c.matches_context? context } + end + + private + + def initialize_variant_definitions(variants) + variants + .select{ |v| v.is_a?(Hash) && v.has_key?("name") } + .map do |v| + VariantDefinition.new( + v.fetch("name", ""), + v.fetch("weight", 0), + v.fetch("payload", nil), + v.fetch("stickiness", nil), + v.fetch("overrides", []) + ) + end end end end diff --git a/lib/unleash/feature_toggle.rb b/lib/unleash/feature_toggle.rb index 4d0d0881..88bd72dc 100644 --- a/lib/unleash/feature_toggle.rb +++ b/lib/unleash/feature_toggle.rb @@ -39,9 +39,12 @@ def get_variant(context, fallback_variant = Unleash::FeatureToggle.disabled_vari toggle_enabled = am_enabled?(context) - variants = am_enabled(context)[:variants] + strategy = am_enabled(context)[:strategy] - variant = resolve_variant(context, toggle_enabled, variants) + group_id = strategy&.params&.fetch('groupId', self.name) || self.name + variants = strategy&.variants || self.variant_definitions + + variant = resolve_variant(context, toggle_enabled, variants, group_id) choice = toggle_enabled ? :yes : :no Unleash.toggle_metrics.increment_variant(self.name, choice, variant.name) unless Unleash.configuration.disable_metrics @@ -54,11 +57,11 @@ def self.disabled_variant private - def resolve_variant(context, toggle_enabled, variants) + def resolve_variant(context, toggle_enabled, variants, group_id) return Unleash::FeatureToggle.disabled_variant unless toggle_enabled return Unleash::FeatureToggle.disabled_variant if sum_variant_defs_weights(variants) <= 0 - variant_from_override_match(context, variants) || variant_from_weights(context, resolve_stickiness(variants), variants) + variant_from_override_match(context, variants) || variant_from_weights(context, resolve_stickiness(variants), variants, group_id) end def resolve_stickiness(variants) @@ -71,27 +74,25 @@ def am_enabled?(context) end def am_enabled(context) - result = false - variants = self.variant_definitions - if self.enabled - if self.strategies.empty? - result = true - else - strategy = self.strategies.find(proc {false}){|s| (strategy_enabled?(s, context) && strategy_constraint_matches?(s, context))} - if strategy - variants = strategy.variants if strategy.variants - result = true - end + returnable = { + result: false, + strategy: nil + } + return returnable unless self.enabled + + if self.strategies.empty? + returnable[:result] = true + else + returnable[:strategy] = self.strategies.find(proc{ nil }) do |s| + (strategy_enabled?(s, context) && strategy_constraint_matches?(s, context)) end + returnable[:result] = true if returnable[:strategy] end Unleash.logger.debug "Unleash::FeatureToggle (enabled:#{self.enabled} " \ - "and Strategies combined with contraints returned #{result})" + "and Strategies combined with contraints returned #{returnable[:result]})" - { - result: result, - variants: variants - } + returnable end def strategy_enabled?(strategy, context) @@ -131,8 +132,11 @@ def variant_from_override_match(context, variants) Unleash::Variant.new(name: variant.name, enabled: true, payload: variant.payload) end - def variant_from_weights(context, stickiness, variants) - variant_weight = Unleash::Strategy::Util.get_normalized_number(variant_salt(context, stickiness), self.name, sum_variant_defs_weights(variants)) + def variant_from_weights(context, stickiness, variants, group_id) + variant_weight = Unleash::Strategy::Util.get_normalized_number( + variant_salt(context, stickiness), group_id, + sum_variant_defs_weights(variants) + ) prev_weights = 0 variant_definition = variants diff --git a/lib/unleash/variant_definition.rb b/lib/unleash/variant_definition.rb index d9cecad0..25a2593a 100644 --- a/lib/unleash/variant_definition.rb +++ b/lib/unleash/variant_definition.rb @@ -2,8 +2,17 @@ module Unleash class VariantDefinition + include Comparable attr_accessor :name, :weight, :payload, :overrides, :stickiness + def ==(other) + self.name == other.name && + self.weight == other.weight && + self.payload == other.payload && + self.stickiness == other.stickiness && + self.overrides == other.overrides + end + def initialize(name, weight = 0, payload = nil, stickiness = nil, overrides = []) # rubocop:disable Metrics/ParameterLists self.name = name self.weight = weight diff --git a/spec/unleash/activation_strategy_spec.rb b/spec/unleash/activation_strategy_spec.rb index 812dbe08..82f0170e 100644 --- a/spec/unleash/activation_strategy_spec.rb +++ b/spec/unleash/activation_strategy_spec.rb @@ -1,4 +1,5 @@ require 'unleash/constraint' +require 'unleash/variant_definition' RSpec.describe Unleash::ActivationStrategy do before do @@ -12,15 +13,17 @@ context 'with correct payload' do let(:params) { Hash.new(test: true) } let(:constraints) { [Unleash::Constraint.new("constraint_name", "IN", ["value"])] } + let(:variants) { [Unleash::VariantDefinition.new("variant_name")] } it 'initializes with correct attributes' do expect(Unleash.logger).to_not receive(:warn) - strategy = Unleash::ActivationStrategy.new(name, params, constraints) + strategy = Unleash::ActivationStrategy.new(name, params, constraints, [{ "name" => "variant_name" }]) expect(strategy.name).to eq name expect(strategy.params).to eq params expect(strategy.constraints).to eq constraints + expect(strategy.variants).to eq variants end end