fix: add deprecated method for old hashing algorithm (#233)

fix: add deprecated method for old hashing algorithm Co-authored-by: Christopher Kolstad <chriswk@getunleash.io>
Unleash · Dec 7, 2023 · f64bc6b · f64bc6b
1 parent 1f1b8c4
commit f64bc6b
Show file tree

Hide file tree

Showing 7 changed files with 311 additions and 0 deletions.
diff --git a/src/main/java/io/getunleash/DefaultUnleash.java b/src/main/java/io/getunleash/DefaultUnleash.java
@@ -219,6 +219,69 @@ private FeatureEvaluationResult getFeatureEvaluationResult(
         }
     }
 
+    /**
+     * Uses the old, statistically broken Variant seed for finding the correct variant
+     * @deprecated
+     * @param toggleName Name of the toggle
+     * @param context The UnleashContext
+     * @param fallbackAction What to do if we fail to find the toggle
+     * @param defaultVariant If we can't resolve a variant, what are we returning
+     * @return A wrapper containing whether the feature was enabled as well which Variant was selected
+     */
+      private FeatureEvaluationResult deprecatedGetFeatureEvaluationResult(
+            String toggleName,
+            UnleashContext context,
+            BiPredicate<String, UnleashContext> fallbackAction,
+            @Nullable Variant defaultVariant) {
+        checkIfToggleMatchesNamePrefix(toggleName);
+        FeatureToggle featureToggle = featureRepository.getToggle(toggleName);
+
+        UnleashContext enhancedContext = context.applyStaticFields(config);
+        if (featureToggle == null) {
+            return new FeatureEvaluationResult(
+                    fallbackAction.test(toggleName, enhancedContext), defaultVariant);
+        } else if (!featureToggle.isEnabled()) {
+            return new FeatureEvaluationResult(false, defaultVariant);
+        } else if (featureToggle.getStrategies().isEmpty()) {
+            return new FeatureEvaluationResult(
+                    true, VariantUtil.selectDeprecatedVariantHashingAlgo(featureToggle, context, defaultVariant));
+        } else {
+            // Dependent toggles, no point in evaluating child strategies if our dependencies are
+            // not satisfied
+            if (isParentDependencySatisfied(featureToggle, context, fallbackAction)) {
+                for (ActivationStrategy strategy : featureToggle.getStrategies()) {
+                    Strategy configuredStrategy = getStrategy(strategy.getName());
+                    if (configuredStrategy == UNKNOWN_STRATEGY) {
+                        LOGGER.warn(
+                                "Unable to find matching strategy for toggle:{} strategy:{}",
+                                toggleName,
+                                strategy.getName());
+                    }
+
+                    FeatureEvaluationResult result =
+                            configuredStrategy.getDeprecatedHashingAlgoResult(
+                                    strategy.getParameters(),
+                                    enhancedContext,
+                                    ConstraintMerger.mergeConstraints(featureRepository, strategy),
+                                    strategy.getVariants());
+
+                    if (result.isEnabled()) {
+                        Variant variant = result.getVariant();
+                        // If strategy variant is null, look for a variant in the featureToggle
+                        if (variant == null) {
+                            variant =
+                                    VariantUtil.selectDeprecatedVariantHashingAlgo(
+                                            featureToggle, context, defaultVariant);
+                        }
+                        result.setVariant(variant);
+                        return result;
+                    }
+                }
+            }
+            return new FeatureEvaluationResult(false, defaultVariant);
+        }
+    }
+
     private boolean isParentDependencySatisfied(
             @Nonnull FeatureToggle featureToggle,
             @Nonnull UnleashContext context,
@@ -323,6 +386,69 @@ public Variant getVariant(String toggleName, Variant defaultValue) {
         return getVariant(toggleName, contextProvider.getContext(), defaultValue);
     }
 
+    /**
+     * Uses the old, statistically broken Variant seed for finding the correct variant
+     * @deprecated
+     * @param toggleName
+     * @param context
+     * @return
+     */
+    @Override
+    public Variant deprecatedGetVariant(String toggleName, UnleashContext context) {
+        return deprecatedGetVariant(toggleName, context, DISABLED_VARIANT);
+    }
+
+    /**
+     * Uses the old, statistically broken Variant seed for finding the correct variant
+     * @deprecated
+     * @param toggleName
+     * @param context
+     * @param defaultValue
+     * @return
+     */
+    @Override
+    public Variant deprecatedGetVariant(String toggleName, UnleashContext context, Variant defaultValue) {
+        return deprecatedGetVariant(toggleName, context, defaultValue, false);
+    }
+
+    private Variant deprecatedGetVariant(
+            String toggleName, UnleashContext context, Variant defaultValue, boolean isParent) {
+        FeatureEvaluationResult result =
+                deprecatedGetFeatureEvaluationResult(toggleName, context, (n, c) -> false, defaultValue);
+        Variant variant = result.getVariant();
+        if (!isParent) {
+            metricService.countVariant(toggleName, variant.getName());
+            // Should count yes/no also when getting variant.
+            metricService.count(toggleName, result.isEnabled());
+        }
+        dispatchVariantImpressionDataIfNeeded(
+                toggleName, variant.getName(), result.isEnabled(), context);
+        return variant;
+    }
+
+    /**
+     * Uses the old, statistically broken Variant seed for finding the correct variant
+     * @deprecated
+     * @param toggleName
+     * @return
+     */
+    @Override
+    public Variant deprecatedGetVariant(String toggleName) {
+        return deprecatedGetVariant(toggleName, contextProvider.getContext());
+    }
+
+    /**
+     * Uses the old, statistically broken Variant seed for finding the correct variant
+     * @deprecated
+     * @param toggleName
+     * @param defaultValue
+     * @return
+     */
+    @Override
+    public Variant deprecatedGetVariant(String toggleName, Variant defaultValue) {
+        return deprecatedGetVariant(toggleName, contextProvider.getContext(), defaultValue);
+    }
+
     /**
      * Use more().getFeatureToggleDefinition() instead
      *

diff --git a/src/main/java/io/getunleash/FakeUnleash.java b/src/main/java/io/getunleash/FakeUnleash.java
@@ -106,6 +106,16 @@ public void disableAllExcept(String... excludedFeatures) {
         }
     }
 
+    @Override
+    public Variant deprecatedGetVariant(String toggleName, UnleashContext context) {
+        return null;
+    }
+
+    @Override
+    public Variant deprecatedGetVariant(String toggleName, UnleashContext context, Variant defaultValue) {
+        return null;
+    }
+
     public void resetAll() {
         disableAll = false;
         enableAll = false;

diff --git a/src/main/java/io/getunleash/Unleash.java b/src/main/java/io/getunleash/Unleash.java
@@ -43,6 +43,19 @@ default Variant getVariant(final String toggleName, final Variant defaultValue)
         return getVariant(toggleName, UnleashContext.builder().build(), defaultValue);
     }
 
+    Variant deprecatedGetVariant(final String toggleName, final UnleashContext context);
+
+    Variant deprecatedGetVariant(
+            final String toggleName, final UnleashContext context, final Variant defaultValue);
+
+    default Variant deprecatedGetVariant(final String toggleName) {
+        return deprecatedGetVariant(toggleName, UnleashContext.builder().build());
+    }
+
+    default Variant deprecatedGetVariant(final String toggleName, final Variant defaultValue) {
+        return deprecatedGetVariant(toggleName, UnleashContext.builder().build(), defaultValue);
+    }
+
     /**
      * Use more().getFeatureToggleNames() instead
      *

diff --git a/src/main/java/io/getunleash/strategy/Strategy.java b/src/main/java/io/getunleash/strategy/Strategy.java
@@ -26,6 +26,27 @@ default FeatureEvaluationResult getResult(
                 enabled ? VariantUtil.selectVariant(parameters, variants, unleashContext) : null);
     }
 
+
+    /**
+     * Uses the old pre 9.0.0 way of hashing for finding the Variant to return
+     * @deprecated
+     * @param parameters
+     * @param unleashContext
+     * @param constraints
+     * @param variants
+     * @return
+     */
+    default FeatureEvaluationResult getDeprecatedHashingAlgoResult(
+            Map<String, String> parameters,
+            UnleashContext unleashContext,
+            List<Constraint> constraints,
+            @Nullable List<VariantDefinition> variants) {
+        boolean enabled = isEnabled(parameters, unleashContext, constraints);
+        return new FeatureEvaluationResult(
+                enabled,
+                enabled ? VariantUtil.selectDeprecatedVariantHashingAlgo(parameters, variants, unleashContext) : null);
+    }
+
     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
@@ -133,4 +133,78 @@ public static Variant selectVariant(
         }
         return null;
     }
+
+    /**
+     * Uses the old pre 9.0.0 way of hashing for finding the Variant to return
+     * @deprecated
+     * @param parameters
+     * @param variants
+     * @param context
+     * @return
+     */
+    public static @Nullable Variant selectDeprecatedVariantHashingAlgo(
+            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,
+                            0);
+
+            int counter = 0;
+            for (VariantDefinition variant : variants) {
+                if (variant.getWeight() != 0) {
+                    counter += variant.getWeight();
+                    if (counter >= target) {
+                        return variant.toVariant();
+                    }
+                }
+            }
+        }
+        return null;
+    }
+
+    /**
+     * Uses the old pre 9.0.0 way of hashing for finding the Variant to return
+     * @deprecated
+     * @param featureToggle
+     * @param context
+     * @param defaultVariant
+     * @return
+     */
+    public static @Nullable Variant selectDeprecatedVariantHashingAlgo(
+        FeatureToggle featureToggle, UnleashContext context, Variant defaultVariant
+    ) {
+        if (featureToggle == null) {
+            return defaultVariant;
+        }
+
+        Variant variant =
+            selectDeprecatedVariantHashingAlgo(
+                Collections.singletonMap("groupId", featureToggle.getName()),
+                featureToggle.getVariants(),
+                context);
+
+        return variant != null ? variant : defaultVariant;
+    }
 }
diff --git a/src/test/java/io/getunleash/UnleashTest.java b/src/test/java/io/getunleash/UnleashTest.java
@@ -431,6 +431,8 @@ public void get_default_variant_without_context() {
         assertThat(result.isEnabled()).isTrue();
     }
 
+
+
     @Test
     public void get_first_variant_with_context_provider() {
 
@@ -477,6 +479,36 @@ public void get_second_variant_with_context_provider() {
         assertThat(result.isEnabled()).isTrue();
     }
 
+    @Test
+    public void get_second_variant_with_context_provider_and_deprecated_algorithm() {
+
+        UnleashContext context = UnleashContext.builder().userId("5").build();
+        when(contextProvider.getContext()).thenReturn(context);
+
+        // Set up a toggleName using UserWithIdStrategy
+        Map<String, String> params = new HashMap<>();
+        params.put("userIds", "123, 5, 121");
+        ActivationStrategy strategy = new ActivationStrategy("userWithId", params);
+        FeatureToggle featureToggle =
+            new FeatureToggle("test", true, asList(strategy), getTestVariantsForDeprecatedHash());
+
+        when(toggleRepository.getToggle("test")).thenReturn(featureToggle);
+
+        final Variant result = unleash.deprecatedGetVariant("test");
+
+        assertThat(result).isNotNull();
+        assertThat(result.getName()).isEqualTo("en");
+        assertThat(result.getPayload().map(Payload::getValue).get()).isEqualTo("en");
+        assertThat(result.isEnabled()).isTrue();
+
+        final Variant newHash = unleash.getVariant("test");
+        assertThat(newHash).isNotNull();
+        assertThat(newHash.getName()).isEqualTo("to");
+        assertThat(newHash.getPayload().map(Payload::getValue).get()).isEqualTo("to");
+        assertThat(newHash.isEnabled()).isTrue();
+
+    }
+
     @Test
     public void should_be_enabled_with_strategy_constraints() {
         List<Constraint> constraints = new ArrayList<>();
@@ -605,4 +637,11 @@ private List<VariantDefinition> getTestVariants() {
                 new VariantDefinition(
                         "to", 50, new Payload("string", "to"), Collections.emptyList()));
     }
+
+    private List<VariantDefinition> getTestVariantsForDeprecatedHash() {
+        return asList(
+            new VariantDefinition("en", 65, new Payload("string", "en"), Collections.emptyList()),
+            new VariantDefinition("to", 35, new Payload("string", "to"), Collections.emptyList())
+        );
+    }
 }
diff --git a/src/test/java/io/getunleash/variant/VariantUtilTest.java b/src/test/java/io/getunleash/variant/VariantUtilTest.java
@@ -387,6 +387,34 @@ public void feature_variants_variant_d_client_spec_tests() {
         assertThat(variantUser537.getName()).isEqualTo("variant3");
     }
 
+    @Test
+    public void feature_variants_variant_d_client_spec_tests_with_deprecated_seed() {
+        List<VariantDefinition> variants = new ArrayList<>();
+        variants.add(
+            new VariantDefinition(
+                "variant1", 1, new Payload("string", "val1"), Collections.emptyList()));
+        variants.add(
+            new VariantDefinition(
+                "variant2", 49, new Payload("string", "val2"), Collections.emptyList()));
+        variants.add(
+            new VariantDefinition(
+                "variant3", 50, new Payload("string", "val3"), Collections.emptyList()));
+        FeatureToggle toggle =
+            new FeatureToggle("Feature.Variants.D", true, Collections.emptyList(), variants);
+        Variant variantUser712 =
+            VariantUtil.selectDeprecatedVariantHashingAlgo(
+                toggle, UnleashContext.builder().userId("712").build(), DISABLED_VARIANT);
+        assertThat(variantUser712.getName()).isEqualTo("variant3");
+        Variant variantUser525 =
+            VariantUtil.selectDeprecatedVariantHashingAlgo(
+                toggle, UnleashContext.builder().userId("525").build(), DISABLED_VARIANT);
+        assertThat(variantUser525.getName()).isEqualTo("variant3");
+        Variant variantUser537 =
+            VariantUtil.selectDeprecatedVariantHashingAlgo(
+                toggle, UnleashContext.builder().userId("537").build(), DISABLED_VARIANT);
+        assertThat(variantUser537.getName()).isEqualTo("variant2");
+    }
+
     @Test
     public void feature_variants_variant_d_with_override_client_spec_tests() {
         List<VariantDefinition> variants = new ArrayList<>();