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

feat: In-memory provider for e2e testing and minimal usage #546

Merged
merged 15 commits into from
Aug 15, 2023
Merged
7 changes: 0 additions & 7 deletions .github/workflows/pullrequest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,6 @@ permissions:
jobs:
build:
runs-on: ubuntu-latest
# TODO: this can be removed with https://github.com/open-feature/java-sdk/issues/523
services:
flagd:
image: ghcr.io/open-feature/flagd-testbed:latest
ports:
- 8013:8013

steps:
- name: Check out the code
uses: actions/checkout@96f53100ba2a5449eb71d2e6604bbcd94b9449b5
Expand Down
9 changes: 2 additions & 7 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,9 @@ If you're adding tests to cover something in the spec, use the `@Specification`

## End-to-End Tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than removing, IMO we should mention that the e2e tests use InMemoryProvider. Consider the GO SDK readme for reference - https://github.com/open-feature/go-sdk/blob/main/e2e/README.md


<!-- TODO: this section should be updated with https://github.com/open-feature/java-sdk/issues/523 -->
The continuous integration runs a set of [gherkin e2e tests](https://github.com/open-feature/test-harness/blob/main/features/evaluation.feature) using `InMemoryProvider`.

The continuous integration runs a set of [gherkin e2e tests](https://github.com/open-feature/test-harness/blob/main/features/evaluation.feature) using [`flagd`](https://github.com/open-feature/flagd). These tests do not run with the default maven profile. If you'd like to run them locally, you can start the flagd testbed with

```
docker run -p 8013:8013 ghcr.io/open-feature/flagd-testbed:latest
```
and then run
to run alone:
```
mvn test -P e2e-test
toddbaert marked this conversation as resolved.
Show resolved Hide resolved
```
Expand Down
67 changes: 28 additions & 39 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
<maven.compiler.source>1.8</maven.compiler.source>
<maven.compiler.target>${maven.compiler.source}</maven.compiler.target>
<junit.jupiter.version>5.10.0</junit.jupiter.version>
<!-- exclusion expression for e2e tests -->
<testExclusions>**/e2e/*.java</testExclusions>
<module-name>${groupId}.${artifactId}</module-name>
</properties>

Expand All @@ -21,10 +19,10 @@
<url>https://openfeature.dev</url>
<developers>
<developer>
<id>abrahms</id>
<name>Justin Abrahms</name>
<organization>eBay</organization>
<url>https://justin.abrah.ms/</url>
<id>abrahms</id>
<name>Justin Abrahms</name>
<organization>eBay</organization>
<url>https://justin.abrah.ms/</url>
</developer>
</developers>
<licenses>
Expand Down Expand Up @@ -120,9 +118,9 @@
</dependency>

<dependency>
<groupId>io.cucumber</groupId>
<artifactId>cucumber-junit-platform-engine</artifactId>
<scope>test</scope>
<groupId>io.cucumber</groupId>
<artifactId>cucumber-junit-platform-engine</artifactId>
<scope>test</scope>
</dependency>

<dependency>
Expand All @@ -139,39 +137,33 @@
<scope>test</scope>
</dependency>

<dependency>
<groupId>dev.openfeature.contrib.providers</groupId>
<artifactId>flagd</artifactId>
<version>0.5.10</version>
<scope>test</scope>
</dependency>

<dependency>
<groupId>org.awaitility</groupId>
<artifactId>awaitility</artifactId>
<version>4.2.0</version>
<scope>test</scope>
</dependency>

</dependencies>

<dependencyManagement>
<dependencies>

<dependency>
<groupId>io.cucumber</groupId>
<artifactId>cucumber-bom</artifactId>
<version>7.13.0</version>
<type>pom</type>
<scope>import</scope>
</dependency>

<dependency>
<groupId>org.junit</groupId>
<artifactId>junit-bom</artifactId>
<version>5.10.0</version>
<type>pom</type>
<scope>import</scope>
</dependency>
<dependency>
<groupId>io.cucumber</groupId>
<artifactId>cucumber-bom</artifactId>
<version>7.13.0</version>
<type>pom</type>
<scope>import</scope>
</dependency>

<dependency>
<groupId>org.junit</groupId>
<artifactId>junit-bom</artifactId>
<version>5.10.0</version>
<type>pom</type>
<scope>import</scope>
</dependency>

</dependencies>
</dependencyManagement>
Expand Down Expand Up @@ -203,7 +195,7 @@
</execution>
</executions>
</plugin>

<plugin>
<artifactId>maven-dependency-plugin</artifactId>
<version>3.6.0</version>
Expand Down Expand Up @@ -249,7 +241,7 @@
<excludes>
<!-- tests to exclude -->
<exclude>${testExclusions}</exclude>
</excludes>
</excludes>
</configuration>
</plugin>

Expand All @@ -271,7 +263,7 @@

<executions>
<execution>
<id>prepare-agent</id>
<id>prepare-agent</id>
<goals>
<goal>prepare-agent</goal>
</goals>
Expand Down Expand Up @@ -319,7 +311,7 @@
</rule>
</rules>
</configuration>
</execution>
</execution>

</executions>
</plugin>
Expand Down Expand Up @@ -496,14 +488,11 @@
</profile>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do not need to remove this profile. It is still needed and works normally

Changes you made at StepDefinisions.java [1] is sufficient to fulfill the comment

[1] - https://github.com/open-feature/java-sdk/pull/546/files#diff-ca6f171ac07058b6e01935480aefe8e8a07379832fc06c334a9ad8887ae3881f

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this section, we do not initialize the test-harness submodule. See the build result - https://github.com/open-feature/java-sdk/actions/runs/5824793560/job/15795407914?pr=546

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By this comment it can be removed, if you still think it should remain update here and I can return it.

<!-- this profile handles running the flagd e2e tests -->
      <!-- TODO: this profile can likely be removed with TODO: this section should be updated with https://github.com/open-feature/java-sdk/issues/523 -->
      <!-- TODO: we should pull the submodule and run these tests unconditionall once flagd isn't required -->

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, we still need them. Tests are now independent from flagd but we need this profile to intialize git submodule and run gherkin tetsts

See the GitHub action - https://github.com/open-feature/java-sdk/blob/main/.github/workflows/pullrequest.yml#L42-L43

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking we could just always pull the submodule and copy the gherkin tests, since now they don't require the external flagd process. I dont really mind if we keep these integration tests on a separate profile or not.

I think if we keep the profile though, we should add back the little bit of documentation in the contributing guide about it.

<profile>
<!-- this profile handles running the flagd e2e tests -->
<!-- TODO: this profile can likely be removed with TODO: this section should be updated with https://github.com/open-feature/java-sdk/issues/523 -->
<!-- TODO: we should pull the submodule and run these tests unconditionall once flagd isn't required -->
<id>e2e-test</id>
<properties>
<!-- run the e2e tests by clearing the exclusions -->
<testExclusions/>
</properties>
</properties>
<build>
<plugins>
<!-- pull the gherkin tests as a git submodule -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public static <T> FlagEvaluationDetails<T> from(ProviderEvaluation<T> providerEv
.value(providerEval.getValue())
.variant(providerEval.getVariant())
.reason(providerEval.getReason())
.errorMessage(providerEval.getErrorMessage())
Kavindu-Dodan marked this conversation as resolved.
Show resolved Hide resolved
.errorCode(providerEval.getErrorCode())
.flagMetadata(providerEval.getFlagMetadata())
.build();
Expand Down
14 changes: 14 additions & 0 deletions src/main/java/dev/openfeature/sdk/Structure.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import java.util.function.Function;
import java.util.stream.Collectors;

import static dev.openfeature.sdk.Value.objectToValue;

/**
* {@link Structure} represents a potentially nested object type which is used to represent
* structured data.
Expand Down Expand Up @@ -123,4 +125,16 @@ default <T extends Structure> Map<String, Value> merge(Function<Map<String, Valu
}
return merged;
}

/**
* Transform an object map to a {@link Structure} type.
*
* @param map map of objects
* @return a Structure object in the SDK format
*/
static Structure mapToStructure(Map<String, Object> map) {
return new MutableStructure(map.entrySet().stream()
.filter(e -> e.getValue() != null)
.collect(Collectors.toMap(Map.Entry::getKey, e -> objectToValue(e.getValue()))));
}
}
37 changes: 37 additions & 0 deletions src/main/java/dev/openfeature/sdk/Value.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@
import java.util.Map;
import java.util.stream.Collectors;

import dev.openfeature.sdk.exceptions.TypeMismatchError;
import lombok.EqualsAndHashCode;
import lombok.SneakyThrows;
import lombok.ToString;

import static dev.openfeature.sdk.Structure.mapToStructure;

/**
* Values serve as a generic return type for structure data from providers.
* Providers may deal in JSON, protobuf, XML or some other data-interchange format.
Expand Down Expand Up @@ -280,4 +283,38 @@
}
return new Value(this.asObject());
}

/**
* Wrap an object into a Value.
*
* @param object the object to wrap
* @return the wrapped object
*/
public static Value objectToValue(Object object) {
if (object instanceof Value) {
return (Value) object;
} else if (object == null) {
return null;

Check warning on line 297 in src/main/java/dev/openfeature/sdk/Value.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/dev/openfeature/sdk/Value.java#L297

Added line #L297 was not covered by tests
} else if (object instanceof String) {
return new Value((String) object);
} else if (object instanceof Boolean) {
return new Value((Boolean) object);
} else if (object instanceof Integer) {
return new Value((Integer) object);
} else if (object instanceof Double) {
return new Value((Double) object);
} else if (object instanceof Structure) {
return new Value((Structure) object);
} else if (object instanceof List) {
return new Value(((List<Object>) object).stream()
.map(o -> objectToValue(o))
.collect(Collectors.toList()));
} else if (object instanceof Instant) {
return new Value((Instant) object);
} else if (object instanceof Map) {
return new Value(mapToStructure((Map<String, Object>) object));
} else {
throw new TypeMismatchError("Flag value " + object + " had unexpected type " + object.getClass() + ".");

Check warning on line 317 in src/main/java/dev/openfeature/sdk/Value.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/dev/openfeature/sdk/Value.java#L317

Added line #L317 was not covered by tests
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package dev.openfeature.sdk.providers.memory;

import dev.openfeature.sdk.EvaluationContext;

/**
* Context evaluator - use for resolving flag according to evaluation context, for handling targeting.
* @param <T> expected value type
*/
public interface ContextEvaluator<T> {

T evaluate(Flag flag, EvaluationContext evaluationContext);
}
21 changes: 21 additions & 0 deletions src/main/java/dev/openfeature/sdk/providers/memory/Flag.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package dev.openfeature.sdk.providers.memory;

import lombok.Builder;
import lombok.Getter;
import lombok.Singular;
import lombok.ToString;

import java.util.Map;

/**
* Flag representation for the in-memory provider.
*/
@ToString
@Builder
@Getter
public class Flag<T> {
@Singular
private Map<String, Object> variants;
private String defaultVariant;
private ContextEvaluator<T> contextEvaluator;
}
Loading
Loading