Skip to content

feat: implement v4 client-side evaluation - #51

Merged
lourens-octopus merged 11 commits into
mainfrom
lourens/bmbb-751
Aug 11, 2026
Merged

feat: implement v4 client-side evaluation#51
lourens-octopus merged 11 commits into
mainfrom
lourens/bmbb-751

Conversation

@lourens-octopus

@lourens-octopus lourens-octopus commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

Resolves BMBB-751. Java counterpart of openfeature-provider-dotnet#93, brought up to date with .NET's current v4 code.

What

Gives the v4 types behaviour: a flag is enabled when any rule matches, and a rule matches when all of its conditions do. ServerSideEvaluation.evaluate(EvaluationContext) returns a ProviderEvaluation<Boolean>, resolving either the server's value or the client-side rules. New alongside it: ClientSideEvaluationContext, ContextAttributes (the lookup shared by both attribute conditions) and EvaluationReasons.

Not wired into the provider — OctopusProvider still serves v3.

Validation happens at evaluation, not parsing

Reporting a specific problem per malformed shape requires the payload to deserialize first, so no property is required at parse time — matching .NET, which enforces no JSON member at all — and lists keep their null elements (WireLists.copyOrNull, not List.copyOf). A malformed flag then costs only itself rather than every other flag in the response.

Every ParseError message was diffed against .NET's and matches exactly, including the reason requirement .NET added after #93 ("The flag has a value but has no reason."). An unrecognised condition type remains the one tolerated case: it fails its own rule and nothing else, so a newer server's capability degrades instead of erroring.

Decisions

  • Nothing is added to the public API. PercentageRollout is package-private in v4, where .NET keeps it. Package access is not hierarchical, so v3 cannot call it and keeps its own copy of the hash — OctopusContext is untouched by this PR. The duplication is deliberate: a public type would be something consumers of 1.0.0 could bind to and the wiring-up ticket could not remove without a breaking change. Both copies run against one shared set of vectors (RolloutVectors), so they cannot drift apart unnoticed; v3's copy goes when v3 does.
  • A custom deserializer rather than @JsonTypeInfo. Jackson coerces "type": 123 to "123", losing the malformed-versus-unrecognised distinction .NET makes, and no coercion setting reaches the type id. A parameterized test pins every unusable shape — number, boolean, null, object, array.
  • Conditions stay in …provider.v4, not a conditions sub-package as .NET has them: ClientSideRule could not see them there without making every condition public.

Two differences from .NET remain, both forced by the Java SDK rather than chosen: ProviderEvaluation has no flag-key field, so the resolved slug does not reach the caller; and serialization falls back to Jackson's defaults where .NET's converter throws — immaterial, as the provider only ever reads.

Deferred to the wiring-up ticket

Mirroring .NET #96: serving v4 from OctopusProvider, retiring the v3 path (which removes its copy of the rollout hash), and #97's v3-level renames.

Tests

532 pass, 0 failures. Suites mirror .NET's, including MalformedEvaluationTests (23 shapes — .NET's 19 plus the no-reason one it covers elsewhere). The count jumps because the 114 shared rollout vectors now run against the v4 implementation as well as v3's. Public API surface confirmed identical to main via javap over target/classes.

🤖 Generated with Claude Code

@lourens-octopus
lourens-octopus requested a review from a team as a code owner August 9, 2026 22:12
@lourens-octopus
lourens-octopus force-pushed the lourens/bmbb-751 branch 2 times, most recently from 38288a8 to 15c95a8 Compare August 10, 2026 19:40
Base automatically changed from lourens/bmbb-747 to main August 10, 2026 20:50
lourens-octopus and others added 6 commits August 11, 2026 09:34
Implements client-side evaluation of the v4 types: a flag is enabled
when any rule matches, and a rule matches when all of its conditions do.

Defers validation from deserialization to evaluation so a malformed
response reports the specific problem as a ParseError rather than
failing the whole response as it is read.

Percentage bucketing is shared with v3 through RolloutBucketing, so a
rollout lands on the same users whichever flag version resolved it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Broadens the non-string discriminator case from the single numeric shape
to number, boolean, null, object and array, all of which must carry no
type so evaluation reports them rather than degrading quietly.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follows the same removal in the base branch, and drops the
case-insensitive discriminator lookup those tests were the only
justification for. The lookup now matches "type" exactly, as the .NET
provider's converter does.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Matches the .NET provider, which added this check after the equivalent
PR: a flag carrying a value but no reason is a response the server does
not send, so it is reported rather than resolved with a null reason.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Renames RolloutBucketing to PercentageRollout and adds includes(), so
the bucket comparison lives in one place, matching the .NET provider's
V4/Conditions/PercentageRollout. getNormalizedNumber stays reachable for
the shared cross-library test vectors and for v3.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Matches the terminology the .NET provider settled on, and drops a
reference to a server-internal condition name that means nothing from
inside the provider library.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
lourens-octopus and others added 2 commits August 11, 2026 10:32
A missing slug threw MismatchedInputException while the response was
being read, which cost every other flag in the array, not just the
malformed one. The .NET provider enforces no JSON member at all, so this
was the last behavioural divergence between the two libraries.

No property in the v4 types is required now: a malformed flag is
reported when it is evaluated, so it costs only itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
PercentageRollout moves into the v4 package as a package-private type,
where the .NET provider keeps it, so this branch adds nothing to the
published API. The library is at 1.0.0 on Maven Central, so a public type
here would be something consumers could bind to and the wiring-up ticket
could not remove without a breaking change.

Package access is not hierarchical, so v3 cannot call it and keeps its
own copy of the hash — restoring OctopusContext to what it was on main.
Both copies now run against one shared set of vectors, extracted to
RolloutVectors, so they cannot drift apart unnoticed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@liamhughes liamhughes left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good.

A few tidying-type comments.

) {
this.name = name;
this.conditions = List.copyOf(conditions);
this.conditions = WireLists.copyOrNull(conditions);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ListUtils?

* <p>Exposed rather than private so the shared cross-library vectors can assert on the bucket
* itself, as the other provider libraries do.
*/
static int getNormalizedNumber(String evaluationKey, String targetingKey) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Confirmed same as v3. 👍

}

@Test
void aMissingValueInTheListThrowsAParseError() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this be folded into the above test?

}

@Test
void aMissingValueInTheListThrowsAParseError() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this be folded into the above test?

* A JSON payload written with single quotes, so the malformed-response cases stay readable on one
* line without escaping. No payload here contains an apostrophe.
*/
static String json(String singleQuoted) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Blergh. Java's lack of string manipulation vs C# makes things...difficult.

@Test
void withoutATargetingKeyOnlyAFullRolloutMatches() {
assertThat(new PercentageByContextCondition(100).matches(Contexts.forRules(null)))
.as("a 100%% rollout matches even without a targeting key").isTrue();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
.as("a 100%% rollout matches even without a targeting key").isTrue();
.as("a 100% rollout matches even without a targeting key").isTrue();

.as("there is no attribute to match").isFalse();
assertThat(deferred(new ClientSideRule("everyone", List.of(new PercentageByContextCondition(100))))
.evaluate(null).getValue())
.as("a 100%% rollout matches without a targeting key").isTrue();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
.as("a 100%% rollout matches without a targeting key").isTrue();
.as("a 100% rollout matches without a targeting key").isTrue();

lourens-octopus and others added 3 commits August 12, 2026 08:48
AssertJ only runs String.format on a description when format arguments
are supplied, so a doubled percent was rendered literally as "100%%" in
a failure message rather than collapsing to one. Passing tests never
showed it, because the description is only rendered when an assertion
fails.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The null-in-list case only stood apart because List.of() rejects null; a
MethodSource can supply Arrays.asList, so it is now one more row of the
cases covering a condition with nothing to match on.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@lourens-octopus
lourens-octopus merged commit 1f9103f into main Aug 11, 2026
7 checks passed
@lourens-octopus
lourens-octopus deleted the lourens/bmbb-751 branch August 11, 2026 21:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants