feat!: DISABLED is a successful evaluation (still defaults)#1800
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the behavior of disabled feature flags to return a disabled reason instead of an error code. However, the current implementation returns a null value for disabled flags, which can lead to NullPointerExceptions in client applications. The reviewer suggests returning the user-provided default value instead of null when a flag is disabled, and updating the corresponding unit tests to assert this behavior.
There was a problem hiding this comment.
Pull request overview
This PR updates the in-process flagd-core evaluation behavior to treat state: DISABLED as a successful evaluation with reason=DISABLED (instead of returning FLAG_NOT_FOUND), and ensures disabled evaluations include flag metadata.
Changes:
FlagdCore: returnreason=DISABLEDfor disabled flags (no errorCode/errorMessage) and include flag metadata.MockEvaluator: align test evaluator behavior with the new disabled-flag semantics.- Tests: update assertions in
flagd-coreandproviders/flagdto expectreason=DISABLEDwith no value/variant.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tools/flagd-core/src/main/java/dev/openfeature/contrib/tools/flagd/core/FlagdCore.java | Changes disabled-flag evaluation to return reason=DISABLED and include metadata. |
| tools/flagd-core/src/test/java/dev/openfeature/contrib/tools/flagd/core/FlagdCoreTest.java | Updates disabled-flag test expectations for FlagdCore. |
| providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/MockEvaluator.java | Updates mock evaluator disabled-flag behavior used by in-process resolver tests. |
| providers/flagd/src/test/java/dev/openfeature/contrib/providers/flagd/resolver/process/InProcessResolverTest.java | Updates disabled-flag assertions for in-process resolver tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Marking as draft for now - we need the flagd and testbed update first. |
af8f129 to
309fb2d
Compare
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
…s, add object test Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
309fb2d to
b2969bd
Compare
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
reason=DISABLEDinstead oferrorCode=FLAG_NOT_FOUND. See the ADR.reason/errorCodeon disabled-flag evaluations.Related Issues
Closes #1799
Part of open-feature/flagd#1965