Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add deprecated method for old hashing algorithm #233

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
126 changes: 126 additions & 0 deletions src/main/java/io/getunleash/DefaultUnleash.java
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
*
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/io/getunleash/FakeUnleash.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 13 additions & 0 deletions src/main/java/io/getunleash/Unleash.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down
21 changes: 21 additions & 0 deletions src/main/java/io/getunleash/strategy/Strategy.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
74 changes: 74 additions & 0 deletions src/main/java/io/getunleash/variant/VariantUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
39 changes: 39 additions & 0 deletions src/test/java/io/getunleash/UnleashTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,8 @@ public void get_default_variant_without_context() {
assertThat(result.isEnabled()).isTrue();
}



@Test
public void get_first_variant_with_context_provider() {

Expand Down Expand Up @@ -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<>();
Expand Down Expand Up @@ -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())
);
}
}
28 changes: 28 additions & 0 deletions src/test/java/io/getunleash/variant/VariantUtilTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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<>();
Expand Down