Repository: Unleash/unleash-client-java
Pull request #205 (2023-07-20 13:10:00)

feat: strategy variants
<!-- Thanks for creating a PR! To make it easier for reviewers and everyone else to understand what your changes relate to, please add some relevant content to the headings below. Feel free to ignore or delete sections that you don't think are relevant. Thank you! ❤️ -->
Implements strategy variants based on the client-specifications v4.3.0
## About the changes
<!-- Describe the changes introduced. What are they and why are they being introduced? Feel free to also add screenshots or steps to view the changes if they're visual. -->

<!-- Does it close an issue? Multiple? -->
Closes #

<!-- (For internal contributors): Does it relate to an issue on public roadmap? -->
<!--
Relates to [roadmap](https://github.com/orgs/Unleash/projects/10) item: #
-->

### Important files
<!-- PRs can contain a lot of changes, but not all changes are equally important. Where should a reviewer start looking to get an overview of the changes? Are any files particularly important?  -->


## Discussion points
<!-- Anything about the PR you'd like to discuss before it gets merged? Got any questions or doubts? -->


sonatype-lift[bot]: ## Sonatype Lift is retiring
Sonatype Lift will be retiring on <b>Sep 12, 2023</b>, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console.
We are extremely grateful and thank you for your support over the years.

<b>[📖 Read about the impacts and timeline](https://www.sonatype.com/lift-retirement)</b>
sonatype-lift[bot]: ## 🛠 Lift Auto-fix
Some of the Lift findings in this PR can be automatically fixed. You can download and apply these changes in your local project directory of your branch to review the suggestions before committing.[^1]
```bash
# Download the patch
curl https://lift.sonatype.com/api/patch/github.com/Unleash/unleash-client-java/205.diff -o lift-autofixes.diff

# Apply the patch with git
git apply lift-autofixes.diff

# Review the changes
git diff
```
Want it all in a single command? Open a terminal in your project's directory and copy and paste the following command:
```bash
curl https://lift.sonatype.com/api/patch/github.com/Unleash/unleash-client-java/205.diff | git apply
```
Once you're satisfied, commit and push your changes in your project.
[^1]: You can preview the patch by opening the [patch URL](https://lift.sonatype.com/api/patch/github.com/Unleash/unleash-client-java/205.diff) in the browser.

Patch:
diff --git a/pom.xml b/pom.xml
index 90ab4438..e395cc23 100644
--- a/pom.xml
+++ b/pom.xml
@@ -11,7 +11,7 @@
         <version.junit5>5.9.0</version.junit5>
         <version.okhttp>4.10.0</version.okhttp>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
-        <version.unleash.specification>4.2.2</version.unleash.specification>
+        <version.unleash.specification>4.3.0</version.unleash.specification>
         <arguments />
         <version.jackson>2.14.0</version.jackson>
     </properties>
diff --git a/src/main/java/io/getunleash/ActivationStrategy.java b/src/main/java/io/getunleash/ActivationStrategy.java
index a08c7fc2..72b5de98 100644
--- a/src/main/java/io/getunleash/ActivationStrategy.java
+++ b/src/main/java/io/getunleash/ActivationStrategy.java
@@ -1,6 +1,7 @@
 package io.getunleash;
 
 import io.getunleash.lang.Nullable;
+import io.getunleash.variant.VariantDefinition;
 import java.util.*;
 
 public final class ActivationStrategy {
@@ -8,20 +9,28 @@ public final class ActivationStrategy {
     private final Map<String, String> parameters;
     private final List<Constraint> constraints;
     private final List<Integer> segments;
+    private final List<VariantDefinition> variants;
 
     public ActivationStrategy(String name, @Nullable Map<String, String> parameters) {
-        this(name, parameters, Collections.emptyList(), Collections.emptyList());
+        this(
+                name,
+                parameters,
+                Collections.emptyList(),
+                Collections.emptyList(),
+                Collections.emptyList());
     }
 
     public ActivationStrategy(
             String name,
             @Nullable Map<String, String> parameters,
             List<Constraint> constraints,
-            List<Integer> segments) {
+            List<Integer> segments,
+            List<VariantDefinition> variants) {
         this.name = name;
         this.parameters = Optional.ofNullable(parameters).orElseGet(Collections::emptyMap);
         this.constraints = Optional.ofNullable(constraints).orElseGet(Collections::emptyList);
         this.segments = Optional.ofNullable(segments).orElseGet(Collections::emptyList);
+        this.variants = Optional.ofNullable(variants).orElseGet(Collections::emptyList);
     }
 
     public String getName() {
@@ -39,4 +48,8 @@ public final class ActivationStrategy {
     public List<Constraint> getConstraints() {
         return constraints;
     }
+
+    public List<VariantDefinition> getVariants() {
+        return variants;
+    }
 }
diff --git a/src/main/java/io/getunleash/DefaultUnleash.java b/src/main/java/io/getunleash/DefaultUnleash.java
index f0373e21..73f68525 100644
--- a/src/main/java/io/getunleash/DefaultUnleash.java
+++ b/src/main/java/io/getunleash/DefaultUnleash.java
@@ -136,11 +136,12 @@ public class DefaultUnleash implements Unleash {
             String toggleName,
             UnleashContext context,
             BiPredicate<String, UnleashContext> fallbackAction) {
-        boolean enabled = checkEnabled(toggleName, context, fallbackAction);
-        count(toggleName, enabled);
-        eventDispatcher.dispatch(new ToggleEvaluated(toggleName, enabled));
-        dispatchEnabledImpressionDataIfNeeded("isEnabled", toggleName, enabled, context);
-        return enabled;
+        FeatureEvaluationResult result =
+                getFeatureEvaluationResult(toggleName, context, fallbackAction, null);
+        count(toggleName, result.isEnabled());
+        eventDispatcher.dispatch(new ToggleEvaluated(toggleName, result.isEnabled()));
+        dispatchEnabledImpressionDataIfNeeded("isEnabled", toggleName, result.isEnabled(), context);
+        return result.isEnabled();
     }
 
     private void dispatchEnabledImpressionDataIfNeeded(
@@ -151,25 +152,27 @@ public class DefaultUnleash implements Unleash {
         }
     }
 
-    private boolean checkEnabled(
+    private FeatureEvaluationResult getFeatureEvaluationResult(
             String toggleName,
             UnleashContext context,
-            BiPredicate<String, UnleashContext> fallbackAction) {
+            BiPredicate<String, UnleashContext> fallbackAction,
+            @Nullable Variant defaultVariant) {
         checkIfToggleMatchesNamePrefix(toggleName);
         FeatureToggle featureToggle = featureRepository.getToggle(toggleName);
-        boolean enabled;
-        UnleashContext enhancedContext = context.applyStaticFields(config);
 
+        UnleashContext enhancedContext = context.applyStaticFields(config);
+        Optional<ActivationStrategy> enabledStrategy = Optional.empty();
+        boolean enabled = false;
         if (featureToggle == null) {
             enabled = fallbackAction.test(toggleName, enhancedContext);
         } else if (!featureToggle.isEnabled()) {
             enabled = false;
         } else if (featureToggle.getStrategies().size() == 0) {
-            return true;
+            enabled = true;
         } else {
-            enabled =
+            enabledStrategy =
                     featureToggle.getStrategies().stream()
-                            .anyMatch(
+                            .filter(
                                     strategy -> {
                                         Strategy configuredStrategy =
                                                 getStrategy(strategy.getName());
@@ -181,13 +184,33 @@ public class DefaultUnleash implements Unleash {
                                         }
 
                                         return configuredStrategy.isEnabled(
-                                                strategy.getParameters(),
-                                                enhancedContext,
-                                                ConstraintMerger.mergeConstraints(
-                                                        featureRepository, strategy));
-                                    });
+                                                strategy.getParameters(), context);
+                                    })
+                            .findFirst();
+        }
+        FeatureEvaluationResult result = new FeatureEvaluationResult(enabled, null);
+        if (enabledStrategy.isPresent()) {
+            Strategy configuredStrategy = getStrategy(enabledStrategy.get().getName());
+            result =
+                    configuredStrategy.getResult(
+                            enabledStrategy.get().getParameters(),
+                            enhancedContext,
+                            ConstraintMerger.mergeConstraints(
+                                    featureRepository, enabledStrategy.get()),
+                            enabledStrategy.get().getVariants());
+        }
+
+        Variant variant = result.isEnabled() ? result.getVariant() : null;
+        // If strategy variant is null, look for a variant in the featureToggle
+        if (variant == null && defaultVariant != null) {
+            variant =
+                    result.isEnabled()
+                            ? VariantUtil.selectVariant(featureToggle, context, defaultVariant)
+                            : defaultVariant;
         }
-        return enabled;
+        result.setVariant(variant);
+
+        return result;
     }
 
     private void checkIfToggleMatchesNamePrefix(String toggleName) {
@@ -208,16 +231,14 @@ public class DefaultUnleash implements Unleash {
 
     @Override
     public Variant getVariant(String toggleName, UnleashContext context, Variant defaultValue) {
-        FeatureToggle featureToggle = featureRepository.getToggle(toggleName);
-        boolean enabled = checkEnabled(toggleName, context, (n, c) -> false);
-        Variant variant =
-                enabled
-                        ? VariantUtil.selectVariant(featureToggle, context, defaultValue)
-                        : defaultValue;
+        FeatureEvaluationResult result =
+                getFeatureEvaluationResult(toggleName, context, (n, c) -> false, defaultValue);
+        Variant variant = result.getVariant();
         metricService.countVariant(toggleName, variant.getName());
         // Should count yes/no also when getting variant.
-        metricService.count(toggleName, enabled);
-        dispatchVariantImpressionDataIfNeeded(toggleName, variant.getName(), enabled, context);
+        metricService.count(toggleName, result.isEnabled());
+        dispatchVariantImpressionDataIfNeeded(
+                toggleName, variant.getName(), result.isEnabled(), context);
         return variant;
     }
 
@@ -316,17 +337,12 @@ public class DefaultUnleash implements Unleash {
             return getFeatureToggleNames().stream()
                     .map(
                             toggleName -> {
-                                boolean enabled =
-                                        checkEnabled(toggleName, context, (n, c) -> false);
-                                FeatureToggle featureToggle =
-                                        featureRepository.getToggle(toggleName);
-                                Variant variant =
-                                        enabled
-                                                ? VariantUtil.selectVariant(
-                                                        featureToggle, context, DISABLED_VARIANT)
-                                                : DISABLED_VARIANT;
-
-                                return new EvaluatedToggle(toggleName, enabled, variant);
+                                FeatureEvaluationResult result =
+                                        getFeatureEvaluationResult(
+                                                toggleName, context, (n, c) -> false, null);
+
+                                return new EvaluatedToggle(
+                                        toggleName, result.isEnabled(), result.getVariant());
                             })
                     .collect(Collectors.toList());
         }
diff --git a/src/main/java/io/getunleash/FeatureEvaluationResult.java b/src/main/java/io/getunleash/FeatureEvaluationResult.java
new file mode 100644
index 00000000..c441552c
--- /dev/null
+++ b/src/main/java/io/getunleash/FeatureEvaluationResult.java
@@ -0,0 +1,33 @@
+package io.getunleash;
+
+import io.getunleash.lang.Nullable;
+
+public class FeatureEvaluationResult {
+    private boolean enabled;
+
+    private Variant variant;
+
+    public FeatureEvaluationResult() {}
+
+    public FeatureEvaluationResult(boolean enabled, @Nullable Variant variant) {
+        this.enabled = enabled;
+        this.variant = variant;
+    }
+
+    public boolean isEnabled() {
+        return enabled;
+    }
+
+    public void setEnabled(boolean enabled) {
+        this.enabled = enabled;
+    }
+
+    @Nullable
+    public Variant getVariant() {
+        return variant;
+    }
+
+    public void setVariant(Variant variant) {
+        this.variant = variant;
+    }
+}
diff --git a/src/main/java/io/getunleash/strategy/Strategy.java b/src/main/java/io/getunleash/strategy/Strategy.java
index 1720f694..694f6997 100644
--- a/src/main/java/io/getunleash/strategy/Strategy.java
+++ b/src/main/java/io/getunleash/strategy/Strategy.java
@@ -1,15 +1,30 @@
 package io.getunleash.strategy;
 
 import io.getunleash.Constraint;
+import io.getunleash.FeatureEvaluationResult;
 import io.getunleash.UnleashContext;
+import io.getunleash.lang.Nullable;
+import io.getunleash.variant.VariantDefinition;
+import io.getunleash.variant.VariantUtil;
 import java.util.List;
 import java.util.Map;
 
 public interface Strategy {
+
     String getName();
 
     boolean isEnabled(Map<String, String> parameters);
 
+    default FeatureEvaluationResult getResult(
+            Map<String, String> parameters,
+            UnleashContext unleashContext,
+            List<Constraint> constraints,
+            @Nullable List<VariantDefinition> variants) {
+        return new FeatureEvaluationResult(
+                isEnabled(parameters, unleashContext, constraints),
+                VariantUtil.selectVariant(parameters, variants, unleashContext));
+    }
+
     default boolean isEnabled(Map<String, String> parameters, UnleashContext unleashContext) {
         return isEnabled(parameters);
     }
diff --git a/src/main/java/io/getunleash/variant/VariantUtil.java b/src/main/java/io/getunleash/variant/VariantUtil.java
index 47ba77cf..969cd3fc 100644
--- a/src/main/java/io/getunleash/variant/VariantUtil.java
+++ b/src/main/java/io/getunleash/variant/VariantUtil.java
@@ -5,13 +5,12 @@ import io.getunleash.UnleashContext;
 import io.getunleash.Variant;
 import io.getunleash.lang.Nullable;
 import io.getunleash.strategy.StrategyUtils;
-import java.util.List;
-import java.util.Optional;
-import java.util.Random;
+import java.util.*;
 import java.util.function.Predicate;
 
 public final class VariantUtil {
-    // Utility class
+    static final String GROUP_ID_KEY = "groupId";
+
     private VariantUtil() {}
 
     private static Predicate<VariantOverride> overrideMatchesContext(UnleashContext context) {
@@ -78,39 +77,54 @@ public final class VariantUtil {
         if (featureToggle == null) {
             return defaultVariant;
         }
-        List<VariantDefinition> variants = featureToggle.getVariants();
-        int totalWeight = variants.stream().mapToInt(VariantDefinition::getWeight).sum();
-        if (totalWeight == 0) {
-            return defaultVariant;
-        }
 
-        Optional<VariantDefinition> variantOverride = getOverride(variants, context);
-        if (variantOverride.isPresent()) {
-            return variantOverride.get().toVariant();
-        }
-        Optional<String> customStickiness =
-                variants.stream()
-                        .filter(
-                                f ->
-                                        f.getStickiness() != null
-                                                && !"default".equals(f.getStickiness()))
-                        .map(VariantDefinition::getStickiness)
-                        .findFirst();
-        int target =
-                StrategyUtils.getNormalizedNumber(
-                        getSeed(context, customStickiness), featureToggle.getName(), totalWeight);
+        Variant variant =
+                selectVariant(
+                        Collections.singletonMap("groupId", featureToggle.getName()),
+                        featureToggle.getVariants(),
+                        context);
+
+        return variant != null ? variant : defaultVariant;
+    }
 
-        int counter = 0;
-        for (final VariantDefinition definition : featureToggle.getVariants()) {
-            if (definition.getWeight() != 0) {
-                counter += definition.getWeight();
-                if (counter >= target) {
-                    return definition.toVariant();
+    public static @Nullable Variant selectVariant(
+            Map<String, String> parameters,
+            @Nullable List<VariantDefinition> variants,
+            UnleashContext context) {
+        if (variants != null) {
+            int totalWeight = variants.stream().mapToInt(VariantDefinition::getWeight).sum();
+            if (totalWeight <= 0) {
+                return null;
+            }
+            Optional<VariantDefinition> variantOverride = getOverride(variants, context);
+            if (variantOverride.isPresent()) {
+                return variantOverride.get().toVariant();
+            }
+
+            Optional<String> customStickiness =
+                    variants.stream()
+                            .filter(
+                                    f ->
+                                            f.getStickiness() != null
+                                                    && !"default".equals(f.getStickiness()))
+                            .map(VariantDefinition::getStickiness)
+                            .findFirst();
+            int target =
+                    StrategyUtils.getNormalizedNumber(
+                            getSeed(context, customStickiness),
+                            parameters.get(GROUP_ID_KEY),
+                            totalWeight);
+
+            int counter = 0;
+            for (VariantDefinition variant : variants) {
+                if (variant.getWeight() != 0) {
+                    counter += variant.getWeight();
+                    if (counter >= target) {
+                        return variant.toVariant();
+                    }
                 }
             }
         }
-
-        // Should not happen
-        return defaultVariant;
+        return null;
     }
 }


Test patch:
diff --git a/src/test/java/io/getunleash/DefaultUnleashTest.java b/src/test/java/io/getunleash/DefaultUnleashTest.java
index 0f6b8e19..f869429c 100644
--- a/src/test/java/io/getunleash/DefaultUnleashTest.java
+++ b/src/test/java/io/getunleash/DefaultUnleashTest.java
@@ -85,7 +85,11 @@ class DefaultUnleashTest {
         Constraint semverConstraint = new Constraint("version", Operator.SEMVER_EQ, semVer);
         ActivationStrategy withMissingSegment =
                 new ActivationStrategy(
-                        "default", Collections.emptyMap(), asList(semverConstraint), asList(404));
+                        "default",
+                        Collections.emptyMap(),
+                        asList(semverConstraint),
+                        asList(404),
+                        Collections.emptyList());
         when(featureRepository.getToggle(toggleName))
                 .thenReturn(new FeatureToggle(toggleName, true, asList(withMissingSegment)));
         when(featureRepository.getSegment(404)).thenReturn(Segment.DENY_SEGMENT);
@@ -103,7 +107,8 @@ class DefaultUnleashTest {
                         "default",
                         Collections.emptyMap(),
                         asList(semverConstraint),
-                        asList(404, 1));
+                        asList(404, 1),
+                        Collections.emptyList());
         when(featureRepository.getToggle(toggleName))
                 .thenReturn(new FeatureToggle(toggleName, true, asList(withMissingSegment)));
         when(featureRepository.getSegment(1))
@@ -147,7 +152,7 @@ class DefaultUnleashTest {
 
         sut.isEnabled("toggle1");
 
-        verify(fallback).isEnabled(any(), any(), any());
+        verify(fallback).isEnabled(any(), any());
     }
 
     @Test
diff --git a/src/test/java/io/getunleash/UnleashTest.java b/src/test/java/io/getunleash/UnleashTest.java
index f6bc7b98..1e39d79e 100644
--- a/src/test/java/io/getunleash/UnleashTest.java
+++ b/src/test/java/io/getunleash/UnleashTest.java
@@ -144,7 +144,7 @@ public class UnleashTest {
 
         unleash.isEnabled("test");
 
-        verify(customStrategy, times(1)).isEnabled(any(), any(UnleashContext.class), any());
+        verify(customStrategy, times(1)).isEnabled(any(), any(UnleashContext.class));
     }
 
     @Test
@@ -441,7 +441,12 @@ public class UnleashTest {
         List<Constraint> constraints = new ArrayList<>();
         constraints.add(new Constraint("environment", Operator.IN, Arrays.asList("test")));
         ActivationStrategy activeStrategy =
-                new ActivationStrategy("default", null, constraints, Collections.emptyList());
+                new ActivationStrategy(
+                        "default",
+                        null,
+                        constraints,
+                        Collections.emptyList(),
+                        Collections.emptyList());
 
         FeatureToggle featureToggle = new FeatureToggle("test", true, asList(activeStrategy));
 
@@ -455,7 +460,12 @@ public class UnleashTest {
         List<Constraint> constraints = new ArrayList<>();
         constraints.add(new Constraint("environment", Operator.IN, Arrays.asList("dev", "prod")));
         ActivationStrategy activeStrategy =
-                new ActivationStrategy("default", null, constraints, Collections.emptyList());
+                new ActivationStrategy(
+                        "default",
+                        null,
+                        constraints,
+                        Collections.emptyList(),
+                        Collections.emptyList());
 
         FeatureToggle featureToggle = new FeatureToggle("test", true, asList(activeStrategy));
 
@@ -528,6 +538,25 @@ public class UnleashTest {
         assertThat(unleash.isEnabled("Test.variants")).isFalse();
     }
 
+    @Test
+    public void should_be_disabled_with_strategy_variants() {
+        List<Constraint> constraints = new ArrayList<>();
+        constraints.add(new Constraint("environment", Operator.IN, Arrays.asList("dev", "prod")));
+        ActivationStrategy activeStrategy =
+                new ActivationStrategy(
+                        "default",
+                        null,
+                        constraints,
+                        Collections.emptyList(),
+                        Collections.emptyList());
+
+        FeatureToggle featureToggle = new FeatureToggle("test", true, asList(activeStrategy));
+
+        when(toggleRepository.getToggle("test")).thenReturn(featureToggle);
+
+        assertThat(unleash.isEnabled("test")).isFalse();
+    }
+
     private List<VariantDefinition> getTestVariants() {
         return asList(
                 new VariantDefinition(