feat: Update fractional logic to support hashing consistency ADR. - #1831
feat: Update fractional logic to support hashing consistency ADR.#1831NeaguGeorgiana23 wants to merge 4 commits into
Conversation
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
📝 WalkthroughWalkthroughThe PR adds structured-value support and canonical CBOR hashing for fractional evaluation. It updates API testkit and provider context steps, null conversion, error assertions, lifecycle state transitions, compliance exclusions, and test harness references. ChangesFractional evaluation
API testkit
Provider lifecycle and end-to-end alignment
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Fractional
participant ObjectMapper
participant CBOR
participant MurmurHash
Fractional->>ObjectMapper: Convert bucket value to JSON tree
ObjectMapper->>CBOR: Encode canonical JSON value
CBOR->>MurmurHash: Provide encoded bytes
MurmurHash-->>Fractional: Return bucket position
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
NeaguGeorgiana23
changed the title
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProviderSyncResources.java`:
- Around line 117-120: Update the fatal-error cleanup flow across
FlagdProviderSyncResources.fatalError and FlagdProvider.shutdown so shutdown
still executes after fatalError marks the provider fatal, rather than returning
solely because isInitialized is false. Use an appropriate cleanup-state check or
adjust the shutdown contract while preserving normal shutdown behavior, and add
a regression test covering a fatal error after initialization that verifies
resolver and executor cleanup and the shut-down state.
In
`@providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java`:
- Around line 18-19: Update the type-conversion logic in Utils to check value ==
null before evaluating the "Null" type or the string "null" sentinel, returning
null immediately for actual null inputs. Preserve the existing sentinel behavior
for non-String types and leave the remaining type conversion unchanged.
In `@providers/flagd/test-harness`:
- Line 1: Replace the orphaned providers/flagd/test-harness gitlink commit with
a valid commit reachable from the configured OpenFeature test-harness/repos, or
migrate the flagd E2E step definitions according to the documented spec-based
Gherkin test approach. Do not leave the test-harness reference pointing to the
unreachable commit.
In
`@tools/flagd-api-testkit/src/main/java/dev/openfeature/contrib/tools/flagd/api/testkit/EvaluatorUtils.java`:
- Around line 25-28: Update the convert method’s Javadoc to document the Null
type and accurately describe null handling: the literal "null" remains the
string value for String, while an empty Object value produces an empty object
rather than null.
In
`@tools/flagd-core/src/main/java/dev/openfeature/contrib/tools/flagd/core/targeting/Fractional.java`:
- Around line 51-57: Update the argument classification in Fractional so a
pre-evaluated List bucket key is accepted alongside primitive, Map, and other
supported values, while preserving the distinction from a distribution list.
Ensure list keys are assigned to bucketBy and only the remaining arguments are
treated as distributions, consistent with convertNode’s CBOR array support.
- Around line 60-65: Update Fractional’s missing fallback targeting-key branch
to return or propagate a GeneralError instead of null, matching the expected
evaluation outcome in FractionalTest.missingBucketKeyReturnsNull. Replace the
current null-return behavior while preserving the existing debug logging and
targeting-key validation.
- Around line 185-193: Update the non-integral-number branch in Fractional’s
numeric encoding so whole-valued doubles, including 1.0 and -0.0, remain encoded
with CBORObject.FromObject(double) rather than being converted to long; retain
integral JSON-number handling, and add fractional bucketing coverage for 1, 1.0,
and -0.0.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8d1f5695-439c-4f3a-a810-b47ec000e9bf
📒 Files selected for processing (17)
providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProviderSyncResources.javaproviders/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/RunInProcessTest.javaproviders/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ContextSteps.javaproviders/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.javaproviders/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/config/ConfigSteps.javaproviders/flagd/test-harnesstools/flagd-api-testkit/src/main/java/dev/openfeature/contrib/tools/flagd/api/testkit/ContextSteps.javatools/flagd-api-testkit/src/main/java/dev/openfeature/contrib/tools/flagd/api/testkit/EvaluationSteps.javatools/flagd-api-testkit/src/main/java/dev/openfeature/contrib/tools/flagd/api/testkit/EvaluatorUtils.javatools/flagd-api-testkit/test-harnesstools/flagd-core/pom.xmltools/flagd-core/src/main/java/dev/openfeature/contrib/tools/flagd/core/targeting/Fractional.javatools/flagd-core/src/test/java/dev/openfeature/contrib/tools/flagd/core/e2e/FlagdCoreEvaluatorTest.javatools/flagd-core/src/test/java/dev/openfeature/contrib/tools/flagd/core/targeting/FractionalTest.javatools/flagd-core/src/test/java/dev/openfeature/contrib/tools/flagd/core/targeting/OperatorTest.javatools/flagd-core/src/test/resources/fractional/selfContainedFractionalB.jsontools/flagd-core/src/test/resources/fractional/string.json
| public synchronized void fatalError(ProviderEventDetails providerEventDetails) { | ||
| isFatal = true; | ||
| isInitialized = false; | ||
| fatalProviderEventDetails = providerEventDetails; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Preserve cleanup after fatal errors.
Line 119 sets isInitialized to false before FlagdProvider.shutdown() runs. FlagdProvider.onFatal calls fatalError(...) and then shutdown(). FlagdProvider.shutdown() returns when !syncResources.isInitialized(), before it calls flagResolver.shutdown(), shuts down errorExecutor, or marks the resource as shut down. This can leave fatal provider resources running and isShutDown false.
Change the shutdown contract so fatal cleanup does not depend on isInitialized, or use a separate cleanup state. Add a regression test for a fatal error after initialization.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@providers/flagd/src/main/java/dev/openfeature/contrib/providers/flagd/FlagdProviderSyncResources.java`
around lines 117 - 120, Update the fatal-error cleanup flow across
FlagdProviderSyncResources.fatalError and FlagdProvider.shutdown so shutdown
still executes after fatalError marks the provider fatal, rather than returning
solely because isInitialized is false. Use an appropriate cleanup-state check or
adjust the shutdown contract while preserving normal shutdown behavior, and add
a regression test covering a fatal error after initialization that verifies
resolver and executor cleanup and the shut-down state.
| if ("Null".equals(type)) return null; | ||
| if (Objects.equals(value, "null") && !"String".equals(type)) return null; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg '(^|/)Utils\.java$|EvaluatorUtils\.ja$|flagd' || true
echo "== relevant Utils.java =="
if [ -f providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java ]; then
cat -n providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java
fi
echo "== compare EvaluatorUtils if present =="
if [ -f tools/flagd-api-testkit/src/main/java/dev/openfeature/contrib/tools/flagd/api/testkit/EvaluatorUtils.java ]; then
cat -n tools/flagd-api-testkit/src/main/java/dev/openfeature/contrib/tools/flagd/api/testkit/EvaluatorUtils.java
fi
echo "== search for Utils method name/call sites =="
rg -n "public static Object|convert|valueOf|Null|\"null\"" providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java tools/flagd-api-testkit/src/main/java/dev/openfeature/contrib/tools/flagd/api/testkit/EvaluatorUtils.java 2>/dev/null || true
echo "== git diff stat/name status if available =="
git diff --stat HEAD~1..HEAD 2>/dev/null || git status --short 2>/dev/null || trueRepository: open-feature/java-sdk-contrib
Length of output: 1944
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral probe: model the Java switch expression from utils/evaluator files using Java semantics.
# This avoids modifying repository files and keeps the probe read-only.
python3 - <<'PY'
class JavaLike:
def convert(self, value, target_type):
if target_type == "Null":
return None
if value == "null" and target_type != "String":
return None
if target_type == "Boolean":
return bool(value.lower() == "true") if isinstance(value, str) else bool(value)
if target_type == "Integer":
if isinstance(value, str):
if value in ("null", "Infinity", "-Infinity"):
raise ValueError("badIntFormat")
return int(value)
raise TypeError("non-string integer")
if target_type == "Number":
if isinstance(value, str):
return float(value)
raise TypeError("non-string number")
if target_type == "String":
return str(value)
raise NotImplementedError(target_type)
cases = [
(None, "Boolean"),
("null", "Boolean"),
(None, "Integer"),
("null", "Integer"),
(None, "Number"),
("null", "Number"),
(None, "Object"),
("null", "Object"),
(None, "String"),
("null", "String"),
]
for value, target_type in cases:
try:
out = JavaLike().convert(value, target_type)
except Exception as ex:
out = f"{type(ex).__name__}: {ex}"
print(f"({value!r}, {target_type!r}) -> {out!r}")
PYRepository: open-feature/java-sdk-contrib
Length of output: 1944
🌐 Web query:
Java OpenFeature flagd e2e Utils.java Null value type conversion providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java
💡 Result:
The file providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java in the java-sdk-contrib repository is part of the end-to-end (e2e) test infrastructure for the flagd Java provider [1][2]. These tests utilize Gherkin scenarios to verify correct provider behavior, including type handling and evaluation consistency [3][4]. Regarding null value type conversion and error handling in flagd: 1. Consistency Initiatives: There has been an ongoing effort to standardize how flagd SDKs, including the Java implementation, handle errors and null values during evaluation [5][6]. Specifically, it has been established that custom operators and targeting rules should return null (or the language equivalent) when encountering invalid inputs, parse failures, or error conditions, rather than returning false or throwing exceptions [5][6]. 2. Evaluation Impact: In the Java SDK, returning null from a targeting rule effectively triggers a fallback to the default variant [5][6]. This behavior ensures that evaluations remain robust when encountering malformed targeting logic or edge cases (e.g., in custom operators like fractional or sem_ver) [5]. 3. Null in Targeting Rules: When a targeting rule evaluates to null and no defaultValue is specified, the provider is expected to revert to the code-defined default [7]. Recent development efforts have addressed issues where such evaluations might fail to correctly set the reason or value, ensuring that null propagation correctly triggers the fallback mechanism as intended by the OpenFeature specification [7]. The Utils.java file and related e2e test steps are used to validate these behaviors by simulating various targeting scenarios and asserting that the outcomes—including null-induced fallbacks—match expected results defined in the test harness [2][4].
Citations:
- 1: https://github.com/open-feature/java-sdk-contrib/tree/main/providers/flagd
- 2: perf(flagd): speed up e2e test execution via container pool and parallel scenarios #1752
- 3: https://central.sonatype.com/artifact/dev.openfeature.contrib.providers/flagd/0.13.2
- 4: 5af4c0f
- 5: [BUG] Custom operator error return values are inconsistent across SDK implementations flagd#1874
- 6: [BUG] In-process evaluation consistency: custom operator edge cases and error handling flagd#1904
- 7: [BUG] null values in targeting rules for object-valued flags when there is no defaultValue should evaluate to the code default flagd#1913
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg '(^|/)Utils\.java$|EvaluatorUtils\.ja$|flagd' || true
echo "== relevant Utils.java =="
if [ -f providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java ]; then
cat -n providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java
fi
echo "== compare EvaluatorUtils if present =="
if [ -f tools/flagd-api-testkit/src/main/java/dev/openfeature/contrib/tools/flagd/api/testkit/EvaluatorUtils.java ]; then
cat -n tools/flagd-api-testkit/src/main/java/dev/openfeature/contrib/tools/flagd/api/testkit/EvaluatorUtils.java
fi
echo "== search for Utils method name/call sites =="
rg -n "public static Object|convert|valueOf|Null|\"null\"" providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java tools/flagd-api-testkit/src/main/java/dev/openfeature/contrib/tools/flagd/api/testkit/EvaluatorUtils.java 2>/dev/null || true
echo "== git diff stat/name status if available =="
git diff --stat HEAD~1..HEAD 2>/dev/null || git status --short 2>/dev/null || trueRepository: open-feature/java-sdk-contrib
Length of output: 1944
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Behavioral probe: model the Java switch expression from utils/evaluator files using Java semantics.
# This avoids modifying repository files and keeps the probe read-only.
python3 - <<'PY'
class JavaLike:
def convert(self, value, target_type):
if target_type == "Null":
return None
if value == "null" and target_type != "String":
return None
if target_type == "Boolean":
return bool(value.lower() == "true") if isinstance(value, str) else bool(value)
if target_type == "Integer":
if isinstance(value, str):
if value in ("null", "Infinity", "-Infinity"):
raise ValueError("badIntFormat")
return int(value)
raise TypeError("non-string integer")
if target_type == "Number":
if isinstance(value, str):
return float(value)
raise TypeError("non-string number")
if target_type == "String":
return str(value)
raise NotImplementedError(target_type)
cases = [
(None, "Boolean"),
("null", "Boolean"),
(None, "Integer"),
("null", "Integer"),
(None, "Number"),
("null", "Number"),
(None, "Object"),
("null", "Object"),
(None, "String"),
("null", "String"),
]
for value, target_type in cases:
try:
out = JavaLike().convert(value, target_type)
except Exception as ex:
out = f"{type(ex).__name__}: {ex}"
print(f"({value!r}, {target_type!r}) -> {out!r}")
PYRepository: open-feature/java-sdk-contrib
Length of output: 1944
🌐 Web query:
openfeature/java-sdk-contrib tools/flagd-api-testkit/src/main/java/dev/openfeature/contrib/tools/flagd/api/testkit/EvaluatorUtils.java value == null
💡 Result:
The class EvaluatorUtils in the open-feature/java-sdk-contrib repository is part of the flagd-api-testkit toolset [1][2][3]. While specific code details regarding a null check on value are not explicitly documented in high-level summaries, the project frequently addresses null pointer exceptions (NPEs) and null handling across its tools and providers to ensure robustness in flag evaluation [4][5][6]. Recent updates in the flagd-related modules have focused on: - Improving null safety: Defensive null checks have been added to prevent NPEs in various evaluation contexts, such as when handling flag metadata [4][5]. - Consistent evaluation: Changes have been made to evaluation logic, such as treating DISABLED flags as successful evaluations [3][7] and fixing custom operator conformance edge cases that previously returned null on errors [6]. If you are investigating a specific null value behavior in EvaluatorUtils.java, it is likely related to these broader efforts to standardize how missing metadata, disabled flags, or evaluation errors are handled within the OpenFeature flagd ecosystem. For the most accurate technical analysis of the code at its current state, please refer to the source file directly on the main branch of the repository [8].
Citations:
- 1: c41be4a
- 2: chore(main): release dev.openfeature.contrib.tools.flagdapitestkit 0.2.0 #1745
- 3: chore(main): release dev.openfeature.contrib.tools.flagdapitestkit 0.3.0 #1782
- 4: [flagd] [BUG] Null Pointer Dereference in getFlagMetadata() #1708
- 5: fix: fixing null pointer exception error #1709
- 6: fix: various custom operator conformance fixes #1778
- 7: d732c35
- 8: https://github.com/open-feature/java-sdk-contrib
🌐 Web query:
github.com/open-feature/java-sdk-contrib providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java
💡 Result:
The file Utils.java located in providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/ is a helper class used within the end-to-end (E2E) testing suite for the OpenFeature flagd Java provider [1][2]. These E2E tests are based on the Gherkin feature files provided by the OpenFeature specification and rely on the flagd-testbed as a submodule to orchestrate the flagd environment (e.g., managing containers, dynamic flag updates) [3]. The Utils.java class typically provides common functionality required by the Cucumber step definitions, such as: 1. Interaction with the test-harness: Handling communication with the flagd instances or the launchpad utility [3]. 2. State Management: Assisting in managing the state of the OpenFeature API, such as setting or resetting the provider between test scenarios [4]. 3. Environment/Context Helpers: Providing utilities for configuring or retrieving context-specific information needed for flag evaluations within the E2E tests [5]. Recent changes to the E2E infrastructure in the java-sdk-contrib repository, including performance optimizations like a pre-warmed container pool, have updated how these tests execute in parallel [4]. Consequently, utility classes like Utils.java are essential for ensuring that test resources, such as container connections or OpenFeature SDK states, are handled safely and consistently across concurrent test executions [4].
Citations:
- 1: https://github.com/open-feature/java-sdk-contrib/tree/main/providers/flagd
- 2: 5af4c0f
- 3: https://github.com/open-feature/flagd-testbed
- 4: perf(flagd): speed up e2e test execution via container pool and parallel scenarios #1752
- 5: c41be4a
🌐 Web query:
github.com/open-feature/java-sdk-contrib raw providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java
💡 Result:
The file located at providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java is part of the flagd provider test suite within the open-feature/java-sdk-contrib repository [1][2]. This utility class typically contains helper methods used for end-to-end (e2e) testing of the flagd provider [3][2]. Recent development in the java-sdk-contrib repository has involved significant architectural changes to these e2e tests to support parallel execution and performance improvements [4]. Specifically, the repository utilizes Cucumber for its e2e testing, and utility classes like this are instrumental in managing test state, container lifecycles, and environment synchronization across test scenarios [2][4]. Due to the active nature of this repository, the contents of specific test utility files are frequently updated to accommodate new features, refactoring of test runners, and dependency management [2][4][5]. For the most up-to-date and accurate code, please refer directly to the official GitHub repository [6].
Citations:
- 1: https://github.com/open-feature/java-sdk-contrib/tree/main/providers/flagd
- 2: 5af4c0f
- 3: c41be4a
- 4: perf(flagd): speed up e2e test execution via container pool and parallel scenarios #1752
- 5: 5362405
- 6: https://github.com/open-feature/java-sdk-contrib/
🌐 Web query:
"EvaluatorUtils.java" "value == null" "flagd-api-testkit"
💡 Result:
The EvaluatorUtils.java file is a component within the OpenFeature flagd Java ecosystem, typically associated with utilities for the flagd-api-testkit or related evaluation logic [1][2][3]. In the context of the flagd-api-testkit and flagd Java evaluators, a check for value == null (often appearing in code like if (value == null)) is used to handle edge cases in flag evaluation [2]. These checks typically ensure that when a resolved flag value is null or missing, the system adheres to OpenFeature specifications regarding default values and error handling [4]. For example: 1. Compliance with Specifications: The flagd-api-testkit is used to verify that flagd evaluators correctly handle various scenarios, including those where values might be null, ensuring consistency across implementations [1][2]. 2. Default Value Handling: Recent developments have focused on allowing null or missing default values, where the system must return a specific result (such as FlagNotFound) if targeting does not resolve a variant and no default value is provided [4]. 3. Type Coercion: Evaluator utilities often include logic to coerce or validate resolved values [2]. Checks for null values are essential before performing type coercion to prevent NullPointerException errors and to correctly identify when a type mismatch or default path should be taken [2]. If you are encountering a specific issue or warning related to value == null in EvaluatorUtils.java within your test suite, it is likely part of the ongoing effort to improve operator conformance, null-handling, and spec compliance within the flagd Java ecosystem [5][6][7].
Citations:
- 1: https://mvnrepository.com/artifact/dev.openfeature.contrib.tools/flagd-api-testkit
- 2: test(java): adopt flagd-api-testkit compliance suite flagd-evaluator#62
- 3: chore(main): release dev.openfeature.contrib.tools.flagdapitestkit 0.2.0 #1745
- 4: feat: allowing null/missing defaultValue flagd#1659
- 5: open-feature/flagd-testbed@604df92
- 6: chore(main): release dev.openfeature.contrib.tools.flagdapitestkit 0.3.0 #1782
- 7: d732c35
🌐 Web query:
github.com/open-feature/java-sdk-contrib "Convert to" "Tools" "Null" "valueOf"
💡 Result:
There is no standard or known operation in the open-feature/java-sdk-contrib repository to "Convert to Tools Null valueOf." Research into the repository indicates that recent efforts have focused on fixing NullPointerExceptions [1] and improving operator conformance [2] within its tools modules, specifically within flagd-core components [1]. The term "valueOf" is a standard Java method often used for type conversion (e.g., String.valueOf(Object)) [3], but it is not associated with a specific migration or conversion utility in this context. If you are encountering an issue related to null handling or value conversion, it is likely related to general Java programming practices or specific bugs within the providers/tools, rather than a documented library feature. If you are looking to resolve null-related issues in your implementation, common approaches in the OpenFeature Java ecosystem include: 1. Ensuring proper null checks when accessing metadata or evaluation results [1]. 2. Using the standard SDK evaluation methods that support default values, which are designed to handle nulls safely [4]. 3. Reviewing project-specific pull requests (such as PR #1709 or #1778) for examples of how to safely handle nulls in the context of the java-sdk-contrib repository [2][1].
Citations:
- 1: fix: fixing null pointer exception error #1709
- 2: fix: various custom operator conformance fixes #1778
- 3: http://k2java.blogspot.com/search/label/null
- 4: https://openfeature.dev/docs/reference/sdks/server/java/
Handle actual null values before type conversion.
This guard handles the "Null" type and the text "null", but not value == null. A null Boolean becomes false, while null numeric and object values fail. Add the actual-null check before the string sentinel check.
Proposed guard
if ("Null".equals(type)) return null;
- if (Objects.equals(value, "null") && !"String".equals(type)) return null;
+ if (value == null || (Objects.equals(value, "null") && !"String".equals(type))) return null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if ("Null".equals(type)) return null; | |
| if (Objects.equals(value, "null") && !"String".equals(type)) return null; | |
| if ("Null".equals(type)) return null; | |
| if (value == null || (Objects.equals(value, "null") && !"String".equals(type))) return null; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/e2e/steps/Utils.java`
around lines 18 - 19, Update the type-conversion logic in Utils to check value
== null before evaluating the "Null" type or the string "null" sentinel,
returning null immediately for actual null inputs. Preserve the existing
sentinel behavior for non-String types and leave the remaining type conversion
unchanged.
| @@ -1 +1 @@ | |||
| Subproject commit 7575a1dc45f176e57e809748a712a555e9aa5d11 | |||
| Subproject commit 82ba89ec8db498fa51368e558e4d87642d9e93c4 | |||
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
path="providers/flagd/test-harness"
old="7575a1dc45f176e57e809748a712a555e9aa5d11"
new="82ba89ec8db498fa51368e558e4d87642d9e93c4"
git ls-tree HEAD -- "$path"
if ! git -C "$path" rev-parse --git-dir >/dev/null 2>&1; then
echo "Initialize the submodule before running this check." >&2
exit 2
fi
test "$(git -C "$path" rev-parse HEAD)" = "$new"
git -C "$path" cat-file -e "$old^{commit}"
git -C "$path" cat-file -e "$new^{commit}"
git -C "$path" diff --stat "$old" "$new"
git -C "$path" diff --name-status "$old" "$new"Repository: open-feature/java-sdk-contrib
Length of output: 251
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
path="providers/flagd/test-harness"
old="7575a1dc45f176e57e809748a712a555e9aa5d11"
new="82ba89ec8db498fa51368e558e4d87642d9e93c4"
echo "Submodule entry:"
git ls-tree HEAD -- "$path"
echo "Submodule config and status:"
git submodule status "$path" || true
git config -f .gitmodules --get submodule."$path".url || true
if git -C "$path" rev-parse --git-dir >/dev/null 2>&1; then
echo "Local HEAD:"
git -C "$path" rev-parse --verify HEAD
echo "Local new commit exists:"
git -C "$path$'\n'cat-file -t "$new" || true
echo "Local old commit exists:"
git -C "$path" cat-file -t "$old" || true
echo "Local diff stat:"
git -C "$path" diff --stat "$old" "$new" || true
else
echo "Submodule object dir is unavailable."
git ls-tree HEAD -- "$path" | awk '{print $3}'
fiRepository: open-feature/java-sdk-contrib
Length of output: 478
🌐 Web query:
open-feature test-harness git commit 82ba89ec8db498fa51368e558e4d87642d9e93c4
💡 Result:
The git commit hash 82ba89ec8db498fa51368e558e4d87642d9e93c4 does not appear to be a valid commit in the OpenFeature 'test-harness' repository [1][2]. The OpenFeature project maintained a repository titled 'test-harness' (also known as 'flagd-testbed') which was used to support Gherkin-based integration tests for OpenFeature SDKs [1][2][3]. However, as of April 2026, this repository has been deprecated or superseded in favor of using the 'spec' repository directly [4]. For instance, the 'js-sdk' repository replaced its 'test-harness' submodule with the 'spec' submodule in April 2026 to remove flagd-specific dependencies and utilize Gherkin feature files directly from the OpenFeature specification [4]. There is no record of the specific commit 82ba89ec8db498fa51368e558e4d87642d9e93c4 in the official OpenFeature 'test-harness' repository, nor does the hash correspond to common OpenFeature project activities [1][2]. The provided hash does not match the format of typical Git commit IDs associated with these repositories.
Citations:
- 1: https://github.com/open-feature/test-harness
- 2: https://github.com/open-feature/flagd-testbed
- 3: https://pkg.go.dev/github.com/open-feature/test-harness/sync
- 4: open-feature/js-sdk@7924205
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
new="82ba89ec8db498fa51368e558e4d87642d9e93c4"
for url in \
"https://github.com/open-feature/test-harness.git" \
"https://github.com/open-feature/flagd-testbed.git" \
"https://github.com/open-feature/spec.git"
do
echo "Check ${new}@${url}"
git ls-remote "$url" "$new" || true
doneRepository: open-feature/java-sdk-contrib
Length of output: 448
🌐 Web query:
"82ba89ec8db498fa51368e558e4d87642d9e93c4"
💡 Result:
The character string 82ba89ec8db498fa51368e558e4d87642d9e93c4 does not appear to correspond to a recognized public blockchain address, transaction hash, or a widely documented technical identifier. A search of this specific 40-character hexadecimal string yielded no direct matches across major blockchain block explorers (such as Etherscan, BscScan, or other network-specific scanners). While the string follows the common hexadecimal format used for Ethereum-style addresses (which are 20 bytes/40 hex characters) and transaction hashes (which are typically 32 bytes/64 hex characters), it is not indexed in public records as an active or notable entity. It is likely a private identifier, a randomly generated hash, or a truncated/incorrectly formatted value.
Citations:
- 1: https://abscan.org/address/0x82ba1ba2be26c1cb695da06aa6518c89dbd53d87
- 2: https://sepolia.easscan.org/address/0x82BAF16207105E53bCa01424405213b074f051A5
- 3: https://explorer.blockscout.com/address/0x82b89e0F9C0695639EB88659D0c306dBC242AF96
- 4: https://uniscan.xyz/address/0x82cf8baF53Af12dD0d6070089288822Fa82dbE98
- 5: https://gnosisscan.io/address/0x82fba55f8cf1e687ed848a75bb3575c37b8e0b38
Replace the invalid test-harness gitlink.
This PR points providers/flagd/test-harness at 82ba89ec8db498fa51368e558e4d87642d9e93c4, but that commit is not reachable from the configured OpenFeature test-harness/repos. Choose a valid commit or migrate the E2E step definitions as documented for spec-based Gherkin tests rather than leaving an orphaned gitlink.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@providers/flagd/test-harness` at line 1, Replace the orphaned
providers/flagd/test-harness gitlink commit with a valid commit reachable from
the configured OpenFeature test-harness/repos, or migrate the flagd E2E step
definitions according to the documented spec-based Gherkin test approach. Do not
leave the test-harness reference pointing to the unreachable commit.
| if ("Null".equals(type)) { | ||
| return null; | ||
| } | ||
| if (value == null || (value.equals("null") && !"String".equals(type))) { |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the convert Javadoc to match the null contract.
The documentation omits the Null type. It also states that "null" and an empty Object value return null. The implementation preserves "null" for String and creates an empty object for an empty Object value.
Proposed documentation update
- * `@param` type the flag type name: Boolean, String, Integer, Float, or Object
- * `@return` the converted value, or {`@code` null} if {`@code` value} is "null" or empty for Object
+ * `@param` type the flag type name: Null, Boolean, String, Integer, Float, or Object
+ * `@return` the converted value; String preserves the literal "null", and Object
+ * converts an empty value to an empty object🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@tools/flagd-api-testkit/src/main/java/dev/openfeature/contrib/tools/flagd/api/testkit/EvaluatorUtils.java`
around lines 25 - 28, Update the convert method’s Javadoc to document the Null
type and accurately describe null handling: the literal "null" remains the
string value for String, while an empty Object value produces an empty object
rather than null.
| } else if (arguments.get(0) instanceof String | ||
| || arguments.get(0) instanceof Boolean | ||
| || arguments.get(0) instanceof Number | ||
| || arguments.get(0) instanceof java.util.Map) { | ||
| // first arg is a primitive or Map, use for bucketing | ||
| bucketBy = arguments.get(0); | ||
| distributions = arguments.subList(1, arguments.size()); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Support list bucket keys.
A list-valued bucket key does not match this type check. It enters the fallback path and is parsed as a fractional distribution. This prevents explicit array attributes from being hashed, even though convertNode supports CBOR arrays.
Preserve the distinction between a pre-evaluated list bucket key and a distribution list before this branch.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@tools/flagd-core/src/main/java/dev/openfeature/contrib/tools/flagd/core/targeting/Fractional.java`
around lines 51 - 57, Update the argument classification in Fractional so a
pre-evaluated List bucket key is accepted alongside primitive, Map, and other
supported values, while preserving the distinction from a distribution list.
Ensure list keys are assigned to bucketBy and only the remaining arguments are
treated as distributions, consistent with convertNode’s CBOR array support.
| if (properties.getTargetingKey() == null) { | ||
| log.debug("Missing fallback targeting key"); | ||
| // if (arguments.size() == 2) { | ||
| // throw new dev.openfeature.sdk.exceptions.GeneralError("Missing fallback targeting key"); | ||
| // } | ||
| return null; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Return GeneralError for a missing fallback targeting key.
Line 65 returns null. FractionalTest.missingBucketKeyReturnsNull now requires GeneralError for this input. The current implementation will fail that test and return the wrong evaluation outcome.
Proposed fix
- return null;
+ throw new dev.openfeature.sdk.exceptions.GeneralError("Missing fallback targeting key");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (properties.getTargetingKey() == null) { | |
| log.debug("Missing fallback targeting key"); | |
| // if (arguments.size() == 2) { | |
| // throw new dev.openfeature.sdk.exceptions.GeneralError("Missing fallback targeting key"); | |
| // } | |
| return null; | |
| if (properties.getTargetingKey() == null) { | |
| log.debug("Missing fallback targeting key"); | |
| // if (arguments.size() == 2) { | |
| // throw new dev.openfeature.sdk.exceptions.GeneralError("Missing fallback targeting key"); | |
| // } | |
| throw new dev.openfeature.sdk.exceptions.GeneralError("Missing fallback targeting key"); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@tools/flagd-core/src/main/java/dev/openfeature/contrib/tools/flagd/core/targeting/Fractional.java`
around lines 60 - 65, Update Fractional’s missing fallback targeting-key branch
to return or propagate a GeneralError instead of null, matching the expected
evaluation outcome in FractionalTest.missingBucketKeyReturnsNull. Replace the
current null-return behavior while preserving the existing debug logging and
targeting-key validation.
| } else if (node.isNumber()) { | ||
| if (node.isIntegralNumber()) { | ||
| return CBORObject.FromObject(node.asLong()); | ||
| } else { | ||
| double val = node.asDouble(); | ||
| if (val == Math.floor(val) && val >= Long.MIN_VALUE && val <= Long.MAX_VALUE) { | ||
| return CBORObject.FromObject((long) val); | ||
| } | ||
| return CBORObject.FromObject(val); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate file =="
fd -a 'Fractional\.java' . || true
echo "== file outline =="
ast-grep outline tools/flagd-core/src/main/java/dev/openfeature/contrib/tools/flagd/core/targeting/Fractional.java --view expanded || true
echo "== relevant lines =="
nl -ba tools/flagd-core/src/main/java/dev/openfeature/contrib/tools/flagd/core/targeting/Fractional.java | sed -n '130,230p'
echo "== CBORObject imports / dependencies =="
rg -n "CBORObject|co\.farrui|net\.sf\.jcbor|jcbor|jackson-databind" tools/flagd-core/src/main/java tools/flagd-core -g '!**/build/**' -g '!**/.gradle/**' || true
echo "== project files =="
git ls-files tools/flagd-core | sed -n '1,120p'Repository: open-feature/java-sdk-contrib
Length of output: 929
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== relevant Fractional.java lines =="
awk 'NR>=130 && NR<=220 {printf "%6d\t%s\n", NR, $0}' tools/flagd-core/src/main/java/dev/openfeature/contrib/tools/flagd/core/targeting/Fractional.java
echo "== CBOR deps and usages =="
rg -n "CBORObject|jcbor|net\.sf\.jcbor|co\.farrui|farrui" -S . || true
echo "== tests mentioning fractional / hash / 1.0 / -0.0 =="
rg -n "Fraction|fractional|1\.0|-0\.0|asDouble|isIntegralNumber|CBORObject|distributeValueFromHash" -S . || true
echo "== build files =="
git ls-files | rg '(^|/)(build\.gradle(\.k ts)?|settings\.gradle(\.k ts)?|pom\.xml|gradle\.properties|gradlew)$' || trueRepository: open-feature/java-sdk-contrib
Length of output: 50385
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== FractionalTest outline =="
ast-grep outline tools/flagd-core/src/test/java/dev/openfeature/contrib/tools/flagd/core/targeting/FractionalTest.java --view expanded || true
echo "== FractionalTest numeric/hash tests =="
awk 'NR>=1 && NR<=220 {printf "%6d\t%s\n", NR, $0}' tools/flagd-core/src/test/java/dev/openfeature/contrib/tools/flagd/core/targeting/FractionalTest.java
echo "== fractional resource files =="
git ls-files tools/flagd-core/src/test/resources/fractional | sort | while read -r f; do
echo "--- ${f#tools/flagd-core/} ---"
awk '{printf "%6d\t%s\n", NR, $0}' "$f"
done
echo "== build dependency for CBOR =="
awk 'NR>=80 && NR<=110 {printf "%6d\t%s\n", NR, $0}' tools/flagd-core/pom.xmlRepository: open-feature/java-sdk-contrib
Length of output: 13501
🌐 Web query:
com.upokecenter.cbor CBORObject.FromObject Object long double semantic
💡 Result:
In the PeterO.Cbor library (com.upokecenter.cbor), the static methods CBORObject.FromObject(double) and CBORObject.FromObject(long) are marked as obsolete [1][2][3]. Users are encouraged to use explicit, type-specific methods to generate CBOR objects [1][3]. Key recommendations: For floating-point numbers: Use CBORObject.FromDouble(double) instead of FromObject(double) [2][3]. For 64-bit integers: Use CBORObject.FromInt64(long) instead of FromObject(long) [2][3]. Regarding "semantic" conversion, the library's FromObject(object) method remains available for general-purpose conversion of arbitrary objects [1][2]. This method uses reflection (on platforms that support it) to determine how to serialize the object based on its type [4][5]. For more granular control over serialization or to define custom "semantic" mappings for specific types, the library provides the CBORTypeMapper class [6]. This allows you to register custom converters that implement ICBORToFromConverter or ICBORConverter to handle serialization and deserialization behavior explicitly [6][7].
Citations:
- 1: https://peteroupc.github.io/CBOR/docs/PeterO.Cbor.CBORObject.html
- 2: https://github.com/peteroupc/CBOR-Java/blob/master/api/com.upokecenter.cbor.CBORObject.md
- 3: https://github.com/peteroupc/CBOR/blob/master/docs/PeterO.Cbor.CBORObject.md
- 4: https://github.com/peteroupc/CBOR-Java/blob/master/src/main/java/com/upokecenter/cbor/CBORObject.java
- 5: https://mojoauth.com/serialize-and-deserialize/serialize-and-deserialize-cbor-with-gwt
- 6: https://peteroupc.github.io/CBOR/docs/PeterO.Cbor.CBORTypeMapper.html
- 7: https://peteroupc.github.io/CBOR/
🌐 Web query:
com.upokecenter.cbor.CBORObject FromObject long double implementation integer type
💡 Result:
In the Upokecenter CBOR library, the CBORObject.FromObject method is a general-purpose factory method used to convert arbitrary objects into CBORObject instances [1][2]. However, it is important to note that many specific overloads of FromObject are considered obsolete and have been replaced by more specific, type-focused factory methods [1][2]. Regarding the specific types mentioned: 1. Long and Double: The library provides FromInt64(long) and FromDouble(double) as the preferred replacements for the older FromObject(long) and FromObject(double) overloads [1][3][2]. Using these dedicated methods is recommended for better performance and clarity [3]. 2. Integer Type Implementation: The CBOR library handles integers according to the CBOR specification [4]. When converting to integer types (such as long or int) from a CBOR object, the library typically requires the CBOR object to have a type of Integer [3]. Methods like AsInt64Value are used to retrieve these values [3]. If a floating-point number is provided where an integer is expected, the implementation generally requires an explicit conversion (such as discarding the fractional part), which may throw an exception if the value is out of range or not representable as an integer (e.g., infinity or NaN) [5]. For developers seeking to avoid the overhead or limitations associated with generic reflection-based conversion, direct usage of type-specific factory methods (e.g., FromInt64, FromDouble) and conversion methods (e.g., AsInt64Value, AsDouble) is the standard best practice for this library [1][3][2].
Citations:
- 1: https://peteroupc.github.io/CBOR/docs/PeterO.Cbor.CBORObject.html
- 2: https://github.com/peteroupc/CBOR/blob/master/docs/PeterO.Cbor.CBORObject.md
- 3: https://github.com/peteroupc/CBOR-Java/blob/master/api/com.upokecenter.cbor.CBORObject.md
- 4: https://github.com/peteroupc/cbor-java
- 5: https://github.com/peteroupc/CBOR-Java/blob/master/src/main/java/com/upokecenter/cbor/CBORObject.java
Encode whole-valued floating-point bucket keys as CBOR floats.
Lines 188-191 convert non-integral values like 1.0 or -0.0 to long before hashing. This preserves value equality but changes the CBOR type, so a floating-point attribute can hash like an integer attribute. Keep these inputs on the floating-point branch and add fractional bucketing cases for 1, 1.0, and -0.0.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@tools/flagd-core/src/main/java/dev/openfeature/contrib/tools/flagd/core/targeting/Fractional.java`
around lines 185 - 193, Update the non-integral-number branch in Fractional’s
numeric encoding so whole-valued doubles, including 1.0 and -0.0, remain encoded
with CBORObject.FromObject(double) rather than being converted to long; retain
integral JSON-number handling, and add fractional bucketing coverage for 1, 1.0,
and -0.0.
5 participants
This PR
flagd-coreto adhere to the hashing consistency ADR.com.upokecenter:cbordependency (v4.5.6) to serialize bucketing keys (flag key, targeting key, primitives, maps, and lists) to canonical CBOR format before MurmurHash3 (32-bit x86) hashing.KEY_COMPARATOR) for map keys based on byte length and lexicographical byte order.test-harnesssubmodules in bothproviders/flagdandtools/flagd-api-testkitto commit82ba89e.ContextSteps,ConfigSteps,EvaluationSteps,Utils) with DataTable context key support, improved type conversions (handlingnullstrings, fallback from Integer to Long), and error reason handling.FlagdProviderSyncResourcesto properly resetisInitializedon fatal errors and shutdowns.fractional-v1->fractional-v2), and test fixture JSONs (selfContainedFractional0.json,string.json) to reflect the new hashing results.Related Issues
Fixes #1662
Notes
Follow-up Tasks