Best practices to use JSON value

[WeihanLi]

We're integrating the LaunchDarkly based on OpenFeature SDK, it did not goes well when we tried to integrate JSON values from LaunchDarkly, are there some best practices about using JSON values?

With my limited research, we should use the GetObject API for getting JSON values, while the value we got is a Structure object, how should we convert it strong typed object performantly?

var result = await _featureClient.GetObjectDetailsAsync(
            flagKey, default, evaluationContext, cancellationToken: cancellationToken);
var obj = result.Value.AsObject;

There's a ToJsonString() method for the LDValue, and implemented a customized JsonConverter for the LdValue in the LaunchDarkly SDK, maybe something similar would be needed for the Value in the open feature SDK also?

[WeihanLi]

A sample JSON converter for the Value drafted

sealed class FeatureValueConverter : JsonConverter<Value>
{
    public override void Write(Utf8JsonWriter writer, Value value, JsonSerializerOptions options) =>
        WriteJsonValue(value, writer);

    public override Value Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
        ReadJsonValue(ref reader);

    private static void WriteJsonValue(Value value, Utf8JsonWriter writer)
    {
        if (value.IsNull)
        {
            writer.WriteNullValue();
            return;
        }

        if (value.IsBoolean)
        {
            writer.WriteBooleanValue(value.AsBoolean.GetValueOrDefault());
            return;
        }

        if (value.IsNumber)
        {
            var intVal = value.AsInteger;
            var doubleVal = value.AsDouble;
            if (intVal.HasValue && Convert.ToDouble(intVal.Value) == doubleVal.GetValueOrDefault())
            {
                writer.WriteNumberValue(intVal.Value);
            }
            else
            {
                writer.WriteNumberValue(value.AsDouble.GetValueOrDefault());
            }
            return;
        }

        if (value.IsString)
        {
            writer.WriteStringValue(value.AsString);
            return;
        }

        if (value.IsList)
        {
            writer.WriteStartArray();
            foreach (var item in value.AsList ?? [])
            {
                WriteJsonValue(item, writer);
            }
            writer.WriteEndArray();
        }

        if (value.IsStructure)
        {
            writer.WriteStartObject();
            var dic = value.AsStructure?.AsDictionary();
            if (dic is { Count: > 0 })
            {
                foreach (var (key, val) in dic)
                {
                    writer.WritePropertyName(key);
                    WriteJsonValue(val, writer);
                }
            }

            writer.WriteEndObject();
        }
    }

    private static Value ReadJsonValue(ref Utf8JsonReader reader)
    {
        switch (reader.TokenType)
        {
            case JsonTokenType.True:
                return new (true);
            case JsonTokenType.False:
                return new (false);
            case JsonTokenType.Number:
                return new (reader.GetDouble());
            case JsonTokenType.String:
                return new(reader.GetString());
            case JsonTokenType.StartArray:
                var list = new List<Value>();
                while (reader.Read())
                {
                    if (reader.TokenType == JsonTokenType.EndArray)
                    {
                        break;
                    }
                    list.Add(ReadJsonValue(ref reader));
                }
                return new(list);
            case JsonTokenType.StartObject:
                var objectBuilder = Structure.Builder();
                while (reader.Read() && reader.TokenType != JsonTokenType.EndObject)
                {
                    var name = reader.GetString();
                    reader.Read();
                    objectBuilder.Set(name, ReadJsonValue(ref reader));
                }
                return new(objectBuilder.Build());

            default:
                return new();
        }
    }
}

[toddbaert]

That looks nice @WeihanLi ! We don't support directly parsing the Structure/Value types to a specific custom type. I'm not 100% sure it's something we want to do in the SDK, though it seems much more reasonable in .NET than in Java for instance, where there's no standard JSON parser like .NET's System.Text.Json (and so we'd have to add a very specific dependency).

We were just discussing doing something like this in Go.

@askpt What do you think... is it a good idea to implement this sort of thing in the SDK? Or should we make it a "userland" concern? I'm somewhat unsure.

[askpt]

@toddbaert and @WeihanLi, it seems like a good idea. My only concern is that for framework users they will need to bring System.Text.Json and I would assume the vast majority of them are still using Newtonsoft.

I would vote to bring it anyway or make it available only for Net Core apps.

Pinging @kylejuliandev and @arttonoyan for this one also.

@toddbaert what is the perspective of the technical group?

[kylejuliandev]

So is the idea the SDK could provide a built-in JsonConverter that would allow user to register and handle deserialization? Will the SDK handle the converter registration (with the [JsonConverter] attribute)?

I don't think it's uncommon for libraries to do this. Examples include:

• NodaTime
• MassTransit
• EF Core - the Cosmos and SQL database providers have some Json converters too
• Serilog - I found an example for a Browser Console sink

[askpt]

@toddbaert if the @open-feature/technical-steering-committee is happy, I would propose we add @WeihanLi implementation to the SDK. The framework concern is minimal and it shouldn't weight that much on the decision.

[lukas-reining]

@toddbaert if the @open-feature/technical-steering-committee is happy, I would propose we add @WeihanLi implementation to the SDK. The framework concern is minimal and it shouldn't weight that much on the decision.

If @open-feature/sdk-dotnet-maintainers support this too, I also support it :D
The improvement for users sounds great!

[kinyoklion]

This works for me. As @WeihanLi mentioned we basically support the same thing in the LD SDK. It isn't the best for the most performance critical cases, but people always have the ability to directly use the Value for those cases.

[askpt]

@WeihanLi Would you mind creating a PR with the suggestion you added in this issue? It would be great to add it to the SDK with some unit tests! 👍