Skip to content

Commit ce8282e

Browse files
liamhughesCopilot
liamhughes
and
Copilot
authored
Fix: client breaks when evaluation contains segments or unknown response fields (#5)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 475f2a7 commit ce8282e

17 files changed

Lines changed: 310 additions & 12 deletions

‎src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,23 +3,27 @@
33
import com.fasterxml.jackson.annotation.JsonCreator;
44
import com.fasterxml.jackson.annotation.JsonProperty;
55

6+
import java.util.Collections;
67
import java.util.ArrayList;
78
import java.util.List;
8-
import java.util.Map;
99

1010
class FeatureToggleEvaluation {
1111
private final String name;
1212
private final String slug;
1313
private final boolean isEnabled;
14-
private final List<Map.Entry<String, String>> segments;
14+
private final List<Segment> segments;
1515

1616
@JsonCreator(mode = JsonCreator.Mode.PROPERTIES)
17-
FeatureToggleEvaluation(@JsonProperty("name") String name, @JsonProperty("slug")String slug, @JsonProperty("isEnabled") boolean isEnabled,
18-
@JsonProperty("segments") List<Map.Entry<String, String>> segments) {
17+
FeatureToggleEvaluation(
18+
@JsonProperty("name") String name,
19+
@JsonProperty("slug") String slug,
20+
@JsonProperty("isEnabled") boolean isEnabled,
21+
@JsonProperty("segments") List<Segment> segments
22+
) {
1923
this.name = name;
2024
this.slug = slug;
2125
this.isEnabled = isEnabled;
22-
26+
2327
this.segments = new ArrayList<>();
2428
if (segments != null) {
2529
this.segments.addAll(segments);
@@ -38,7 +42,7 @@ public boolean isEnabled() {
3842
return isEnabled;
3943
}
4044

41-
public List<Map.Entry<String, String>> getSegments() {
42-
return segments;
45+
public List<Segment> getSegments() {
46+
return Collections.unmodifiableList(segments);
4347
}
4448
}

‎src/main/java/com/octopus/openfeature/provider/OctopusClient.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ Boolean haveFeatureTogglesChanged(byte[] contentHash)
3636
.build();
3737
try {
3838
HttpResponse<String> httpResponse = client.send(request, HttpResponse.BodyHandlers.ofString());
39-
FeatureToggleCheckResponse checkResponse = new ObjectMapper().readValue(httpResponse.body(), FeatureToggleCheckResponse.class);
39+
FeatureToggleCheckResponse checkResponse = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), FeatureToggleCheckResponse.class);
4040
return !Arrays.equals(checkResponse.contentHash, contentHash);
4141
} catch (Exception e) {
4242
logger.log(System.Logger.Level.WARNING, String.format("Unable to query Octopus Feature Toggle service. URI: %s", checkURI.toString()), e);
@@ -65,7 +65,7 @@ FeatureToggles getFeatureToggleEvaluationManifest()
6565
logger.log(System.Logger.Level.WARNING,String.format("Feature toggle response from %s did not contain expected ContentHash header", manifestURI.toString()));
6666
return null;
6767
}
68-
List<FeatureToggleEvaluation> evaluations = new ObjectMapper().readValue(httpResponse.body(), new TypeReference<List<FeatureToggleEvaluation>>(){});
68+
List<FeatureToggleEvaluation> evaluations = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), new TypeReference<List<FeatureToggleEvaluation>>(){});
6969
return new FeatureToggles(evaluations, Base64.getDecoder().decode(contentHashHeader.get()));
7070
} catch (Exception e) {
7171
logger.log(System.Logger.Level.WARNING, "Unable to query Octopus Feature Toggle service", e);

‎src/main/java/com/octopus/openfeature/provider/OctopusContext.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,13 @@ ProviderEvaluation<Boolean> evaluate(String slug, Boolean defaultValue, Evaluati
4848
.build();
4949
}
5050

51-
private Boolean MatchesSegment(EvaluationContext evaluationContext, List<Map.Entry<String, String>> segments) {
51+
private Boolean MatchesSegment(EvaluationContext evaluationContext, List<Segment> segments) {
5252
if (evaluationContext == null) {
5353
return false;
5454
}
5555

5656
var contextEntries = evaluationContext.asMap();
57-
var groupedByKey = segments.stream().collect(groupingBy(Map.Entry::getKey));
57+
var groupedByKey = segments.stream().collect(groupingBy(Segment::getKey));
5858
return groupedByKey.keySet().stream().allMatch(k -> {
5959
var values = groupedByKey.get(k);
6060

‎src/main/java/com/octopus/openfeature/provider/OctopusObjectMapper.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
package com.octopus.openfeature.provider;
2+
3+
import com.fasterxml.jackson.databind.DeserializationFeature;
4+
import com.fasterxml.jackson.databind.MapperFeature;
5+
import com.fasterxml.jackson.databind.ObjectMapper;
6+
import com.fasterxml.jackson.databind.json.JsonMapper;
7+
8+
class OctopusObjectMapper {
9+
static final ObjectMapper INSTANCE = JsonMapper.builder()
10+
.enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES)
11+
.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
12+
.build();
13+
}

‎src/main/java/com/octopus/openfeature/provider/Segment.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package com.octopus.openfeature.provider;
2+
3+
import com.fasterxml.jackson.annotation.JsonCreator;
4+
import com.fasterxml.jackson.annotation.JsonProperty;
5+
6+
class Segment {
7+
private final String key;
8+
private final String value;
9+
10+
@JsonCreator(mode = JsonCreator.Mode.PROPERTIES)
11+
public Segment(
12+
@JsonProperty(value = "key", required = true) String key,
13+
@JsonProperty(value = "value", required = true) String value
14+
) {
15+
this.key = key;
16+
this.value = value;
17+
}
18+
19+
public String getKey() {
20+
return key;
21+
}
22+
23+
public String getValue() {
24+
return value;
25+
}
26+
}

‎src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
package com.octopus.openfeature.provider;
2+
3+
import com.fasterxml.jackson.core.type.TypeReference;
4+
import com.fasterxml.jackson.databind.ObjectMapper;
5+
import com.fasterxml.jackson.databind.exc.MismatchedInputException;
6+
import org.junit.jupiter.api.Test;
7+
8+
import java.io.InputStream;
9+
import java.util.List;
10+
import java.util.Map;
11+
12+
import static org.assertj.core.api.Assertions.assertThat;
13+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
14+
15+
class FeatureToggleEvaluationDeserializationTests {
16+
17+
private final ObjectMapper objectMapper = OctopusObjectMapper.INSTANCE;
18+
19+
private InputStream resource(String name) {
20+
return getClass().getResourceAsStream(name);
21+
}
22+
23+
private void assertSegmentsContain(List<Segment> segments, Segment... expected) {
24+
assertThat(segments).usingRecursiveFieldByFieldElementComparator().contains(expected);
25+
}
26+
27+
@Test
28+
void shouldDeserializeEnabledToggle() throws Exception {
29+
FeatureToggleEvaluation result = objectMapper.readValue(resource("toggle-enabled-no-segments.json"), FeatureToggleEvaluation.class);
30+
31+
assertThat(result.getName()).isEqualTo("My Feature");
32+
assertThat(result.getSlug()).isEqualTo("my-feature");
33+
assertThat(result.isEnabled()).isTrue();
34+
assertThat(result.getSegments()).isEmpty();
35+
}
36+
37+
@Test
38+
void shouldDeserializeDisabledToggle() throws Exception {
39+
FeatureToggleEvaluation result = objectMapper.readValue(resource("toggle-disabled.json"), FeatureToggleEvaluation.class);
40+
41+
assertThat(result.isEnabled()).isFalse();
42+
}
43+
44+
@Test
45+
void shouldDeserializeToggleWithMissingSegmentsField() throws Exception {
46+
FeatureToggleEvaluation result = objectMapper.readValue(resource("toggle-missing-segments.json"), FeatureToggleEvaluation.class);
47+
48+
assertThat(result.getSegments()).isNotNull().isEmpty();
49+
}
50+
51+
@Test
52+
void shouldDeserializeToggleWithSegments() throws Exception {
53+
FeatureToggleEvaluation result = objectMapper.readValue(
54+
resource("toggle-with-segments.json"), FeatureToggleEvaluation.class);
55+
56+
assertThat(result.getSegments()).hasSize(2);
57+
assertSegmentsContain(result.getSegments(),
58+
new Segment("license-type", "free"),
59+
new Segment("country", "au")
60+
);
61+
}
62+
63+
@Test
64+
void shouldDeserializeListOfToggles() throws Exception {
65+
List<FeatureToggleEvaluation> result = objectMapper.readValue(
66+
resource("toggle-list.json"),
67+
new TypeReference<>() {
68+
}
69+
);
70+
71+
assertThat(result).hasSize(2);
72+
assertThat(result.get(0).getSlug()).isEqualTo("feature-a");
73+
assertThat(result.get(0).isEnabled()).isTrue();
74+
assertThat(result.get(1).getSlug()).isEqualTo("feature-b");
75+
assertThat(result.get(1).isEnabled()).isFalse();
76+
}
77+
78+
@Test
79+
void shouldDeserializeListOfTogglesWithVariousFieldCasings() throws Exception {
80+
List<FeatureToggleEvaluation> result = objectMapper.readValue(
81+
resource("toggles-with-different-field-capitalisation.json"),
82+
new TypeReference<>() {
83+
}
84+
);
85+
86+
assertThat(result).hasSize(3);
87+
assertThat(result.get(0).getSlug()).isEqualTo("feature-a");
88+
assertThat(result.get(0).isEnabled()).isTrue();
89+
assertSegmentsContain(result.get(0).getSegments(), new Segment("license-type", "free"));
90+
assertThat(result.get(1).getSlug()).isEqualTo("feature-b");
91+
assertThat(result.get(1).isEnabled()).isTrue();
92+
assertSegmentsContain(result.get(1).getSegments(), new Segment("plan", "enterprise"));
93+
assertThat(result.get(2).getSlug()).isEqualTo("feature-c");
94+
assertThat(result.get(2).isEnabled()).isTrue();
95+
assertSegmentsContain(result.get(2).getSegments(), new Segment("country", "au"));
96+
}
97+
98+
@Test
99+
void shouldFailDeserializationWhenSegmentKeyIsMissing() {
100+
assertThatThrownBy(() -> objectMapper.readValue(
101+
resource("toggle-segment-missing-key.json"), FeatureToggleEvaluation.class))
102+
.isInstanceOf(MismatchedInputException.class);
103+
}
104+
105+
@Test
106+
void shouldFailDeserializationWhenSegmentValueIsMissing() {
107+
assertThatThrownBy(() -> objectMapper.readValue(
108+
resource("toggle-segment-missing-value.json"), FeatureToggleEvaluation.class))
109+
.isInstanceOf(MismatchedInputException.class);
110+
}
111+
112+
@Test
113+
void shouldFailDeserializationWhenSegmentKeyAndValueAreMissing() {
114+
assertThatThrownBy(() -> objectMapper.readValue(
115+
resource("toggle-segment-missing-key-and-value.json"), FeatureToggleEvaluation.class))
116+
.isInstanceOf(MismatchedInputException.class);
117+
}
118+
119+
@Test
120+
void shouldIgnoreExtraneousProperties() throws Exception {
121+
FeatureToggleEvaluation result = objectMapper.readValue(
122+
resource("toggle-with-extraneous-properties.json"), FeatureToggleEvaluation.class);
123+
124+
assertThat(result.getName()).isEqualTo("My Feature");
125+
assertThat(result.getSlug()).isEqualTo("my-feature");
126+
assertThat(result.isEnabled()).isTrue();
127+
assertSegmentsContain(result.getSegments(), new Segment("license-type", "free"));
128+
}
129+
}

‎src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ class OctopusContextTests {
2020
Arrays.asList(
2121
new FeatureToggleEvaluation("Enabled Feature", "enabled-feature", true, null),
2222
new FeatureToggleEvaluation("Disabled Feature", "disabled-feature", false, null),
23-
new FeatureToggleEvaluation("Feature With Segments", "feature-with-segments", true, Arrays.asList(Map.entry("license-type", "free"), Map.entry("country", "au")) )
23+
new FeatureToggleEvaluation("Feature With Segments", "feature-with-segments", true, Arrays.asList(new Segment("license-type", "free"), new Segment("country", "au")) )
2424
),
2525
new byte[0]
2626
);

‎src/test/resources/com/octopus/openfeature/provider/toggle-disabled.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"name": "My Feature",
3+
"slug": "my-feature",
4+
"isEnabled": false,
5+
"segments": null
6+
}

‎src/test/resources/com/octopus/openfeature/provider/toggle-enabled-no-segments.json‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"name": "My Feature",
3+
"slug": "my-feature",
4+
"isEnabled": true,
5+
"segments": null
6+
}

‎src/test/resources/com/octopus/openfeature/provider/toggle-list.json‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
[
2+
{
3+
"name": "Feature A",
4+
"slug": "feature-a",
5+
"isEnabled": true,
6+
"segments": null
7+
},
8+
{
9+
"name": "Feature B",
10+
"slug": "feature-b",
11+
"isEnabled": false,
12+
"segments": null
13+
}
14+
]

0 commit comments

Comments
 (0)