Repository: Unleash/unleash-client-java
Pull request #248 (2024-07-29 09:45:39)

fix: Always check parent dependency if present
Fixes: #246




Patch:
diff --git a/src/main/java/io/getunleash/DefaultUnleash.java b/src/main/java/io/getunleash/DefaultUnleash.java
index 7480ec1e..9abcdb9b 100644
--- a/src/main/java/io/getunleash/DefaultUnleash.java
+++ b/src/main/java/io/getunleash/DefaultUnleash.java
@@ -179,56 +179,53 @@ public class DefaultUnleash implements Unleash {
                     fallbackAction.test(toggleName, enhancedContext), defaultVariant);
         } else if (!featureToggle.isEnabled()) {
             return new FeatureEvaluationResult(false, defaultVariant);
-        } else if (featureToggle.getStrategies().isEmpty()) {
-            return new FeatureEvaluationResult(
-                    true, VariantUtil.selectVariant(featureToggle, context, defaultVariant));
-        } else {
+        } else if (isParentDependencySatisfied(featureToggle, context, fallbackAction)) {
             // 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());
-                    }
+            if (featureToggle.getStrategies().isEmpty()) {
+                return new FeatureEvaluationResult(
+                        true, VariantUtil.selectVariant(featureToggle, context, defaultVariant));
+            }
+            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.getResult(
-                                    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.selectVariant(
-                                            featureToggle, context, defaultVariant);
-                        }
-                        result.setVariant(variant);
-                        return result;
+                FeatureEvaluationResult result =
+                        configuredStrategy.getResult(
+                                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.selectVariant(featureToggle, context, defaultVariant);
                     }
+                    result.setVariant(variant);
+                    return result;
                 }
             }
-            return new FeatureEvaluationResult(false, defaultVariant);
         }
+        return new FeatureEvaluationResult(false, defaultVariant);
     }
 
     /**
      * 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
+     * @deprecated
      */
     private FeatureEvaluationResult deprecatedGetFeatureEvaluationResult(
             String toggleName,
@@ -244,46 +241,43 @@ public class DefaultUnleash implements Unleash {
                     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());
-                    }
+        } else if (isParentDependencySatisfied(featureToggle, context, fallbackAction)) {
+            if (featureToggle.getStrategies().isEmpty()) {
+                return new FeatureEvaluationResult(
+                        true,
+                        VariantUtil.selectDeprecatedVariantHashingAlgo(
+                                featureToggle, context, defaultVariant));
+            }
+            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;
+                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);
         }
+        return new FeatureEvaluationResult(false, defaultVariant);
     }
 
     private boolean isParentDependencySatisfied(
@@ -393,10 +387,10 @@ public class DefaultUnleash implements Unleash {
     /**
      * Uses the old, statistically broken Variant seed for finding the correct variant
      *
-     * @deprecated
      * @param toggleName
      * @param context
      * @return
+     * @deprecated
      */
     @Override
     public Variant deprecatedGetVariant(String toggleName, UnleashContext context) {
@@ -406,11 +400,11 @@ public class DefaultUnleash implements Unleash {
     /**
      * Uses the old, statistically broken Variant seed for finding the correct variant
      *
-     * @deprecated
      * @param toggleName
      * @param context
      * @param defaultValue
      * @return
+     * @deprecated
      */
     @Override
     public Variant deprecatedGetVariant(
@@ -437,9 +431,9 @@ public class DefaultUnleash implements Unleash {
     /**
      * Uses the old, statistically broken Variant seed for finding the correct variant
      *
-     * @deprecated
      * @param toggleName
      * @return
+     * @deprecated
      */
     @Override
     public Variant deprecatedGetVariant(String toggleName) {
@@ -449,10 +443,10 @@ public class DefaultUnleash implements Unleash {
     /**
      * Uses the old, statistically broken Variant seed for finding the correct variant
      *
-     * @deprecated
      * @param toggleName
      * @param defaultValue
      * @return
+     * @deprecated
      */
     @Override
     public Variant deprecatedGetVariant(String toggleName, Variant defaultValue) {


Test patch:
diff --git a/src/test/java/io/getunleash/DependentFeatureToggleTest.java b/src/test/java/io/getunleash/DependentFeatureToggleTest.java
index 84761aa9..194bf671 100644
--- a/src/test/java/io/getunleash/DependentFeatureToggleTest.java
+++ b/src/test/java/io/getunleash/DependentFeatureToggleTest.java
@@ -176,4 +176,30 @@ public class DependentFeatureToggleTest {
         when(featureRepository.getToggle(parentName)).thenReturn(parent);
         assertThat(sut.isEnabled(childName, UnleashContext.builder().build())).isFalse();
     }
+
+    @Test
+    public void childIsDisabledWhenChildDoesNotHaveStrategiesAndParentIsDisabled() {
+        FeatureToggle parent =
+                new FeatureToggle(
+                        "parent", false, singletonList(new ActivationStrategy("default", null)));
+        FeatureDependency childDependsOnParent = new FeatureDependency("parant", true, emptyList());
+        FeatureToggle child =
+                new FeatureToggle(
+                        "child",
+                        true,
+                        emptyList(),
+                        emptyList(),
+                        true,
+                        singletonList(childDependsOnParent));
+        when(featureRepository.getToggle("child")).thenReturn(child);
+        when(featureRepository.getToggle("parent")).thenReturn(parent);
+        assertThat(sut.isEnabled("child", UnleashContext.builder().build())).isFalse();
+    }
+
+    @Test
+    public void shouldBeEnabledWhenMissingStrategies() {
+        FeatureToggle c = new FeatureToggle("c", true, emptyList());
+        when(featureRepository.getToggle("c")).thenReturn(c);
+        assertThat(sut.isEnabled("c", UnleashContext.builder().build())).isTrue();
+    }
 }