From 7693ce2a8e4d200346bd3b09f998a3af1e75d653 Mon Sep 17 00:00:00 2001 From: Lourens de Jager Date: Tue, 11 Aug 2026 14:24:04 +1200 Subject: [PATCH 01/10] refactor: flatten the v4 package into the provider package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The switch to v4 needs the evaluator, which lives in the provider package, to call ServerSideEvaluation.evaluate(). Java package access is not hierarchical, so keeping the v4 types in a sub-package would mean making them public — permanently, since nothing later removes the split. With v4 becoming the only contract, the sub-package has served its purpose: it kept an unreleased contract out of the way while v3 was live. Everything stays package-private in one package, and the public API is unchanged. A pure move: no behaviour changes, and the two test-only shims that only existed to cross the package boundary are gone. Co-Authored-By: Claude Opus 5 (1M context) --- .../{v4 => }/ClientSideCondition.java | 2 +- .../ClientSideConditionDeserializer.java | 2 +- .../{v4 => }/ClientSideEvaluationContext.java | 2 +- .../provider/{v4 => }/ClientSideRule.java | 2 +- .../provider/{v4 => }/ConditionTypeNames.java | 2 +- .../ContextAttributeIsNotOneOfCondition.java | 2 +- .../ContextAttributeIsOneOfCondition.java | 2 +- .../provider/{v4 => }/ContextAttributes.java | 2 +- .../provider/{v4 => }/EvaluationReasons.java | 2 +- .../provider/{v4 => }/ListUtils.java | 2 +- .../PercentageByContextCondition.java | 2 +- .../provider/{v4 => }/PercentageRollout.java | 9 +++----- .../{v4 => }/ServerSideEvaluation.java | 2 +- .../provider/{v4 => }/UnknownCondition.java | 2 +- ...ientSideConditionDeserializationTests.java | 17 +++++++-------- .../{v4 => }/ClientSideRuleTests.java | 5 ++--- ...textAttributeIsNotOneOfConditionTests.java | 2 +- ...ContextAttributeIsOneOfConditionTests.java | 2 +- .../provider/{v4 => }/Contexts.java | 2 +- .../{v4 => }/MalformedEvaluationTests.java | 5 ++--- .../PercentageByContextConditionTests.java | 2 +- .../{v4 => }/PercentageRolloutTests.java | 2 +- .../openfeature/provider/RolloutVectors.java | 7 ++----- ...verSideEvaluationDeserializationTests.java | 21 +++++++++---------- .../{v4 => }/ServerSideEvaluationTests.java | 2 +- .../provider/TestObjectMapper.java | 17 --------------- .../{v4 => }/UnknownConditionTests.java | 2 +- .../{v4 => }/UnrecognisedConditionTests.java | 5 ++--- ...tion-context-attribute-is-not-one-of.json} | 0 ...ondition-context-attribute-is-one-of.json} | 0 ...ixed.json => v4-condition-list-mixed.json} | 0 ...pe.json => v4-condition-missing-type.json} | 0 ...> v4-condition-percentage-by-context.json} | 0 ...pe.json => v4-condition-unknown-type.json} | 0 ... => v4-evaluation-deferred-to-client.json} | 0 ... v4-evaluation-list-one-missing-slug.json} | 0 ...tion-list.json => v4-evaluation-list.json} | 0 ...g.json => v4-evaluation-missing-slug.json} | 0 ...son => v4-evaluation-server-resolved.json} | 0 ...valuation-with-extraneous-properties.json} | 0 ...v4-evaluation-with-unknown-condition.json} | 0 41 files changed, 49 insertions(+), 77 deletions(-) rename src/main/java/com/octopus/openfeature/provider/{v4 => }/ClientSideCondition.java (96%) rename src/main/java/com/octopus/openfeature/provider/{v4 => }/ClientSideConditionDeserializer.java (98%) rename src/main/java/com/octopus/openfeature/provider/{v4 => }/ClientSideEvaluationContext.java (94%) rename src/main/java/com/octopus/openfeature/provider/{v4 => }/ClientSideRule.java (97%) rename src/main/java/com/octopus/openfeature/provider/{v4 => }/ConditionTypeNames.java (91%) rename src/main/java/com/octopus/openfeature/provider/{v4 => }/ContextAttributeIsNotOneOfCondition.java (96%) rename src/main/java/com/octopus/openfeature/provider/{v4 => }/ContextAttributeIsOneOfCondition.java (96%) rename src/main/java/com/octopus/openfeature/provider/{v4 => }/ContextAttributes.java (97%) rename src/main/java/com/octopus/openfeature/provider/{v4 => }/EvaluationReasons.java (91%) rename src/main/java/com/octopus/openfeature/provider/{v4 => }/ListUtils.java (94%) rename src/main/java/com/octopus/openfeature/provider/{v4 => }/PercentageByContextCondition.java (98%) rename src/main/java/com/octopus/openfeature/provider/{v4 => }/PercentageRollout.java (77%) rename src/main/java/com/octopus/openfeature/provider/{v4 => }/ServerSideEvaluation.java (98%) rename src/main/java/com/octopus/openfeature/provider/{v4 => }/UnknownCondition.java (97%) rename src/test/java/com/octopus/openfeature/provider/{v4 => }/ClientSideConditionDeserializationTests.java (86%) rename src/test/java/com/octopus/openfeature/provider/{v4 => }/ClientSideRuleTests.java (95%) rename src/test/java/com/octopus/openfeature/provider/{v4 => }/ContextAttributeIsNotOneOfConditionTests.java (98%) rename src/test/java/com/octopus/openfeature/provider/{v4 => }/ContextAttributeIsOneOfConditionTests.java (98%) rename src/test/java/com/octopus/openfeature/provider/{v4 => }/Contexts.java (97%) rename src/test/java/com/octopus/openfeature/provider/{v4 => }/MalformedEvaluationTests.java (98%) rename src/test/java/com/octopus/openfeature/provider/{v4 => }/PercentageByContextConditionTests.java (98%) rename src/test/java/com/octopus/openfeature/provider/{v4 => }/PercentageRolloutTests.java (97%) rename src/test/java/com/octopus/openfeature/provider/{v4 => }/ServerSideEvaluationDeserializationTests.java (87%) rename src/test/java/com/octopus/openfeature/provider/{v4 => }/ServerSideEvaluationTests.java (99%) delete mode 100644 src/test/java/com/octopus/openfeature/provider/TestObjectMapper.java rename src/test/java/com/octopus/openfeature/provider/{v4 => }/UnknownConditionTests.java (94%) rename src/test/java/com/octopus/openfeature/provider/{v4 => }/UnrecognisedConditionTests.java (92%) rename src/test/resources/com/octopus/openfeature/provider/{v4/condition-context-attribute-is-not-one-of.json => v4-condition-context-attribute-is-not-one-of.json} (100%) rename src/test/resources/com/octopus/openfeature/provider/{v4/condition-context-attribute-is-one-of.json => v4-condition-context-attribute-is-one-of.json} (100%) rename src/test/resources/com/octopus/openfeature/provider/{v4/condition-list-mixed.json => v4-condition-list-mixed.json} (100%) rename src/test/resources/com/octopus/openfeature/provider/{v4/condition-missing-type.json => v4-condition-missing-type.json} (100%) rename src/test/resources/com/octopus/openfeature/provider/{v4/condition-percentage-by-context.json => v4-condition-percentage-by-context.json} (100%) rename src/test/resources/com/octopus/openfeature/provider/{v4/condition-unknown-type.json => v4-condition-unknown-type.json} (100%) rename src/test/resources/com/octopus/openfeature/provider/{v4/evaluation-deferred-to-client.json => v4-evaluation-deferred-to-client.json} (100%) rename src/test/resources/com/octopus/openfeature/provider/{v4/evaluation-list-one-missing-slug.json => v4-evaluation-list-one-missing-slug.json} (100%) rename src/test/resources/com/octopus/openfeature/provider/{v4/evaluation-list.json => v4-evaluation-list.json} (100%) rename src/test/resources/com/octopus/openfeature/provider/{v4/evaluation-missing-slug.json => v4-evaluation-missing-slug.json} (100%) rename src/test/resources/com/octopus/openfeature/provider/{v4/evaluation-server-resolved.json => v4-evaluation-server-resolved.json} (100%) rename src/test/resources/com/octopus/openfeature/provider/{v4/evaluation-with-extraneous-properties.json => v4-evaluation-with-extraneous-properties.json} (100%) rename src/test/resources/com/octopus/openfeature/provider/{v4/evaluation-with-unknown-condition.json => v4-evaluation-with-unknown-condition.json} (100%) diff --git a/src/main/java/com/octopus/openfeature/provider/v4/ClientSideCondition.java b/src/main/java/com/octopus/openfeature/provider/ClientSideCondition.java similarity index 96% rename from src/main/java/com/octopus/openfeature/provider/v4/ClientSideCondition.java rename to src/main/java/com/octopus/openfeature/provider/ClientSideCondition.java index 708a3bd..5fc9b39 100644 --- a/src/main/java/com/octopus/openfeature/provider/v4/ClientSideCondition.java +++ b/src/main/java/com/octopus/openfeature/provider/ClientSideCondition.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; diff --git a/src/main/java/com/octopus/openfeature/provider/v4/ClientSideConditionDeserializer.java b/src/main/java/com/octopus/openfeature/provider/ClientSideConditionDeserializer.java similarity index 98% rename from src/main/java/com/octopus/openfeature/provider/v4/ClientSideConditionDeserializer.java rename to src/main/java/com/octopus/openfeature/provider/ClientSideConditionDeserializer.java index fab619f..10e23c7 100644 --- a/src/main/java/com/octopus/openfeature/provider/v4/ClientSideConditionDeserializer.java +++ b/src/main/java/com/octopus/openfeature/provider/ClientSideConditionDeserializer.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import com.fasterxml.jackson.core.JsonParser; import com.fasterxml.jackson.core.ObjectCodec; diff --git a/src/main/java/com/octopus/openfeature/provider/v4/ClientSideEvaluationContext.java b/src/main/java/com/octopus/openfeature/provider/ClientSideEvaluationContext.java similarity index 94% rename from src/main/java/com/octopus/openfeature/provider/v4/ClientSideEvaluationContext.java rename to src/main/java/com/octopus/openfeature/provider/ClientSideEvaluationContext.java index 73a334c..796a4e0 100644 --- a/src/main/java/com/octopus/openfeature/provider/v4/ClientSideEvaluationContext.java +++ b/src/main/java/com/octopus/openfeature/provider/ClientSideEvaluationContext.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import dev.openfeature.sdk.EvaluationContext; diff --git a/src/main/java/com/octopus/openfeature/provider/v4/ClientSideRule.java b/src/main/java/com/octopus/openfeature/provider/ClientSideRule.java similarity index 97% rename from src/main/java/com/octopus/openfeature/provider/v4/ClientSideRule.java rename to src/main/java/com/octopus/openfeature/provider/ClientSideRule.java index 90f19dd..d821317 100644 --- a/src/main/java/com/octopus/openfeature/provider/v4/ClientSideRule.java +++ b/src/main/java/com/octopus/openfeature/provider/ClientSideRule.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; diff --git a/src/main/java/com/octopus/openfeature/provider/v4/ConditionTypeNames.java b/src/main/java/com/octopus/openfeature/provider/ConditionTypeNames.java similarity index 91% rename from src/main/java/com/octopus/openfeature/provider/v4/ConditionTypeNames.java rename to src/main/java/com/octopus/openfeature/provider/ConditionTypeNames.java index f8f8cff..a7897fb 100644 --- a/src/main/java/com/octopus/openfeature/provider/v4/ConditionTypeNames.java +++ b/src/main/java/com/octopus/openfeature/provider/ConditionTypeNames.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; /** * Discriminator values for the polymorphic v4 client-side conditions. These mirror the values in the diff --git a/src/main/java/com/octopus/openfeature/provider/v4/ContextAttributeIsNotOneOfCondition.java b/src/main/java/com/octopus/openfeature/provider/ContextAttributeIsNotOneOfCondition.java similarity index 96% rename from src/main/java/com/octopus/openfeature/provider/v4/ContextAttributeIsNotOneOfCondition.java rename to src/main/java/com/octopus/openfeature/provider/ContextAttributeIsNotOneOfCondition.java index 6a1f037..1dea7fa 100644 --- a/src/main/java/com/octopus/openfeature/provider/v4/ContextAttributeIsNotOneOfCondition.java +++ b/src/main/java/com/octopus/openfeature/provider/ContextAttributeIsNotOneOfCondition.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; diff --git a/src/main/java/com/octopus/openfeature/provider/v4/ContextAttributeIsOneOfCondition.java b/src/main/java/com/octopus/openfeature/provider/ContextAttributeIsOneOfCondition.java similarity index 96% rename from src/main/java/com/octopus/openfeature/provider/v4/ContextAttributeIsOneOfCondition.java rename to src/main/java/com/octopus/openfeature/provider/ContextAttributeIsOneOfCondition.java index 9d50ac5..dbb6bc3 100644 --- a/src/main/java/com/octopus/openfeature/provider/v4/ContextAttributeIsOneOfCondition.java +++ b/src/main/java/com/octopus/openfeature/provider/ContextAttributeIsOneOfCondition.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; diff --git a/src/main/java/com/octopus/openfeature/provider/v4/ContextAttributes.java b/src/main/java/com/octopus/openfeature/provider/ContextAttributes.java similarity index 97% rename from src/main/java/com/octopus/openfeature/provider/v4/ContextAttributes.java rename to src/main/java/com/octopus/openfeature/provider/ContextAttributes.java index 5f8f2fe..afc6bbf 100644 --- a/src/main/java/com/octopus/openfeature/provider/v4/ContextAttributes.java +++ b/src/main/java/com/octopus/openfeature/provider/ContextAttributes.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import dev.openfeature.sdk.EvaluationContext; import dev.openfeature.sdk.exceptions.ParseError; diff --git a/src/main/java/com/octopus/openfeature/provider/v4/EvaluationReasons.java b/src/main/java/com/octopus/openfeature/provider/EvaluationReasons.java similarity index 91% rename from src/main/java/com/octopus/openfeature/provider/v4/EvaluationReasons.java rename to src/main/java/com/octopus/openfeature/provider/EvaluationReasons.java index 072c0d0..fd65101 100644 --- a/src/main/java/com/octopus/openfeature/provider/v4/EvaluationReasons.java +++ b/src/main/java/com/octopus/openfeature/provider/EvaluationReasons.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; /** * Reasons returned alongside a client-side evaluation. Both match the strings the Feature Flags service diff --git a/src/main/java/com/octopus/openfeature/provider/v4/ListUtils.java b/src/main/java/com/octopus/openfeature/provider/ListUtils.java similarity index 94% rename from src/main/java/com/octopus/openfeature/provider/v4/ListUtils.java rename to src/main/java/com/octopus/openfeature/provider/ListUtils.java index fad17f4..ded497d 100644 --- a/src/main/java/com/octopus/openfeature/provider/v4/ListUtils.java +++ b/src/main/java/com/octopus/openfeature/provider/ListUtils.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import java.util.ArrayList; import java.util.Collections; diff --git a/src/main/java/com/octopus/openfeature/provider/v4/PercentageByContextCondition.java b/src/main/java/com/octopus/openfeature/provider/PercentageByContextCondition.java similarity index 98% rename from src/main/java/com/octopus/openfeature/provider/v4/PercentageByContextCondition.java rename to src/main/java/com/octopus/openfeature/provider/PercentageByContextCondition.java index 4dc156a..0ea1304 100644 --- a/src/main/java/com/octopus/openfeature/provider/v4/PercentageByContextCondition.java +++ b/src/main/java/com/octopus/openfeature/provider/PercentageByContextCondition.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonIgnoreProperties; diff --git a/src/main/java/com/octopus/openfeature/provider/v4/PercentageRollout.java b/src/main/java/com/octopus/openfeature/provider/PercentageRollout.java similarity index 77% rename from src/main/java/com/octopus/openfeature/provider/v4/PercentageRollout.java rename to src/main/java/com/octopus/openfeature/provider/PercentageRollout.java index 06814a7..9a1c9f9 100644 --- a/src/main/java/com/octopus/openfeature/provider/v4/PercentageRollout.java +++ b/src/main/java/com/octopus/openfeature/provider/PercentageRollout.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import org.apache.commons.codec.digest.MurmurHash3; @@ -9,11 +9,8 @@ * the targeting key keeps a bucket stable across evaluations, while giving each flag an independent * spread of targeting keys. * - *

The v3 path has its own copy of this hash in {@code OctopusContext}, because Java package access - * is not hierarchical and a package-private type here is invisible to that package. Keeping both - * package-private is worth the duplication: the alternative is a public type that consumers could - * bind to, and this one is due to disappear along with v3. Both copies are pinned to the same shared - * vectors — see {@code RolloutVectors} in the tests — so the two cannot drift apart unnoticed. + *

Shared by every percentage rollout in the library, and matching the equivalent implementations in + * the other Octopus OpenFeature provider libraries — see {@code RolloutVectors} in the tests. */ final class PercentageRollout { diff --git a/src/main/java/com/octopus/openfeature/provider/v4/ServerSideEvaluation.java b/src/main/java/com/octopus/openfeature/provider/ServerSideEvaluation.java similarity index 98% rename from src/main/java/com/octopus/openfeature/provider/v4/ServerSideEvaluation.java rename to src/main/java/com/octopus/openfeature/provider/ServerSideEvaluation.java index 5a4c99b..d6de652 100644 --- a/src/main/java/com/octopus/openfeature/provider/v4/ServerSideEvaluation.java +++ b/src/main/java/com/octopus/openfeature/provider/ServerSideEvaluation.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; diff --git a/src/main/java/com/octopus/openfeature/provider/v4/UnknownCondition.java b/src/main/java/com/octopus/openfeature/provider/UnknownCondition.java similarity index 97% rename from src/main/java/com/octopus/openfeature/provider/v4/UnknownCondition.java rename to src/main/java/com/octopus/openfeature/provider/UnknownCondition.java index 058846d..d016b96 100644 --- a/src/main/java/com/octopus/openfeature/provider/v4/UnknownCondition.java +++ b/src/main/java/com/octopus/openfeature/provider/UnknownCondition.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; diff --git a/src/test/java/com/octopus/openfeature/provider/v4/ClientSideConditionDeserializationTests.java b/src/test/java/com/octopus/openfeature/provider/ClientSideConditionDeserializationTests.java similarity index 86% rename from src/test/java/com/octopus/openfeature/provider/v4/ClientSideConditionDeserializationTests.java rename to src/test/java/com/octopus/openfeature/provider/ClientSideConditionDeserializationTests.java index d03baf0..aced930 100644 --- a/src/test/java/com/octopus/openfeature/provider/v4/ClientSideConditionDeserializationTests.java +++ b/src/test/java/com/octopus/openfeature/provider/ClientSideConditionDeserializationTests.java @@ -1,8 +1,7 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; -import com.octopus.openfeature.provider.TestObjectMapper; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -19,7 +18,7 @@ */ class ClientSideConditionDeserializationTests { - private final ObjectMapper objectMapper = TestObjectMapper.INSTANCE; + private final ObjectMapper objectMapper = OctopusObjectMapper.INSTANCE; private InputStream resource(String name) { return getClass().getResourceAsStream(name); @@ -28,7 +27,7 @@ private InputStream resource(String name) { @Test void shouldDeserializePercentageByContextConditionToConcreteType() throws Exception { var condition = objectMapper.readValue( - resource("condition-percentage-by-context.json"), ClientSideCondition.class); + resource("v4-condition-percentage-by-context.json"), ClientSideCondition.class); assertThat(condition) .isInstanceOfSatisfying(PercentageByContextCondition.class, @@ -38,7 +37,7 @@ void shouldDeserializePercentageByContextConditionToConcreteType() throws Except @Test void shouldDeserializeContextAttributeIsOneOfConditionToConcreteType() throws Exception { var condition = objectMapper.readValue( - resource("condition-context-attribute-is-one-of.json"), ClientSideCondition.class); + resource("v4-condition-context-attribute-is-one-of.json"), ClientSideCondition.class); assertThat(condition) .isInstanceOfSatisfying(ContextAttributeIsOneOfCondition.class, isOneOf -> { @@ -50,7 +49,7 @@ void shouldDeserializeContextAttributeIsOneOfConditionToConcreteType() throws Ex @Test void shouldDeserializeContextAttributeIsNotOneOfConditionToConcreteType() throws Exception { var condition = objectMapper.readValue( - resource("condition-context-attribute-is-not-one-of.json"), ClientSideCondition.class); + resource("v4-condition-context-attribute-is-not-one-of.json"), ClientSideCondition.class); assertThat(condition) .isInstanceOfSatisfying(ContextAttributeIsNotOneOfCondition.class, isNotOneOf -> { @@ -62,7 +61,7 @@ void shouldDeserializeContextAttributeIsNotOneOfConditionToConcreteType() throws @Test void shouldDeserializeMixedConditionListToConcreteTypes() throws Exception { var conditions = objectMapper.readValue( - resource("condition-list-mixed.json"), + resource("v4-condition-list-mixed.json"), new TypeReference>() {} ); @@ -76,7 +75,7 @@ void shouldDeserializeMixedConditionListToConcreteTypes() throws Exception { @Test void shouldDeserializeUnknownConditionTypeToUnknownConditionInsteadOfThrowing() throws Exception { var condition = objectMapper.readValue( - resource("condition-unknown-type.json"), ClientSideCondition.class); + resource("v4-condition-unknown-type.json"), ClientSideCondition.class); assertThat(condition) .isInstanceOfSatisfying(UnknownCondition.class, @@ -86,7 +85,7 @@ void shouldDeserializeUnknownConditionTypeToUnknownConditionInsteadOfThrowing() @Test void shouldDeserializeConditionWithoutTypeDiscriminatorToUnknownCondition() throws Exception { var condition = objectMapper.readValue( - resource("condition-missing-type.json"), ClientSideCondition.class); + resource("v4-condition-missing-type.json"), ClientSideCondition.class); assertThat(condition) .isInstanceOfSatisfying(UnknownCondition.class, diff --git a/src/test/java/com/octopus/openfeature/provider/v4/ClientSideRuleTests.java b/src/test/java/com/octopus/openfeature/provider/ClientSideRuleTests.java similarity index 95% rename from src/test/java/com/octopus/openfeature/provider/v4/ClientSideRuleTests.java rename to src/test/java/com/octopus/openfeature/provider/ClientSideRuleTests.java index a4d3296..59b5b84 100644 --- a/src/test/java/com/octopus/openfeature/provider/v4/ClientSideRuleTests.java +++ b/src/test/java/com/octopus/openfeature/provider/ClientSideRuleTests.java @@ -1,7 +1,6 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import com.fasterxml.jackson.databind.ObjectMapper; -import com.octopus.openfeature.provider.TestObjectMapper; import dev.openfeature.sdk.exceptions.ParseError; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -17,7 +16,7 @@ class ClientSideRuleTests { - private final ObjectMapper objectMapper = TestObjectMapper.INSTANCE; + private final ObjectMapper objectMapper = OctopusObjectMapper.INSTANCE; private static ClientSideRule rule(ClientSideCondition... conditions) { return new ClientSideRule("Rule 1", Arrays.asList(conditions)); diff --git a/src/test/java/com/octopus/openfeature/provider/v4/ContextAttributeIsNotOneOfConditionTests.java b/src/test/java/com/octopus/openfeature/provider/ContextAttributeIsNotOneOfConditionTests.java similarity index 98% rename from src/test/java/com/octopus/openfeature/provider/v4/ContextAttributeIsNotOneOfConditionTests.java rename to src/test/java/com/octopus/openfeature/provider/ContextAttributeIsNotOneOfConditionTests.java index 317c815..dfd0cec 100644 --- a/src/test/java/com/octopus/openfeature/provider/v4/ContextAttributeIsNotOneOfConditionTests.java +++ b/src/test/java/com/octopus/openfeature/provider/ContextAttributeIsNotOneOfConditionTests.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import dev.openfeature.sdk.MutableContext; import dev.openfeature.sdk.exceptions.ParseError; diff --git a/src/test/java/com/octopus/openfeature/provider/v4/ContextAttributeIsOneOfConditionTests.java b/src/test/java/com/octopus/openfeature/provider/ContextAttributeIsOneOfConditionTests.java similarity index 98% rename from src/test/java/com/octopus/openfeature/provider/v4/ContextAttributeIsOneOfConditionTests.java rename to src/test/java/com/octopus/openfeature/provider/ContextAttributeIsOneOfConditionTests.java index 674eb62..0bdecce 100644 --- a/src/test/java/com/octopus/openfeature/provider/v4/ContextAttributeIsOneOfConditionTests.java +++ b/src/test/java/com/octopus/openfeature/provider/ContextAttributeIsOneOfConditionTests.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import dev.openfeature.sdk.MutableContext; import dev.openfeature.sdk.exceptions.ParseError; diff --git a/src/test/java/com/octopus/openfeature/provider/v4/Contexts.java b/src/test/java/com/octopus/openfeature/provider/Contexts.java similarity index 97% rename from src/test/java/com/octopus/openfeature/provider/v4/Contexts.java rename to src/test/java/com/octopus/openfeature/provider/Contexts.java index 92fc0fb..8b56fbb 100644 --- a/src/test/java/com/octopus/openfeature/provider/v4/Contexts.java +++ b/src/test/java/com/octopus/openfeature/provider/Contexts.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import dev.openfeature.sdk.EvaluationContext; import dev.openfeature.sdk.MutableContext; diff --git a/src/test/java/com/octopus/openfeature/provider/v4/MalformedEvaluationTests.java b/src/test/java/com/octopus/openfeature/provider/MalformedEvaluationTests.java similarity index 98% rename from src/test/java/com/octopus/openfeature/provider/v4/MalformedEvaluationTests.java rename to src/test/java/com/octopus/openfeature/provider/MalformedEvaluationTests.java index 763ba43..5345707 100644 --- a/src/test/java/com/octopus/openfeature/provider/v4/MalformedEvaluationTests.java +++ b/src/test/java/com/octopus/openfeature/provider/MalformedEvaluationTests.java @@ -1,8 +1,7 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; -import com.octopus.openfeature.provider.TestObjectMapper; import dev.openfeature.sdk.ErrorCode; import dev.openfeature.sdk.EvaluationContext; import dev.openfeature.sdk.exceptions.ParseError; @@ -27,7 +26,7 @@ */ class MalformedEvaluationTests { - private final ObjectMapper objectMapper = TestObjectMapper.INSTANCE; + private final ObjectMapper objectMapper = OctopusObjectMapper.INSTANCE; /** * Satisfies every rule below, so a flag that failed to throw would visibly turn on. diff --git a/src/test/java/com/octopus/openfeature/provider/v4/PercentageByContextConditionTests.java b/src/test/java/com/octopus/openfeature/provider/PercentageByContextConditionTests.java similarity index 98% rename from src/test/java/com/octopus/openfeature/provider/v4/PercentageByContextConditionTests.java rename to src/test/java/com/octopus/openfeature/provider/PercentageByContextConditionTests.java index 09a8ad9..d5331f1 100644 --- a/src/test/java/com/octopus/openfeature/provider/v4/PercentageByContextConditionTests.java +++ b/src/test/java/com/octopus/openfeature/provider/PercentageByContextConditionTests.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import dev.openfeature.sdk.exceptions.ParseError; import org.junit.jupiter.api.Test; diff --git a/src/test/java/com/octopus/openfeature/provider/v4/PercentageRolloutTests.java b/src/test/java/com/octopus/openfeature/provider/PercentageRolloutTests.java similarity index 97% rename from src/test/java/com/octopus/openfeature/provider/v4/PercentageRolloutTests.java rename to src/test/java/com/octopus/openfeature/provider/PercentageRolloutTests.java index ba03dde..aaea725 100644 --- a/src/test/java/com/octopus/openfeature/provider/v4/PercentageRolloutTests.java +++ b/src/test/java/com/octopus/openfeature/provider/PercentageRolloutTests.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; diff --git a/src/test/java/com/octopus/openfeature/provider/RolloutVectors.java b/src/test/java/com/octopus/openfeature/provider/RolloutVectors.java index 2f8360c..97f7a8f 100644 --- a/src/test/java/com/octopus/openfeature/provider/RolloutVectors.java +++ b/src/test/java/com/octopus/openfeature/provider/RolloutVectors.java @@ -14,16 +14,13 @@ * NOT modify the input arguments or expected values — doing so would mask a real divergence in * evaluation behaviour between libraries and defeat the purpose of these cases. * - *

Public and in this package so both the v3 path and the v4 rollout can consume one list: the two - * hold separate copies of the hash, and a single set of vectors is what stops them drifting apart. - * Test-only, so it is never published. */ -public final class RolloutVectors { +final class RolloutVectors { private RolloutVectors() { } - public static Stream cases() { + static Stream cases() { return Stream.of( Arguments.of("ef70b4c0-1773-44a3-9b95-f239ae97d9db", "780c4b16-a510-49fa-a2b2-bbd1c38dbe31", 48), Arguments.of("055665f0-fbb3-484e-8ef1-52125425b7b2", "6a33c211-5af8-4c34-ba82-4d79846cb045", 85), diff --git a/src/test/java/com/octopus/openfeature/provider/v4/ServerSideEvaluationDeserializationTests.java b/src/test/java/com/octopus/openfeature/provider/ServerSideEvaluationDeserializationTests.java similarity index 87% rename from src/test/java/com/octopus/openfeature/provider/v4/ServerSideEvaluationDeserializationTests.java rename to src/test/java/com/octopus/openfeature/provider/ServerSideEvaluationDeserializationTests.java index cd207d8..42f2e14 100644 --- a/src/test/java/com/octopus/openfeature/provider/v4/ServerSideEvaluationDeserializationTests.java +++ b/src/test/java/com/octopus/openfeature/provider/ServerSideEvaluationDeserializationTests.java @@ -1,8 +1,7 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import com.fasterxml.jackson.core.type.TypeReference; import com.fasterxml.jackson.databind.ObjectMapper; -import com.octopus.openfeature.provider.TestObjectMapper; import org.junit.jupiter.api.Test; import java.io.InputStream; @@ -18,7 +17,7 @@ */ class ServerSideEvaluationDeserializationTests { - private final ObjectMapper objectMapper = TestObjectMapper.INSTANCE; + private final ObjectMapper objectMapper = OctopusObjectMapper.INSTANCE; private InputStream resource(String name) { return getClass().getResourceAsStream(name); @@ -27,7 +26,7 @@ private InputStream resource(String name) { @Test void shouldDeserializeServerResolvedEvaluation() throws Exception { var evaluation = objectMapper.readValue( - resource("evaluation-server-resolved.json"), ServerSideEvaluation.class); + resource("v4-evaluation-server-resolved.json"), ServerSideEvaluation.class); assertThat(evaluation.getSlug()).isEqualTo("my-feature"); assertThat(evaluation.getValue()).hasValue(true); @@ -39,7 +38,7 @@ void shouldDeserializeServerResolvedEvaluation() throws Exception { @Test void shouldDeserializeEvaluationDeferredToTheClientWithPolymorphicConditions() throws Exception { var evaluation = objectMapper.readValue( - resource("evaluation-deferred-to-client.json"), ServerSideEvaluation.class); + resource("v4-evaluation-deferred-to-client.json"), ServerSideEvaluation.class); assertThat(evaluation.getSlug()).isEqualTo("my-feature"); assertThat(evaluation.getEvaluationKey()).hasValue("0f8fad5b-d9cb-469f-a165-70867728950e"); @@ -66,7 +65,7 @@ void shouldDeserializeEvaluationDeferredToTheClientWithPolymorphicConditions() t @Test void shouldPreserveUnknownConditionAlongsideKnownConditionsWithoutFailingTheResponse() throws Exception { var evaluation = objectMapper.readValue( - resource("evaluation-with-unknown-condition.json"), ServerSideEvaluation.class); + resource("v4-evaluation-with-unknown-condition.json"), ServerSideEvaluation.class); var conditions = evaluation.getRules().orElseThrow().get(0).getConditions(); @@ -79,7 +78,7 @@ void shouldPreserveUnknownConditionAlongsideKnownConditionsWithoutFailingTheResp @Test void shouldDeserializeEvaluationsResponseAsListOfEvaluations() throws Exception { var evaluations = objectMapper.readValue( - resource("evaluation-list.json"), + resource("v4-evaluation-list.json"), new TypeReference>() {} ); @@ -102,7 +101,7 @@ void shouldDeserializeEvaluationWithoutASlugRatherThanFailingTheResponse() throw // No property is required at parse time: a malformed flag is reported when it is evaluated, so // it costs only itself rather than every other flag in the response. var evaluation = objectMapper.readValue( - resource("evaluation-missing-slug.json"), ServerSideEvaluation.class); + resource("v4-evaluation-missing-slug.json"), ServerSideEvaluation.class); assertThat(evaluation.getSlug()).isNull(); assertThat(evaluation.getValue()).hasValue(true); @@ -111,7 +110,7 @@ void shouldDeserializeEvaluationWithoutASlugRatherThanFailingTheResponse() throw @Test void shouldDeserializeEveryFlagWhenOneOfThemIsMissingItsSlug() throws Exception { var evaluations = objectMapper.readValue( - resource("evaluation-list-one-missing-slug.json"), + resource("v4-evaluation-list-one-missing-slug.json"), new TypeReference>() {} ); @@ -123,7 +122,7 @@ void shouldDeserializeEveryFlagWhenOneOfThemIsMissingItsSlug() throws Exception @Test void shouldIgnoreExtraneousProperties() throws Exception { var evaluation = objectMapper.readValue( - resource("evaluation-with-extraneous-properties.json"), ServerSideEvaluation.class); + resource("v4-evaluation-with-extraneous-properties.json"), ServerSideEvaluation.class); assertThat(evaluation.getSlug()).isEqualTo("my-feature"); @@ -136,7 +135,7 @@ void shouldIgnoreExtraneousProperties() throws Exception { @Test void shouldExposeConditionsAsImmutableLists() throws Exception { var evaluation = objectMapper.readValue( - resource("evaluation-deferred-to-client.json"), ServerSideEvaluation.class); + resource("v4-evaluation-deferred-to-client.json"), ServerSideEvaluation.class); var rules = evaluation.getRules().orElseThrow(); diff --git a/src/test/java/com/octopus/openfeature/provider/v4/ServerSideEvaluationTests.java b/src/test/java/com/octopus/openfeature/provider/ServerSideEvaluationTests.java similarity index 99% rename from src/test/java/com/octopus/openfeature/provider/v4/ServerSideEvaluationTests.java rename to src/test/java/com/octopus/openfeature/provider/ServerSideEvaluationTests.java index 8c818c2..3444253 100644 --- a/src/test/java/com/octopus/openfeature/provider/v4/ServerSideEvaluationTests.java +++ b/src/test/java/com/octopus/openfeature/provider/ServerSideEvaluationTests.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import dev.openfeature.sdk.ErrorCode; import dev.openfeature.sdk.exceptions.ParseError; diff --git a/src/test/java/com/octopus/openfeature/provider/TestObjectMapper.java b/src/test/java/com/octopus/openfeature/provider/TestObjectMapper.java deleted file mode 100644 index 0a34a97..0000000 --- a/src/test/java/com/octopus/openfeature/provider/TestObjectMapper.java +++ /dev/null @@ -1,17 +0,0 @@ -package com.octopus.openfeature.provider; - -import com.fasterxml.jackson.databind.ObjectMapper; - -/** - * Exposes the provider's package-private {@link OctopusObjectMapper} to tests in other packages, so - * they can exercise deserialization with the same mapper the client uses in production rather than - * a re-declared copy that could drift from it. - * - *

Test-only: this lives in the test sources and is never published. - */ -public final class TestObjectMapper { - public static final ObjectMapper INSTANCE = OctopusObjectMapper.INSTANCE; - - private TestObjectMapper() { - } -} diff --git a/src/test/java/com/octopus/openfeature/provider/v4/UnknownConditionTests.java b/src/test/java/com/octopus/openfeature/provider/UnknownConditionTests.java similarity index 94% rename from src/test/java/com/octopus/openfeature/provider/v4/UnknownConditionTests.java rename to src/test/java/com/octopus/openfeature/provider/UnknownConditionTests.java index cf68f58..421cba1 100644 --- a/src/test/java/com/octopus/openfeature/provider/v4/UnknownConditionTests.java +++ b/src/test/java/com/octopus/openfeature/provider/UnknownConditionTests.java @@ -1,4 +1,4 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import dev.openfeature.sdk.exceptions.ParseError; import org.junit.jupiter.api.Test; diff --git a/src/test/java/com/octopus/openfeature/provider/v4/UnrecognisedConditionTests.java b/src/test/java/com/octopus/openfeature/provider/UnrecognisedConditionTests.java similarity index 92% rename from src/test/java/com/octopus/openfeature/provider/v4/UnrecognisedConditionTests.java rename to src/test/java/com/octopus/openfeature/provider/UnrecognisedConditionTests.java index 78e1631..a668aec 100644 --- a/src/test/java/com/octopus/openfeature/provider/v4/UnrecognisedConditionTests.java +++ b/src/test/java/com/octopus/openfeature/provider/UnrecognisedConditionTests.java @@ -1,7 +1,6 @@ -package com.octopus.openfeature.provider.v4; +package com.octopus.openfeature.provider; import com.fasterxml.jackson.databind.ObjectMapper; -import com.octopus.openfeature.provider.TestObjectMapper; import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; @@ -14,7 +13,7 @@ */ class UnrecognisedConditionTests { - private final ObjectMapper objectMapper = TestObjectMapper.INSTANCE; + private final ObjectMapper objectMapper = OctopusObjectMapper.INSTANCE; private ServerSideEvaluation flag(String singleQuotedJson) throws Exception { return objectMapper.readValue(Contexts.json(singleQuotedJson), ServerSideEvaluation.class); diff --git a/src/test/resources/com/octopus/openfeature/provider/v4/condition-context-attribute-is-not-one-of.json b/src/test/resources/com/octopus/openfeature/provider/v4-condition-context-attribute-is-not-one-of.json similarity index 100% rename from src/test/resources/com/octopus/openfeature/provider/v4/condition-context-attribute-is-not-one-of.json rename to src/test/resources/com/octopus/openfeature/provider/v4-condition-context-attribute-is-not-one-of.json diff --git a/src/test/resources/com/octopus/openfeature/provider/v4/condition-context-attribute-is-one-of.json b/src/test/resources/com/octopus/openfeature/provider/v4-condition-context-attribute-is-one-of.json similarity index 100% rename from src/test/resources/com/octopus/openfeature/provider/v4/condition-context-attribute-is-one-of.json rename to src/test/resources/com/octopus/openfeature/provider/v4-condition-context-attribute-is-one-of.json diff --git a/src/test/resources/com/octopus/openfeature/provider/v4/condition-list-mixed.json b/src/test/resources/com/octopus/openfeature/provider/v4-condition-list-mixed.json similarity index 100% rename from src/test/resources/com/octopus/openfeature/provider/v4/condition-list-mixed.json rename to src/test/resources/com/octopus/openfeature/provider/v4-condition-list-mixed.json diff --git a/src/test/resources/com/octopus/openfeature/provider/v4/condition-missing-type.json b/src/test/resources/com/octopus/openfeature/provider/v4-condition-missing-type.json similarity index 100% rename from src/test/resources/com/octopus/openfeature/provider/v4/condition-missing-type.json rename to src/test/resources/com/octopus/openfeature/provider/v4-condition-missing-type.json diff --git a/src/test/resources/com/octopus/openfeature/provider/v4/condition-percentage-by-context.json b/src/test/resources/com/octopus/openfeature/provider/v4-condition-percentage-by-context.json similarity index 100% rename from src/test/resources/com/octopus/openfeature/provider/v4/condition-percentage-by-context.json rename to src/test/resources/com/octopus/openfeature/provider/v4-condition-percentage-by-context.json diff --git a/src/test/resources/com/octopus/openfeature/provider/v4/condition-unknown-type.json b/src/test/resources/com/octopus/openfeature/provider/v4-condition-unknown-type.json similarity index 100% rename from src/test/resources/com/octopus/openfeature/provider/v4/condition-unknown-type.json rename to src/test/resources/com/octopus/openfeature/provider/v4-condition-unknown-type.json diff --git a/src/test/resources/com/octopus/openfeature/provider/v4/evaluation-deferred-to-client.json b/src/test/resources/com/octopus/openfeature/provider/v4-evaluation-deferred-to-client.json similarity index 100% rename from src/test/resources/com/octopus/openfeature/provider/v4/evaluation-deferred-to-client.json rename to src/test/resources/com/octopus/openfeature/provider/v4-evaluation-deferred-to-client.json diff --git a/src/test/resources/com/octopus/openfeature/provider/v4/evaluation-list-one-missing-slug.json b/src/test/resources/com/octopus/openfeature/provider/v4-evaluation-list-one-missing-slug.json similarity index 100% rename from src/test/resources/com/octopus/openfeature/provider/v4/evaluation-list-one-missing-slug.json rename to src/test/resources/com/octopus/openfeature/provider/v4-evaluation-list-one-missing-slug.json diff --git a/src/test/resources/com/octopus/openfeature/provider/v4/evaluation-list.json b/src/test/resources/com/octopus/openfeature/provider/v4-evaluation-list.json similarity index 100% rename from src/test/resources/com/octopus/openfeature/provider/v4/evaluation-list.json rename to src/test/resources/com/octopus/openfeature/provider/v4-evaluation-list.json diff --git a/src/test/resources/com/octopus/openfeature/provider/v4/evaluation-missing-slug.json b/src/test/resources/com/octopus/openfeature/provider/v4-evaluation-missing-slug.json similarity index 100% rename from src/test/resources/com/octopus/openfeature/provider/v4/evaluation-missing-slug.json rename to src/test/resources/com/octopus/openfeature/provider/v4-evaluation-missing-slug.json diff --git a/src/test/resources/com/octopus/openfeature/provider/v4/evaluation-server-resolved.json b/src/test/resources/com/octopus/openfeature/provider/v4-evaluation-server-resolved.json similarity index 100% rename from src/test/resources/com/octopus/openfeature/provider/v4/evaluation-server-resolved.json rename to src/test/resources/com/octopus/openfeature/provider/v4-evaluation-server-resolved.json diff --git a/src/test/resources/com/octopus/openfeature/provider/v4/evaluation-with-extraneous-properties.json b/src/test/resources/com/octopus/openfeature/provider/v4-evaluation-with-extraneous-properties.json similarity index 100% rename from src/test/resources/com/octopus/openfeature/provider/v4/evaluation-with-extraneous-properties.json rename to src/test/resources/com/octopus/openfeature/provider/v4-evaluation-with-extraneous-properties.json diff --git a/src/test/resources/com/octopus/openfeature/provider/v4/evaluation-with-unknown-condition.json b/src/test/resources/com/octopus/openfeature/provider/v4-evaluation-with-unknown-condition.json similarity index 100% rename from src/test/resources/com/octopus/openfeature/provider/v4/evaluation-with-unknown-condition.json rename to src/test/resources/com/octopus/openfeature/provider/v4-evaluation-with-unknown-condition.json From 27aea2c0ad1dd11fc0827d5cf83ed8f5fa4aed28 Mon Sep 17 00:00:00 2001 From: Lourens de Jager Date: Tue, 11 Aug 2026 14:25:28 +1200 Subject: [PATCH 02/10] feat!: evaluate feature flags through the v4 endpoints Switches the provider from the v3 toggle endpoints to the v4 feature-flag endpoints, and from v3's segment and rollout matching to the rules-based evaluation added earlier. - GET api/feature-flags/check/v4/ and api/feature-flags/evaluations/v4/ - EvaluationResponse replaces FeatureToggles - evaluate(slug, context) resolves a ServerSideEvaluation, so the value and reason come from the server or from the client-side rules it deferred, and a malformed response reports the problem it found - an unrecognised slug throws FlagNotFoundError with the message the other provider libraries use - specification fixtures move to the v4 evaluations set v3's types remain, unused, for BMBB-780. BREAKING CHANGE: the provider now calls the v4 feature-flag endpoints and evaluates rules-based flags. A server that only serves the v3 toggle endpoints is no longer supported. Co-Authored-By: Claude Opus 5 (1M context) --- specification | 2 +- .../provider/EvaluationResponse.java | 25 ++ .../provider/FeatureToggleEvaluation.java | 1 + .../openfeature/provider/FeatureToggles.java | 1 + .../openfeature/provider/OctopusClient.java | 26 +- .../openfeature/provider/OctopusContext.java | 137 ++------ .../provider/OctopusContextProvider.java | 18 +- .../openfeature/provider/OctopusProvider.java | 9 +- .../octopus/openfeature/provider/Segment.java | 1 + .../provider/OctopusClientTests.java | 81 +++++ .../provider/OctopusContextProviderTests.java | 83 ++--- .../provider/OctopusContextTests.java | 301 ++++-------------- .../provider/OctopusProviderTests.java | 7 +- .../octopus/openfeature/provider/Server.java | 4 +- .../provider/SpecificationTests.java | 23 +- 15 files changed, 286 insertions(+), 433 deletions(-) create mode 100644 src/main/java/com/octopus/openfeature/provider/EvaluationResponse.java diff --git a/specification b/specification index ef84509..67890d8 160000 --- a/specification +++ b/specification @@ -1 +1 @@ -Subproject commit ef8450956f7619b742b0983fb99f664426b36c8d +Subproject commit 67890d834996dc65e130716e9246c8754ad1aeda diff --git a/src/main/java/com/octopus/openfeature/provider/EvaluationResponse.java b/src/main/java/com/octopus/openfeature/provider/EvaluationResponse.java new file mode 100644 index 0000000..9022a72 --- /dev/null +++ b/src/main/java/com/octopus/openfeature/provider/EvaluationResponse.java @@ -0,0 +1,25 @@ +package com.octopus.openfeature.provider; + +import java.util.List; + +/** + * One response from the v4 evaluations endpoint: the server-side evaluation of every flag, and the + * content hash identifying that set. + */ +class EvaluationResponse { + private final List evaluations; + private final byte[] contentHash; + + EvaluationResponse(List evaluations, byte[] contentHash) { + this.evaluations = evaluations; + this.contentHash = contentHash; + } + + public List getEvaluations() { + return evaluations; + } + + public byte[] getContentHash() { + return contentHash; + } +} diff --git a/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java b/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java index 3d2db59..21b698a 100644 --- a/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java +++ b/src/main/java/com/octopus/openfeature/provider/FeatureToggleEvaluation.java @@ -6,6 +6,7 @@ import java.util.List; import java.util.Optional; +// TODO(BMBB-780): a v3 type, unused since the switch to v4. class FeatureToggleEvaluation { private final String slug; private final boolean isEnabled; diff --git a/src/main/java/com/octopus/openfeature/provider/FeatureToggles.java b/src/main/java/com/octopus/openfeature/provider/FeatureToggles.java index 987c77b..1500ea9 100644 --- a/src/main/java/com/octopus/openfeature/provider/FeatureToggles.java +++ b/src/main/java/com/octopus/openfeature/provider/FeatureToggles.java @@ -2,6 +2,7 @@ import java.util.List; +// TODO(BMBB-780): a v3 type, unused since the switch to v4. class FeatureToggles { private final List evaluations; private final byte[] contentHash; diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java index 59ea702..badc8c3 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java @@ -43,7 +43,7 @@ private static String loadProviderVersion() { this.config = config; } - Boolean haveFeatureTogglesChanged(byte[] contentHash) throws IOException, InterruptedException { + Boolean haveFeatureFlagsChanged(byte[] contentHash) throws IOException, InterruptedException { if (contentHash.length == 0) { return true; } @@ -56,31 +56,31 @@ Boolean haveFeatureTogglesChanged(byte[] contentHash) throws IOException, Interr .header("X-Octopus-Client", buildOctopusClientHeaderValue()) .build(); HttpResponse httpResponse = client.send(request, HttpResponse.BodyHandlers.ofString()); - FeatureToggleCheckResponse checkResponse = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), FeatureToggleCheckResponse.class); + FeatureFlagCheckResponse checkResponse = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), FeatureFlagCheckResponse.class); return !Arrays.equals(checkResponse.contentHash, contentHash); } - FeatureToggles getFeatureToggleEvaluationManifest() throws IOException, InterruptedException { - URI manifestURI = getManifestURI(); + EvaluationResponse getServerSideEvaluations() throws IOException, InterruptedException { + URI evaluationsURI = getEvaluationsURI(); HttpClient client = HttpClient.newHttpClient(); HttpRequest request = HttpRequest.newBuilder() .GET() - .uri(manifestURI) + .uri(evaluationsURI) .header("Authorization", String.format("Bearer %s", config.getClientIdentifier())) .header("X-Octopus-Client", buildOctopusClientHeaderValue()) .build(); HttpResponse httpResponse = client.send(request, HttpResponse.BodyHandlers.ofString()); if (httpResponse.statusCode() == StatusCodeNotFound) { - logger.log(System.Logger.Level.WARNING, String.format("Failed to retrieve feature toggles for client identifier %s from %s", config.getClientIdentifier(), manifestURI.toString())); + logger.log(System.Logger.Level.WARNING, String.format("Failed to retrieve feature flags for client identifier %s from %s", config.getClientIdentifier(), evaluationsURI.toString())); return null; } Optional contentHashHeader = httpResponse.headers().firstValue("ContentHash"); if (contentHashHeader.isEmpty()) { - logger.log(System.Logger.Level.WARNING, String.format("Feature toggle response from %s did not contain expected ContentHash header", manifestURI.toString())); + logger.log(System.Logger.Level.WARNING, String.format("Feature flag response from %s did not contain expected ContentHash header", evaluationsURI.toString())); return null; } - var evaluations = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), new TypeReference>() {}); - return new FeatureToggles(evaluations, Base64.getDecoder().decode(contentHashHeader.get())); + var evaluations = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), new TypeReference>() {}); + return new EvaluationResponse(evaluations, Base64.getDecoder().decode(contentHashHeader.get())); } String buildOctopusClientHeaderValue() { @@ -95,16 +95,16 @@ String buildOctopusClientHeaderValue() { private URI getCheckURI() { try { - return new URL(config.getServerUri().toURL(), "/api/featuretoggles/check/v3/").toURI(); + return new URL(config.getServerUri().toURL(), "/api/feature-flags/check/v4/").toURI(); } catch (MalformedURLException | URISyntaxException ignored) // we know this URL is well-formed { } return null; } - private URI getManifestURI() { + private URI getEvaluationsURI() { try { - return new URL(config.getServerUri().toURL(), "/api/toggles/evaluations/v3/").toURI(); + return new URL(config.getServerUri().toURL(), "/api/feature-flags/evaluations/v4/").toURI(); } catch (MalformedURLException | URISyntaxException ignored) // we know this URL is well-formed { } @@ -112,7 +112,7 @@ private URI getManifestURI() { } // This class needs to be static to allow deserialization - private static class FeatureToggleCheckResponse { + private static class FeatureFlagCheckResponse { public byte[] contentHash; } } diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusContext.java b/src/main/java/com/octopus/openfeature/provider/OctopusContext.java index 2bd33a3..065212a 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusContext.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusContext.java @@ -1,133 +1,58 @@ package com.octopus.openfeature.provider; -import dev.openfeature.sdk.*; +import dev.openfeature.sdk.EvaluationContext; +import dev.openfeature.sdk.ProviderEvaluation; import dev.openfeature.sdk.exceptions.FlagNotFoundError; -import dev.openfeature.sdk.exceptions.ParseError; -import org.apache.commons.codec.digest.MurmurHash3; -import java.nio.charset.StandardCharsets; import java.util.List; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; -import static java.util.stream.Collectors.groupingBy; - +/** + * Holds one evaluation response and resolves a flag from it, applying any client-side rules the + * server deferred. + */ class OctopusContext { - private static final System.Logger logger = System.getLogger(OctopusClient.class.getName()); - private final FeatureToggles featureToggles; + private static final System.Logger logger = System.getLogger(OctopusContext.class.getName()); + + private final EvaluationResponse evaluationResponse; + private final Set warnedSlugs = ConcurrentHashMap.newKeySet(); - OctopusContext(FeatureToggles featureToggles) { - this.featureToggles = featureToggles; + OctopusContext(EvaluationResponse evaluationResponse) { + this.evaluationResponse = evaluationResponse; } static OctopusContext empty() { - return new OctopusContext(new FeatureToggles(List.of(), new byte[0])); + return new OctopusContext(new EvaluationResponse(List.of(), new byte[0])); } byte[] getContentHash() { - return featureToggles.getContentHash(); + return evaluationResponse.getContentHash(); } - FeatureToggleEvaluation findFeatureToggleBySlug(String slug) { - return featureToggles.getEvaluations().stream() - .filter(f -> f.getSlug().equalsIgnoreCase(slug)) + ServerSideEvaluation findEvaluationBySlug(String slug) { + return evaluationResponse.getEvaluations().stream() + .filter(evaluation -> slug.equalsIgnoreCase(evaluation.getSlug())) .findFirst().orElse(null); } - ProviderEvaluation evaluate(String slug, Boolean defaultValue, EvaluationContext evaluationContext) { - var toggleValue = findFeatureToggleBySlug(slug); - - if (toggleValue == null) { - throw new FlagNotFoundError(); - } - - if (missingRequiredPropertiesForClientSideEvaluation(toggleValue)) { - throw new ParseError("Feature toggle " + toggleValue.getSlug() + " is missing necessary information for client-side evaluation."); - } - - if (!toggleValue.isEnabled()) { - return ProviderEvaluation.builder() - .value(false) - .reason(Reason.DEFAULT.toString()) - .build(); - } - - // EvaluationKey and ClientRolloutPercentage are guaranteed non-null here via missingRequiredPropertiesForClientSideEvaluation() - String evaluationKey = toggleValue.getEvaluationKey().orElseThrow(); - int rolloutPercentage = toggleValue.getClientRolloutPercentage().orElseThrow(); - String targetingKey = evaluationContext != null ? evaluationContext.getTargetingKey() : null; + ProviderEvaluation evaluate(String slug, EvaluationContext evaluationContext) { + var serverSideEvaluation = findEvaluationBySlug(slug); - if (targetingKey == null || targetingKey.isEmpty()) { - if (rolloutPercentage < 100) { - return ProviderEvaluation.builder() - .value(false) - .reason(Reason.TARGETING_MATCH.toString()) - .build(); + if (serverSideEvaluation == null) { + // Warned once per slug: an unrecognised slug is usually a typo, which would otherwise log on + // every evaluation of it. + if (warnedSlugs.add(slug.toLowerCase())) { + logger.log(System.Logger.Level.WARNING, String.format( + "The slug %s did not match any of your Octopus Feature Flags. Please double check your slug and try again.", + slug)); } - // rolloutPercentage == 100: fall through to segment check - } else { - if (getNormalizedNumber(evaluationKey, targetingKey) > rolloutPercentage) { - return ProviderEvaluation.builder() - .value(false) - .reason(Reason.TARGETING_MATCH.toString()) - .build(); - } - } - - if (!toggleValue.hasSegments()) { - return ProviderEvaluation.builder() - .value(true) - .reason(Reason.DEFAULT.toString()) - .build(); - } - - var segments = toggleValue.getSegments().orElseThrow(); - return ProviderEvaluation.builder() - .value(matchesSegment(evaluationContext, segments)) - .reason(Reason.TARGETING_MATCH.toString()) - .build(); - } - - private boolean missingRequiredPropertiesForClientSideEvaluation(FeatureToggleEvaluation evaluation) { - if (!evaluation.isEnabled()) { - return false; - } - - return evaluation.getClientRolloutPercentage().isEmpty() - || evaluation.getEvaluationKey().isEmpty() - || evaluation.getSegments().isEmpty(); - } - - static int getNormalizedNumber(String evaluationKey, String targetingKey) { - byte[] bytes = (evaluationKey + ":" + targetingKey).getBytes(StandardCharsets.UTF_8); - - // MurmurHash3 32-bit, seed 0. hash32x86 processes tail bytes in little-endian order, - // matching the reference C spec and equivalent to .NET's MurmurHash.Create32() + - // BinaryPrimitives.ReadUInt32LittleEndian(). - int hash = MurmurHash3.hash32x86(bytes, 0, bytes.length, 0); - - // Java has no unsigned integer type. Integer.toUnsignedLong() reinterprets the signed - // int as an unsigned 32-bit value (widened to long) — equivalent to casting to uint in C#. - long unsignedHash = Integer.toUnsignedLong(hash); - - return (int) (unsignedHash % 100) + 1; - } - - static boolean matchesSegment(EvaluationContext evaluationContext, List segments) { - if (evaluationContext == null) { - return false; + throw new FlagNotFoundError( + "The slug provided did not match any of your Octopus Feature Flags. Please double check your slug and try again."); } - var contextEntries = evaluationContext.asMap(); - var groupedByKey = segments.stream().collect(groupingBy(Segment::getKey)); - return groupedByKey.keySet().stream().allMatch(k -> { - var values = groupedByKey.get(k); - - return contextEntries.keySet().stream().anyMatch( - c -> c.equalsIgnoreCase(k) && values.stream().anyMatch( - v -> v.getValue().equalsIgnoreCase(contextEntries.get(c).asString()))); - - }); + return serverSideEvaluation.evaluate(evaluationContext); } - } diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java b/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java index c5c042a..aba4004 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java @@ -21,10 +21,10 @@ void initialize() { } try { - var toggles = client.getFeatureToggleEvaluationManifest(); - currentContext = toggles == null ? OctopusContext.empty() : new OctopusContext(toggles); + var evaluationResponse = client.getServerSideEvaluations(); + currentContext = evaluationResponse == null ? OctopusContext.empty() : new OctopusContext(evaluationResponse); } catch (Exception e) { - logger.log(System.Logger.Level.ERROR, "Failed to retrieve feature manifest during initialization. Falling back to empty context, defaults will be used during evaluation.", e); + logger.log(System.Logger.Level.ERROR, "Failed to retrieve feature flag evaluations during initialization. Falling back to empty context, defaults will be used during evaluation.", e); currentContext = OctopusContext.empty(); } @@ -45,19 +45,19 @@ void refresh() { try { Thread.sleep(config.getCacheDuration().toMillis()); - if (client.haveFeatureTogglesChanged(currentContext.getContentHash())) { - var toggles = client.getFeatureToggleEvaluationManifest(); - if (toggles != null) { - currentContext = new OctopusContext(toggles); + if (client.haveFeatureFlagsChanged(currentContext.getContentHash())) { + var evaluationResponse = client.getServerSideEvaluations(); + if (evaluationResponse != null) { + currentContext = new OctopusContext(evaluationResponse); } else { - logger.log(System.Logger.Level.ERROR, "Failed to retrieve updated feature manifest. Retaining existing context which may be stale."); + logger.log(System.Logger.Level.ERROR, "Failed to retrieve updated feature flag evaluations. Retaining existing context which may be stale."); } } } catch (InterruptedException e) { // the loop will be terminated and the thread will finish Thread.currentThread().interrupt(); } catch (Exception e) { - logger.log(System.Logger.Level.ERROR, "Failed to retrieve updated feature manifest. Retaining existing context which may be stale.", e); + logger.log(System.Logger.Level.ERROR, "Failed to retrieve updated feature flag evaluations. Retaining existing context which may be stale.", e); } } } diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusProvider.java b/src/main/java/com/octopus/openfeature/provider/OctopusProvider.java index 0d169a0..b66a409 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusProvider.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusProvider.java @@ -37,7 +37,7 @@ public void shutdown() { @Override public ProviderEvaluation getBooleanEvaluation(String flagKey, Boolean defaultValue, EvaluationContext evaluationContext) { - return contextProvider.getOctopusContext().evaluate(flagKey, defaultValue, evaluationContext); + return contextProvider.getOctopusContext().evaluate(flagKey, evaluationContext); } @Override @@ -61,9 +61,10 @@ public ProviderEvaluation getObjectEvaluation(String flagKey, Value defau } private RuntimeException rejectNonBooleanEvaluation(String flagKey) { - var toggle = contextProvider.getOctopusContext().findFeatureToggleBySlug(flagKey); - if (toggle == null) { - return new FlagNotFoundError(flagKey); + var evaluation = contextProvider.getOctopusContext().findEvaluationBySlug(flagKey); + if (evaluation == null) { + return new FlagNotFoundError( + "The slug provided did not match any of your Octopus Feature Flags. Please double check your slug and try again."); } return new TypeMismatchError("Octopus only supports boolean flags."); } diff --git a/src/main/java/com/octopus/openfeature/provider/Segment.java b/src/main/java/com/octopus/openfeature/provider/Segment.java index 0ca3fda..da06270 100644 --- a/src/main/java/com/octopus/openfeature/provider/Segment.java +++ b/src/main/java/com/octopus/openfeature/provider/Segment.java @@ -3,6 +3,7 @@ import com.fasterxml.jackson.annotation.JsonCreator; import com.fasterxml.jackson.annotation.JsonProperty; +// TODO(BMBB-780): a v3 type, unused since the switch to v4. class Segment { private final String key; private final String value; diff --git a/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java b/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java index a80dfb9..0a04696 100644 --- a/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java +++ b/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java @@ -1,10 +1,20 @@ package com.octopus.openfeature.provider; +import com.github.tomakehurst.wiremock.WireMockServer; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import java.io.IOException; +import java.net.URI; +import java.util.Base64; import java.util.Properties; +import java.util.stream.Collectors; +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl; +import static com.github.tomakehurst.wiremock.client.WireMock.get; +import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; import static org.assertj.core.api.Assertions.assertThat; class OctopusClientTests { @@ -55,4 +65,75 @@ void buildOctopusClientHeaderValue_withNameContainingUnsupportedChars_stripsChar assertThat(client.buildOctopusClientHeaderValue()) .isEqualTo("MyProduct openfeature-provider-java/" + PROVIDER_VERSION); } + + private static final String CHECK_PATH = "/api/feature-flags/check/v4/"; + private static final String EVALUATIONS_PATH = "/api/feature-flags/evaluations/v4/"; + private static final String CONTENT_HASH = Base64.getEncoder().encodeToString(new byte[]{0x01, 0x02}); + + private WireMockServer wireMock; + + @BeforeEach + void startServer() { + wireMock = new WireMockServer(wireMockConfig().dynamicPort()); + wireMock.start(); + } + + @AfterEach + void stopServer() { + wireMock.stop(); + } + + private OctopusClient clientForServer() { + var config = new OctopusConfiguration("test-id", new ProductMetadata("MyProduct")); + config.setServerUri(URI.create(wireMock.baseUrl())); + return new OctopusClient(config); + } + + private String requestedPaths() { + return wireMock.getAllServeEvents().stream() + .map(event -> event.getRequest().getUrl()) + .collect(Collectors.joining(", ")); + } + + @Test + void haveFeatureFlagsChanged_requestsTheV4CheckEndpoint() throws Exception { + wireMock.stubFor(get(anyUrl()).willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Type", "application/json") + .withBody("{\"contentHash\":\"" + CONTENT_HASH + "\"}"))); + + var haveChanged = clientForServer().haveFeatureFlagsChanged(new byte[]{0x03, 0x04}); + + assertThat(requestedPaths()).isEqualTo(CHECK_PATH); + assertThat(haveChanged).isTrue(); + } + + @Test + void haveFeatureFlagsChanged_whenTheContentHashIsUnchanged_reportsNoChange() throws Exception { + wireMock.stubFor(get(anyUrl()).willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Type", "application/json") + .withBody("{\"contentHash\":\"" + CONTENT_HASH + "\"}"))); + + var haveChanged = clientForServer().haveFeatureFlagsChanged(new byte[]{0x01, 0x02}); + + assertThat(haveChanged).isFalse(); + } + + @Test + void getServerSideEvaluations_requestsTheV4EvaluationsEndpoint() throws Exception { + wireMock.stubFor(get(anyUrl()).willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Type", "application/json") + .withHeader("ContentHash", CONTENT_HASH) + .withBody("[{\"slug\":\"test-feature\",\"value\":true,\"reason\":\"The flag is enabled for this environment.\"}]"))); + + var response = clientForServer().getServerSideEvaluations(); + + assertThat(requestedPaths()).isEqualTo(EVALUATIONS_PATH); + assertThat(response).isNotNull(); + assertThat(response.getContentHash()).isEqualTo(new byte[]{0x01, 0x02}); + assertThat(response.getEvaluations()).singleElement() + .satisfies(evaluation -> assertThat(evaluation.getSlug()).isEqualTo("test-feature")); + } } diff --git a/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java b/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java index 3bfd7d6..28b3c75 100644 --- a/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java +++ b/src/test/java/com/octopus/openfeature/provider/OctopusContextProviderTests.java @@ -15,28 +15,36 @@ class OctopusContextProviderTests { static class MockOctopusFeatureClient extends OctopusClient { - private volatile FeatureToggles toggles; + private volatile EvaluationResponse evaluationResponse; - MockOctopusFeatureClient(FeatureToggles toggles) { + MockOctopusFeatureClient(EvaluationResponse evaluationResponse) { super(null); - this.toggles = toggles; + this.evaluationResponse = evaluationResponse; } - void changeToggles(FeatureToggles toggles) { - this.toggles = toggles; + void changeEvaluations(EvaluationResponse evaluationResponse) { + this.evaluationResponse = evaluationResponse; } @Override - Boolean haveFeatureTogglesChanged(byte[] contentHash) { + Boolean haveFeatureFlagsChanged(byte[] contentHash) { return true; } @Override - FeatureToggles getFeatureToggleEvaluationManifest() { - return toggles; + EvaluationResponse getServerSideEvaluations() { + return evaluationResponse; } } + private static EvaluationResponse response(boolean value, byte[] contentHash) { + return new EvaluationResponse( + List.of(new ServerSideEvaluation("test-feature", value, + value ? "The flag is enabled for this environment." : "The flag is disabled for this environment.", + null, null)), + contentHash); + } + private final OctopusConfiguration configuration = configure(); private static OctopusConfiguration configure() { @@ -51,10 +59,7 @@ void whenInitialized_RefreshesCacheAfterCacheDurationExpires() throws Interrupte byte[] initialHash = {0x01, 0x02, 0x03, 0x04}; byte[] updatedHash = {0x01, 0x02, 0x03, 0x05}; - var client = new MockOctopusFeatureClient(new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", true, "evaluation-key", Collections.emptyList(), 100)), - initialHash - )); + var client = new MockOctopusFeatureClient(response(true, initialHash)); var provider = new OctopusContextProvider(configuration, client); provider.initialize(); @@ -62,20 +67,17 @@ void whenInitialized_RefreshesCacheAfterCacheDurationExpires() throws Interrupte try { // Validate the initial state assertThat(provider.getOctopusContext().getContentHash()).isEqualTo(initialHash); - assertThat(provider.getOctopusContext().evaluate("test-feature", false, null).getValue()).isTrue(); + assertThat(provider.getOctopusContext().evaluate("test-feature", null).getValue()).isTrue(); // Simulate a change in the available feature toggles - client.changeToggles(new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", false, "evaluation-key", Collections.emptyList(), 100)), - updatedHash - )); + client.changeEvaluations(response(false, updatedHash)); // Wait for the cache to expire Thread.sleep(500); // Validate the updated toggles are available assertThat(provider.getOctopusContext().getContentHash()).isEqualTo(updatedHash); - assertThat(provider.getOctopusContext().evaluate("test-feature", true, null).getValue()).isFalse(); + assertThat(provider.getOctopusContext().evaluate("test-feature", null).getValue()).isFalse(); } finally { provider.shutdown(); @@ -87,10 +89,7 @@ void whenInitialized_AndRefreshFails_RetainsExistingContextAndLogsError() throws byte[] contentHash = {0x01, 0x02, 0x03, 0x04}; - var client = new MockOctopusFeatureClient(new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", true, "evaluation-key", Collections.emptyList(), 100)), - contentHash - )); + var client = new MockOctopusFeatureClient(response(true, contentHash)); var logMessages = new ArrayList(); var julLogger = Logger.getLogger(OctopusClient.class.getName()); @@ -110,15 +109,15 @@ void whenInitialized_AndRefreshFails_RetainsExistingContextAndLogsError() throws julLogger.setUseParentHandlers(false); // Simulate a failed fetch - client.changeToggles(null); + client.changeEvaluations(null); // Wait for the cache to expire Thread.sleep(500); // Validate that the existing context is retained and an error was logged assertThat(provider.getOctopusContext().getContentHash()).isEqualTo(contentHash); - assertThat(provider.getOctopusContext().evaluate("test-feature", false, null).getValue()).isTrue(); - assertThat(logMessages).anyMatch(m -> m.startsWith("Failed to retrieve updated feature manifest")); + assertThat(provider.getOctopusContext().evaluate("test-feature", null).getValue()).isTrue(); + assertThat(logMessages).anyMatch(m -> m.startsWith("Failed to retrieve updated feature flag evaluations")); } finally { julLogger.removeHandler(handler); @@ -145,10 +144,7 @@ void whenInitialFetchReturnsNothing_AndRefreshSucceeds_ContextIsPopulated() thro assertThat(provider.getOctopusContext().getContentHash()).isEmpty(); // Update client to return valid toggles and wait for refresh - client.changeToggles(new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", false, "evaluation-key", Collections.emptyList(), 100)), - contentHash - )); + client.changeEvaluations(response(false, contentHash)); Thread.sleep(5000); // Assert that the context is now correctly populated @@ -176,10 +172,7 @@ void whenRefreshReturnsNothing_AndSubsequentRefreshSucceeds_ContextIsUpdated() t julLogger.addHandler(handler); // initialize with a client that returns valid toggles - var client = new MockOctopusFeatureClient(new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", true, "evaluation-key", Collections.emptyList(), 100)), - initialHash - )); + var client = new MockOctopusFeatureClient(response(true, initialHash)); var provider = new OctopusContextProvider(configuration, client); provider.initialize(); @@ -188,18 +181,15 @@ void whenRefreshReturnsNothing_AndSubsequentRefreshSucceeds_ContextIsUpdated() t try { // Switch to a null client and wait for refresh to fail - client.changeToggles(null); + client.changeEvaluations(null); Thread.sleep(5000); // Assert that failed refresh is logged and old context is retained - assertThat(logMessages).anyMatch(m -> m.startsWith("Failed to retrieve updated feature manifest")); + assertThat(logMessages).anyMatch(m -> m.startsWith("Failed to retrieve updated feature flag evaluations")); assertThat(provider.getOctopusContext().getContentHash()).isEqualTo(initialHash); // Update client to return valid toggles again and wait for refresh - client.changeToggles(new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", false, "evaluation-key", Collections.emptyList(), 100)), - updatedHash - )); + client.changeEvaluations(response(false, updatedHash)); Thread.sleep(5000); assertThat(provider.getOctopusContext().getContentHash()).isEqualTo(updatedHash); @@ -214,20 +204,20 @@ void whenRefreshReturnsNothing_AndSubsequentRefreshSucceeds_ContextIsUpdated() t static class ThrowsOnRefreshClient extends OctopusClient { static final String ERROR_MESSAGE = "Oops! Simulated refresh error"; - private final FeatureToggles initial; + private final EvaluationResponse initial; - ThrowsOnRefreshClient(FeatureToggles initial) { + ThrowsOnRefreshClient(EvaluationResponse initial) { super(null); this.initial = initial; } @Override - Boolean haveFeatureTogglesChanged(byte[] contentHash) { + Boolean haveFeatureFlagsChanged(byte[] contentHash) { throw new RuntimeException(ERROR_MESSAGE); } @Override - FeatureToggles getFeatureToggleEvaluationManifest() { + EvaluationResponse getServerSideEvaluations() { return initial; } } @@ -249,10 +239,7 @@ void whenAnExceptionIsThrownDuringRefresh_LogsErrorDetails() throws InterruptedE julLogger.setUseParentHandlers(false); // Initialize with a client that will throw on refresh - var client = new ThrowsOnRefreshClient(new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", true, "evaluation-key", Collections.emptyList(), 100)), - contentHash - )); + var client = new ThrowsOnRefreshClient(response(true, contentHash)); var provider = new OctopusContextProvider(configuration, client); provider.initialize(); @@ -261,7 +248,7 @@ void whenAnExceptionIsThrownDuringRefresh_LogsErrorDetails() throws InterruptedE Thread.sleep(500); assertThat(logRecords).anyMatch(r -> - r.getMessage().startsWith("Failed to retrieve updated feature manifest") + r.getMessage().startsWith("Failed to retrieve updated feature flag evaluations") && r.getThrown() != null && r.getThrown().getMessage().contains(ThrowsOnRefreshClient.ERROR_MESSAGE) ); diff --git a/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java b/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java index c2e00f3..d3437fa 100644 --- a/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java +++ b/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java @@ -1,291 +1,108 @@ package com.octopus.openfeature.provider; -import dev.openfeature.sdk.EvaluationContext; +import dev.openfeature.sdk.ErrorCode; import dev.openfeature.sdk.MutableContext; import dev.openfeature.sdk.exceptions.FlagNotFoundError; -import dev.openfeature.sdk.exceptions.ParseError; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; -import java.util.*; -import java.util.stream.Stream; +import java.util.List; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertThrows; - +import static org.assertj.core.api.Assertions.assertThatThrownBy; +/** + * Resolving a flag from an evaluation response. The evaluation itself — server-resolved values, + * client-side rules and malformed responses — is covered by {@link ServerSideEvaluationTests} and the + * suites around it; these cases cover finding the flag, and what happens when it is not there. + */ class OctopusContextTests { - @Test - void evaluatesToTrue_IfFeatureIsContainedWithinTheSet_AndFeatureIsEnabled() { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", true, "evaluation-key", Collections.emptyList(), 100)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - assertThat(subject.evaluate("test-feature", false, null).getValue()).isTrue(); - } - - @Test - void whenEvaluatedWithCasingDifferences_EvaluationIsInsensitiveToCase() { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", true, "evaluation-key", Collections.emptyList(), 100)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - assertThat(subject.evaluate("Test-Feature", false, null).getValue()).isTrue(); - } - - @Test - void evaluatesToFalse_IfFeatureIsContainedWithinTheSet_AndFeatureIsNotEnabled() { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", false, "evaluation-key", Collections.emptyList(), 100)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - assertThat(subject.evaluate("test-feature", false, null).getValue()).isFalse(); - } - - @Test - void givenAFlagKeyThatIsNotASlug_ThrowsFlagNotFound() { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("this-is-clearly-not-a-slug", true, "evaluation-key", Collections.emptyList(), 100)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - assertThrows(FlagNotFoundError.class, () -> subject.evaluate("This is clearly not a slug!", true, null)); - } - - @Test - void throwsFlagNotFound_IfFeatureIsNotContainedWithinSet() { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("testfeature", false, "evaluation-key", Collections.emptyList(), 100)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - assertThrows(FlagNotFoundError.class, () -> subject.evaluate("anotherfeature", true, null)); - } - - @Test - void whenAFeatureIsToggledOnForASpecificSegment_EvaluatesToTrueWhenSegmentIsSpecified() { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("testfeature", true, "evaluation-key", List.of(new Segment("license", "trial")), 100)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - - assertThat(subject.evaluate("testfeature", false, buildContext(List.of(Map.entry("license", "trial")))).getValue()).isTrue(); - assertThat(subject.evaluate("testfeature", false, buildContext(List.of(Map.entry("other", "segment")))).getValue()).isFalse(); - assertThat(subject.evaluate("testfeature", false, null).getValue()).isFalse(); + private static ServerSideEvaluation serverResolved(String slug, boolean value) { + return new ServerSideEvaluation(slug, value, "the server said so", null, null); } - @Test - void whenFeatureIsNotToggledOnForSpecificSegments_EvaluatesToTrueRegardlessOfSegmentSpecified() { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("testfeature", true, "evaluation-key", Collections.emptyList(), 100)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - - assertThat(subject.evaluate("testfeature", false, buildContext(List.of(Map.entry("license", "trial")))).getValue()).isTrue(); - assertThat(subject.evaluate("testfeature", false, null).getValue()).isTrue(); + private static OctopusContext contextWith(ServerSideEvaluation... evaluations) { + return new OctopusContext(new EvaluationResponse(List.of(evaluations), new byte[0])); } @Test - void whenAFeatureIsToggledOnForMultipleSegments_EvaluatesCorrectly() { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation( - "testfeature", true, "evaluation-key", - Arrays.asList(new Segment("license", "trial"), new Segment("region", "au"), new Segment("region", "us")), - 100 - )), - new byte[0] - ); - var subject = new OctopusContext(toggles); + void resolvesAFlagFromTheResponse() { + var result = contextWith(serverResolved("feature-a", true)).evaluate("feature-a", null); - // A matching context value is present for each toggled segment - assertThat(subject.evaluate("testfeature", false, buildContext(Arrays.asList(Map.entry("license", "trial"), Map.entry("region", "us")))).getValue()) - .isTrue(); - - // A context value is present for each toggled segment, but it is not toggled on for one of the supplied values - assertThat(subject.evaluate("testfeature", false, buildContext(Arrays.asList(Map.entry("license", "trial"), Map.entry("region", "eu")))).getValue()) - .isFalse(); - - // A matching context value is present for each toggled segment, and an additional segment is present in the provided context values - assertThat(subject.evaluate("testfeature", false, buildContext(Arrays.asList(Map.entry("license", "trial"), Map.entry("region", "us"), Map.entry("language", "english")))).getValue()) - .isTrue(); - - // A context value is present for only one of the two toggled segments - assertThat(subject.evaluate("testfeature", false, buildContext(List.of(Map.entry("license", "trial")))).getValue()) - .isFalse(); - - // No context values are present for the two toggled segments - assertThat(subject.evaluate("testfeature", true, buildContext(List.of(Map.entry("other", "segment")))).getValue()) - .isFalse(); - - // None specified - assertThat(subject.evaluate("testfeature", true, null).getValue()) - .isFalse(); + assertThat(result.getValue()).isTrue(); + assertThat(result.getReason()).isEqualTo("the server said so"); + assertThat(result.getErrorCode()).isNull(); } @Test - void whenAFeatureIsToggledOnForASpecificSegment_ToleratesNullValuesInContext() { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("testfeature", true, "evaluation-key", List.of(new Segment("license", "trial")), 100)), - new byte[0] - ); - var subject = new OctopusContext(toggles); + void resolvesADisabledFlagWithoutError() { + // Off, not defaulted: the flag resolved, it just resolved to false. + var result = contextWith(serverResolved("feature-a", false)).evaluate("feature-a", null); - // null value for the segment key does not match - var ctxNullLicense = new MutableContext(); - ctxNullLicense.add("license", (String) null); - assertThat(subject.evaluate("testfeature", false, ctxNullLicense).getValue()).isFalse(); - - assertThat(subject.evaluate("testfeature", false, buildContext(List.of(Map.entry("other", "segment")))).getValue()).isFalse(); - assertThat(subject.evaluate("testfeature", false, null).getValue()).isFalse(); + assertThat(result.getValue()).isFalse(); + assertThat(result.getErrorCode()).isNull(); } @Test - void whenTargetingKeyFallsWithinRolloutPercentage_AndFeatureIsNotToggledForSegments_ResolvesToTrue() { - // "evaluation-key:targeting-key" hashes to bucket 13, which is within the rollout of 13, so the feature is enabled - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", true, "evaluation-key", Collections.emptyList(), 13)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - assertThat(subject.evaluate("test-feature", false, buildContext(Collections.emptyList(), "targeting-key")).getValue()).isTrue(); - } + void matchesTheSlugWithoutRegardToCase() { + var context = contextWith(serverResolved("Feature-A", true)); - @Test - void whenTargetingKeyFallsOutsideRolloutPercentage_AndFeatureIsNotToggledForSegments_ResolvesToFalse() { - // "evaluation-key:targeting-key" hashes to bucket 13, which exceeds the rollout of 12, so the feature is disabled - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", true, "evaluation-key", Collections.emptyList(), 12)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - assertThat(subject.evaluate("test-feature", false, buildContext(Collections.emptyList(), "targeting-key")).getValue()).isFalse(); + assertThat(context.evaluate("feature-a", null).getValue()).isTrue(); + assertThat(context.evaluate("FEATURE-A", null).getValue()).isTrue(); } @Test - void whenTargetingKeyFallsWithinRolloutPercentage_AndSegmentMatchesRequiredSegments_EvaluatesToTrue() { - // "evaluation-key:targeting-key" hashes to bucket 13, which is within the rollout of 13, and the segment license=trial matches - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", true, "evaluation-key", List.of(new Segment("license", "trial")), 13)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - assertThat(subject.evaluate("test-feature", false, buildContext(List.of(Map.entry("license", "trial")), "targeting-key")).getValue()).isTrue(); - } + void picksTheRequestedFlagOutOfSeveral() { + var context = contextWith( + serverResolved("feature-a", true), + serverResolved("feature-b", false)); - @Test - void whenTargetingKeyFallsWithinRolloutPercentage_AndSegmentValueDoesNotMatchRequiredSegment_EvaluatesToFalse() { - // "evaluation-key:targeting-key" hashes to bucket 13, which is within the rollout of 99, but the required - // segment license=enterprise does not match the provided license=trial, so the feature is disabled - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", true, "evaluation-key", List.of(new Segment("license", "enterprise")), 99)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - assertThat(subject.evaluate("test-feature", false, buildContext(List.of(Map.entry("license", "trial")), "targeting-key")).getValue()).isFalse(); + assertThat(context.evaluate("feature-a", null).getValue()).isTrue(); + assertThat(context.evaluate("feature-b", null).getValue()).isFalse(); } @Test - void whenTargetingKeyFallsOutsideRolloutPercentage_AndSegmentValueDoesNotMatchRequiredSegment_EvaluatesToFalse() { - // "evaluation-key:targeting-key" hashes to bucket 13, which exceeds the rollout of 12, and the segment also does not match - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", true, "evaluation-key", List.of(new Segment("license", "enterprise")), 12)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - assertThat(subject.evaluate("test-feature", false, buildContext(List.of(Map.entry("license", "trial")), "targeting-key")).getValue()).isFalse(); + void anUnknownSlugThrowsFlagNotFound() { + assertThatThrownBy(() -> contextWith(serverResolved("feature-a", true)).evaluate("no-such-flag", null)) + .isInstanceOf(FlagNotFoundError.class) + .hasMessage("The slug provided did not match any of your Octopus Feature Flags. Please double check your slug and try again.") + .extracting(thrown -> ((FlagNotFoundError) thrown).getErrorCode()) + .isEqualTo(ErrorCode.FLAG_NOT_FOUND); } @Test - void whenNoTargetingKey_RolloutIsLessThanOneHundredPercent_ResolvesToFalse() { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", true, "evaluation-key", Collections.emptyList(), 99)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - assertThat(subject.evaluate("test-feature", false, buildContext(Collections.emptyList(), null)).getValue()).isFalse(); + void anEmptyResponseThrowsFlagNotFoundForEveryFlag() { + assertThatThrownBy(() -> OctopusContext.empty().evaluate("feature-a", null)) + .isInstanceOf(FlagNotFoundError.class); } @Test - void whenNoTargetingKey_RolloutIsEqualToOneHundredPercent_ResolvesToTrue() { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("test-feature", true, "evaluation-key", Collections.emptyList(), 100)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - assertThat(subject.evaluate("test-feature", false, buildContext(Collections.emptyList(), null)).getValue()).isTrue(); - } + void appliesTheClientSideRulesTheServerDeferred() { + var deferred = new ServerSideEvaluation("feature-a", null, null, "evaluation-key", + List.of(new ClientSideRule("Pro plans", + List.of(new ContextAttributeIsOneOfCondition("plan", List.of("pro")))))); + var context = contextWith(deferred); - @Test - void shouldThrowParseErrorWhenEnabledToggleIsMissingEvaluationKey() { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("feature-a", true, null, Collections.emptyList(), 100)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - var ex = assertThrows(ParseError.class, () -> subject.evaluate("feature-a", false, null)); - assertThat(ex.getMessage()).contains("feature-a"); + assertThat(context.evaluate("feature-a", new MutableContext().add("plan", "pro")).getValue()) + .as("the rule matches").isTrue(); + assertThat(context.evaluate("feature-a", new MutableContext().add("plan", "free")).getValue()) + .as("the rule does not match").isFalse(); } @Test - void shouldThrowParseErrorWhenEnabledToggleIsMissingSegments() { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("feature-b", true, "evaluation-key", null, 100)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - var ex = assertThrows(ParseError.class, () -> subject.evaluate("feature-b", false, null)); - assertThat(ex.getMessage()).contains("feature-b"); - } + void exposesTheContentHashOfTheResponseItHolds() { + byte[] contentHash = {0x01, 0x02}; - @Test - void shouldThrowParseErrorWhenEnabledToggleIsMissingClientRolloutPercentage() { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("feature-c", true, "evaluation-key", Collections.emptyList(), null)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - var ex = assertThrows(ParseError.class, () -> subject.evaluate("feature-c", false, null)); - assertThat(ex.getMessage()).contains("feature-c"); + assertThat(new OctopusContext(new EvaluationResponse(List.of(), contentHash)).getContentHash()) + .isEqualTo(contentHash); + assertThat(OctopusContext.empty().getContentHash()).isEmpty(); } @Test - void shouldThrowParseErrorWhenEnabledToggleIsMissingAllClientEvaluationFields() { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("feature-d", true, null, null, null)), - new byte[0] - ); - var subject = new OctopusContext(toggles); - var ex = assertThrows(ParseError.class, () -> subject.evaluate("feature-d", true, null)); - assertThat(ex.getMessage()).contains("feature-d"); - } + void findsAnEvaluationBySlugWithoutEvaluatingIt() { + var context = contextWith(serverResolved("feature-a", true)); - private EvaluationContext buildContext(List> entries) { - return buildContext(entries, null); + assertThat(context.findEvaluationBySlug("FEATURE-A")).isNotNull(); + assertThat(context.findEvaluationBySlug("no-such-flag")).isNull(); } - - private EvaluationContext buildContext(List> entries, String targetingKey) { - var context = new MutableContext(); - entries.forEach(entry -> context.add(entry.getKey(), entry.getValue())); - if (targetingKey != null) { - context.setTargetingKey(targetingKey); - } - return context; - } - - // The vectors live in RolloutVectors, shared with the v4 rollout so both implementations are held - // to the same expected buckets. - @ParameterizedTest(name = "[{index}] ({0}, {1}) -> {2}") - @MethodSource("com.octopus.openfeature.provider.RolloutVectors#cases") - void getNormalizedNumberMatchesExpectedValue(String evaluationKey, String targetingKey, int expected) { - assertThat(OctopusContext.getNormalizedNumber(evaluationKey, targetingKey)).isEqualTo(expected); - } - } diff --git a/src/test/java/com/octopus/openfeature/provider/OctopusProviderTests.java b/src/test/java/com/octopus/openfeature/provider/OctopusProviderTests.java index 333c22e..fea82c5 100644 --- a/src/test/java/com/octopus/openfeature/provider/OctopusProviderTests.java +++ b/src/test/java/com/octopus/openfeature/provider/OctopusProviderTests.java @@ -8,7 +8,6 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -import java.util.Collections; import java.util.List; import static org.assertj.core.api.Assertions.assertThat; @@ -19,11 +18,11 @@ class OctopusProviderTests { @BeforeEach void setup() throws Exception { - var toggles = new FeatureToggles( - List.of(new FeatureToggleEvaluation("feature-a", true, "key", Collections.emptyList(), 100)), + var response = new EvaluationResponse( + List.of(new ServerSideEvaluation("feature-a", true, "The flag is enabled for this environment.", null, null)), new byte[0] ); - var provider = new OctopusProvider(new FakeOctopusContextProvider(new OctopusContext(toggles))); + var provider = new OctopusProvider(new FakeOctopusContextProvider(new OctopusContext(response))); OpenFeatureAPI.getInstance().setProviderAndWait(provider); client = OpenFeatureAPI.getInstance().getClient(); } diff --git a/src/test/java/com/octopus/openfeature/provider/Server.java b/src/test/java/com/octopus/openfeature/provider/Server.java index ea2d431..d121d98 100644 --- a/src/test/java/com/octopus/openfeature/provider/Server.java +++ b/src/test/java/com/octopus/openfeature/provider/Server.java @@ -37,12 +37,12 @@ class Server { /** * Registers the given JSON as the response body for a new unique client token. * - * @param responseJson the JSON array that the toggle API would return + * @param responseJson the JSON array that the evaluations endpoint would return * @return the client identifier (Bearer token) to use in OctopusConfiguration */ String configure(String responseJson) { String token = UUID.randomUUID().toString(); - wireMock.stubFor(get(urlPathEqualTo("/api/toggles/evaluations/v3/")) + wireMock.stubFor(get(urlPathEqualTo("/api/feature-flags/evaluations/v4/")) .withHeader("Authorization", equalTo("Bearer " + token)) .willReturn(aResponse() .withStatus(200) diff --git a/src/test/java/com/octopus/openfeature/provider/SpecificationTests.java b/src/test/java/com/octopus/openfeature/provider/SpecificationTests.java index 1316a03..ba3f611 100644 --- a/src/test/java/com/octopus/openfeature/provider/SpecificationTests.java +++ b/src/test/java/com/octopus/openfeature/provider/SpecificationTests.java @@ -21,6 +21,7 @@ import java.net.URI; import java.nio.file.Files; import java.nio.file.Path; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -71,6 +72,14 @@ void evaluate(String fileName, String description, String responseJson, FixtureC assertThat(result.getErrorCode()) .as("[%s] %s → errorCode", fileName, description) .isEqualTo(mapErrorCode(testCase.expected.errorCode)); + + // Fixtures only state a reason where the specification pins one down, so an absent reason is not + // an assertion that the provider returned none. + if (testCase.expected.reason != null) { + assertThat(result.getReason()) + .as("[%s] %s → reason", fileName, description) + .isEqualTo(testCase.expected.reason); + } } static Stream fixtureTestCases() throws IOException { @@ -104,11 +113,16 @@ static Stream fixtureTestCases() throws IOException { } private static EvaluationContext buildContext(Map context) { - MutableContext ctx = new MutableContext(); - if (context != null) { - context.forEach(ctx::add); + if (context == null) { + return new MutableContext(); } - return ctx; + + // A null attribute is present in the context but holds no string, which is what a fixture means + // by a null value — not an attribute the caller left out. MutableContext.add() has no overload + // for a bare Value, so the attributes go in through the map constructor. + Map attributes = new LinkedHashMap<>(); + context.forEach((key, value) -> attributes.put(key, value == null ? new Value() : new Value(value))); + return new MutableContext(attributes); } private static ErrorCode mapErrorCode(String code) { @@ -155,6 +169,7 @@ static class FixtureConfiguration { static class FixtureExpected { public boolean value; + public String reason; public String errorCode; } From 3aa1911d064df4bdf4d3c180e92b9b958a6f621b Mon Sep 17 00:00:00 2001 From: Lourens de Jager Date: Wed, 12 Aug 2026 09:29:26 +1200 Subject: [PATCH 03/10] fix: keep a null in the response from failing every flag A null array entry made findEvaluationBySlug throw NullPointerException, which is not an OpenFeatureError, so every flag in the response fell back to the caller's default with ErrorCode.GENERAL until the content hash changed. A null entry carries no slug and can never be the flag being asked for, so it is now skipped. A null response body was worse: it produced a context whose content hash looked valid, so the check endpoint reported no change and the provider never recovered. It is now treated as a failed fetch, like every other unusable response. Co-Authored-By: Claude Opus 5 (1M context) --- .../openfeature/provider/OctopusClient.java | 7 ++++++ .../openfeature/provider/OctopusContext.java | 11 +++++++- .../provider/OctopusContextTests.java | 25 +++++++++++++++++++ 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java index badc8c3..87c37eb 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java @@ -80,6 +80,13 @@ EvaluationResponse getServerSideEvaluations() throws IOException, InterruptedExc return null; } var evaluations = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), new TypeReference>() {}); + if (evaluations == null) { + // Returning null leaves the cache on its previous context, or on the empty one, both of + // which keep refetching. Storing a response with a usable content hash would not: the check + // endpoint would report no change and the provider would never recover. + logger.log(System.Logger.Level.WARNING, String.format("Feature flag response content from %s was empty", evaluationsURI.toString())); + return null; + } return new EvaluationResponse(evaluations, Base64.getDecoder().decode(contentHashHeader.get())); } diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusContext.java b/src/main/java/com/octopus/openfeature/provider/OctopusContext.java index 065212a..aef71f0 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusContext.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusContext.java @@ -5,6 +5,7 @@ import dev.openfeature.sdk.exceptions.FlagNotFoundError; import java.util.List; +import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -32,7 +33,15 @@ byte[] getContentHash() { } ServerSideEvaluation findEvaluationBySlug(String slug) { - return evaluationResponse.getEvaluations().stream() + var evaluations = evaluationResponse.getEvaluations(); + if (evaluations == null) { + return null; + } + + // A null entry carries no slug, so it can never be the flag being asked for. Skipping it keeps a + // malformed entry from costing every other flag in the response. + return evaluations.stream() + .filter(Objects::nonNull) .filter(evaluation -> slug.equalsIgnoreCase(evaluation.getSlug())) .findFirst().orElse(null); } diff --git a/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java b/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java index d3437fa..2150171 100644 --- a/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java +++ b/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java @@ -1,5 +1,6 @@ package com.octopus.openfeature.provider; +import com.fasterxml.jackson.core.type.TypeReference; import dev.openfeature.sdk.ErrorCode; import dev.openfeature.sdk.MutableContext; import dev.openfeature.sdk.exceptions.FlagNotFoundError; @@ -98,6 +99,30 @@ void exposesTheContentHashOfTheResponseItHolds() { assertThat(OctopusContext.empty().getContentHash()).isEmpty(); } + @Test + void aNullEntryInTheResponseDoesNotCostTheOtherFlags() throws Exception { + // A malformed entry only fails its own flag, so a null alongside a well-formed flag must not take + // the lookup down with it. + List evaluations = OctopusObjectMapper.INSTANCE.readValue( + Contexts.json("[ null, { 'slug': 'feature-a', 'value': true, 'reason': 'Enabled.' } ]"), + new TypeReference>() {}); + var context = new OctopusContext(new EvaluationResponse(evaluations, new byte[0])); + + assertThat(context.evaluate("feature-a", null).getValue()).isTrue(); + assertThatThrownBy(() -> context.evaluate("no-such-flag", null)) + .as("the null entry is skipped rather than matched").isInstanceOf(FlagNotFoundError.class); + } + + @Test + void aResponseWithNoEvaluationsResolvesNothingRatherThanFailing() { + // Defence in depth: the client turns a null body into a failed fetch, so this shape should not + // reach the evaluator — but if it does, every flag is not-found rather than an NPE. + var context = new OctopusContext(new EvaluationResponse(null, new byte[0])); + + assertThatThrownBy(() -> context.evaluate("feature-a", null)).isInstanceOf(FlagNotFoundError.class); + assertThat(context.findEvaluationBySlug("feature-a")).isNull(); + } + @Test void findsAnEvaluationBySlugWithoutEvaluatingIt() { var context = contextWith(serverResolved("feature-a", true)); From 29901b3f04ed9ef8a37862c705cce8d59f107b2e Mon Sep 17 00:00:00 2001 From: Lourens de Jager Date: Wed, 12 Aug 2026 09:30:28 +1200 Subject: [PATCH 04/10] fix: read the evaluations one at a time Deserializing the array in one call aborted on the first wrongly-typed field anywhere in it, so "percentage": "lots" on one flag left every flag in the response falling back to the caller's default. Missing fields were already contained to the flag that carried them, because they are reported when that flag is evaluated; a field of the wrong type never gets that far, because it fails while the response is being read. An evaluation that cannot be read is left out and its slug logged, so it resolves as not found rather than being guessed at, and the rest of the response is unaffected. Note this is stricter than the .NET provider, which still deserializes the array in one call and so retains the original behaviour. Worth raising there. Co-Authored-By: Claude Opus 5 (1M context) --- .../openfeature/provider/OctopusClient.java | 39 +++++++++++++++++-- .../provider/OctopusClientTests.java | 32 +++++++++++++++ 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java index 87c37eb..ad67a82 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java @@ -1,6 +1,6 @@ package com.octopus.openfeature.provider; -import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.core.JsonProcessingException; import java.io.IOException; import java.net.MalformedURLException; @@ -79,17 +79,50 @@ EvaluationResponse getServerSideEvaluations() throws IOException, InterruptedExc logger.log(System.Logger.Level.WARNING, String.format("Feature flag response from %s did not contain expected ContentHash header", evaluationsURI.toString())); return null; } - var evaluations = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), new TypeReference>() {}); + var evaluations = readEvaluations(httpResponse.body(), evaluationsURI); if (evaluations == null) { // Returning null leaves the cache on its previous context, or on the empty one, both of // which keep refetching. Storing a response with a usable content hash would not: the check // endpoint would report no change and the provider would never recover. - logger.log(System.Logger.Level.WARNING, String.format("Feature flag response content from %s was empty", evaluationsURI.toString())); return null; } return new EvaluationResponse(evaluations, Base64.getDecoder().decode(contentHashHeader.get())); } + /** + * Reads the evaluations one at a time, so a flag whose payload cannot be read costs only itself. + * + *

Deserializing the array in one call would abort on the first wrongly-typed field anywhere in it + * — {@code "percentage": "lots"} on one flag would leave every flag in the response falling back to + * the caller's default. Missing fields are already reported per flag, when the flag is evaluated; + * this extends the same containment to fields of the wrong type, which cannot get that far because + * they fail while being read. + * + *

Returns null when the response itself is unusable, which the caller treats as a failed fetch. + */ + private List readEvaluations(String body, URI evaluationsURI) throws IOException { + var root = OctopusObjectMapper.INSTANCE.readTree(body); + if (root == null || !root.isArray()) { + logger.log(System.Logger.Level.WARNING, String.format("Feature flag response content from %s was not a list of evaluations", evaluationsURI.toString())); + return null; + } + + var evaluations = new ArrayList(); + for (var element : root) { + try { + evaluations.add(OctopusObjectMapper.INSTANCE.treeToValue(element, ServerSideEvaluation.class)); + } catch (JsonProcessingException e) { + // Left out of the response rather than guessed at: the flag resolves as not found, which + // the caller sees as their default value, and the slug is logged so it can be traced. + var slug = element.path("slug").isTextual() ? element.path("slug").asText() : ""; + logger.log(System.Logger.Level.WARNING, String.format( + "Could not read the evaluation for feature flag %s from %s, so it will resolve as not found: %s", + slug, evaluationsURI.toString(), e.getOriginalMessage())); + } + } + return evaluations; + } + String buildOctopusClientHeaderValue() { var clientHeaderValueBuilder = new StringBuilder(this.config.getProductMetadata().getName()); diff --git a/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java b/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java index 0a04696..a26ee60 100644 --- a/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java +++ b/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java @@ -120,6 +120,38 @@ void haveFeatureFlagsChanged_whenTheContentHashIsUnchanged_reportsNoChange() thr assertThat(haveChanged).isFalse(); } + @Test + void getServerSideEvaluations_whenOneEvaluationCannotBeRead_stillReturnsTheOthers() throws Exception { + // A wrongly-typed field fails while the response is being read, so it cannot be reported per flag + // at evaluation time the way a missing field is. Reading the evaluations one at a time keeps it + // from costing every other flag in the response. + wireMock.stubFor(get(anyUrl()).willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Type", "application/json") + .withHeader("ContentHash", CONTENT_HASH) + .withBody("[{\"slug\":\"broken-feature\",\"evaluationKey\":\"k\",\"rules\":[{\"name\":\"r\",\"conditions\":[{\"type\":\"percentage-by-context\",\"percentage\":\"lots\"}]}]}," + + "{\"slug\":\"well-formed-feature\",\"value\":true,\"reason\":\"The flag is enabled for this environment.\"}]"))); + + var response = clientForServer().getServerSideEvaluations(); + + assertThat(response).isNotNull(); + assertThat(response.getEvaluations()).singleElement() + .satisfies(evaluation -> assertThat(evaluation.getSlug()).isEqualTo("well-formed-feature")); + } + + @Test + void getServerSideEvaluations_whenTheBodyIsNotAListOfEvaluations_reportsAFailedFetch() throws Exception { + wireMock.stubFor(get(anyUrl()).willReturn(aResponse() + .withStatus(200) + .withHeader("Content-Type", "application/json") + .withHeader("ContentHash", CONTENT_HASH) + .withBody("null"))); + + // Null rather than an empty response: the cache keeps refetching instead of settling on a + // content hash that the check endpoint will report as unchanged forever. + assertThat(clientForServer().getServerSideEvaluations()).isNull(); + } + @Test void getServerSideEvaluations_requestsTheV4EvaluationsEndpoint() throws Exception { wireMock.stubFor(get(anyUrl()).willReturn(aResponse() From a932c9d1caf36e6442b54ee85b772dafb322eddf Mon Sep 17 00:00:00 2001 From: Lourens de Jager Date: Wed, 12 Aug 2026 09:33:16 +1200 Subject: [PATCH 05/10] fix: answer FLAG_NOT_FOUND for a null flag key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ordering the slug comparison so a null slug in the payload is tolerated moved the null-intolerance onto the caller's flag key, which is the one that actually arrives null — an unset configuration value, say. v3 answered FLAG_NOT_FOUND; a NullPointerException is not an OpenFeatureError, so the SDK reported ErrorCode.GENERAL with a raw exception message instead. Co-Authored-By: Claude Opus 5 (1M context) --- .../openfeature/provider/OctopusContext.java | 4 ++-- .../openfeature/provider/OctopusContextTests.java | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusContext.java b/src/main/java/com/octopus/openfeature/provider/OctopusContext.java index aef71f0..6b295b4 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusContext.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusContext.java @@ -34,7 +34,7 @@ byte[] getContentHash() { ServerSideEvaluation findEvaluationBySlug(String slug) { var evaluations = evaluationResponse.getEvaluations(); - if (evaluations == null) { + if (slug == null || evaluations == null) { return null; } @@ -52,7 +52,7 @@ ProviderEvaluation evaluate(String slug, EvaluationContext evaluationCo if (serverSideEvaluation == null) { // Warned once per slug: an unrecognised slug is usually a typo, which would otherwise log on // every evaluation of it. - if (warnedSlugs.add(slug.toLowerCase())) { + if (slug != null && warnedSlugs.add(slug.toLowerCase())) { logger.log(System.Logger.Level.WARNING, String.format( "The slug %s did not match any of your Octopus Feature Flags. Please double check your slug and try again.", slug)); diff --git a/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java b/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java index 2150171..85509c9 100644 --- a/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java +++ b/src/test/java/com/octopus/openfeature/provider/OctopusContextTests.java @@ -71,6 +71,19 @@ void anUnknownSlugThrowsFlagNotFound() { .isEqualTo(ErrorCode.FLAG_NOT_FOUND); } + @Test + void aNullFlagKeyThrowsFlagNotFoundRatherThanFailing() { + // The slug comes from the caller, so an unset config value arrives here as null. v3 answered + // FLAG_NOT_FOUND; anything else surfaces as ErrorCode.GENERAL with a raw exception message. + var context = contextWith(serverResolved("feature-a", true)); + + assertThatThrownBy(() -> context.evaluate(null, null)) + .isInstanceOf(FlagNotFoundError.class) + .extracting(thrown -> ((FlagNotFoundError) thrown).getErrorCode()) + .isEqualTo(ErrorCode.FLAG_NOT_FOUND); + assertThat(context.findEvaluationBySlug(null)).isNull(); + } + @Test void anEmptyResponseThrowsFlagNotFoundForEveryFlag() { assertThatThrownBy(() -> OctopusContext.empty().evaluate("feature-a", null)) From 5de1f475cd3539c1585ca4967eb9e427c073f5c6 Mon Sep 17 00:00:00 2001 From: Lourens de Jager Date: Wed, 12 Aug 2026 09:34:22 +1200 Subject: [PATCH 06/10] fix: log the slug when an evaluation cannot be read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ParseError messages are shared verbatim with the other provider libraries, so they name the problem but not the flag — v3's message did name it, and the tests deleted with the v3 path asserted exactly that. Rather than diverge from the shared messages, the slug is logged beside the problem, which is what makes a malformed response traceable when a server rollout affects several flags at once. Co-Authored-By: Claude Opus 5 (1M context) --- .../octopus/openfeature/provider/OctopusContext.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusContext.java b/src/main/java/com/octopus/openfeature/provider/OctopusContext.java index 6b295b4..f544831 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusContext.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusContext.java @@ -3,6 +3,7 @@ import dev.openfeature.sdk.EvaluationContext; import dev.openfeature.sdk.ProviderEvaluation; import dev.openfeature.sdk.exceptions.FlagNotFoundError; +import dev.openfeature.sdk.exceptions.ParseError; import java.util.List; import java.util.Objects; @@ -62,6 +63,15 @@ ProviderEvaluation evaluate(String slug, EvaluationContext evaluationCo "The slug provided did not match any of your Octopus Feature Flags. Please double check your slug and try again."); } - return serverSideEvaluation.evaluate(evaluationContext); + try { + return serverSideEvaluation.evaluate(evaluationContext); + } catch (ParseError e) { + // The message is shared verbatim with the other provider libraries, so it names the problem + // but not the flag. Logging the slug beside it is what makes a malformed response traceable + // when a rollout affects several flags at once. + logger.log(System.Logger.Level.WARNING, String.format( + "Could not evaluate feature flag %s: %s", slug, e.getMessage())); + throw e; + } } } From dc303860b88c3a247d505cd21bbd72d804e5ab25 Mon Sep 17 00:00:00 2001 From: Lourens de Jager Date: Wed, 12 Aug 2026 09:34:54 +1200 Subject: [PATCH 07/10] fix: warn once per unrecognised slug for the life of the provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The set of already-warned slugs lived on the evaluator, which is replaced whenever the response changes, so it forgot every slug about once per cache interval and warned repeatedly for a single typo — the opposite of what it was for. It now belongs to the cache, which outlives the evaluators it builds. It is also bounded. Slugs come from the caller, who may derive them (one per tenant, say), and an unbounded set would have held one string per distinct unknown slug for the life of the process. Keys are lowercased with Locale.ROOT rather than the default locale: under a Turkish locale "MY-FLAG-I" lowercases to a dotless "my-flag-ı", which would not match "my-flag-i" and so would warn twice for one slug. Co-Authored-By: Claude Opus 5 (1M context) --- .../openfeature/provider/OctopusContext.java | 19 ++++-- .../provider/OctopusContextProvider.java | 11 ++-- .../openfeature/provider/UnknownSlugs.java | 40 ++++++++++++ .../provider/UnknownSlugsTests.java | 65 +++++++++++++++++++ 4 files changed, 124 insertions(+), 11 deletions(-) create mode 100644 src/main/java/com/octopus/openfeature/provider/UnknownSlugs.java create mode 100644 src/test/java/com/octopus/openfeature/provider/UnknownSlugsTests.java diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusContext.java b/src/main/java/com/octopus/openfeature/provider/OctopusContext.java index f544831..4d1a0c1 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusContext.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusContext.java @@ -7,8 +7,6 @@ import java.util.List; import java.util.Objects; -import java.util.Set; -import java.util.concurrent.ConcurrentHashMap; /** * Holds one evaluation response and resolves a flag from it, applying any client-side rules the @@ -19,16 +17,25 @@ class OctopusContext { private static final System.Logger logger = System.getLogger(OctopusContext.class.getName()); private final EvaluationResponse evaluationResponse; - private final Set warnedSlugs = ConcurrentHashMap.newKeySet(); + private final UnknownSlugs unknownSlugs; OctopusContext(EvaluationResponse evaluationResponse) { + this(evaluationResponse, new UnknownSlugs()); + } + + OctopusContext(EvaluationResponse evaluationResponse, UnknownSlugs unknownSlugs) { this.evaluationResponse = evaluationResponse; + this.unknownSlugs = unknownSlugs; } static OctopusContext empty() { return new OctopusContext(new EvaluationResponse(List.of(), new byte[0])); } + static OctopusContext empty(UnknownSlugs unknownSlugs) { + return new OctopusContext(new EvaluationResponse(List.of(), new byte[0]), unknownSlugs); + } + byte[] getContentHash() { return evaluationResponse.getContentHash(); } @@ -51,9 +58,7 @@ ProviderEvaluation evaluate(String slug, EvaluationContext evaluationCo var serverSideEvaluation = findEvaluationBySlug(slug); if (serverSideEvaluation == null) { - // Warned once per slug: an unrecognised slug is usually a typo, which would otherwise log on - // every evaluation of it. - if (slug != null && warnedSlugs.add(slug.toLowerCase())) { + if (unknownSlugs.shouldWarnAbout(slug)) { logger.log(System.Logger.Level.WARNING, String.format( "The slug %s did not match any of your Octopus Feature Flags. Please double check your slug and try again.", slug)); @@ -68,7 +73,7 @@ ProviderEvaluation evaluate(String slug, EvaluationContext evaluationCo } catch (ParseError e) { // The message is shared verbatim with the other provider libraries, so it names the problem // but not the flag. Logging the slug beside it is what makes a malformed response traceable - // when a rollout affects several flags at once. + // when several flags are affected at once. logger.log(System.Logger.Level.WARNING, String.format( "Could not evaluate feature flag %s: %s", slug, e.getMessage())); throw e; diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java b/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java index aba4004..172c0fc 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusContextProvider.java @@ -4,7 +4,8 @@ class OctopusContextProvider { private final OctopusConfiguration config; private final OctopusClient client; private boolean initialized = false; - private OctopusContext currentContext = OctopusContext.empty(); + private final UnknownSlugs unknownSlugs = new UnknownSlugs(); + private OctopusContext currentContext = OctopusContext.empty(unknownSlugs); private Thread refreshThread; private static final System.Logger logger = System.getLogger(OctopusClient.class.getName()); @@ -22,10 +23,12 @@ void initialize() { try { var evaluationResponse = client.getServerSideEvaluations(); - currentContext = evaluationResponse == null ? OctopusContext.empty() : new OctopusContext(evaluationResponse); + currentContext = evaluationResponse == null + ? OctopusContext.empty(unknownSlugs) + : new OctopusContext(evaluationResponse, unknownSlugs); } catch (Exception e) { logger.log(System.Logger.Level.ERROR, "Failed to retrieve feature flag evaluations during initialization. Falling back to empty context, defaults will be used during evaluation.", e); - currentContext = OctopusContext.empty(); + currentContext = OctopusContext.empty(unknownSlugs); } // run the refresh loop in the background @@ -48,7 +51,7 @@ void refresh() { if (client.haveFeatureFlagsChanged(currentContext.getContentHash())) { var evaluationResponse = client.getServerSideEvaluations(); if (evaluationResponse != null) { - currentContext = new OctopusContext(evaluationResponse); + currentContext = new OctopusContext(evaluationResponse, unknownSlugs); } else { logger.log(System.Logger.Level.ERROR, "Failed to retrieve updated feature flag evaluations. Retaining existing context which may be stale."); } diff --git a/src/main/java/com/octopus/openfeature/provider/UnknownSlugs.java b/src/main/java/com/octopus/openfeature/provider/UnknownSlugs.java new file mode 100644 index 0000000..8974aff --- /dev/null +++ b/src/main/java/com/octopus/openfeature/provider/UnknownSlugs.java @@ -0,0 +1,40 @@ +package com.octopus.openfeature.provider; + +import java.util.Locale; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; + +/** + * Remembers the slugs already warned about, so an unrecognised slug is reported once rather than on + * every evaluation of it. + * + *

Owned by the cache rather than by an evaluator, because an evaluator is replaced whenever the + * response changes — putting this on one would forget every slug about every cache interval, which is + * the opposite of the intent. Bounded because slugs come from the caller, who may derive them (say, + * one per tenant) and would otherwise grow this without limit for the life of the process. + */ +class UnknownSlugs { + + /** + * Enough that a real application's flags all fit, small enough to stay negligible. Once full, + * warnings simply stop being deduplicated rather than the set growing. + */ + private static final int LIMIT = 1_000; + + private final Set warned = ConcurrentHashMap.newKeySet(); + + /** + * Whether this slug should be warned about now, recording it so the next call returns false. + */ + boolean shouldWarnAbout(String slug) { + // Locale.ROOT, not the default locale: under a Turkish locale "MY-FLAG-I" lowercases to a dotless + // "my-flag-ı", which would not match "my-flag-i" and so would warn twice for one slug. + var key = slug == null ? "" : slug.toLowerCase(Locale.ROOT); + + if (warned.size() >= LIMIT) { + return !warned.contains(key); + } + + return warned.add(key); + } +} diff --git a/src/test/java/com/octopus/openfeature/provider/UnknownSlugsTests.java b/src/test/java/com/octopus/openfeature/provider/UnknownSlugsTests.java new file mode 100644 index 0000000..1e04b50 --- /dev/null +++ b/src/test/java/com/octopus/openfeature/provider/UnknownSlugsTests.java @@ -0,0 +1,65 @@ +package com.octopus.openfeature.provider; + +import org.junit.jupiter.api.Test; + +import java.util.Locale; + +import static org.assertj.core.api.Assertions.assertThat; + +class UnknownSlugsTests { + + @Test + void warnsOncePerSlug() { + var unknownSlugs = new UnknownSlugs(); + + assertThat(unknownSlugs.shouldWarnAbout("feature-a")).isTrue(); + assertThat(unknownSlugs.shouldWarnAbout("feature-a")).isFalse(); + assertThat(unknownSlugs.shouldWarnAbout("feature-b")).as("a different slug still warns").isTrue(); + } + + @Test + void treatsCasingDifferencesAsTheSameSlug() { + var unknownSlugs = new UnknownSlugs(); + + assertThat(unknownSlugs.shouldWarnAbout("Feature-A")).isTrue(); + assertThat(unknownSlugs.shouldWarnAbout("feature-a")).isFalse(); + } + + @Test + void treatsCasingDifferencesAsTheSameSlugWhateverTheDefaultLocale() { + // Under a Turkish default locale, "MY-FLAG-I".toLowerCase() is a dotless "my-flag-ı", which would + // not match "my-flag-i" and so would warn twice for one slug. + var previous = Locale.getDefault(); + try { + Locale.setDefault(new Locale("tr", "TR")); + var unknownSlugs = new UnknownSlugs(); + + assertThat(unknownSlugs.shouldWarnAbout("MY-FLAG-I")).isTrue(); + assertThat(unknownSlugs.shouldWarnAbout("my-flag-i")).isFalse(); + } finally { + Locale.setDefault(previous); + } + } + + @Test + void toleratesANullSlug() { + var unknownSlugs = new UnknownSlugs(); + + assertThat(unknownSlugs.shouldWarnAbout(null)).isTrue(); + assertThat(unknownSlugs.shouldWarnAbout(null)).isFalse(); + } + + @Test + void stopsGrowingOnceFull() { + var unknownSlugs = new UnknownSlugs(); + for (int i = 0; i < 1_000; i++) { + unknownSlugs.shouldWarnAbout("feature-" + i); + } + + // Full: a slug already seen still does not warn, and a new one warns every time rather than being + // remembered, so the set cannot grow without bound. + assertThat(unknownSlugs.shouldWarnAbout("feature-0")).isFalse(); + assertThat(unknownSlugs.shouldWarnAbout("feature-new")).isTrue(); + assertThat(unknownSlugs.shouldWarnAbout("feature-new")).isTrue(); + } +} From 61e9a5bb585803ae829af1d5db0df0ec3a1f759a Mon Sep 17 00:00:00 2001 From: Lourens de Jager Date: Wed, 12 Aug 2026 09:35:09 +1200 Subject: [PATCH 08/10] docs: record that the reason strings are a contract change The reasons are no longer OpenFeature's Reason values, which v3 returned; a caller branching on Reason.TARGETING_MATCH or keying metrics on the reason sees free-text sentences instead, with a cardinality that grows with the number of rule names. Nothing about that surfaces as a compile error, so it belongs in the breaking-change note rather than only in the code. BREAKING CHANGE: evaluation reasons are no longer OpenFeature's Reason values. A server-resolved flag reports the service's own reason, a client-side match reports "Matched rule ''." and a flag that matched no rule reports "Did not match any rules." Co-Authored-By: Claude Opus 5 (1M context) --- .../com/octopus/openfeature/provider/EvaluationReasons.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/com/octopus/openfeature/provider/EvaluationReasons.java b/src/main/java/com/octopus/openfeature/provider/EvaluationReasons.java index fd65101..6dadc74 100644 --- a/src/main/java/com/octopus/openfeature/provider/EvaluationReasons.java +++ b/src/main/java/com/octopus/openfeature/provider/EvaluationReasons.java @@ -3,6 +3,11 @@ /** * Reasons returned alongside a client-side evaluation. Both match the strings the Feature Flags service * produces server-side, so a flag reads the same whichever side resolved it. + * + *

These are deliberately not OpenFeature's {@link dev.openfeature.sdk.Reason} values, which is a + * change in contract from v3: a caller that branched on {@code Reason.TARGETING_MATCH}, or that used the + * reason as a metrics dimension, will see these sentences instead — and the set of them now grows with + * the number of rule names. */ final class EvaluationReasons { From 5d7f28fed1390a32453fe1ae30b883562ae2f2c9 Mon Sep 17 00:00:00 2001 From: Lourens de Jager Date: Wed, 12 Aug 2026 09:59:14 +1200 Subject: [PATCH 09/10] fix: report an unreadable flag as a parse error, not as missing An evaluation that could not be read was left out of the response, so the flag resolved as not found. Every other malformed shape in the shared specification resolves with PARSE_ERROR against the caller's default, and a flag the server did send should not look like one it did not, so the flag is now kept and reports a parse error of its own. This is also what a specification fixture for a wrongly-typed field would expect, if one is added. Co-Authored-By: Claude Opus 5 (1M context) --- .../openfeature/provider/OctopusClient.java | 17 ++++++++--- .../provider/ServerSideEvaluation.java | 30 +++++++++++++++++++ .../provider/OctopusClientTests.java | 12 ++++++-- 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java index ad67a82..758ae34 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java @@ -112,12 +112,21 @@ private List readEvaluations(String body, URI evaluationsU try { evaluations.add(OctopusObjectMapper.INSTANCE.treeToValue(element, ServerSideEvaluation.class)); } catch (JsonProcessingException e) { - // Left out of the response rather than guessed at: the flag resolves as not found, which - // the caller sees as their default value, and the slug is logged so it can be traced. - var slug = element.path("slug").isTextual() ? element.path("slug").asText() : ""; + // Kept, not dropped: the flag reports a parse error of its own, as every other malformed + // shape does, rather than looking like a flag the server never sent. Without a slug there + // is nothing to report it against, so it can only be left out. + if (!element.path("slug").isTextual()) { + logger.log(System.Logger.Level.WARNING, String.format( + "Could not read an unnamed evaluation from %s, so it has been left out: %s", + evaluationsURI.toString(), e.getOriginalMessage())); + continue; + } + + var slug = element.path("slug").asText(); logger.log(System.Logger.Level.WARNING, String.format( - "Could not read the evaluation for feature flag %s from %s, so it will resolve as not found: %s", + "Could not read the evaluation for feature flag %s from %s: %s", slug, evaluationsURI.toString(), e.getOriginalMessage())); + evaluations.add(ServerSideEvaluation.unreadable(slug, "The flag could not be read.")); } } return evaluations; diff --git a/src/main/java/com/octopus/openfeature/provider/ServerSideEvaluation.java b/src/main/java/com/octopus/openfeature/provider/ServerSideEvaluation.java index d6de652..19c7043 100644 --- a/src/main/java/com/octopus/openfeature/provider/ServerSideEvaluation.java +++ b/src/main/java/com/octopus/openfeature/provider/ServerSideEvaluation.java @@ -21,6 +21,11 @@ final class ServerSideEvaluation { private final String evaluationKey; private final List rules; + /** + * Why this flag's entry could not be read, or null if it was read normally. + */ + private final String unreadableBecause; + @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) ServerSideEvaluation( @JsonProperty("slug") String slug, @@ -34,6 +39,27 @@ final class ServerSideEvaluation { this.reason = reason; this.evaluationKey = evaluationKey; this.rules = ListUtils.copyOrNull(rules); + this.unreadableBecause = null; + } + + private ServerSideEvaluation(String slug, String unreadableBecause) { + this.slug = slug; + this.value = null; + this.reason = null; + this.evaluationKey = null; + this.rules = null; + this.unreadableBecause = unreadableBecause; + } + + /** + * A flag whose entry could not be read at all — a field of the wrong type, say, which fails while + * the response is being parsed rather than when the flag is evaluated. + * + *

Kept in the response rather than dropped so that the flag reports a parse error of its own, + * as every other malformed shape does, instead of looking like a flag the server never sent. + */ + static ServerSideEvaluation unreadable(String slug, String problem) { + return new ServerSideEvaluation(slug, problem); } public String getSlug() { @@ -64,6 +90,10 @@ public Optional> getRules() { * the caller's default value. */ ProviderEvaluation evaluate(EvaluationContext context) { + if (unreadableBecause != null) { + throw new ParseError(unreadableBecause); + } + if (value != null) { if (reason == null) { throw new ParseError("The flag has a value but has no reason."); diff --git a/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java b/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java index a26ee60..c91f306 100644 --- a/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java +++ b/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java @@ -1,6 +1,7 @@ package com.octopus.openfeature.provider; import com.github.tomakehurst.wiremock.WireMockServer; +import dev.openfeature.sdk.exceptions.ParseError; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -16,6 +17,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.get; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; class OctopusClientTests { @@ -135,8 +137,14 @@ void getServerSideEvaluations_whenOneEvaluationCannotBeRead_stillReturnsTheOther var response = clientForServer().getServerSideEvaluations(); assertThat(response).isNotNull(); - assertThat(response.getEvaluations()).singleElement() - .satisfies(evaluation -> assertThat(evaluation.getSlug()).isEqualTo("well-formed-feature")); + assertThat(response.getEvaluations()).hasSize(2); + + var evaluations = new OctopusContext(response); + assertThat(evaluations.evaluate("well-formed-feature", null).getValue()) + .as("the well-formed flag is unaffected").isTrue(); + assertThatThrownBy(() -> evaluations.evaluate("broken-feature", null)) + .as("the unreadable flag reports a parse error of its own, as other malformed shapes do") + .isInstanceOf(ParseError.class); } @Test From 9ad055be33de2b07cfcbafa69dea3cf7b7e679f4 Mon Sep 17 00:00:00 2001 From: Lourens de Jager Date: Wed, 12 Aug 2026 10:49:55 +1200 Subject: [PATCH 10/10] revert: read the evaluations in one call again, as .NET does Reverts the per-entry parsing so Java and .NET behave identically: a field of the wrong type anywhere in the response fails the whole manifest in both. That is a real defect in both libraries, tracked as BMBB-788 and deferred rather than fixed here, so this branch is not the one place it gets fixed. The null-body guard stays, along with its test: that was a separate finding and is not part of what BMBB-788 covers. Reverts 29901b3 and 5d7f28f. The fix is preserved on lourens/bmbb-788. Co-Authored-By: Claude Opus 5 (1M context) --- .../openfeature/provider/OctopusClient.java | 48 ++----------------- .../provider/ServerSideEvaluation.java | 30 ------------ .../provider/OctopusClientTests.java | 33 ++----------- 3 files changed, 6 insertions(+), 105 deletions(-) diff --git a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java index 758ae34..87c37eb 100644 --- a/src/main/java/com/octopus/openfeature/provider/OctopusClient.java +++ b/src/main/java/com/octopus/openfeature/provider/OctopusClient.java @@ -1,6 +1,6 @@ package com.octopus.openfeature.provider; -import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.core.type.TypeReference; import java.io.IOException; import java.net.MalformedURLException; @@ -79,59 +79,17 @@ EvaluationResponse getServerSideEvaluations() throws IOException, InterruptedExc logger.log(System.Logger.Level.WARNING, String.format("Feature flag response from %s did not contain expected ContentHash header", evaluationsURI.toString())); return null; } - var evaluations = readEvaluations(httpResponse.body(), evaluationsURI); + var evaluations = OctopusObjectMapper.INSTANCE.readValue(httpResponse.body(), new TypeReference>() {}); if (evaluations == null) { // Returning null leaves the cache on its previous context, or on the empty one, both of // which keep refetching. Storing a response with a usable content hash would not: the check // endpoint would report no change and the provider would never recover. + logger.log(System.Logger.Level.WARNING, String.format("Feature flag response content from %s was empty", evaluationsURI.toString())); return null; } return new EvaluationResponse(evaluations, Base64.getDecoder().decode(contentHashHeader.get())); } - /** - * Reads the evaluations one at a time, so a flag whose payload cannot be read costs only itself. - * - *

Deserializing the array in one call would abort on the first wrongly-typed field anywhere in it - * — {@code "percentage": "lots"} on one flag would leave every flag in the response falling back to - * the caller's default. Missing fields are already reported per flag, when the flag is evaluated; - * this extends the same containment to fields of the wrong type, which cannot get that far because - * they fail while being read. - * - *

Returns null when the response itself is unusable, which the caller treats as a failed fetch. - */ - private List readEvaluations(String body, URI evaluationsURI) throws IOException { - var root = OctopusObjectMapper.INSTANCE.readTree(body); - if (root == null || !root.isArray()) { - logger.log(System.Logger.Level.WARNING, String.format("Feature flag response content from %s was not a list of evaluations", evaluationsURI.toString())); - return null; - } - - var evaluations = new ArrayList(); - for (var element : root) { - try { - evaluations.add(OctopusObjectMapper.INSTANCE.treeToValue(element, ServerSideEvaluation.class)); - } catch (JsonProcessingException e) { - // Kept, not dropped: the flag reports a parse error of its own, as every other malformed - // shape does, rather than looking like a flag the server never sent. Without a slug there - // is nothing to report it against, so it can only be left out. - if (!element.path("slug").isTextual()) { - logger.log(System.Logger.Level.WARNING, String.format( - "Could not read an unnamed evaluation from %s, so it has been left out: %s", - evaluationsURI.toString(), e.getOriginalMessage())); - continue; - } - - var slug = element.path("slug").asText(); - logger.log(System.Logger.Level.WARNING, String.format( - "Could not read the evaluation for feature flag %s from %s: %s", - slug, evaluationsURI.toString(), e.getOriginalMessage())); - evaluations.add(ServerSideEvaluation.unreadable(slug, "The flag could not be read.")); - } - } - return evaluations; - } - String buildOctopusClientHeaderValue() { var clientHeaderValueBuilder = new StringBuilder(this.config.getProductMetadata().getName()); diff --git a/src/main/java/com/octopus/openfeature/provider/ServerSideEvaluation.java b/src/main/java/com/octopus/openfeature/provider/ServerSideEvaluation.java index 19c7043..d6de652 100644 --- a/src/main/java/com/octopus/openfeature/provider/ServerSideEvaluation.java +++ b/src/main/java/com/octopus/openfeature/provider/ServerSideEvaluation.java @@ -21,11 +21,6 @@ final class ServerSideEvaluation { private final String evaluationKey; private final List rules; - /** - * Why this flag's entry could not be read, or null if it was read normally. - */ - private final String unreadableBecause; - @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) ServerSideEvaluation( @JsonProperty("slug") String slug, @@ -39,27 +34,6 @@ final class ServerSideEvaluation { this.reason = reason; this.evaluationKey = evaluationKey; this.rules = ListUtils.copyOrNull(rules); - this.unreadableBecause = null; - } - - private ServerSideEvaluation(String slug, String unreadableBecause) { - this.slug = slug; - this.value = null; - this.reason = null; - this.evaluationKey = null; - this.rules = null; - this.unreadableBecause = unreadableBecause; - } - - /** - * A flag whose entry could not be read at all — a field of the wrong type, say, which fails while - * the response is being parsed rather than when the flag is evaluated. - * - *

Kept in the response rather than dropped so that the flag reports a parse error of its own, - * as every other malformed shape does, instead of looking like a flag the server never sent. - */ - static ServerSideEvaluation unreadable(String slug, String problem) { - return new ServerSideEvaluation(slug, problem); } public String getSlug() { @@ -90,10 +64,6 @@ public Optional> getRules() { * the caller's default value. */ ProviderEvaluation evaluate(EvaluationContext context) { - if (unreadableBecause != null) { - throw new ParseError(unreadableBecause); - } - if (value != null) { if (reason == null) { throw new ParseError("The flag has a value but has no reason."); diff --git a/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java b/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java index c91f306..f99bba5 100644 --- a/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java +++ b/src/test/java/com/octopus/openfeature/provider/OctopusClientTests.java @@ -1,7 +1,6 @@ package com.octopus.openfeature.provider; import com.github.tomakehurst.wiremock.WireMockServer; -import dev.openfeature.sdk.exceptions.ParseError; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -17,7 +16,6 @@ import static com.github.tomakehurst.wiremock.client.WireMock.get; import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.wireMockConfig; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.assertThatThrownBy; class OctopusClientTests { @@ -123,40 +121,15 @@ void haveFeatureFlagsChanged_whenTheContentHashIsUnchanged_reportsNoChange() thr } @Test - void getServerSideEvaluations_whenOneEvaluationCannotBeRead_stillReturnsTheOthers() throws Exception { - // A wrongly-typed field fails while the response is being read, so it cannot be reported per flag - // at evaluation time the way a missing field is. Reading the evaluations one at a time keeps it - // from costing every other flag in the response. - wireMock.stubFor(get(anyUrl()).willReturn(aResponse() - .withStatus(200) - .withHeader("Content-Type", "application/json") - .withHeader("ContentHash", CONTENT_HASH) - .withBody("[{\"slug\":\"broken-feature\",\"evaluationKey\":\"k\",\"rules\":[{\"name\":\"r\",\"conditions\":[{\"type\":\"percentage-by-context\",\"percentage\":\"lots\"}]}]}," - + "{\"slug\":\"well-formed-feature\",\"value\":true,\"reason\":\"The flag is enabled for this environment.\"}]"))); - - var response = clientForServer().getServerSideEvaluations(); - - assertThat(response).isNotNull(); - assertThat(response.getEvaluations()).hasSize(2); - - var evaluations = new OctopusContext(response); - assertThat(evaluations.evaluate("well-formed-feature", null).getValue()) - .as("the well-formed flag is unaffected").isTrue(); - assertThatThrownBy(() -> evaluations.evaluate("broken-feature", null)) - .as("the unreadable flag reports a parse error of its own, as other malformed shapes do") - .isInstanceOf(ParseError.class); - } - - @Test - void getServerSideEvaluations_whenTheBodyIsNotAListOfEvaluations_reportsAFailedFetch() throws Exception { + void getServerSideEvaluations_whenTheBodyIsEmpty_reportsAFailedFetch() throws Exception { wireMock.stubFor(get(anyUrl()).willReturn(aResponse() .withStatus(200) .withHeader("Content-Type", "application/json") .withHeader("ContentHash", CONTENT_HASH) .withBody("null"))); - // Null rather than an empty response: the cache keeps refetching instead of settling on a - // content hash that the check endpoint will report as unchanged forever. + // Null rather than an empty response: the cache keeps refetching instead of settling on a content + // hash that the check endpoint will report as unchanged forever. assertThat(clientForServer().getServerSideEvaluations()).isNull(); }