refactoring

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

lib/unleash/activation_strategy.rb

@@ -15,31 +15,35 @@ 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})"
         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
+      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

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

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

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