Skip to content
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.

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

Add ValueKind property in System.Text.Json.Nodes.JsonNode class #53406

Closed
kingcean opened this issue May 28, 2021 · 5 comments
Closed

Add ValueKind property in System.Text.Json.Nodes.JsonNode class #53406

kingcean opened this issue May 28, 2021 · 5 comments
Assignees
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Milestone

Comments

@kingcean
Copy link

Background and Motivation

System.Text.Json.Node.JsonNode has provided a way to as JsonObject, JsonArray and JsonValue, but has no way to get its value kind directly.

Proposed API

namespace System.Text.Json.Nodes
{
    public abstract class JsonNode : IDynamicMetaObjectProvider {
+        public abstract JsonValueKind ValueKind { get; }
    }
}

Usage Examples

var json = JsonNode.Parse("{ \"name\": \"Kingcean\" }");
Assert.AreEqual(JsonValueKind.Object, json.ValueKind );
Assert.AreEqual(JsonValueKind.String, json["name"].ValueKind ;

Implementation for Reference

// namespace System.Text.Json.Nodes
public abstract class JsonNode
{
    public abstract JsonValueKind ValueKind { get; }
    // ...
}
public sealed class JsonArray : JsonNode, IList<JsonNode?>, ICollection<JsonNode?>, IEnumerable<JsonNode?>, IEnumerable
{
    public override JsonValueKind ValueKind => JsonValueKind.Array;
    // ...
}
public sealed class JsonObject : JsonNode, IDictionary<string, JsonNode?>, ICollection<KeyValuePair<string, JsonNode?>>, IEnumerable<KeyValuePair<string, JsonNode?>>, IEnumerable
{
    public override JsonValueKind ValueKind => JsonValueKind.Object;
    // ...
}
internal abstract partial class JsonValue<TValue> : JsonValue
{
    public override JsonValueKind ValueKind
    {
        get
        {
            if (_value is null) return JsonValueKind.Null;
            if (_value is JsonElement element) return element.ValueKind;
            if (_value is string || _value is Guid || _value is DateTime || _value is DateTimeOffset) return JsonValueKind.String;
            if (_value is bool b) return b ? JsonValueKind.True : JsonValueKind.False;
            if (_value is int || _value is long || _value is uint || _value is ulong || _value is short || _value is ushort || _value is double || _value is float || _value is decimal || _value is byte ||  || _value is sbyte) return JsonValueKind.Number;
            throw new NotSupportedException();
        }
    }
    // ...
}
@kingcean kingcean added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label May 28, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels May 28, 2021
@ghost
Copy link

ghost commented May 28, 2021

Tagging subscribers to this area: @eiriktsarpalis, @layomia
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and Motivation

System.Text.Json.Node.JsonNode has provided a way to as JsonObject, JsonArray and JsonValue, but has no way to get its value kind directly.

Proposed API

namespace System.Text.Json.Nodes
{
    public abstract class JsonNode : IDynamicMetaObjectProvider {
+        public abstract JsonValueKind ValueKind { get; }
    }
}

Usage Examples

var json = JsonNode.Parse("{ \"name\": \"Kingcean\" }");
Assert.AreEqual(JsonValueKind.Object, json.ValueKind );
Assert.AreEqual(JsonValueKind.String, json["name"].ValueKind ;

Implementation for Reference

// namespace System.Text.Json.Nodes
public abstract class JsonNode
{
    public abstract JsonValueKind ValueKind { get; }
    // ...
}
public sealed class JsonArray : JsonNode, IList<JsonNode?>, ICollection<JsonNode?>, IEnumerable<JsonNode?>, IEnumerable
{
    public override JsonValueKind ValueKind => JsonValueKind.Array;
    // ...
}
public sealed class JsonObject : JsonNode, IDictionary<string, JsonNode?>, ICollection<KeyValuePair<string, JsonNode?>>, IEnumerable<KeyValuePair<string, JsonNode?>>, IEnumerable
{
    public override JsonValueKind ValueKind => JsonValueKind.Object;
    // ...
}
internal abstract partial class JsonValue<TValue> : JsonValue
{
    public override JsonValueKind ValueKind
    {
        get
        {
            if (_value is null) return JsonValueKind.Null;
            if (_value is JsonElement element) return element.ValueKind;
            if (_value is string || _value is Guid || _value is DateTime || _value is DateTimeOffset) return JsonValueKind.String;
            if (_value is bool b) return b ? JsonValueKind.True : JsonValueKind.False;
            if (_value is int || _value is long || _value is uint || _value is ulong || _value is short || _value is ushort || _value is double || _value is float || _value is decimal || _value is byte ||  || _value is sbyte) return JsonValueKind.Number;
            throw new NotSupportedException();
        }
    }
    // ...
}
Author: kingcean
Assignees: -
Labels:

api-suggestion, area-System.Text.Json, untriaged

Milestone: -

@kevinchalet
Copy link
Contributor

Having such a property would be extremely useful! 👏🏻

/cc @steveharter

@eiriktsarpalis eiriktsarpalis added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed untriaged New issue has not been triaged by the area owner labels Jul 16, 2021
@eiriktsarpalis eiriktsarpalis added this to the 7.0.0 milestone Jul 16, 2021
@kevinchalet
Copy link
Contributor

As mentioned in #52611, this property would be very useful in OpenIddict. Any chance it could be discussed for 6.0?

@steveharter steveharter self-assigned this Jul 30, 2021
@steveharter
Copy link
Member

The main reason that this isn't supported is that a JsonValue in "edit mode" can be assigned to any CLR type - a POCO, a Dictionary, an Enum, etc and in JSON they may be serialized in an arbitrary format depending on the converter. In "read mode" (after a Parse()) the underlying JsonElement.ValueKind can be used.

However, I did create a JsonNode extension sample for ValueKind in #55827 (comment) that handles this pretty well, but is not cheap when in "edit" mode and these types of values are assigned to a JsonValue. I plan on formalizing the sample soon (in STJ tests) and depending on feedback may make it into 7.0.

Also, note that JsonNode.GetValue<object>() can be called to get the underlying value. This may be useful in certain scenarios, like ValueKind, where inspection of the value is necessary. I'm also updating the doc to make this more clear.

Closing as it should be addressed by #55827.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 29, 2021
@sebastienros
Copy link
Member

sebastienros commented Nov 1, 2021

Also, note that JsonNode.GetValue<object>() can be called to get the underlying value

Is it possible that it returns a JsonElement in some cases instead?

Update

Just assume I had not read the doc correctly ... thanks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Projects
None yet
Development

No branches or pull requests

5 participants