From 7f6d66f45136ee95be6a465db563ab4add5757eb Mon Sep 17 00:00:00 2001 From: sighphyre Date: Tue, 14 Dec 2021 09:03:52 +0200 Subject: [PATCH] feat: Implement custom stickiness (#69) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat: Implement custom stickiness - Add custom stickiness for feature variants - Add custom stickiness for flexible rollout strategies Co-authored-by: Simon Hornby Co-authored-by: Renato Arruda Co-authored-by: Ivar Conradi Østhus --- .github/workflows/pull_request.yml | 2 +- lib/unleash/feature_toggle.rb | 15 ++- lib/unleash/strategy/flexible_rollout.rb | 10 +- lib/unleash/variant_definition.rb | 9 +- spec/unleash/client_spec.rb | 1 + spec/unleash/feature_toggle_spec.rb | 103 +++++++++++++++++- .../unleash/strategy/flexible_rollout_spec.rb | 32 ++++++ 7 files changed, 152 insertions(+), 20 deletions(-) diff --git a/.github/workflows/pull_request.yml b/.github/workflows/pull_request.yml index 2bad3040..09a80502 100644 --- a/.github/workflows/pull_request.yml +++ b/.github/workflows/pull_request.yml @@ -31,7 +31,7 @@ jobs: - name: Install dependencies run: bundle install - name: Download test cases - run: git clone --depth 5 --branch v3.3.0 https://github.com/Unleash/client-specification.git client-specification + run: git clone --depth 5 --branch v4.0.0 https://github.com/Unleash/client-specification.git client-specification - name: rubocop uses: reviewdog/action-rubocop@v2 with: diff --git a/lib/unleash/feature_toggle.rb b/lib/unleash/feature_toggle.rb index 443ddd78..a3dc4ede 100644 --- a/lib/unleash/feature_toggle.rb +++ b/lib/unleash/feature_toggle.rb @@ -40,8 +40,7 @@ def get_variant(context, fallback_variant = Unleash::FeatureToggle.disabled_vari return Unleash::FeatureToggle.disabled_variant unless self.enabled && am_enabled?(context, true) return Unleash::FeatureToggle.disabled_variant if sum_variant_defs_weights <= 0 - variant = variant_from_override_match(context) - variant = variant_from_weights(context) if variant.nil? + variant = variant_from_override_match(context) || variant_from_weights(context, resolve_stickiness) Unleash.toggle_metrics.increment_variant(self.name, variant.name) unless Unleash.configuration.disable_metrics variant @@ -53,6 +52,10 @@ def self.disabled_variant private + def resolve_stickiness + self.variant_definitions&.map(&:stickiness)&.compact&.first || "default" + end + # only check if it is enabled, do not do metrics def am_enabled?(context, default_result) result = @@ -85,7 +88,8 @@ def sum_variant_defs_weights self.variant_definitions.map(&:weight).reduce(0, :+) end - def variant_salt(context) + def variant_salt(context, stickiness = "default") + return context.get_by_name(stickiness) unless stickiness == "default" return context.user_id unless context.user_id.to_s.empty? return context.session_id unless context.session_id.to_s.empty? return context.remote_address unless context.remote_address.to_s.empty? @@ -100,8 +104,8 @@ def variant_from_override_match(context) Unleash::Variant.new(name: variant.name, enabled: true, payload: variant.payload) end - def variant_from_weights(context) - variant_weight = Unleash::Strategy::Util.get_normalized_number(variant_salt(context), self.name, sum_variant_defs_weights) + def variant_from_weights(context, stickiness) + variant_weight = Unleash::Strategy::Util.get_normalized_number(variant_salt(context, stickiness), self.name, sum_variant_defs_weights) prev_weights = 0 variant_definition = self.variant_definitions @@ -150,6 +154,7 @@ def initialize_variant_definitions(params) v.fetch('name', ''), v.fetch('weight', 0), v.fetch('payload', nil), + v.fetch('stickiness', nil), v.fetch('overrides', []) ) end || [] diff --git a/lib/unleash/strategy/flexible_rollout.rb b/lib/unleash/strategy/flexible_rollout.rb index f25196c9..871a81a2 100644 --- a/lib/unleash/strategy/flexible_rollout.rb +++ b/lib/unleash/strategy/flexible_rollout.rb @@ -38,16 +38,16 @@ def random def resolve_stickiness(stickiness, context) case stickiness - when 'userId' - context.user_id - when 'sessionId' - context.session_id when 'random' random when 'default' context.user_id || context.session_id || random else - nil + begin + context.get_by_name(stickiness) + rescue KeyError + nil + end end end end diff --git a/lib/unleash/variant_definition.rb b/lib/unleash/variant_definition.rb index a4c9910b..3491250e 100644 --- a/lib/unleash/variant_definition.rb +++ b/lib/unleash/variant_definition.rb @@ -2,13 +2,13 @@ module Unleash class VariantDefinition - attr_accessor :name, :weight, :payload, :overrides + attr_accessor :name, :weight, :payload, :overrides, :stickiness - def initialize(name, weight = 0, payload = nil, overrides = []) + def initialize(name, weight = 0, payload = nil, stickiness = nil, overrides = []) self.name = name self.weight = weight self.payload = payload - # self.overrides = overrides + self.stickiness = stickiness self.overrides = (overrides || []) .select{ |v| v.is_a?(Hash) && v.has_key?('contextName') } .map{ |v| VariantOverride.new(v.fetch('contextName', ''), v.fetch('values', [])) } || [] @@ -19,7 +19,8 @@ def override_matches_context?(context) end def to_s - "" + "" end end end diff --git a/spec/unleash/client_spec.rb b/spec/unleash/client_spec.rb index 95e7ad75..f64a4644 100644 --- a/spec/unleash/client_spec.rb +++ b/spec/unleash/client_spec.rb @@ -376,6 +376,7 @@ { name: "a", weight: 50, + stickiness: "default", payload: { type: "string", value: "" diff --git a/spec/unleash/feature_toggle_spec.rb b/spec/unleash/feature_toggle_spec.rb index b83982be..a17d7932 100644 --- a/spec/unleash/feature_toggle_spec.rb +++ b/spec/unleash/feature_toggle_spec.rb @@ -156,11 +156,13 @@ "variants" => [ { "name" => "variant1", - "weight" => 50 + "weight" => 50, + "stickiness" => "default" }, { "name" => "variant2", - "weight" => 50 + "weight" => 50, + "stickiness" => "default" } ], "createdAt" => "2019-01-24T10:41:45.236Z" @@ -207,15 +209,18 @@ "variants" => [ { "name" => "variantA", - "weight" => 0 + "weight" => 0, + "stickiness" => "default" }, { "name" => "variantB", - "weight" => 10 + "weight" => 10, + "stickiness" => "default" }, { "name" => "variantC", - "weight" => 20 + "weight" => 20, + "stickiness" => "default" } ], "createdAt" => "2019-01-24T10:41:45.236Z" @@ -303,6 +308,7 @@ { "name" => "variant1", "weight" => 50, + "stickiness" => "default", "payload" => { "type" => "string", "value" => "val1" @@ -315,6 +321,7 @@ { "name" => "variant2", "weight" => 50, + "stickiness" => "default", "payload" => { "type" => "string", "value" => "val2" @@ -528,4 +535,90 @@ expect(ret.name).to eq 'disabled' end end + + describe 'FeatureToggle with custom stickiness' do + let(:feature_toggle) do + Unleash::FeatureToggle.new( + "name" => "toggleName", + "description" => nil, + "enabled" => true, + "variants" => [ + { + "name" => "variant1", + "weight" => 25, + "stickiness" => "organization" + }, + { + "name" => "variant2", + "weight" => 25, + "stickiness" => "organization" + }, + { + "name" => "variant3", + "weight" => 25, + "stickiness" => "organization" + }, + { + "name" => "variant4", + "weight" => 25, + "stickiness" => "organization" + } + + ], + "createdAt" => "2019-01-24T10:41:45.236Z" + ) + end + + it 'should return variant1 organization 726' do + context = Unleash::Context.new( + properties: { + organization: '726' + } + ) + + expect(feature_toggle.get_variant(context)).to have_attributes( + name: "variant1", + enabled: true + ) + end + + it 'should return variant2 organization 48' do + context = Unleash::Context.new( + properties: { + organization: '48' + } + ) + + expect(feature_toggle.get_variant(context)).to have_attributes( + name: "variant2", + enabled: true + ) + end + + it 'should return variant3 organization 381' do + context = Unleash::Context.new( + properties: { + organization: '381' + } + ) + + expect(feature_toggle.get_variant(context)).to have_attributes( + name: "variant3", + enabled: true + ) + end + + it 'should return variant4 organization 222' do + context = Unleash::Context.new( + properties: { + organization: '222' + } + ) + + expect(feature_toggle.get_variant(context)).to have_attributes( + name: "variant4", + enabled: true + ) + end + end end diff --git a/spec/unleash/strategy/flexible_rollout_spec.rb b/spec/unleash/strategy/flexible_rollout_spec.rb index 97263bfb..73e6b25a 100644 --- a/spec/unleash/strategy/flexible_rollout_spec.rb +++ b/spec/unleash/strategy/flexible_rollout_spec.rb @@ -29,5 +29,37 @@ expect(strategy.is_enabled?(params.merge({ 'rollout' => 15 }), unleash_context)).to be_truthy expect(strategy.is_enabled?(params.merge({ 'rollout' => 16 }), unleash_context)).to be_truthy end + + it 'should be enabled when stickiness=customerId and customerId=61 and rollout=10' do + params = { + 'groupId' => 'Demo', + 'rollout' => 10, + 'stickiness' => 'customerId' + } + + custom_context = Unleash::Context.new( + properties: { + customer_id: '61' + } + ) + + expect(strategy.is_enabled?(params, custom_context)).to be_truthy + end + + it 'should be disabled when stickiness=customerId and customerId=63 and rollout=10' do + params = { + 'groupId' => 'Demo', + 'rollout' => 10, + 'stickiness' => 'customerId' + } + + custom_context = Unleash::Context.new( + properties: { + customer_id: '63' + } + ) + + expect(strategy.is_enabled?(params, custom_context)).to be_falsey + end end end