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(flipt): set variant attachment as value for object evaluation #956

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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
3 changes: 2 additions & 1 deletion providers/flipt/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
## Concepts

* Boolean evaluation gets feature boolean evaluation / enabled status.
* Non-boolean evaluation gets feature variant key.
* Object evaluation gets variant attachment.
* Other evaluations gets feature variant key.

## Usage

Expand Down
15 changes: 15 additions & 0 deletions providers/flipt/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,15 @@
<javadoc.failOnWarnings>false</javadoc.failOnWarnings>
</properties>

<developers>
<developer>
<id>markphelps</id>
<name>Mark Phelps</name>
<organization>Flipt Software</organization>
<url>https://flipt.io/</url>
</developer>
</developers>

<dependencies>
<dependency>
<groupId>io.flipt</groupId>
Expand All @@ -33,6 +42,12 @@
<version>2.0.16</version>
</dependency>

<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<version>2.17.2</version>
</dependency>

<dependency>
<groupId>com.github.tomakehurst</groupId>
<artifactId>wiremock-jre8</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public class FliptProvider extends EventProvider {
@Getter
private ProviderState state = ProviderState.NOT_READY;

private AtomicBoolean isInitialized = new AtomicBoolean(false);
private final AtomicBoolean isInitialized = new AtomicBoolean(false);

/**
* Constructor.
Expand Down Expand Up @@ -123,7 +123,8 @@ public ProviderEvaluation<Boolean> getBooleanEvaluation(String key, Boolean defa

@Override
public ProviderEvaluation<String> getStringEvaluation(String key, String defaultValue, EvaluationContext ctx) {
ProviderEvaluation<Value> valueProviderEvaluation = getObjectEvaluation(key, new Value(defaultValue), ctx);
ProviderEvaluation<Value> valueProviderEvaluation =
evaluateVariant(String.class, key, new Value(defaultValue), ctx);
return ProviderEvaluation.<String>builder()
.value(valueProviderEvaluation.getValue().asString())
.variant(valueProviderEvaluation.getVariant())
Expand All @@ -135,7 +136,8 @@ public ProviderEvaluation<String> getStringEvaluation(String key, String default

@Override
public ProviderEvaluation<Integer> getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx) {
ProviderEvaluation<Value> valueProviderEvaluation = getObjectEvaluation(key, new Value(defaultValue), ctx);
ProviderEvaluation<Value> valueProviderEvaluation =
evaluateVariant(Integer.class, key, new Value(defaultValue), ctx);
Integer value = getIntegerValue(valueProviderEvaluation, defaultValue);
return ProviderEvaluation.<Integer>builder()
.value(value)
Expand All @@ -157,7 +159,8 @@ private static Integer getIntegerValue(ProviderEvaluation<Value> valueProviderEv

@Override
public ProviderEvaluation<Double> getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) {
ProviderEvaluation<Value> valueProviderEvaluation = getObjectEvaluation(key, new Value(defaultValue), ctx);
ProviderEvaluation<Value> valueProviderEvaluation =
evaluateVariant(Double.class, key, new Value(defaultValue), ctx);
Double value = getDoubleValue(valueProviderEvaluation, defaultValue);
return ProviderEvaluation.<Double>builder()
.value(value)
Expand All @@ -179,12 +182,18 @@ private static Double getDoubleValue(ProviderEvaluation<Value> valueProviderEval

@Override
public ProviderEvaluation<Value> getObjectEvaluation(String key, Value defaultValue, EvaluationContext ctx) {
return evaluateVariant(Value.class, key, defaultValue, ctx);
}

private <T> ProviderEvaluation<Value> evaluateVariant(Class<T> clazz, String key, Value defaultValue,
EvaluationContext ctx) {
if (!ProviderState.READY.equals(state)) {
if (ProviderState.NOT_READY.equals(state)) {
throw new ProviderNotReadyError(PROVIDER_NOT_YET_INITIALIZED);
}
throw new GeneralError(UNKNOWN_ERROR);
}

Map<String, String> contextMap = ContextTransformer.transform(ctx);
EvaluationRequest request = EvaluationRequest.builder().namespaceKey(fliptProviderConfig.getNamespace())
.flagKey(key).entityId(ctx.getTargetingKey()).context(contextMap).build();
Expand All @@ -205,17 +214,22 @@ public ProviderEvaluation<Value> getObjectEvaluation(String key, Value defaultVa
.build();
}

Value value = new Value(response.getVariantKey());
ImmutableMetadata.ImmutableMetadataBuilder flagMetadataBuilder = ImmutableMetadata.builder();
if (response.getVariantAttachment() != null) {
if (response.getVariantAttachment() != null && !response.getVariantAttachment().isEmpty()) {
flagMetadataBuilder.addString("variant-attachment", response.getVariantAttachment());

if (clazz.isAssignableFrom(Value.class)) {
value = new Value(response.getVariantAttachment());
}
}

return ProviderEvaluation.<Value>builder()
.value(new Value(response.getVariantKey()))
.variant(response.getVariantKey())
.reason(TARGETING_MATCH.name())
.flagMetadata(flagMetadataBuilder.build())
.build();
.value(value)
.variant(response.getVariantKey())
.reason(TARGETING_MATCH.name())
.flagMetadata(flagMetadataBuilder.build())
.build();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import dev.openfeature.sdk.ProviderEvaluation;
import dev.openfeature.sdk.ProviderEventDetails;
import dev.openfeature.sdk.ProviderState;
import dev.openfeature.sdk.Value;
import dev.openfeature.sdk.exceptions.GeneralError;
import dev.openfeature.sdk.exceptions.ProviderNotReadyError;
import lombok.SneakyThrows;
Expand All @@ -31,8 +32,7 @@
import static com.github.tomakehurst.wiremock.client.WireMock.post;
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.*;

/**
* FliptProvider test, based on APIs mocking.
Expand All @@ -50,9 +50,10 @@ class FliptProviderTest {
public static final Double DOUBLE_FLAG_VALUE = 1.23;
public static final String USERS_FLAG_NAME = "users-flag";
public static final String TARGETING_KEY = "targeting_key";
public static final String OBJECT_FLAG_NAME = "object-flag";

private static FliptProvider fliptProvider;
private static Client client;

private String apiUrl;

@BeforeAll
Expand Down Expand Up @@ -101,7 +102,6 @@ void getBooleanEvaluation() {
mockFliptAPI("/evaluate/v1/boolean", "boolean.json", FLAG_NAME);
MutableContext evaluationContext = new MutableContext();
evaluationContext.setTargetingKey(TARGETING_KEY);
assertEquals(true, fliptProvider.getBooleanEvaluation(FLAG_NAME, false, evaluationContext).getValue());
assertEquals(true, client.getBooleanValue(FLAG_NAME, false, evaluationContext));
assertEquals(false, client.getBooleanValue("non-existing", false, evaluationContext));
}
Expand Down Expand Up @@ -171,7 +171,25 @@ void getEvaluationMetadataTest() {
assertEquals("attachment-1", flagMetadata.getString("variant-attachment"));
FlagEvaluationDetails<String> nonExistingFlagEvaluation = client.getStringDetails("non-existing", "",
evaluationContext);
assertEquals(null, nonExistingFlagEvaluation.getFlagMetadata().getBoolean("variant-attachment"));
assertNull(nonExistingFlagEvaluation.getFlagMetadata().getBoolean("variant-attachment"));
}

@SneakyThrows
@Test
void getObjectEvaluationTest() {
mockFliptAPI("/evaluate/v1/variant", "variant-object.json", OBJECT_FLAG_NAME);
MutableContext evaluationContext = new MutableContext();
evaluationContext.setTargetingKey(TARGETING_KEY);
evaluationContext.add("userId", "object");

Value expectedValue = new Value("{\"key1\":\"value1\",\"key2\":42,\"key3\":true}");
Value emptyValue = new Value();

assertEquals(expectedValue, client.getObjectValue(OBJECT_FLAG_NAME, emptyValue, evaluationContext));
assertEquals(emptyValue, client.getObjectValue("non-existing", emptyValue, evaluationContext));

// non-object flag value
assertEquals(emptyValue, client.getObjectValue(VARIANT_FLAG_NAME, emptyValue, evaluationContext));
}

@SneakyThrows
Expand Down
6 changes: 6 additions & 0 deletions providers/flipt/src/test/resources/variant-object.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"flagKey": "object-flag",
"match": true,
"variantKey": "object-variant",
"variantAttachment": "{\"key1\":\"value1\",\"key2\":42,\"key3\":true}"
}