feat: Support retrieving numeric metadata as either integers or decimals

[lager95]

This PR

• adds this new feature

Related Issues

Fixes #443

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.92%. Comparing base (e18ad50) to head (5034ea5).
Report is 30 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #490      +/-   ##
==========================================
+ Coverage   87.08%   87.92%   +0.84%     
==========================================
  Files          45       48       +3     
  Lines        1757     1980     +223     
  Branches      184      226      +42     
==========================================
+ Hits         1530     1741     +211     
- Misses        187      191       +4     
- Partials       40       48       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[alexandraoberaigner]

Looks good to me! 🎉

[arttonoyan]

Just a quick thought - do we want to allow implicit conversion here (e.g., from double) using Convert.ToInt32? Since it can silently round or truncate, I’m curious if that’s acceptable for our use case.

Would it make sense to add an explicit check like this to avoid unexpected rounding?

if (value is double d && d % 1 == 0)
    return Convert.ToInt32(d);

[chrfwow]

I tend to agree. Otherwise we might see values that do not correspond to any variant. However, I am not sure if the implementation you suggested couldn't lead to decimal precision errors

[arttonoyan]

@chrfwow You're right - the example was more to illustrate the idea rather than a final implementation. I believe it should work, but we definitely need a unit test to validate the behavior and catch any potential edge cases. Alternatively, we might explore a more robust approach if there's a better algorithm for handling this conversion safely.

[aepfli]

This is actually somewhat interesting, because, eg, in Java we try to cast, and you can't cast a float to an int. Maybe we should put a little bit more effort into specing this behavior - @open-feature/technical-steering-committee wdyt?

//Edit: ran some tests in java, and we can not use it in a different type, the cast will simply fail.

unit test code java

code

    void TestingIntAndLongAndDoubleAndFloatCasts() {
        ImmutableMetadata i1 =
                ImmutableMetadata.builder()
                        .addInteger("int", 1)
                        .addDouble("double", 1.1d)
                        .addDouble("doubleZero", 1.0d)
                        .addFloat("float", 1.2f)
                        .addFloat("floatZero", 1.0f)
                        .addLong("long", 1L)
                        .build();

        SoftAssertions softly = new SoftAssertions();

        softly.assertThat(i1.getInteger("int")).as("integer to integer").isEqualTo(1);
        softly.assertThat(i1.getInteger("double")).as("double to integer").isEqualTo(1);
        softly.assertThat(i1.getInteger("doubleZero")).as("doubleZero to integer").isEqualTo(1);
        softly.assertThat(i1.getInteger("float")).as("float to integer").isEqualTo(1);
        softly.assertThat(i1.getInteger("long")).as("long to integer").isEqualTo(1);


        softly.assertThat(i1.getDouble("int")).as("integer to double").isEqualTo(1);
        softly.assertThat(i1.getDouble("double")).as("double to double").isEqualTo(1.1);
        softly.assertThat(i1.getDouble("doubleZero")).as("doubleZero to double").isEqualTo(1);
        softly.assertThat(i1.getDouble("float")).as("float to double").isEqualTo(1.2);
        softly.assertThat(i1.getDouble("floatZero")).as("float to double").isEqualTo(1);
        softly.assertThat(i1.getDouble("long")).as("long to double").isEqualTo(1);
        softly.assertAll();
    }

result

Multiple Failures (8 failures)
-- failure 1 --
[double to integer] 
Expecting actual not to be null
at ImmutableMetadataTest.TestingIntAndLongAndDoubleAndFloatCasts(ImmutableMetadataTest.java:37)
-- failure 2 --
[doubleZero to integer] 
Expecting actual not to be null
at ImmutableMetadataTest.TestingIntAndLongAndDoubleAndFloatCasts(ImmutableMetadataTest.java:38)
-- failure 3 --
[float to integer] 
Expecting actual not to be null
at ImmutableMetadataTest.TestingIntAndLongAndDoubleAndFloatCasts(ImmutableMetadataTest.java:39)
-- failure 4 --
[long to integer] 
Expecting actual not to be null
at ImmutableMetadataTest.TestingIntAndLongAndDoubleAndFloatCasts(ImmutableMetadataTest.java:40)
-- failure 5 --
[integer to double] 
Expecting actual not to be null
at ImmutableMetadataTest.TestingIntAndLongAndDoubleAndFloatCasts(ImmutableMetadataTest.java:43)
-- failure 6 --
[float to double] 
Expecting actual not to be null
at ImmutableMetadataTest.TestingIntAndLongAndDoubleAndFloatCasts(ImmutableMetadataTest.java:46)
-- failure 7 --
[float to double] 
Expecting actual not to be null
at ImmutableMetadataTest.TestingIntAndLongAndDoubleAndFloatCasts(ImmutableMetadataTest.java:47)
-- failure 8 --
[long to double] 
Expecting actual not to be null
at ImmutableMetadataTest.TestingIntAndLongAndDoubleAndFloatCasts(ImmutableMetadataTest.java:48)

[chrfwow]

I tend to agree. Otherwise we might see values that do not correspond to any variant. However, I am not sure if the implementation you suggested couldn't lead to decimal precision errors

[chrfwow]

It seems like you added a duplicate either to the PR title by accident