Skip to content

Commit

Permalink
Feat/custom stickiness (#139)
Browse files Browse the repository at this point in the history
* feat: Support custom stickiness

- Adds support for custom stickiness field to both variants
  and FlexibleRolloutStrategies

fixes: #133, #132
  • Loading branch information
Christopher Kolstad authored Jun 8, 2021
1 parent 4477553 commit 323877c
Show file tree
Hide file tree
Showing 15 changed files with 184 additions and 50 deletions.
8 changes: 4 additions & 4 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
<properties>
<version.slf4j>1.7.30</version.slf4j>
<version.log4j2>2.13.3</version.log4j2>
<version.junit5>5.7.1</version.junit5>
<version.junit5>5.7.2</version.junit5>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<version.unleash.specification>3.3.0</version.unleash.specification>
<version.unleash.specification>4.0.0</version.unleash.specification>
<arguments />
</properties>

Expand Down Expand Up @@ -143,7 +143,7 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-surefire-plugin</artifactId>
<version>2.22.2</version>
<version>3.0.0-M5</version>
<configuration>
<forkCount>1</forkCount>
</configuration>
Expand Down Expand Up @@ -177,7 +177,7 @@
<plugin>
<groupId>org.jacoco</groupId>
<artifactId>jacoco-maven-plugin</artifactId>
<version>0.8.6</version>
<version>0.8.7</version>
<executions>
<execution>
<id>prepare-agent</id>
Expand Down
16 changes: 16 additions & 0 deletions src/main/java/no/finn/unleash/Variant.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,28 @@ public class Variant {
private final String name;
@Nullable private final Payload payload;
private final boolean enabled;
@Nullable private final String stickiness;

public Variant(String name, @Nullable Payload payload, boolean enabled) {
this(name, payload, enabled, null);
}

public Variant(String name, @Nullable Payload payload, boolean enabled, String stickiness) {
this.name = name;
this.payload = payload;
this.enabled = enabled;
this.stickiness = stickiness;
}

public Variant(String name, @Nullable String payload, boolean enabled) {
this(name, payload, enabled, null);
}

public Variant(String name, @Nullable String payload, boolean enabled, String stickiness) {
this.name = name;
this.payload = new Payload("string", payload);
this.enabled = enabled;
this.stickiness = stickiness;
}

public String getName() {
Expand All @@ -36,6 +47,11 @@ public boolean isEnabled() {
return enabled;
}

@Nullable
public String getStickiness() {
return stickiness;
}

@Override
public String toString() {
return "Variant{"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,11 @@ public final class HttpToggleFetcher implements ToggleFetcher {

public HttpToggleFetcher(UnleashConfig unleashConfig) {
this.unleashConfig = unleashConfig;
this.toggleUrl = unleashConfig
.getUnleashURLs()
.getFetchTogglesURL(unleashConfig.getProjectName(), unleashConfig.getNamePrefix());
this.toggleUrl =
unleashConfig
.getUnleashURLs()
.getFetchTogglesURL(
unleashConfig.getProjectName(), unleashConfig.getNamePrefix());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,13 @@ private Optional<String> resolveStickiness(String stickiness, UnleashContext con
return context.getSessionId();
case "random":
return Optional.of(randomGenerator.get());
default:
case "default":
String value =
context.getUserId()
.orElse(context.getSessionId().orElse(this.randomGenerator.get()));
return Optional.of(value);
default:
return context.getByName(stickiness);
}
}

Expand All @@ -52,13 +54,10 @@ public boolean isEnabled(Map<String, String> parameters, UnleashContext unleashC
final int percentage = StrategyUtils.getPercentage(parameters.get(PERCENTAGE));
final String groupId = parameters.getOrDefault(GROUP_ID, "");

if (stickinessId.isPresent()) {
final int normalizedUserId =
StrategyUtils.getNormalizedNumber(stickinessId.get(), groupId);
return percentage > 0 && normalizedUserId <= percentage;
} else {
return false;
}
return stickinessId
.map(stick -> StrategyUtils.getNormalizedNumber(stick, groupId))
.map(norm -> percentage > 0 && norm <= percentage)
.orElse(false);
}

private String getStickiness(Map<String, String> parameters) {
Expand Down
13 changes: 9 additions & 4 deletions src/main/java/no/finn/unleash/util/UnleashConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,8 @@ static class SystemProxyAuthenticator extends Authenticator {

// Only apply PasswordAuthentication to requests to the proxy itself - if not set
// just ignore
if (getRequestingHost().equalsIgnoreCase(proxyHost) && Integer.parseInt(proxyPort) == getRequestingPort()) {
if (getRequestingHost().equalsIgnoreCase(proxyHost)
&& Integer.parseInt(proxyPort) == getRequestingPort()) {
return new PasswordAuthentication(proxyUser, proxyPassword.toCharArray());
}
}
Expand All @@ -267,14 +268,17 @@ public CustomProxyAuthenticator(Proxy proxy, String proxyUser, String proxyPassw

@Override
protected @Nullable PasswordAuthentication getPasswordAuthentication() {
if (getRequestorType() == RequestorType.PROXY && proxy.type() == Proxy.Type.HTTP && proxy.address() instanceof InetSocketAddress) {
if (getRequestorType() == RequestorType.PROXY
&& proxy.type() == Proxy.Type.HTTP
&& proxy.address() instanceof InetSocketAddress) {
final String proxyHost = ((InetSocketAddress) proxy.address()).getHostName();
final int proxyPort = ((InetSocketAddress) proxy.address()).getPort();

// Only apply PasswordAuthentication to requests to the proxy
// itself - if not set
// just ignore
if (getRequestingHost().equalsIgnoreCase(proxyHost) && proxyPort == getRequestingPort()) {
if (getRequestingHost().equalsIgnoreCase(proxyHost)
&& proxyPort == getRequestingPort()) {
return new PasswordAuthentication(proxyUser, proxyPassword.toCharArray());
}
}
Expand Down Expand Up @@ -435,7 +439,8 @@ public Builder proxy(
this.proxy = proxy;

if (proxyUser != null && proxyPassword != null) {
this.proxyAuthenticator = new CustomProxyAuthenticator(proxy, proxyUser, proxyPassword);
this.proxyAuthenticator =
new CustomProxyAuthenticator(proxy, proxyUser, proxyPassword);
}
return this;
}
Expand Down
15 changes: 6 additions & 9 deletions src/main/java/no/finn/unleash/util/UnleashURLs.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package no.finn.unleash.util;

import no.finn.unleash.lang.Nullable;

import java.net.MalformedURLException;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URL;
import no.finn.unleash.lang.Nullable;

public class UnleashURLs {
private final URL fetchTogglesURL;
Expand Down Expand Up @@ -43,20 +41,19 @@ public URL getFetchTogglesURL(@Nullable String projectName, @Nullable String nam

try {
return URI.create(fetchTogglesURL + suffix.toString()).normalize().toURL();
} catch (IllegalArgumentException|MalformedURLException e) {
throw new IllegalArgumentException("fetchTogglesURL [" + fetchTogglesURL + suffix + "] was not URL friendly.", e);
} catch (IllegalArgumentException | MalformedURLException e) {
throw new IllegalArgumentException(
"fetchTogglesURL [" + fetchTogglesURL + suffix + "] was not URL friendly.", e);
}
}

private void appendParam(StringBuilder suffix, String key, @Nullable String value) {
if(value == null) return;
if(suffix.length() == 0){
if (value == null) return;
if (suffix.length() == 0) {
suffix.append("?");
} else {
suffix.append("&");
}
suffix.append(key).append("=").append(value);
}


}
19 changes: 17 additions & 2 deletions src/main/java/no/finn/unleash/variant/VariantDefinition.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,20 +11,31 @@ public class VariantDefinition {
private final int weight;
@Nullable private final Payload payload;
@Nullable private final List<VariantOverride> overrides;
@Nullable private final String stickiness;

public VariantDefinition(
String name,
int weight,
@Nullable Payload payload,
@Nullable List<VariantOverride> overrides) {
this(name, weight, payload, overrides, null);
}

public VariantDefinition(
String name,
int weight,
@Nullable Payload payload,
@Nullable List<VariantOverride> overrides,
@Nullable String stickiness) {
this.name = name;
this.weight = weight;
this.payload = payload;
this.overrides = overrides;
this.stickiness = stickiness;
}

VariantDefinition(String name, int weight) {
this(name, weight, null, Collections.emptyList());
this(name, weight, null, Collections.emptyList(), null);
}

public String getName() {
Expand All @@ -47,7 +58,11 @@ List<VariantOverride> getOverrides() {
}
}

public @Nullable String getStickiness() {
return stickiness;
}

Variant toVariant() {
return new Variant(name, payload, true);
return new Variant(name, payload, true, stickiness);
}
}
20 changes: 18 additions & 2 deletions src/main/java/no/finn/unleash/variant/VariantUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.util.List;
import java.util.Optional;
import java.util.Random;
import java.util.function.Predicate;
import no.finn.unleash.FeatureToggle;
import no.finn.unleash.UnleashContext;
Expand Down Expand Up @@ -61,6 +62,17 @@ private static String getIdentifier(UnleashContext context) {
.orElse(Double.toString(Math.random()))));
}

private static String randomString() {
int randSeed = new Random().nextInt(100000);
return "" + randSeed;
}

private static String getSeed(UnleashContext unleashContext, Optional<String> stickiness) {
return stickiness
.map(s -> unleashContext.getByName(s).orElse(randomString()))
.orElse(getIdentifier(unleashContext));
}

public static Variant selectVariant(
@Nullable FeatureToggle featureToggle, UnleashContext context, Variant defaultVariant) {
if (featureToggle == null) {
Expand All @@ -76,10 +88,14 @@ public static Variant selectVariant(
if (variantOverride.isPresent()) {
return variantOverride.get().toVariant();
}

Optional<String> customStickiness =
variants.stream()
.filter(f -> f.getStickiness() != null)
.map(VariantDefinition::getStickiness)
.findFirst();
int target =
StrategyUtils.getNormalizedNumber(
getIdentifier(context), featureToggle.getName(), totalWeight);
getSeed(context, customStickiness), featureToggle.getName(), totalWeight);

int counter = 0;
for (final VariantDefinition definition : featureToggle.getVariants()) {
Expand Down
4 changes: 3 additions & 1 deletion src/test/java/no/finn/unleash/ManualTesting.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,9 @@ public boolean isEnabled(Map<String, String> parameters) {
.appName("java-test")
.instanceId("instance y")
.unleashAPI("https://unleash.herokuapp.com/api/")
.customHttpHeader("Authorization", "3bd74da5b341d868443134377ba5d802ea1e6fa2d2a948276ade1f092bec8d92")
.customHttpHeader(
"Authorization",
"3bd74da5b341d868443134377ba5d802ea1e6fa2d2a948276ade1f092bec8d92")
.subscriber(
new UnleashSubscriber() {
@Override
Expand Down
14 changes: 8 additions & 6 deletions src/test/java/no/finn/unleash/event/SubscriberTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
import no.finn.unleash.metric.ClientMetrics;
import no.finn.unleash.metric.ClientRegistration;
import no.finn.unleash.repository.FeatureToggleResponse;
import no.finn.unleash.repository.HttpToggleFetcher;
import no.finn.unleash.repository.ToggleBootstrapHandler;
import no.finn.unleash.util.UnleashConfig;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -65,14 +63,18 @@ void subscriberAreNotified() {
assertThat(testSubscriber.toggleEnabled).isFalse();
assertThat(testSubscriber.errors).hasSize(2);

// assertThat(testSubscriber.events).filteredOn(e -> e instanceof ToggleBootstrapHandler.ToggleBootstrapRead).hasSize(1);
// assertThat(testSubscriber.events).filteredOn(e -> e instanceof
// ToggleBootstrapHandler.ToggleBootstrapRead).hasSize(1);
assertThat(testSubscriber.events).filteredOn(e -> e instanceof UnleashReady).hasSize(1);
assertThat(testSubscriber.events).filteredOn(e -> e instanceof ToggleEvaluated).hasSize(3);
assertThat(testSubscriber.events).filteredOn(e -> e instanceof FeatureToggleResponse).hasSize(2);
assertThat(testSubscriber.events).filteredOn(e -> e instanceof ClientRegistration).hasSize(1);
assertThat(testSubscriber.events)
.filteredOn(e -> e instanceof FeatureToggleResponse)
.hasSize(2);
assertThat(testSubscriber.events)
.filteredOn(e -> e instanceof ClientRegistration)
.hasSize(1);
assertThat(testSubscriber.events).filteredOn(e -> e instanceof ClientMetrics).hasSize(1);
assertThat(testSubscriber.events).hasSize(9);

}

private class TestSubscriber implements UnleashSubscriber {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,6 @@ public void should_throw_an_exception_if_project_name_is_not_url_friendly()
UnleashConfig.builder().appName("test").unleashAPI(uri).projectName(name).build();
org.assertj.core.api.Assertions.assertThatThrownBy(() -> new HttpToggleFetcher(config))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("?project="+name+"] was not URL friendly.");
.hasMessageContaining("?project=" + name + "] was not URL friendly.");
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package no.finn.unleash.strategy;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.*;

import java.util.HashMap;
Expand Down Expand Up @@ -141,4 +142,46 @@ public void should_NOT_be_enabled_for_rollout_10_and_randomId_1() {
boolean enabled = strategy.isEnabled(params, context);
assertFalse(enabled);
}

@Test
public void should_not_be_enabled_for_custom_field_402() {
Supplier<String> randomGenerator = () -> "2";
FlexibleRolloutStrategy strategy = new FlexibleRolloutStrategy(randomGenerator);
Map<String, String> params = new HashMap<>();
params.put("rollout", "50");
params.put("stickiness", "customField");
params.put("groupId", "Feature.flexible.rollout.custom.stickiness_50");
UnleashContext context = UnleashContext.builder().addProperty("customField", "402").build();
boolean enabled = strategy.isEnabled(params, context);
assertThat(enabled).isFalse();
}

@Test
public void should_not_be_enabled_for_custom_field_388_and_39() {
Supplier<String> randomGenerator = () -> "2";
FlexibleRolloutStrategy strategy = new FlexibleRolloutStrategy(randomGenerator);
Map<String, String> params = new HashMap<>();
params.put("rollout", "50");
params.put("stickiness", "customField");
params.put("groupId", "Feature.flexible.rollout.custom.stickiness_50");
UnleashContext context = UnleashContext.builder().addProperty("customField", "388").build();
boolean enabled = strategy.isEnabled(params, context);
assertThat(enabled).isTrue();
context = UnleashContext.builder().addProperty("customField", "39").build();
enabled = strategy.isEnabled(params, context);
assertThat(enabled).isTrue();
}

@Test
public void should_not_be_enabled_with_custom_stickiness_if_custom_field_is_missing() {
Supplier<String> randomGenerator = () -> "2";
FlexibleRolloutStrategy strategy = new FlexibleRolloutStrategy(randomGenerator);
Map<String, String> params = new HashMap<>();
params.put("rollout", "50");
params.put("stickiness", "customField");
params.put("groupId", "Feature.flexible.rollout.custom.stickiness_50");
UnleashContext context = UnleashContext.builder().build();
boolean enabled = strategy.isEnabled(params, context);
assertThat(enabled).isFalse();
}
}
Loading

0 comments on commit 323877c

Please sign in to comment.