Replies: 1 comment 1 reply
-
|
A possible alternative solution could be to utilise the Value type as in the ImmutableMetadata. For example: private readonly ImmutableDictionary<string, Value> _metadata;Rationale
Proposed Changes
- public bool? GetBool(string key)
- public int? GetInt(string key)
- public double? GetDouble(string key)
- public string? GetString(string key)
+ public Value? TryGetValue(string key, out Value value)
- public ImmutableMetadata(Dictionary<string, object> metadata)
+ public ImmutableMetadata(Dictionary<string, Value> metadata)Breaking ChangesThis would drastically change how
Example UsageWith the proposed change you can access metadata with the following pattern: if (_metadata.TryGetValue("notes", out var v))
{
if (v.IsNull)
{
// present and explicitly null
}
else
{
// present with a concrete value
}
}
else
{
// key not found
}Migration and compatibilityInstead of removing the existing accessor methods ( public bool? GetBool(string key)
{
var value = GetValue(key);
return value.IsBoolean ? value.AsBoolean : null;
}
private Value GetValue(string key)
{
if (!this._metadata.ContainsKey(key))
{
return new Value();
}
return this._metadata[key];
}Additionally, we could keep the existing constructor and chain constructors together as public ImmutableMetadata(Dictionary<string, Value> metadata)
{
}
public ImmutableMetadata(Dictionary<string, object> metadata)
: this(metadata.ToDictionary(kvp => kvp.Key, kvp => new Value(kvp.Value)))
{
} |
Beta Was this translation helpful? Give feedback.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
Summary
The current
ImmutableMetadataimplementation in the .NET SDK usesDictionary<string, object>for storing metadata values. This means there's no way to distinguish between a metadata key that doesn't exist and a metadata key that explicitly has anullvalue.This issue was identified during the review of open-feature/dotnet-sdk-contrib#524, where
JsonElementvalues withJsonValueKind.Nullare being converted to empty strings:As noted by @chrfwow: "Shouldn't there be a difference between an empty string and a null value?"
Current Behavior
The
ImmutableMetadataclass stores values in anImmutableDictionary<string, object>:When retrieving values, there's no distinction between:
nullvalueFor example,
GetString("key")returnsnullfor both missing keys and keys with null values.OpenFeature Specification Consideration
Per Requirement 2.2.10 in the OpenFeature specification:
The current spec does not include
nullas a valid metadata value type. This raises the question of whether this change should:Proposed Changes
Update the internal dictionary type to support nullable values:
Add a
ContainsKeymethod to allow consumers to check if a key exists:Consider adding
TryGetValuemethods for more explicit value retrieval:Update the constructor to accept nullable values:
Breaking Changes
This is a breaking change because:
Dictionary<string, object>toDictionary<string, object?>Questions for Discussion
open-feature/specto includenullas a valid metadata value type?Related
Beta Was this translation helpful? Give feedback.
All reactions