From 1b0dffa7fc96df5bcbac3c1ff39ae88d64c0d1d6 Mon Sep 17 00:00:00 2001 From: Liam Hughes Date: Thu, 19 Mar 2026 13:18:20 +1100 Subject: [PATCH 01/12] JSON deserialisation tests --- ...eToggleEvaluationDeserializationTests.java | 82 +++++++++++++++++++ .../openfeature/provider/toggle-disabled.json | 6 ++ .../provider/toggle-enabled-no-segments.json | 6 ++ .../openfeature/provider/toggle-list.json | 14 ++++ .../provider/toggle-missing-segments.json | 5 ++ .../toggle-with-extraneous-properties.json | 17 ++++ .../provider/toggle-with-segments.json | 15 ++++ 7 files changed, 145 insertions(+) create mode 100644 src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java create mode 100644 src/test/resources/com/octopus/openfeature/provider/toggle-disabled.json create mode 100644 src/test/resources/com/octopus/openfeature/provider/toggle-enabled-no-segments.json create mode 100644 src/test/resources/com/octopus/openfeature/provider/toggle-list.json create mode 100644 src/test/resources/com/octopus/openfeature/provider/toggle-missing-segments.json create mode 100644 src/test/resources/com/octopus/openfeature/provider/toggle-with-extraneous-properties.json create mode 100644 src/test/resources/com/octopus/openfeature/provider/toggle-with-segments.json diff --git a/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java b/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java new file mode 100644 index 0000000..52a514f --- /dev/null +++ b/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java @@ -0,0 +1,82 @@ +package com.octopus.openfeature.provider; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.ObjectMapper; +import org.junit.jupiter.api.Test; + +import java.io.InputStream; +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +class FeatureToggleEvaluationDeserializationTests { + + private final ObjectMapper objectMapper = new ObjectMapper(); + + private InputStream resource(String name) { + return getClass().getResourceAsStream(name); + } + + @Test + void shouldDeserializeEnabledToggle() throws Exception { + FeatureToggleEvaluation result = objectMapper.readValue(resource("toggle-enabled-no-segments.json"), FeatureToggleEvaluation.class); + + assertThat(result.getName()).isEqualTo("My Feature"); + assertThat(result.getSlug()).isEqualTo("my-feature"); + assertThat(result.isEnabled()).isTrue(); + assertThat(result.getSegments()).isEmpty(); + } + + @Test + void shouldDeserializeDisabledToggle() throws Exception { + FeatureToggleEvaluation result = objectMapper.readValue(resource("toggle-disabled.json"), FeatureToggleEvaluation.class); + + assertThat(result.isEnabled()).isFalse(); + } + + @Test + void shouldDeserializeToggleWithMissingSegmentsField() throws Exception { + FeatureToggleEvaluation result = objectMapper.readValue(resource("toggle-missing-segments.json"), FeatureToggleEvaluation.class); + + assertThat(result.getSegments()).isNotNull().isEmpty(); + } + + @Test + void shouldDeserializeToggleWithSegments() throws Exception { + FeatureToggleEvaluation result = objectMapper.readValue( + resource("toggle-with-segments.json"), FeatureToggleEvaluation.class); + + assertThat(result.getSegments()).hasSize(2); + assertThat(result.getSegments()).contains( + Map.entry("license-type", "free"), + Map.entry("country", "au") + ); + } + + @Test + void shouldDeserializeListOfToggles() throws Exception { + List result = objectMapper.readValue( + resource("toggle-list.json"), + new TypeReference<>() { + } + ); + + assertThat(result).hasSize(2); + assertThat(result.get(0).getSlug()).isEqualTo("feature-a"); + assertThat(result.get(0).isEnabled()).isTrue(); + assertThat(result.get(1).getSlug()).isEqualTo("feature-b"); + assertThat(result.get(1).isEnabled()).isFalse(); + } + + @Test + void shouldIgnoreExtraneousProperties() throws Exception { + FeatureToggleEvaluation result = objectMapper.readValue( + resource("toggle-with-extraneous-properties.json"), FeatureToggleEvaluation.class); + + assertThat(result.getName()).isEqualTo("My Feature"); + assertThat(result.getSlug()).isEqualTo("my-feature"); + assertThat(result.isEnabled()).isTrue(); + assertThat(result.getSegments()).contains(Map.entry("license-type", "free")); + } +} diff --git a/src/test/resources/com/octopus/openfeature/provider/toggle-disabled.json b/src/test/resources/com/octopus/openfeature/provider/toggle-disabled.json new file mode 100644 index 0000000..f09370d --- /dev/null +++ b/src/test/resources/com/octopus/openfeature/provider/toggle-disabled.json @@ -0,0 +1,6 @@ +{ + "name": "My Feature", + "slug": "my-feature", + "isEnabled": false, + "segments": null +} diff --git a/src/test/resources/com/octopus/openfeature/provider/toggle-enabled-no-segments.json b/src/test/resources/com/octopus/openfeature/provider/toggle-enabled-no-segments.json new file mode 100644 index 0000000..e9d17e9 --- /dev/null +++ b/src/test/resources/com/octopus/openfeature/provider/toggle-enabled-no-segments.json @@ -0,0 +1,6 @@ +{ + "name": "My Feature", + "slug": "my-feature", + "isEnabled": true, + "segments": null +} diff --git a/src/test/resources/com/octopus/openfeature/provider/toggle-list.json b/src/test/resources/com/octopus/openfeature/provider/toggle-list.json new file mode 100644 index 0000000..ea6d15f --- /dev/null +++ b/src/test/resources/com/octopus/openfeature/provider/toggle-list.json @@ -0,0 +1,14 @@ +[ + { + "name": "Feature A", + "slug": "feature-a", + "isEnabled": true, + "segments": null + }, + { + "name": "Feature B", + "slug": "feature-b", + "isEnabled": false, + "segments": null + } +] diff --git a/src/test/resources/com/octopus/openfeature/provider/toggle-missing-segments.json b/src/test/resources/com/octopus/openfeature/provider/toggle-missing-segments.json new file mode 100644 index 0000000..285d13e --- /dev/null +++ b/src/test/resources/com/octopus/openfeature/provider/toggle-missing-segments.json @@ -0,0 +1,5 @@ +{ + "name": "My Feature", + "slug": "my-feature", + "isEnabled": true +} diff --git a/src/test/resources/com/octopus/openfeature/provider/toggle-with-extraneous-properties.json b/src/test/resources/com/octopus/openfeature/provider/toggle-with-extraneous-properties.json new file mode 100644 index 0000000..39b78d1 --- /dev/null +++ b/src/test/resources/com/octopus/openfeature/provider/toggle-with-extraneous-properties.json @@ -0,0 +1,17 @@ +{ + "name": "My Feature", + "slug": "my-feature", + "isEnabled": true, + "segments": [ + { + "key": "license-type", + "value": "free", + "more": "data" + } + ], + "foo": "bar", + "qux": 123, + "wux": { + "nested": "value" + } +} \ No newline at end of file diff --git a/src/test/resources/com/octopus/openfeature/provider/toggle-with-segments.json b/src/test/resources/com/octopus/openfeature/provider/toggle-with-segments.json new file mode 100644 index 0000000..88ae4f0 --- /dev/null +++ b/src/test/resources/com/octopus/openfeature/provider/toggle-with-segments.json @@ -0,0 +1,15 @@ +{ + "name": "My Feature", + "slug": "my-feature", + "isEnabled": true, + "segments": [ + { + "key": "license-type", + "value": "free" + }, + { + "key": "country", + "value": "au" + } + ] +} \ No newline at end of file From f6a013c61fd6f2b11f6b3eae9e7d7269506cdafe Mon Sep 17 00:00:00 2001 From: Liam Hughes Date: Thu, 26 Mar 2026 09:38:34 +1100 Subject: [PATCH 02/12] Apply fixes --- .../provider/FeatureToggleEvaluation.java | 13 +++++++++--- .../provider/SegmentDeserializer.java | 20 +++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java diff --git a/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java b/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java index d9db7be..b43f99c 100644 --- a/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java +++ b/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java @@ -1,12 +1,15 @@ package com.octopus.openfeature.provider; import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import java.util.ArrayList; import java.util.List; import java.util.Map; +@JsonIgnoreProperties(ignoreUnknown = true) class FeatureToggleEvaluation { private final String name; private final String slug; @@ -14,12 +17,16 @@ class FeatureToggleEvaluation { private final List> segments; @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) - FeatureToggleEvaluation(@JsonProperty("name") String name, @JsonProperty("slug")String slug, @JsonProperty("isEnabled") boolean isEnabled, - @JsonProperty("segments") List> segments) { + FeatureToggleEvaluation( + @JsonProperty("name") String name, + @JsonProperty("slug") String slug, + @JsonProperty("isEnabled") boolean isEnabled, + @JsonDeserialize(contentUsing = SegmentDeserializer.class) @JsonProperty("segments") List> segments + ) { this.name = name; this.slug = slug; this.isEnabled = isEnabled; - + this.segments = new ArrayList<>(); if (segments != null) { this.segments.addAll(segments); diff --git a/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java b/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java new file mode 100644 index 0000000..3eba90d --- /dev/null +++ b/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java @@ -0,0 +1,20 @@ +package com.octopus.openfeature.provider; + +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.databind.DeserializationContext; +import com.fasterxml.jackson.databind.JsonDeserializer; +import com.fasterxml.jackson.databind.JsonNode; + +import java.io.IOException; +import java.util.AbstractMap; +import java.util.Map; + +class SegmentDeserializer extends JsonDeserializer> { + @Override + public Map.Entry deserialize(JsonParser p, DeserializationContext ctxt) throws IOException { + JsonNode node = p.getCodec().readTree(p); + String key = node.get("key").asText(); + String value = node.get("value").asText(); + return new AbstractMap.SimpleEntry<>(key, value); + } +} From c6a88555b8bfbb55772375269c4495dec8a4950e Mon Sep 17 00:00:00 2001 From: Liam Hughes Date: Thu, 26 Mar 2026 09:57:01 +1100 Subject: [PATCH 03/12] Add more checks during segment deserialisation Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../provider/SegmentDeserializer.java | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java b/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java index 3eba90d..0dbdb6d 100644 --- a/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java +++ b/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java @@ -13,8 +13,34 @@ class SegmentDeserializer extends JsonDeserializer> { @Override public Map.Entry deserialize(JsonParser p, DeserializationContext ctxt) throws IOException { JsonNode node = p.getCodec().readTree(p); - String key = node.get("key").asText(); - String value = node.get("value").asText(); + if (node == null || !node.isObject()) { + ctxt.reportInputMismatch( + Map.Entry.class, + "Expected JSON object for Segment but got: %s", + node + ); + } + + JsonNode keyNode = node.get("key"); + if (keyNode == null || keyNode.isNull() || !keyNode.isTextual()) { + ctxt.reportInputMismatch( + Map.Entry.class, + "Expected non-null string 'key' field but got: %s", + keyNode + ); + } + + JsonNode valueNode = node.get("value"); + if (valueNode == null || valueNode.isNull() || !valueNode.isTextual()) { + ctxt.reportInputMismatch( + Map.Entry.class, + "Expected non-null string 'value' field but got: %s", + valueNode + ); + } + + String key = keyNode.asText(); + String value = valueNode.asText(); return new AbstractMap.SimpleEntry<>(key, value); } } From 2310f22d7e68fb10f6ea4fa7b2aa09f5964d81ac Mon Sep 17 00:00:00 2001 From: Liam Hughes Date: Thu, 26 Mar 2026 10:18:33 +1100 Subject: [PATCH 04/12] Add SegmentDeserializerTests --- .../provider/SegmentDeserializer.java | 5 + .../provider/SegmentDeserializerTests.java | 95 +++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 src/test/java/com/octopus/openfeature/provider/SegmentDeserializerTests.java diff --git a/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java b/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java index 0dbdb6d..87764fa 100644 --- a/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java +++ b/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java @@ -21,6 +21,7 @@ public Map.Entry deserialize(JsonParser p, DeserializationContex ); } + assert node != null; JsonNode keyNode = node.get("key"); if (keyNode == null || keyNode.isNull() || !keyNode.isTextual()) { ctxt.reportInputMismatch( @@ -39,8 +40,12 @@ public Map.Entry deserialize(JsonParser p, DeserializationContex ); } + assert keyNode != null; String key = keyNode.asText(); + + assert valueNode != null; String value = valueNode.asText(); + return new AbstractMap.SimpleEntry<>(key, value); } } diff --git a/src/test/java/com/octopus/openfeature/provider/SegmentDeserializerTests.java b/src/test/java/com/octopus/openfeature/provider/SegmentDeserializerTests.java new file mode 100644 index 0000000..222c3a9 --- /dev/null +++ b/src/test/java/com/octopus/openfeature/provider/SegmentDeserializerTests.java @@ -0,0 +1,95 @@ +package com.octopus.openfeature.provider; + +import com.fasterxml.jackson.databind.JsonMappingException; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.module.SimpleModule; +import org.junit.jupiter.api.Test; + +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class SegmentDeserializerTests { + private final ObjectMapper objectMapper = new ObjectMapper() + .registerModule(new SimpleModule().addDeserializer( + Map.Entry.class, + new SegmentDeserializer() + )); + + @Test + void shouldDeserializeSegmentWithKeyAndValue() throws Exception { + Map.Entry result = objectMapper.readValue( + "{\"key\":\"license-type\",\"value\":\"free\"}", + Map.Entry.class + ); + + assertThat(result.getKey()).isEqualTo("license-type"); + assertThat(result.getValue()).isEqualTo("free"); + } + + @Test + void shouldIgnoreExtraFields() throws Exception { + Map.Entry result = objectMapper.readValue( + "{\"key\":\"k\",\"value\":\"v\",\"extra\":\"ignored\"}", + Map.Entry.class + ); + + assertThat(result.getKey()).isEqualTo("k"); + assertThat(result.getValue()).isEqualTo("v"); + } + + @Test + void shouldRejectMissingKey() { + assertThatThrownBy(() -> objectMapper.readValue("{\"value\":\"v\"}", Map.Entry.class)) + .isInstanceOf(JsonMappingException.class) + .hasMessageContaining("key"); + } + + @Test + void shouldRejectNullKey() { + assertThatThrownBy(() -> objectMapper.readValue("{\"key\":null,\"value\":\"v\"}", Map.Entry.class)) + .isInstanceOf(JsonMappingException.class) + .hasMessageContaining("key"); + } + + @Test + void shouldRejectNonStringKey() { + assertThatThrownBy(() -> objectMapper.readValue("{\"key\":42,\"value\":\"v\"}", Map.Entry.class)) + .isInstanceOf(JsonMappingException.class) + .hasMessageContaining("key"); + } + + @Test + void shouldRejectMissingValue() { + assertThatThrownBy(() -> objectMapper.readValue("{\"key\":\"k\"}", Map.Entry.class)) + .isInstanceOf(JsonMappingException.class) + .hasMessageContaining("value"); + } + + @Test + void shouldRejectNullValue() { + assertThatThrownBy(() -> objectMapper.readValue("{\"key\":\"k\",\"value\":null}", Map.Entry.class)) + .isInstanceOf(JsonMappingException.class) + .hasMessageContaining("value"); + } + + @Test + void shouldRejectNonStringValue() { + assertThatThrownBy(() -> objectMapper.readValue("{\"key\":\"k\",\"value\":42}", Map.Entry.class)) + .isInstanceOf(JsonMappingException.class) + .hasMessageContaining("value"); + } + + @Test + void shouldRejectArrayInput() { + assertThatThrownBy(() -> objectMapper.readValue("[\"k\",\"v\"]", Map.Entry.class)) + .isInstanceOf(JsonMappingException.class); + } + + @Test + void shouldRejectScalarInput() { + assertThatThrownBy(() -> objectMapper.readValue("\"just-a-string\"", Map.Entry.class)) + .isInstanceOf(JsonMappingException.class); + } +} From d4b0cfbe0ee7ccf609bf52c322983ec47745c0da Mon Sep 17 00:00:00 2001 From: Liam Hughes Date: Thu, 26 Mar 2026 10:24:05 +1100 Subject: [PATCH 05/12] Idiomatic handling of null warnings --- .../octopus/openfeature/provider/SegmentDeserializer.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java b/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java index 87764fa..cb55d1c 100644 --- a/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java +++ b/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java @@ -19,9 +19,9 @@ public Map.Entry deserialize(JsonParser p, DeserializationContex "Expected JSON object for Segment but got: %s", node ); + return null; } - assert node != null; JsonNode keyNode = node.get("key"); if (keyNode == null || keyNode.isNull() || !keyNode.isTextual()) { ctxt.reportInputMismatch( @@ -29,6 +29,7 @@ public Map.Entry deserialize(JsonParser p, DeserializationContex "Expected non-null string 'key' field but got: %s", keyNode ); + return null; } JsonNode valueNode = node.get("value"); @@ -38,12 +39,10 @@ public Map.Entry deserialize(JsonParser p, DeserializationContex "Expected non-null string 'value' field but got: %s", valueNode ); + return null; } - assert keyNode != null; String key = keyNode.asText(); - - assert valueNode != null; String value = valueNode.asText(); return new AbstractMap.SimpleEntry<>(key, value); From 633c0264825fc17a05de9d3bdea01e6f1f7964ec Mon Sep 17 00:00:00 2001 From: Liam Hughes Date: Thu, 26 Mar 2026 10:30:39 +1100 Subject: [PATCH 06/12] SimpleImmutableEntry --- .../com/octopus/openfeature/provider/SegmentDeserializer.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java b/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java index cb55d1c..62ae758 100644 --- a/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java +++ b/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java @@ -45,6 +45,6 @@ public Map.Entry deserialize(JsonParser p, DeserializationContex String key = keyNode.asText(); String value = valueNode.asText(); - return new AbstractMap.SimpleEntry<>(key, value); + return new AbstractMap.SimpleImmutableEntry<>(key, value); } } From 10465084013feb594d1b5d2d6c9bcd77ce713db8 Mon Sep 17 00:00:00 2001 From: Liam Hughes Date: Thu, 26 Mar 2026 15:27:38 +1100 Subject: [PATCH 07/12] Make deserialisation case insensitive --- .../provider/FeatureToggleEvaluation.java | 2 -- .../openfeature/provider/OctopusClient.java | 4 +-- .../provider/OctopusObjectMapper.java | 15 ++++++++ .../provider/SegmentDeserializer.java | 16 +++++++-- ...eToggleEvaluationDeserializationTests.java | 22 +++++++++++- ...s-with-different-field-capitalisation.json | 35 +++++++++++++++++++ 6 files changed, 87 insertions(+), 7 deletions(-) create mode 100644 src/main/java/com/octopus/openfeature/provider/OctopusObjectMapper.java create mode 100644 src/test/resources/com/octopus/openfeature/provider/toggles-with-different-field-capitalisation.json diff --git a/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java b/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java index b43f99c..1d13a66 100644 --- a/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java +++ b/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java @@ -1,7 +1,6 @@ package com.octopus.openfeature.provider; import com.fasterxml.jackson.annotation.JsonCreator; -import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; @@ -9,7 +8,6 @@ import java.util.List; import java.util.Map; -@JsonIgnoreProperties(ignoreUnknown = true) class FeatureToggleEvaluation { private final String name; private final String slug; diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java index c2b2b56..6fb2e9e 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java @@ -36,7 +36,7 @@ Boolean haveFeatureTogglesChanged(byte[] contentHash) .build(); try { HttpResponse httpResponse = client.send(request, HttpResponse.BodyHandlers.ofString()); - FeatureToggleCheckResponse checkResponse = new ObjectMapper().readValue(httpResponse.body(), FeatureToggleCheckResponse.class); + FeatureToggleCheckResponse checkResponse = OctopusObjectMapper.create().readValue(httpResponse.body(), FeatureToggleCheckResponse.class); return !Arrays.equals(checkResponse.contentHash, contentHash); } catch (Exception e) { 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() logger.log(System.Logger.Level.WARNING,String.format("Feature toggle response from %s did not contain expected ContentHash header", manifestURI.toString())); return null; } - List evaluations = new ObjectMapper().readValue(httpResponse.body(), new TypeReference>(){}); + List evaluations = OctopusObjectMapper.create().readValue(httpResponse.body(), new TypeReference>(){}); return new FeatureToggles(evaluations, Base64.getDecoder().decode(contentHashHeader.get())); } catch (Exception e) { logger.log(System.Logger.Level.WARNING, "Unable to query Octopus Feature Toggle service", e); diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusObjectMapper.java b/src/main/java/com/octopus/openfeature/provider/OctopusObjectMapper.java new file mode 100644 index 0000000..3ac4b0e --- /dev/null +++ b/src/main/java/com/octopus/openfeature/provider/OctopusObjectMapper.java @@ -0,0 +1,15 @@ +package com.octopus.openfeature.provider; + +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.MapperFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.json.JsonMapper; + +class OctopusObjectMapper { + static ObjectMapper create() { + return JsonMapper.builder() + .enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES) + .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + .build(); + } +} diff --git a/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java b/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java index 62ae758..c47ae22 100644 --- a/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java +++ b/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java @@ -7,6 +7,7 @@ import java.io.IOException; import java.util.AbstractMap; +import java.util.Iterator; import java.util.Map; class SegmentDeserializer extends JsonDeserializer> { @@ -22,7 +23,7 @@ public Map.Entry deserialize(JsonParser p, DeserializationContex return null; } - JsonNode keyNode = node.get("key"); + JsonNode keyNode = getIgnoreCase(node, "key"); if (keyNode == null || keyNode.isNull() || !keyNode.isTextual()) { ctxt.reportInputMismatch( Map.Entry.class, @@ -32,7 +33,7 @@ public Map.Entry deserialize(JsonParser p, DeserializationContex return null; } - JsonNode valueNode = node.get("value"); + JsonNode valueNode = getIgnoreCase(node, "value"); if (valueNode == null || valueNode.isNull() || !valueNode.isTextual()) { ctxt.reportInputMismatch( Map.Entry.class, @@ -47,4 +48,15 @@ public Map.Entry deserialize(JsonParser p, DeserializationContex return new AbstractMap.SimpleImmutableEntry<>(key, value); } + + private static JsonNode getIgnoreCase(JsonNode node, String fieldName) { + Iterator> fields = node.fields(); + while (fields.hasNext()) { + Map.Entry field = fields.next(); + if (field.getKey().equalsIgnoreCase(fieldName)) { + return field.getValue(); + } + } + return null; + } } diff --git a/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java b/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java index 52a514f..440882c 100644 --- a/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java +++ b/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java @@ -12,7 +12,7 @@ class FeatureToggleEvaluationDeserializationTests { - private final ObjectMapper objectMapper = new ObjectMapper(); + private final ObjectMapper objectMapper = OctopusObjectMapper.create(); private InputStream resource(String name) { return getClass().getResourceAsStream(name); @@ -69,6 +69,26 @@ void shouldDeserializeListOfToggles() throws Exception { assertThat(result.get(1).isEnabled()).isFalse(); } + @Test + void shouldDeserializeListOfTogglesWithVariousFieldCasings() throws Exception { + List result = objectMapper.readValue( + resource("toggles-with-different-field-capitalisation.json"), + new TypeReference<>() { + } + ); + + assertThat(result).hasSize(3); + assertThat(result.get(0).getSlug()).isEqualTo("feature-a"); + assertThat(result.get(0).isEnabled()).isTrue(); + assertThat(result.get(0).getSegments()).contains(Map.entry("license-type", "free")); + assertThat(result.get(1).getSlug()).isEqualTo("feature-b"); + assertThat(result.get(1).isEnabled()).isTrue(); + assertThat(result.get(1).getSegments()).contains(Map.entry("plan", "enterprise")); + assertThat(result.get(2).getSlug()).isEqualTo("feature-c"); + assertThat(result.get(2).isEnabled()).isTrue(); + assertThat(result.get(2).getSegments()).contains(Map.entry("country", "au")); + } + @Test void shouldIgnoreExtraneousProperties() throws Exception { FeatureToggleEvaluation result = objectMapper.readValue( diff --git a/src/test/resources/com/octopus/openfeature/provider/toggles-with-different-field-capitalisation.json b/src/test/resources/com/octopus/openfeature/provider/toggles-with-different-field-capitalisation.json new file mode 100644 index 0000000..6f0e5d2 --- /dev/null +++ b/src/test/resources/com/octopus/openfeature/provider/toggles-with-different-field-capitalisation.json @@ -0,0 +1,35 @@ +[ + { + "NAME": "Feature A", + "SLUG": "feature-a", + "ISENABLED": true, + "SEGMENTS": [ + { + "KEY": "license-type", + "VALUE": "free" + } + ] + }, + { + "Name": "Feature B", + "Slug": "feature-b", + "IsEnabled": true, + "Segments": [ + { + "Key": "plan", + "Value": "enterprise" + } + ] + }, + { + "nAmE": "Feature C", + "sLuG": "feature-c", + "iSeNaBlEd": true, + "sEgMeNtS": [ + { + "kEy": "country", + "vAlUe": "au" + } + ] + } +] From a839dfe511468812840265e58559143fa91ddc97 Mon Sep 17 00:00:00 2001 From: Liam Hughes Date: Thu, 26 Mar 2026 15:37:47 +1100 Subject: [PATCH 08/12] Replace Map.Entry with Segment --- .../provider/FeatureToggleEvaluation.java | 6 +- .../openfeature/provider/OctopusContext.java | 4 +- .../octopus/openfeature/provider/Segment.java | 27 ++++++ .../provider/SegmentDeserializer.java | 62 ------------ ...eToggleEvaluationDeserializationTests.java | 18 ++-- .../provider/OctopusContextTests.java | 2 +- .../provider/SegmentDeserializerTests.java | 95 ------------------- 7 files changed, 44 insertions(+), 170 deletions(-) create mode 100644 src/main/java/com/octopus/openfeature/provider/Segment.java delete mode 100644 src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java delete mode 100644 src/test/java/com/octopus/openfeature/provider/SegmentDeserializerTests.java diff --git a/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java b/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java index 1d13a66..bd5255a 100644 --- a/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java +++ b/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java @@ -12,14 +12,14 @@ class FeatureToggleEvaluation { private final String name; private final String slug; private final boolean isEnabled; - private final List> segments; + private final List segments; @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) FeatureToggleEvaluation( @JsonProperty("name") String name, @JsonProperty("slug") String slug, @JsonProperty("isEnabled") boolean isEnabled, - @JsonDeserialize(contentUsing = SegmentDeserializer.class) @JsonProperty("segments") List> segments + @JsonProperty("segments") List segments ) { this.name = name; this.slug = slug; @@ -43,7 +43,7 @@ public boolean isEnabled() { return isEnabled; } - public List> getSegments() { + public List getSegments() { return segments; } } diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusContext.java b/src/main/java/com/octopus/openfeature/provider/OctopusContext.java index 6521e9c..7ce50d5 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusContext.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusContext.java @@ -48,13 +48,13 @@ ProviderEvaluation evaluate(String slug, Boolean defaultValue, Evaluati .build(); } - private Boolean MatchesSegment(EvaluationContext evaluationContext, List> segments) { + private Boolean MatchesSegment(EvaluationContext evaluationContext, List segments) { if (evaluationContext == null) { return false; } var contextEntries = evaluationContext.asMap(); - var groupedByKey = segments.stream().collect(groupingBy(Map.Entry::getKey)); + var groupedByKey = segments.stream().collect(groupingBy(Segment::getKey)); return groupedByKey.keySet().stream().allMatch(k -> { var values = groupedByKey.get(k); diff --git a/src/main/java/com/octopus/openfeature/provider/Segment.java b/src/main/java/com/octopus/openfeature/provider/Segment.java new file mode 100644 index 0000000..dc2e9a3 --- /dev/null +++ b/src/main/java/com/octopus/openfeature/provider/Segment.java @@ -0,0 +1,27 @@ +package com.octopus.openfeature.provider; + + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; + +public class Segment { + private final String key; + private final String value; + + @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) + public Segment( + @JsonProperty("key") String key, + @JsonProperty("value") String value + ) { + this.key = key; + this.value = value; + } + + public String getKey() { + return key; + } + + public String getValue() { + return value; + } +} diff --git a/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java b/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java deleted file mode 100644 index c47ae22..0000000 --- a/src/main/java/com/octopus/openfeature/provider/SegmentDeserializer.java +++ /dev/null @@ -1,62 +0,0 @@ -package com.octopus.openfeature.provider; - -import com.fasterxml.jackson.core.JsonParser; -import com.fasterxml.jackson.databind.DeserializationContext; -import com.fasterxml.jackson.databind.JsonDeserializer; -import com.fasterxml.jackson.databind.JsonNode; - -import java.io.IOException; -import java.util.AbstractMap; -import java.util.Iterator; -import java.util.Map; - -class SegmentDeserializer extends JsonDeserializer> { - @Override - public Map.Entry deserialize(JsonParser p, DeserializationContext ctxt) throws IOException { - JsonNode node = p.getCodec().readTree(p); - if (node == null || !node.isObject()) { - ctxt.reportInputMismatch( - Map.Entry.class, - "Expected JSON object for Segment but got: %s", - node - ); - return null; - } - - JsonNode keyNode = getIgnoreCase(node, "key"); - if (keyNode == null || keyNode.isNull() || !keyNode.isTextual()) { - ctxt.reportInputMismatch( - Map.Entry.class, - "Expected non-null string 'key' field but got: %s", - keyNode - ); - return null; - } - - JsonNode valueNode = getIgnoreCase(node, "value"); - if (valueNode == null || valueNode.isNull() || !valueNode.isTextual()) { - ctxt.reportInputMismatch( - Map.Entry.class, - "Expected non-null string 'value' field but got: %s", - valueNode - ); - return null; - } - - String key = keyNode.asText(); - String value = valueNode.asText(); - - return new AbstractMap.SimpleImmutableEntry<>(key, value); - } - - private static JsonNode getIgnoreCase(JsonNode node, String fieldName) { - Iterator> fields = node.fields(); - while (fields.hasNext()) { - Map.Entry field = fields.next(); - if (field.getKey().equalsIgnoreCase(fieldName)) { - return field.getValue(); - } - } - return null; - } -} diff --git a/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java b/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java index 440882c..cc2d1c0 100644 --- a/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java +++ b/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java @@ -18,6 +18,10 @@ private InputStream resource(String name) { return getClass().getResourceAsStream(name); } + private void assertSegmentsContain(List segments, Segment... expected) { + assertThat(segments).usingRecursiveFieldByFieldElementComparator().contains(expected); + } + @Test void shouldDeserializeEnabledToggle() throws Exception { FeatureToggleEvaluation result = objectMapper.readValue(resource("toggle-enabled-no-segments.json"), FeatureToggleEvaluation.class); @@ -48,9 +52,9 @@ void shouldDeserializeToggleWithSegments() throws Exception { resource("toggle-with-segments.json"), FeatureToggleEvaluation.class); assertThat(result.getSegments()).hasSize(2); - assertThat(result.getSegments()).contains( - Map.entry("license-type", "free"), - Map.entry("country", "au") + assertSegmentsContain(result.getSegments(), + new Segment("license-type", "free"), + new Segment("country", "au") ); } @@ -80,13 +84,13 @@ void shouldDeserializeListOfTogglesWithVariousFieldCasings() throws Exception { assertThat(result).hasSize(3); assertThat(result.get(0).getSlug()).isEqualTo("feature-a"); assertThat(result.get(0).isEnabled()).isTrue(); - assertThat(result.get(0).getSegments()).contains(Map.entry("license-type", "free")); + assertSegmentsContain(result.get(0).getSegments(), new Segment("license-type", "free")); assertThat(result.get(1).getSlug()).isEqualTo("feature-b"); assertThat(result.get(1).isEnabled()).isTrue(); - assertThat(result.get(1).getSegments()).contains(Map.entry("plan", "enterprise")); + assertSegmentsContain(result.get(1).getSegments(), new Segment("plan", "enterprise")); assertThat(result.get(2).getSlug()).isEqualTo("feature-c"); assertThat(result.get(2).isEnabled()).isTrue(); - assertThat(result.get(2).getSegments()).contains(Map.entry("country", "au")); + assertSegmentsContain(result.get(2).getSegments(), new Segment("country", "au")); } @Test @@ -97,6 +101,6 @@ void shouldIgnoreExtraneousProperties() throws Exception { assertThat(result.getName()).isEqualTo("My Feature"); assertThat(result.getSlug()).isEqualTo("my-feature"); assertThat(result.isEnabled()).isTrue(); - assertThat(result.getSegments()).contains(Map.entry("license-type", "free")); + assertSegmentsContain(result.getSegments(), new Segment("license-type", "free")); } } diff --git a/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java b/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java index 529a4ca..5cdea37 100644 --- a/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java +++ b/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java @@ -20,7 +20,7 @@ class OctopusContextTests { Arrays.asList( new FeatureToggleEvaluation("Enabled Feature", "enabled-feature", true, null), new FeatureToggleEvaluation("Disabled Feature", "disabled-feature", false, null), - new FeatureToggleEvaluation("Feature With Segments", "feature-with-segments", true, Arrays.asList(Map.entry("license-type", "free"), Map.entry("country", "au")) ) + new FeatureToggleEvaluation("Feature With Segments", "feature-with-segments", true, Arrays.asList(new Segment("license-type", "free"), new Segment("country", "au")) ) ), new byte[0] ); diff --git a/src/test/java/com/octopus/openfeature/provider/SegmentDeserializerTests.java b/src/test/java/com/octopus/openfeature/provider/SegmentDeserializerTests.java deleted file mode 100644 index 222c3a9..0000000 --- a/src/test/java/com/octopus/openfeature/provider/SegmentDeserializerTests.java +++ /dev/null @@ -1,95 +0,0 @@ -package com.octopus.openfeature.provider; - -import com.fasterxml.jackson.databind.JsonMappingException; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.module.SimpleModule; -import org.junit.jupiter.api.Test; - -import java.util.Map; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; - -class SegmentDeserializerTests { - private final ObjectMapper objectMapper = new ObjectMapper() - .registerModule(new SimpleModule().addDeserializer( - Map.Entry.class, - new SegmentDeserializer() - )); - - @Test - void shouldDeserializeSegmentWithKeyAndValue() throws Exception { - Map.Entry result = objectMapper.readValue( - "{\"key\":\"license-type\",\"value\":\"free\"}", - Map.Entry.class - ); - - assertThat(result.getKey()).isEqualTo("license-type"); - assertThat(result.getValue()).isEqualTo("free"); - } - - @Test - void shouldIgnoreExtraFields() throws Exception { - Map.Entry result = objectMapper.readValue( - "{\"key\":\"k\",\"value\":\"v\",\"extra\":\"ignored\"}", - Map.Entry.class - ); - - assertThat(result.getKey()).isEqualTo("k"); - assertThat(result.getValue()).isEqualTo("v"); - } - - @Test - void shouldRejectMissingKey() { - assertThatThrownBy(() -> objectMapper.readValue("{\"value\":\"v\"}", Map.Entry.class)) - .isInstanceOf(JsonMappingException.class) - .hasMessageContaining("key"); - } - - @Test - void shouldRejectNullKey() { - assertThatThrownBy(() -> objectMapper.readValue("{\"key\":null,\"value\":\"v\"}", Map.Entry.class)) - .isInstanceOf(JsonMappingException.class) - .hasMessageContaining("key"); - } - - @Test - void shouldRejectNonStringKey() { - assertThatThrownBy(() -> objectMapper.readValue("{\"key\":42,\"value\":\"v\"}", Map.Entry.class)) - .isInstanceOf(JsonMappingException.class) - .hasMessageContaining("key"); - } - - @Test - void shouldRejectMissingValue() { - assertThatThrownBy(() -> objectMapper.readValue("{\"key\":\"k\"}", Map.Entry.class)) - .isInstanceOf(JsonMappingException.class) - .hasMessageContaining("value"); - } - - @Test - void shouldRejectNullValue() { - assertThatThrownBy(() -> objectMapper.readValue("{\"key\":\"k\",\"value\":null}", Map.Entry.class)) - .isInstanceOf(JsonMappingException.class) - .hasMessageContaining("value"); - } - - @Test - void shouldRejectNonStringValue() { - assertThatThrownBy(() -> objectMapper.readValue("{\"key\":\"k\",\"value\":42}", Map.Entry.class)) - .isInstanceOf(JsonMappingException.class) - .hasMessageContaining("value"); - } - - @Test - void shouldRejectArrayInput() { - assertThatThrownBy(() -> objectMapper.readValue("[\"k\",\"v\"]", Map.Entry.class)) - .isInstanceOf(JsonMappingException.class); - } - - @Test - void shouldRejectScalarInput() { - assertThatThrownBy(() -> objectMapper.readValue("\"just-a-string\"", Map.Entry.class)) - .isInstanceOf(JsonMappingException.class); - } -} From 6626ea6bbcbdde7b533fa1caac90352cdb120128 Mon Sep 17 00:00:00 2001 From: Liam Hughes Date: Thu, 26 Mar 2026 15:38:30 +1100 Subject: [PATCH 09/12] Make getSegments return immutable --- .../octopus/openfeature/provider/FeatureToggleEvaluation.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java b/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java index bd5255a..56746f9 100644 --- a/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java +++ b/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java @@ -4,6 +4,7 @@ import com.fasterxml.jackson.annotation.JsonProperty; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import java.util.Collections; import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -44,6 +45,6 @@ public boolean isEnabled() { } public List getSegments() { - return segments; + return Collections.unmodifiableList(segments); } } From 5cca0e0d36aaecc3effb03b36924663f1f161379 Mon Sep 17 00:00:00 2001 From: Liam Hughes Date: Thu, 26 Mar 2026 15:43:49 +1100 Subject: [PATCH 10/12] Clean up --- .../openfeature/provider/FeatureToggleEvaluation.java | 10 ++++------ .../java/com/octopus/openfeature/provider/Segment.java | 7 +++---- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java b/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java index 56746f9..c394baa 100644 --- a/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java +++ b/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java @@ -2,12 +2,10 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -import com.fasterxml.jackson.databind.annotation.JsonDeserialize; import java.util.Collections; import java.util.ArrayList; import java.util.List; -import java.util.Map; class FeatureToggleEvaluation { private final String name; @@ -17,10 +15,10 @@ class FeatureToggleEvaluation { @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) FeatureToggleEvaluation( - @JsonProperty("name") String name, - @JsonProperty("slug") String slug, - @JsonProperty("isEnabled") boolean isEnabled, - @JsonProperty("segments") List segments + @JsonProperty("name") String name, + @JsonProperty("slug") String slug, + @JsonProperty("isEnabled") boolean isEnabled, + @JsonProperty("segments") List segments ) { this.name = name; this.slug = slug; diff --git a/src/main/java/com/octopus/openfeature/provider/Segment.java b/src/main/java/com/octopus/openfeature/provider/Segment.java index dc2e9a3..c3cb90e 100644 --- a/src/main/java/com/octopus/openfeature/provider/Segment.java +++ b/src/main/java/com/octopus/openfeature/provider/Segment.java @@ -1,17 +1,16 @@ package com.octopus.openfeature.provider; - import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; -public class Segment { +class Segment { private final String key; private final String value; @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) public Segment( - @JsonProperty("key") String key, - @JsonProperty("value") String value + @JsonProperty("key") String key, + @JsonProperty("value") String value ) { this.key = key; this.value = value; From 12f7edbd09fd830314aad03eeb956fbb6dd28289 Mon Sep 17 00:00:00 2001 From: Liam Hughes Date: Thu, 26 Mar 2026 15:45:38 +1100 Subject: [PATCH 11/12] Make ObjectMapper static for performance --- .../octopus/openfeature/provider/OctopusClient.java | 4 ++-- .../openfeature/provider/OctopusObjectMapper.java | 10 ++++------ .../FeatureToggleEvaluationDeserializationTests.java | 2 +- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java index 6fb2e9e..f22e455 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java @@ -36,7 +36,7 @@ Boolean haveFeatureTogglesChanged(byte[] contentHash) .build(); try { HttpResponse httpResponse = client.send(request, HttpResponse.BodyHandlers.ofString()); - FeatureToggleCheckResponse checkResponse = OctopusObjectMapper.create().readValue(httpResponse.body(), FeatureToggleCheckResponse.class); + FeatureToggleCheckResponse checkResponse = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), FeatureToggleCheckResponse.class); return !Arrays.equals(checkResponse.contentHash, contentHash); } catch (Exception e) { 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() logger.log(System.Logger.Level.WARNING,String.format("Feature toggle response from %s did not contain expected ContentHash header", manifestURI.toString())); return null; } - List evaluations = OctopusObjectMapper.create().readValue(httpResponse.body(), new TypeReference>(){}); + List evaluations = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), new TypeReference>(){}); return new FeatureToggles(evaluations, Base64.getDecoder().decode(contentHashHeader.get())); } catch (Exception e) { logger.log(System.Logger.Level.WARNING, "Unable to query Octopus Feature Toggle service", e); diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusObjectMapper.java b/src/main/java/com/octopus/openfeature/provider/OctopusObjectMapper.java index 3ac4b0e..6930dff 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusObjectMapper.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusObjectMapper.java @@ -6,10 +6,8 @@ import com.fasterxml.jackson.databind.json.JsonMapper; class OctopusObjectMapper { - static ObjectMapper create() { - return JsonMapper.builder() - .enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES) - .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) - .build(); - } + static final ObjectMapper INSTANCE = JsonMapper.builder() + .enable(MapperFeature.ACCEPT_CASE_INSENSITIVE_PROPERTIES) + .disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES) + .build(); } diff --git a/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java b/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java index cc2d1c0..bd391be 100644 --- a/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java +++ b/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java @@ -12,7 +12,7 @@ class FeatureToggleEvaluationDeserializationTests { - private final ObjectMapper objectMapper = OctopusObjectMapper.create(); + private final ObjectMapper objectMapper = OctopusObjectMapper.INSTANCE; private InputStream resource(String name) { return getClass().getResourceAsStream(name); From 6d04e1fc0f3d1df4196386c4587bb2e55239717f Mon Sep 17 00:00:00 2001 From: Liam Hughes Date: Thu, 26 Mar 2026 15:55:43 +1100 Subject: [PATCH 12/12] Make Segment key and value required --- .../octopus/openfeature/provider/Segment.java | 4 ++-- ...eToggleEvaluationDeserializationTests.java | 23 +++++++++++++++++++ .../toggle-segment-missing-key-and-value.json | 8 +++++++ .../provider/toggle-segment-missing-key.json | 10 ++++++++ .../toggle-segment-missing-value.json | 10 ++++++++ 5 files changed, 53 insertions(+), 2 deletions(-) create mode 100644 src/test/resources/com/octopus/openfeature/provider/toggle-segment-missing-key-and-value.json create mode 100644 src/test/resources/com/octopus/openfeature/provider/toggle-segment-missing-key.json create mode 100644 src/test/resources/com/octopus/openfeature/provider/toggle-segment-missing-value.json diff --git a/src/main/java/com/octopus/openfeature/provider/Segment.java b/src/main/java/com/octopus/openfeature/provider/Segment.java index c3cb90e..0ca3fda 100644 --- a/src/main/java/com/octopus/openfeature/provider/Segment.java +++ b/src/main/java/com/octopus/openfeature/provider/Segment.java @@ -9,8 +9,8 @@ class Segment { @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) public Segment( - @JsonProperty("key") String key, - @JsonProperty("value") String value + @JsonProperty(value = "key", required = true) String key, + @JsonProperty(value = "value", required = true) String value ) { this.key = key; this.value = value; diff --git a/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java b/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java index bd391be..4607b62 100644 --- a/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java +++ b/src/test/java/com/octopus/openfeature/provider/FeatureToggleEvaluationDeserializationTests.java @@ -2,6 +2,7 @@ import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.exc.MismatchedInputException; import org.junit.jupiter.api.Test; import java.io.InputStream; @@ -9,6 +10,7 @@ import java.util.Map; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; class FeatureToggleEvaluationDeserializationTests { @@ -93,6 +95,27 @@ void shouldDeserializeListOfTogglesWithVariousFieldCasings() throws Exception { assertSegmentsContain(result.get(2).getSegments(), new Segment("country", "au")); } + @Test + void shouldFailDeserializationWhenSegmentKeyIsMissing() { + assertThatThrownBy(() -> objectMapper.readValue( + resource("toggle-segment-missing-key.json"), FeatureToggleEvaluation.class)) + .isInstanceOf(MismatchedInputException.class); + } + + @Test + void shouldFailDeserializationWhenSegmentValueIsMissing() { + assertThatThrownBy(() -> objectMapper.readValue( + resource("toggle-segment-missing-value.json"), FeatureToggleEvaluation.class)) + .isInstanceOf(MismatchedInputException.class); + } + + @Test + void shouldFailDeserializationWhenSegmentKeyAndValueAreMissing() { + assertThatThrownBy(() -> objectMapper.readValue( + resource("toggle-segment-missing-key-and-value.json"), FeatureToggleEvaluation.class)) + .isInstanceOf(MismatchedInputException.class); + } + @Test void shouldIgnoreExtraneousProperties() throws Exception { FeatureToggleEvaluation result = objectMapper.readValue( diff --git a/src/test/resources/com/octopus/openfeature/provider/toggle-segment-missing-key-and-value.json b/src/test/resources/com/octopus/openfeature/provider/toggle-segment-missing-key-and-value.json new file mode 100644 index 0000000..11669cf --- /dev/null +++ b/src/test/resources/com/octopus/openfeature/provider/toggle-segment-missing-key-and-value.json @@ -0,0 +1,8 @@ +{ + "name": "My Feature", + "slug": "my-feature", + "isEnabled": true, + "segments": [ + {} + ] +} diff --git a/src/test/resources/com/octopus/openfeature/provider/toggle-segment-missing-key.json b/src/test/resources/com/octopus/openfeature/provider/toggle-segment-missing-key.json new file mode 100644 index 0000000..7bf7e21 --- /dev/null +++ b/src/test/resources/com/octopus/openfeature/provider/toggle-segment-missing-key.json @@ -0,0 +1,10 @@ +{ + "name": "My Feature", + "slug": "my-feature", + "isEnabled": true, + "segments": [ + { + "value": "free" + } + ] +} diff --git a/src/test/resources/com/octopus/openfeature/provider/toggle-segment-missing-value.json b/src/test/resources/com/octopus/openfeature/provider/toggle-segment-missing-value.json new file mode 100644 index 0000000..b4f2137 --- /dev/null +++ b/src/test/resources/com/octopus/openfeature/provider/toggle-segment-missing-value.json @@ -0,0 +1,10 @@ +{ + "name": "My Feature", + "slug": "my-feature", + "isEnabled": true, + "segments": [ + { + "key": "license-type" + } + ] +}