Skip to content

feat: add getNumberEvaluation method delegating to getDoubleEvaluation (#501) #1536

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

feat: add getNumberEvaluation method delegating to getDoubleEvaluation (#501) #1536

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
27e7a3f
feat: add getNumberEvaluation method delegating to getDoubleEvaluatio…
carla-was-not-available Jul 31, 2025
d1a48d4
feat: add getNumberEvaluation method delegating to getDoubleEvaluatio…
carla-was-not-available Jul 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions src/main/java/dev/openfeature/sdk/FeatureProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,43 @@ default List<Hook> getProviderHooks() {
return new ArrayList<>();
}

/**
* Resolves a feature flag value as a {@link Number}.
*
* @param key the unique identifier for the flag
* @param defaultValue the default value to return if the flag cannot be resolved
* @param ctx the evaluation context containing any relevant information for resolution
* @return a {@link ProviderEvaluation} containing the resolved {@code Number} value and additional eval details
*/
default ProviderEvaluation<Number> getNumberEvaluation(String key, Number defaultValue, EvaluationContext ctx) {
ProviderEvaluation<Double> dep = getDoubleEvaluation(key, defaultValue.doubleValue(), ctx);
return new ProviderEvaluation<>(
dep.getValue(),
dep.getReason(),
dep.getVariant(),
dep.getErrorCode(),
dep.getErrorMessage(),
dep.getFlagMetadata());
Comment on lines +28 to +34
Copy link

Choose a reason for hiding this comment

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

critical

There's a bug in the constructor call for ProviderEvaluation. The arguments for variant and reason are swapped. According to the ProviderEvaluation class definition, the constructor expects variant before reason. This will lead to incorrect evaluation details.

Suggested change
return new ProviderEvaluation<>(
dep.getValue(),
dep.getReason(),
dep.getVariant(),
dep.getErrorCode(),
dep.getErrorMessage(),
dep.getFlagMetadata());
return new ProviderEvaluation<>(
dep.getValue(),
dep.getVariant(),
dep.getReason(),
dep.getErrorCode(),
dep.getErrorMessage(),
dep.getFlagMetadata());

}

ProviderEvaluation<Boolean> getBooleanEvaluation(String key, Boolean defaultValue, EvaluationContext ctx);

ProviderEvaluation<String> getStringEvaluation(String key, String defaultValue, EvaluationContext ctx);

/**
* Evaluate feature flag returning an integer value.
*
* @deprecated please use {@link #getNumberEvaluation(String, Number, EvaluationContext)}
*/
@Deprecated
ProviderEvaluation<Integer> getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx);

/**
* Evaluate a feature flag returning a double value.
*
* @deprecated please use {@link #getNumberEvaluation(String, Number, EvaluationContext)}
*/
@Deprecated
ProviderEvaluation<Double> getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx);

ProviderEvaluation<Value> getObjectEvaluation(String key, Value defaultValue, EvaluationContext ctx);
Expand Down
1 change: 1 addition & 0 deletions src/main/java/dev/openfeature/sdk/FlagValueType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
@SuppressWarnings("checkstyle:MissingJavadocType")
public enum FlagValueType {
STRING,
NUMBER,
INTEGER,
DOUBLE,
OBJECT,
Expand Down
2 changes: 2 additions & 0 deletions src/main/java/dev/openfeature/sdk/NoOpProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ public ProviderEvaluation<String> getStringEvaluation(String key, String default
}

@Override
@Deprecated
public ProviderEvaluation<Integer> getIntegerEvaluation(String key, Integer defaultValue, EvaluationContext ctx) {
return ProviderEvaluation.<Integer>builder()
.value(defaultValue)
Expand All @@ -50,6 +51,7 @@ public ProviderEvaluation<Integer> getIntegerEvaluation(String key, Integer defa
}

@Override
@Deprecated
public ProviderEvaluation<Double> getDoubleEvaluation(String key, Double defaultValue, EvaluationContext ctx) {
return ProviderEvaluation.<Double>builder()
.value(defaultValue)
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/dev/openfeature/sdk/OpenFeatureClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,9 +287,11 @@ private <T> ProviderEvaluation<?> createProviderEvaluation(
case STRING:
return provider.getStringEvaluation(key, (String) defaultValue, invocationContext);
case INTEGER:
return provider.getIntegerEvaluation(key, (Integer) defaultValue, invocationContext);
return provider.getNumberEvaluation(key, (Integer) defaultValue, invocationContext);
Copy link
Member

Choose a reason for hiding this comment

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

do we need to modify/cast here the resolved value to an Integer before returning, for backwards compatibility?

case NUMBER:
return provider.getNumberEvaluation(key, (Number) defaultValue, invocationContext);
case DOUBLE:
return provider.getDoubleEvaluation(key, (Double) defaultValue, invocationContext);
return provider.getNumberEvaluation(key, (Double) defaultValue, invocationContext);
case OBJECT:
return provider.getObjectEvaluation(key, (Value) defaultValue, invocationContext);
default:
Expand Down
7 changes: 7 additions & 0 deletions src/test/java/dev/openfeature/sdk/NoOpProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,11 @@ void value() {
ProviderEvaluation<Value> eval = p.getObjectEvaluation("key", s, null);
assertEquals(s, eval.getValue());
}

@Test
void noOpNumber() {
NoOpProvider p = new NoOpProvider();
ProviderEvaluation<Number> eval = p.getNumberEvaluation("key", 123456789L, null);
assertEquals(123456789.0, eval.getValue());
Copy link

Choose a reason for hiding this comment

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

medium

This test is good for verifying the returned value. To make it more robust, I suggest adding assertions for the variant and reason fields as well. This would help catch potential issues like swapped fields in the evaluation details.

        assertEquals(123456789.0, eval.getValue());
        assertEquals(NoOpProvider.PASSED_IN_DEFAULT, eval.getVariant());
        assertEquals(Reason.DEFAULT.toString(), eval.getReason());

}
}
6 changes: 6 additions & 0 deletions src/test/java/dev/openfeature/sdk/ProviderSpecTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ void flag_value_set() {
ProviderEvaluation<Integer> int_result = p.getIntegerEvaluation("key", 4, new ImmutableContext());
assertNotNull(int_result.getValue());

ProviderEvaluation<Number> number_result = p.getNumberEvaluation("key", 2L, new ImmutableContext());
assertNotNull(number_result.getValue());

ProviderEvaluation<Double> double_result = p.getDoubleEvaluation("key", 0.4, new ImmutableContext());
assertNotNull(double_result.getValue());

Expand Down Expand Up @@ -97,6 +100,9 @@ void variant_set() {
ProviderEvaluation<Integer> int_result = p.getIntegerEvaluation("key", 4, new ImmutableContext());
assertNotNull(int_result.getReason());

ProviderEvaluation<Number> number_result = p.getNumberEvaluation("key", 2L, new ImmutableContext());
assertNotNull(number_result.getReason());
Copy link

Choose a reason for hiding this comment

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

medium

The test method is named variant_set and refers to specification 2.2.4, which is about the variant field. However, the assertion checks getReason(). To align with the test's intent, it should assert on getVariant() instead. I've noticed this pattern for other assertions in this test, but I'm only commenting on the line you've added.

Suggested change
assertNotNull(number_result.getReason());
assertNotNull(number_result.getVariant());


ProviderEvaluation<Double> double_result = p.getDoubleEvaluation("key", 0.4, new ImmutableContext());
assertNotNull(double_result.getReason());

Expand Down
Loading