Skip to content

Commit

Permalink
variants in strategies passes tests
Browse files Browse the repository at this point in the history
Just ready for early review
  • Loading branch information
gardleopard committed Aug 22, 2023
1 parent 4e714a7 commit 617dacc
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 28 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,7 @@ You can also run `bin/console` for an interactive prompt that will allow you to
This SDK is also built against the Unleash Client Specification tests.
To run the Ruby SDK against this test suite, you'll need to have a copy on your machine, you can clone the repository directly using:

`git clone --depth 5 --branch v4.2.2 https://github.com/Unleash/client-specification.git client-specification`
`git clone --depth 5 --branch v4.3.1 https://github.com/Unleash/client-specification.git client-specification`

After doing this, `rake spec` will also run the client specification tests.

Expand Down
22 changes: 18 additions & 4 deletions lib/unleash/activation_strategy.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
module Unleash
class ActivationStrategy
attr_accessor :name, :params, :constraints, :disabled
attr_accessor :name, :params, :constraints, :disabled, :variants

def initialize(name, params, constraints = [])
def initialize(name, params, constraints = [], variants = [])
self.name = name
self.disabled = false

Expand All @@ -15,17 +15,31 @@ def initialize(name, params, constraints = [])
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})"
self.disabled = true
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
end

def matches_context?(context)
self.constraints.any?{ |c| c.matches_context? context }
self.constraints.any? { |c| c.matches_context? context }
end
end
end
59 changes: 37 additions & 22 deletions lib/unleash/feature_toggle.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ def get_variant(context, fallback_variant = Unleash::FeatureToggle.disabled_vari
context = ensure_valid_context(context)

toggle_enabled = am_enabled?(context)
variant = resolve_variant(context, toggle_enabled)

variants = am_enabled(context)[:variants]

variant = resolve_variant(context, toggle_enabled, variants)

choice = toggle_enabled ? :yes : :no
Unleash.toggle_metrics.increment_variant(self.name, choice, variant.name) unless Unleash.configuration.disable_metrics
Expand All @@ -51,33 +54,44 @@ def self.disabled_variant

private

def resolve_variant(context, toggle_enabled)
def resolve_variant(context, toggle_enabled, variants)
return Unleash::FeatureToggle.disabled_variant unless toggle_enabled
return Unleash::FeatureToggle.disabled_variant if sum_variant_defs_weights <= 0
return Unleash::FeatureToggle.disabled_variant if sum_variant_defs_weights(variants) <= 0

variant_from_override_match(context) || variant_from_weights(context, resolve_stickiness)
variant_from_override_match(context, variants) || variant_from_weights(context, resolve_stickiness(variants), variants)
end

def resolve_stickiness
self.variant_definitions&.map(&:stickiness)&.compact&.first || "default"
def resolve_stickiness(variants)
variants&.map(&:stickiness)&.compact&.first || "default"
end

# only check if it is enabled, do not do metrics
def am_enabled?(context)
result =
if self.enabled
self.strategies.empty? ||
self.strategies.any? do |s|
strategy_enabled?(s, context) && strategy_constraint_matches?(s, context)
end
am_enabled(context)[:result]
end

def am_enabled(context)
result = false
variants = self.variant_definitions
if self.enabled
if self.strategies.empty?
result = true
else
false
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
end
end

Unleash.logger.debug "Unleash::FeatureToggle (enabled:#{self.enabled} " \
"and Strategies combined with contraints returned #{result})"

result
{
result: result,
variants: variants
}
end

def strategy_enabled?(strategy, context)
Expand All @@ -92,8 +106,8 @@ def strategy_constraint_matches?(strategy, context)
strategy.constraints.empty? || strategy.constraints.all?{ |c| c.matches_context?(context) }
end

def sum_variant_defs_weights
self.variant_definitions.map(&:weight).reduce(0, :+)
def sum_variant_defs_weights(variants)
variants.map(&:weight).reduce(0, :+)
end

def variant_salt(context, stickiness = "default")
Expand All @@ -110,18 +124,18 @@ def variant_salt(context, stickiness = "default")
SecureRandom.random_number
end

def variant_from_override_match(context)
variant = self.variant_definitions.find{ |vd| vd.override_matches_context?(context) }
def variant_from_override_match(context, variants)
variant = variants.find{ |vd| vd.override_matches_context?(context) }
return nil if variant.nil?

Unleash::Variant.new(name: variant.name, enabled: true, payload: variant.payload)
end

def variant_from_weights(context, stickiness)
variant_weight = Unleash::Strategy::Util.get_normalized_number(variant_salt(context, stickiness), self.name, sum_variant_defs_weights)
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))
prev_weights = 0

variant_definition = self.variant_definitions
variant_definition = variants
.find do |v|
res = (prev_weights + v.weight >= variant_weight)
prev_weights += v.weight
Expand All @@ -148,7 +162,8 @@ def initialize_strategies(params, segment_map)
ActivationStrategy.new(
s['name'],
s['parameters'],
resolve_constraints(s, segment_map)
resolve_constraints(s, segment_map),
s['variants']
)
end || []
end
Expand Down
2 changes: 1 addition & 1 deletion spec/unleash/feature_toggle_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@
it 'get_variant_with_matching_override should for user_id:61' do
# NOTE: Use send method, as we are testing a private method
context = Unleash::Context.new(user_id: 61)
expect(feature_toggle.send(:variant_from_override_match, context)).to have_attributes(
expect(feature_toggle.send(:variant_from_override_match, context, feature_toggle.variant_definitions)).to have_attributes(
name: "variant1",
payload: { "type" => "string", "value" => "val1" }
)
Expand Down

0 comments on commit 617dacc

Please sign in to comment.