diff --git a/src/main/java/io/getunleash/DefaultUnleash.java b/src/main/java/io/getunleash/DefaultUnleash.java index 7480ec1e9..9abcdb9ba 100644 --- a/src/main/java/io/getunleash/DefaultUnleash.java +++ b/src/main/java/io/getunleash/DefaultUnleash.java @@ -179,56 +179,53 @@ private FeatureEvaluationResult getFeatureEvaluationResult( 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 @@ private FeatureEvaluationResult deprecatedGetFeatureEvaluationResult( 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 Variant getVariant(String toggleName, Variant defaultValue) { /** * 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 Variant deprecatedGetVariant(String toggleName, UnleashContext context) { /** * 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 @@ private Variant deprecatedGetVariant( /** * 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 Variant deprecatedGetVariant(String toggleName) { /** * 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) { diff --git a/src/test/java/io/getunleash/DependentFeatureToggleTest.java b/src/test/java/io/getunleash/DependentFeatureToggleTest.java index 84761aa91..194bf671f 100644 --- a/src/test/java/io/getunleash/DependentFeatureToggleTest.java +++ b/src/test/java/io/getunleash/DependentFeatureToggleTest.java @@ -176,4 +176,30 @@ public void should_trigger_impression_event_for_parent_variant_when_checking_chi 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(); + } }