feat: strategy variants (#148)

* comply with client-specification 4.3.1
* fixes bug with group_id in variant resolving

.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:

README.md

@@ -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.
 

lib/unleash/activation_strategy.rb

@@ -1,8 +1,8 @@
 module Unleash
   class ActivationStrategy
-    attr_accessor :name, :params, :constraints, :disabled
+    attr_accessor :name, :params, :constraints, :disabled, :variant_definitions
 
-    def initialize(name, params, constraints = [])
+    def initialize(name, params, constraints = [], variant_definitions = [])
       self.name = name
       self.disabled = false
 
@@ -15,17 +15,30 @@ 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.all?{ |c| c.is_a?(Constraint) }
         self.constraints = constraints
       else
-        Unleash.logger.warn "Invalid constraints provided for ActivationStrategy (contraints: #{constraints})"
+        Unleash.logger.warn "Invalid constraints provided for ActivationStrategy (constraints: #{constraints})"
         self.disabled = true
         self.constraints = []
       end
+
+      self.variant_definitions = valid_variant_definitions(variant_definitions)
     end
 
     def matches_context?(context)
       self.constraints.any?{ |c| c.matches_context? context }
     end
+
+    private
+
+    def valid_variant_definitions(variant_definitions)
+      if variant_definitions.is_a?(Array) && variant_definitions.all?{ |variant_definition| variant_definition.is_a?(VariantDefinition) }
+        variant_definitions
+      else
+        Unleash.logger.warn "Invalid variant_definitions provided for ActivationStrategy (variant_definitions: #{variant_definitions})"
+        []
+      end
+    end
   end
 end

lib/unleash/feature_toggle.rb

@@ -9,6 +9,8 @@ module Unleash
   class FeatureToggle
     attr_accessor :name, :enabled, :strategies, :variant_definitions
 
+    FeatureEvaluationResult = Struct.new(:enabled?, :strategy)
+
     def initialize(params = {}, segment_map = {})
       params = {} if params.nil?
 
@@ -37,10 +39,13 @@ 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)
+      evaluation_result = evaluate(context)
+
+      group_id = evaluation_result[:strategy]&.params.to_h['groupId'] || self.name
+
+      variant = resolve_variant(context, evaluation_result, group_id)
 
-      choice = toggle_enabled ? :yes : :no
+      choice = evaluation_result[:enabled?] ? :yes : :no
       Unleash.toggle_metrics.increment_variant(self.name, choice, variant.name) unless Unleash.configuration.disable_metrics
       variant
     end
@@ -51,33 +56,40 @@ def self.disabled_variant
 
     private
 
-    def resolve_variant(context, toggle_enabled)
-      return Unleash::FeatureToggle.disabled_variant unless toggle_enabled
-      return Unleash::FeatureToggle.disabled_variant if sum_variant_defs_weights <= 0
+    def resolve_variant(context, evaluation_result, group_id)
+      variant_definitions = evaluation_result[:strategy]&.variant_definitions
+      variant_definitions = self.variant_definitions if variant_definitions.nil? || variant_definitions.empty?
+      return Unleash::FeatureToggle.disabled_variant unless evaluation_result[:enabled?]
+      return Unleash::FeatureToggle.disabled_variant if sum_variant_defs_weights(variant_definitions) <= 0
 
-      variant_from_override_match(context) || variant_from_weights(context, resolve_stickiness)
+      variant_from_override_match(context, variant_definitions) ||
+        variant_from_weights(context, resolve_stickiness(variant_definitions), variant_definitions, group_id)
     end
 
-    def resolve_stickiness
-      self.variant_definitions&.map(&:stickiness)&.compact&.first || "default"
+    def resolve_stickiness(variant_definitions)
+      variant_definitions&.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
+      evaluate(context)[:enabled?]
+    end
+
+    def evaluate(context)
+      evaluation_result =
+        if !self.enabled
+          FeatureEvaluationResult.new(false, nil)
+        elsif self.strategies.empty?
+          FeatureEvaluationResult.new(true, nil)
         else
-          false
+          strategy = self.strategies.find{ |s| strategy_enabled?(s, context) && strategy_constraint_matches?(s, context) }
+          FeatureEvaluationResult.new(!strategy.nil?, strategy)
         end
 
       Unleash.logger.debug "Unleash::FeatureToggle (enabled:#{self.enabled} " \
-        "and Strategies combined with contraints returned #{result})"
+        "and Strategies combined with contraints returned #{evaluation_result})"
 
-      result
+      evaluation_result
     end
 
     def strategy_enabled?(strategy, context)
@@ -92,8 +104,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(variant_definitions)
+      variant_definitions.map(&:weight).reduce(0, :+)
     end
 
     def variant_salt(context, stickiness = "default")
@@ -110,18 +122,22 @@ 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) }
-      return nil if variant.nil?
+    def variant_from_override_match(context, variant_definitions)
+      variant_definition = variant_definitions.find{ |vd| vd.override_matches_context?(context) }
+      return nil if variant_definition.nil?
 
-      Unleash::Variant.new(name: variant.name, enabled: true, payload: variant.payload)
+      Unleash::Variant.new(name: variant_definition.name, enabled: true, payload: variant_definition.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, variant_definitions, group_id)
+      variant_weight = Unleash::Strategy::Util.get_normalized_number(
+        variant_salt(context, stickiness),
+        group_id,
+        sum_variant_defs_weights(variant_definitions)
+      )
       prev_weights = 0
 
-      variant_definition = self.variant_definitions
+      variant_definition = variant_definitions
         .find do |v|
           res = (prev_weights + v.weight >= variant_weight)
           prev_weights += v.weight
@@ -148,11 +164,26 @@ def initialize_strategies(params, segment_map)
           ActivationStrategy.new(
             s['name'],
             s['parameters'],
-            resolve_constraints(s, segment_map)
+            resolve_constraints(s, segment_map),
+            resolve_variants(s)
           )
         end || []
     end
 
+    def resolve_variants(strategy)
+      strategy.fetch("variants", [])
+        .select{ |variant| variant.is_a?(Hash) && variant.has_key?("name") }
+        .map do |variant|
+          VariantDefinition.new(
+            variant.fetch("name", ""),
+            variant.fetch("weight", 0),
+            variant.fetch("payload", nil),
+            variant.fetch("stickiness", nil),
+            variant.fetch("overrides", [])
+          )
+        end
+    end
+
     def resolve_constraints(strategy, segment_map)
       segment_constraints = (strategy["segments"] || []).map do |segment_id|
         segment_map[segment_id]&.fetch("constraints")

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,6 +13,7 @@
     context 'with correct payload' do
       let(:params) { Hash.new(test: true) }
       let(:constraints) { [Unleash::Constraint.new("constraint_name", "IN", ["value"])] }
+      let(:variant_definitions) { [Unleash::VariantDefinition.new("variant_name")] }
 
       it 'initializes with correct attributes' do
         expect(Unleash.logger).to_not receive(:warn)
@@ -22,6 +24,43 @@
         expect(strategy.params).to eq params
         expect(strategy.constraints).to eq constraints
       end
+
+      it 'fallbacks to empty array if variant definitions are invalid' do
+        expect(Unleash.logger).to receive(:warn)
+
+        strategy = Unleash::ActivationStrategy.new(
+          name,
+          params,
+          constraints,
+          [variant_definitions.first, "I am not a valid variant definition"]
+        )
+
+        expect(strategy.variant_definitions).to eq []
+      end
+
+      it 'fallbacks to empty array if constraint definitions are invalid' do
+        expect(Unleash.logger).to receive(:warn)
+
+        strategy = Unleash::ActivationStrategy.new(
+          name,
+          params,
+          [constraints.first, "I am not a valid variant definition"],
+          variant_definitions
+        )
+
+        expect(strategy.constraints).to eq []
+      end
+
+      it 'initializes with variant definitions as well' do
+        expect(Unleash.logger).to_not receive(:warn)
+
+        strategy = Unleash::ActivationStrategy.new(name, params, constraints, variant_definitions)
+
+        expect(strategy.name).to eq name
+        expect(strategy.params).to eq params
+        expect(strategy.constraints).to eq constraints
+        expect(strategy.variant_definitions).to eq variant_definitions
+      end
     end
 
     context 'with incorrect payload' do

spec/unleash/feature_toggle_spec.rb

@@ -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" }
       )
@@ -660,4 +660,69 @@
       end
     end
   end
+
+  describe 'FeatureToggle strategy variant overrides feature variant' do
+    let(:feature_toggle) do
+      Unleash::FeatureToggle.new(
+        "name" => "Feature.strategy.variant.overrides.feature.variant",
+        "description" => "Strategy variants with feature variants",
+        "enabled" => true,
+        "strategies" => [
+          {
+            "name" => "flexibleRollout",
+            "parameters" => {
+              "rollout" => "100",
+              "stickiness" => "default",
+              "groupId" => "a"
+            },
+            "variants" => [
+              {
+                "name" => "variantName",
+                "weight" => 1,
+                "payload" => {
+                  "type" => "string",
+                  "value" => "variantValue"
+                }
+              }
+            ],
+            "constraints" => [
+              {
+                "contextName" => "environment",
+                "operator" => "IN",
+                "values" => [
+                  "dev"
+                ]
+              }
+            ]
+          }
+        ],
+        "variants" => [
+          {
+            "name" => "featureVariant",
+            "weight" => 1,
+            "payload" => {
+              "type" => "string",
+              "value" => "willBeIgnored"
+            }
+          }
+        ]
+      )
+    end
+
+    let(:expected_variant) do
+      {
+        name: "variantName",
+        enabled: true,
+        payload: {
+          "type" => "string",
+          "value" => "variantValue"
+        }
+      }
+    end
+
+    it 'should return the variant from the strategy' do
+      context = Unleash::Context.new({ environment: 'dev' })
+      expect(feature_toggle.get_variant(context)).to have_attributes(expected_variant)
+    end
+  end
 end