From 5fddde582ef0b43d8886ea451f5ae203eac7c9c5 Mon Sep 17 00:00:00 2001 From: Christopher Kolstad Date: Wed, 8 May 2024 15:43:08 +0200 Subject: [PATCH] chore: Use client spec 5.1.5 instead of 5.0.2 (#242) * chore: Use client spec 5.1.5 instead of 5.0.2 * fix: inherit strategy stickiness setting for variants --- pom.xml | 2 +- .../strategy/FlexibleRolloutStrategy.java | 4 - .../java/io/getunleash/strategy/Strategy.java | 13 +- .../io/getunleash/variant/VariantUtil.java | 20 ++- .../strategy/StrategyVariantTest.java | 128 ++++++++++++++++++ 5 files changed, 159 insertions(+), 8 deletions(-) create mode 100644 src/test/java/io/getunleash/strategy/StrategyVariantTest.java diff --git a/pom.xml b/pom.xml index 0d5b10517..b95cd793b 100644 --- a/pom.xml +++ b/pom.xml @@ -11,7 +11,7 @@ 5.10.0 4.10.0 UTF-8 - 5.0.2 + 5.1.5 2.14.3 diff --git a/src/main/java/io/getunleash/strategy/FlexibleRolloutStrategy.java b/src/main/java/io/getunleash/strategy/FlexibleRolloutStrategy.java index 40fbc44ab..5c951869c 100644 --- a/src/main/java/io/getunleash/strategy/FlexibleRolloutStrategy.java +++ b/src/main/java/io/getunleash/strategy/FlexibleRolloutStrategy.java @@ -59,8 +59,4 @@ public boolean isEnabled(Map parameters, UnleashContext unleashC .map(norm -> percentage > 0 && norm <= percentage) .orElse(false); } - - private String getStickiness(Map parameters) { - return parameters.getOrDefault("stickiness", "default"); - } } diff --git a/src/main/java/io/getunleash/strategy/Strategy.java b/src/main/java/io/getunleash/strategy/Strategy.java index fe940303b..e0e2fe7b7 100644 --- a/src/main/java/io/getunleash/strategy/Strategy.java +++ b/src/main/java/io/getunleash/strategy/Strategy.java @@ -21,9 +21,13 @@ default FeatureEvaluationResult getResult( List constraints, @Nullable List variants) { boolean enabled = isEnabled(parameters, unleashContext, constraints); + String strategyStickiness = getStickiness(parameters); return new FeatureEvaluationResult( enabled, - enabled ? VariantUtil.selectVariant(parameters, variants, unleashContext) : null); + enabled + ? VariantUtil.selectVariant( + parameters, variants, unleashContext, strategyStickiness) + : null); } /** @@ -61,4 +65,11 @@ default boolean isEnabled( return ConstraintUtil.validate(constraints, unleashContext) && isEnabled(parameters, unleashContext); } + + default String getStickiness(@Nullable Map parameters) { + if (parameters != null) { + return parameters.getOrDefault("stickiness", "default"); + } + return null; + } } diff --git a/src/main/java/io/getunleash/variant/VariantUtil.java b/src/main/java/io/getunleash/variant/VariantUtil.java index 4e3796bf7..dcd6ce925 100644 --- a/src/main/java/io/getunleash/variant/VariantUtil.java +++ b/src/main/java/io/getunleash/variant/VariantUtil.java @@ -95,7 +95,8 @@ public static Variant selectVariant( public static @Nullable Variant selectVariant( Map parameters, @Nullable List variants, - UnleashContext context) { + UnleashContext context, + @Nullable String strategyStickiness) { if (variants != null) { int totalWeight = variants.stream().mapToInt(VariantDefinition::getWeight).sum(); if (totalWeight <= 0) { @@ -106,7 +107,7 @@ public static Variant selectVariant( return variantOverride.get().toVariant(); } - Optional customStickiness = + Optional variantCustomStickiness = variants.stream() .filter( f -> @@ -114,6 +115,14 @@ public static Variant selectVariant( && !"default".equals(f.getStickiness())) .map(VariantDefinition::getStickiness) .findFirst(); + Optional customStickiness; + if (!variantCustomStickiness.isPresent()) { + customStickiness = + Optional.ofNullable(strategyStickiness) + .filter(stickiness -> !stickiness.equalsIgnoreCase("default")); + } else { + customStickiness = variantCustomStickiness; + } int target = StrategyUtils.getNormalizedNumber( getSeed(context, customStickiness), @@ -134,6 +143,13 @@ public static Variant selectVariant( return null; } + public static @Nullable Variant selectVariant( + Map parameters, + @Nullable List variants, + UnleashContext context) { + return selectVariant(parameters, variants, context, null); + } + /** * Uses the old pre 9.0.0 way of hashing for finding the Variant to return * diff --git a/src/test/java/io/getunleash/strategy/StrategyVariantTest.java b/src/test/java/io/getunleash/strategy/StrategyVariantTest.java new file mode 100644 index 000000000..6508c6238 --- /dev/null +++ b/src/test/java/io/getunleash/strategy/StrategyVariantTest.java @@ -0,0 +1,128 @@ +package io.getunleash.strategy; + +import static org.assertj.core.api.Assertions.assertThat; + +import io.getunleash.FeatureEvaluationResult; +import io.getunleash.UnleashContext; +import io.getunleash.Variant; +import io.getunleash.variant.Payload; +import io.getunleash.variant.VariantDefinition; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import org.junit.jupiter.api.Test; + +public class StrategyVariantTest { + + @Test + public void should_inherit_stickiness_from_the_strategy_first_variant() { + HashMap params = new HashMap<>(); + params.put("rollout", "100"); + params.put("stickiness", "clientId"); + params.put("groupId", "a"); + FlexibleRolloutStrategy strategy = new FlexibleRolloutStrategy(); + VariantDefinition varA = + new VariantDefinition( + "variantNameA", + 1, + new Payload("string", "variantValueA"), + Collections.emptyList()); + VariantDefinition varB = + new VariantDefinition( + "variantNameB", + 1, + new Payload("string", "variantValueB"), + Collections.emptyList()); + + UnleashContext context = UnleashContext.builder().addProperty("clientId", "1").build(); + FeatureEvaluationResult result = + strategy.getResult( + params, context, Collections.emptyList(), Arrays.asList(varA, varB)); + Variant selectedVariant = result.getVariant(); + assert selectedVariant != null; + assertThat(selectedVariant.getName()).isEqualTo("variantNameA"); + } + + @Test + public void should_inherit_stickiness_from_the_strategy_second_variant() { + HashMap params = new HashMap<>(); + params.put("rollout", "100"); + params.put("stickiness", "clientId"); + params.put("groupId", "a"); + FlexibleRolloutStrategy strategy = new FlexibleRolloutStrategy(); + VariantDefinition varA = + new VariantDefinition( + "variantNameA", + 1, + new Payload("string", "variantValueA"), + Collections.emptyList()); + VariantDefinition varB = + new VariantDefinition( + "variantNameB", + 1, + new Payload("string", "variantValueB"), + Collections.emptyList()); + + UnleashContext context = UnleashContext.builder().addProperty("clientId", "2").build(); + FeatureEvaluationResult result = + strategy.getResult( + params, context, Collections.emptyList(), Arrays.asList(varA, varB)); + Variant selectedVariant = result.getVariant(); + assert selectedVariant != null; + assertThat(selectedVariant.getName()).isEqualTo("variantNameB"); + } + + @Test + public void multiple_variants_should_choose_first_variant() { + HashMap params = new HashMap<>(); + params.put("rollout", "100"); + params.put("groupId", "a"); + params.put("stickiness", "default"); + FlexibleRolloutStrategy strategy = new FlexibleRolloutStrategy(); + VariantDefinition varA = + new VariantDefinition( + "variantNameA", + 1, + new Payload("string", "variantValueA"), + Collections.emptyList()); + VariantDefinition varB = + new VariantDefinition( + "variantNameB", + 1, + new Payload("string", "variantValueB"), + Collections.emptyList()); + UnleashContext context = UnleashContext.builder().userId("5").build(); + FeatureEvaluationResult result = + strategy.getResult( + params, context, Collections.emptyList(), Arrays.asList(varA, varB)); + Variant selectedVariant = result.getVariant(); + assertThat(selectedVariant.getName()).isEqualTo("variantNameA"); + } + + @Test + public void multiple_variants_should_choose_second_variant() { + HashMap params = new HashMap<>(); + params.put("rollout", "100"); + params.put("groupId", "a"); + params.put("stickiness", "default"); + FlexibleRolloutStrategy strategy = new FlexibleRolloutStrategy(); + VariantDefinition varA = + new VariantDefinition( + "variantNameA", + 1, + new Payload("string", "variantValueA"), + Collections.emptyList()); + VariantDefinition varB = + new VariantDefinition( + "variantNameB", + 1, + new Payload("string", "variantValueB"), + Collections.emptyList()); + UnleashContext context = UnleashContext.builder().userId("0").build(); + FeatureEvaluationResult result = + strategy.getResult( + params, context, Collections.emptyList(), Arrays.asList(varA, varB)); + Variant selectedVariant = result.getVariant(); + assertThat(selectedVariant.getName()).isEqualTo("variantNameB"); + } +}