Skip to content

Commit

Permalink
fix: Always check parent dependency if present (#248)
Browse files Browse the repository at this point in the history 
Fixes: #246
  • Loading branch information
@chriswk
chriswk authored Jul 29, 2024
1 parent efef588 commit 60a135c
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 71 deletions.
136 changes: 65 additions & 71 deletions src/main/java/io/getunleash/DefaultUnleash.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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(
Expand Down Expand Up @@ -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) {
Expand All @@ -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(
Expand All @@ -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) {
Expand All @@ -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) {
Expand Down
26 changes: 26 additions & 0 deletions src/test/java/io/getunleash/DependentFeatureToggleTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}

0 comments on commit 60a135c

Please sign in to comment.