-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
System.Text.Json.Nodes.JsonValue is almost unusable #64472
Comments
Tagging subscribers to this area: @dotnet/area-system-text-json Issue DetailsBackground and motivationAs far as I can see, there's almost no way to do common and necessary operations on JsonValues. I'll give four examples:
API Proposal
API UsageSee use cases in the Background and Motivation section. Alternative DesignsFor greater backwards compatibility, just add JsonNumber, JsonValue.Type, JsonValue.AsBoolean(), JsonValue.AsNumber(), and JsonValue.AsString() and leave everything else the same. (int)JsonValue.Create(5L) will still fail, but (int)JsonValue.Create(5L).AsNumber() will succeed. Risks
|
Hi @AdamMil, thank you for your feedback.
That's a known issue with JsonValue.Parse(JsonValue.Create(5L).ToJsonString()).GetValue<int>(); @steveharter might be aware of a more efficient way to do this.
That's good feedback. I would recommend sharing it in #56592 which tracks improvements to
Related to #62585 (comment). Defining equality in DOM types comes with its own set of problems, which is why both Closing in favor of #56592. |
Background and motivation
As far as I can see, there's almost no way to do common and necessary operations on JsonValues. I'll give four examples:
I got a JsonValue from somewhere. I know the value is supposed to be an int and I want to extract it.
But (int)value or value.TryGetValue<int>() may not work, depending on how it was created or parsed. Even though JsonValue.Create(5) and JsonValue.Create(5L) and JsonValue.Create(5.0) represent the exact same JSON value, (int)value will only work on one of them. If a value is representable as an int, I should be able to do (int)value. But for now I can't, so let's say I want to write my own code to get the type and value. That brings us to...
I want to get the type and value of a JsonValue.
Unfortunately, there's no public property on a JsonValue to tell you what the type or value is! In practice, you can only get the JSON type and value by serializing the JsonValue into JSON and then examining the JSON string. That's ugly and inefficient, but moreover it's not sufficient to understand a JsonValue. As mentioned above, JsonValue.Create(5) and JsonValue.Create(5L) represent the exact same JSON value and have identical serializations but one is actually a JsonValue<int> and the other is actually a JsonValue<long>, and the cast operators require you to know the exact internal type.
But there's no feasible way to get that internal type. You can't type-check against JsonValue<int>, for example, because JsonValue<T> is an internal type. Even if you had the internal type, it's not clear what the JSON type is. JsonValue<Guid> has a JSON type of string. JsonValue<Foo> could be anything. You're back to looking at the JSON string yourself.
The only way to get the internal type with the public API is to enumerate all types in all loaded assemblies (including all possible generic type arguments), and make sure to use a breadth-first search because it's an infinite type space, and then use reflection (!) with every type to invoke the JsonValue.TryGetValue<T>() method. That's crazy, so you're left with using reflection on private members to extract the internal type and value.
Okay, you can forego JsonValue entirely and just parse the JSON string yourself to get the type and value. But then what's the point of JsonValue?
I want to copy a JsonValue from one place to another.
This kind of thing should work:
o1["a"] = o2["a"];
but it fails with InvalidOperationException because the JsonValue already has a parent. Frankly, I don't think it's valuable to have a Parent reference (especially without a corresponding property name or array index). JsonValues are immutable and should be able to be copied around without being cloned.But for now we have to clone it, meaning I need to create a new JsonValue with the same type and value. But there's no easy way to get the type and value, as described above. And if you can get the type and value, then you have to use reflection again to invoke the correct JsonValue.Create method. (There is no Create override that takes a Type rather than a generic type parameter.) Also, JsonValue.Create<int>(5) is not the same as JsonValue.Create(5) - one creates a JsonValueNotTrimmable<int> and the other creates a JsonValueTrimmable<int>, which is another distinction that must be replicated. Properly cloning a JsonValue so we can copy a property value is just too hard.
I want to compare JsonValues for equality.
There doesn't seem to be a good way to do this. If I extract the type and value as above, it doesn't directly help me. As mentioned, there's no obvious connection between the JsonValue<T> type and the JSON value. JsonValue<Guid> is a JSON string, but Guid is not a string. Furthermore, a JsonValue might be a JsonValue<JsonElement> or even a JsonValue<Foo>, which could be anything, and how can I compare Foos? Two Foos might be different but have identical JSON serializations.
So right now you have to compare the serialized representations, which is inefficient and far from easy. JsonValue.Create(1.0).ToJsonString() is "1" but JsonValue.Parse("1.0").ToJsonString() is "1.0", so it's not a simple string comparison. Effectively you have to render them into JSON and then do all your own JSON parsing and comparison with JSON semantics.
API Proposal
I got a JsonValue from somewhere. I know the value is supposed to be an int and I want to extract it.
I want to get the type and value of a JsonValue.
For # 1, it is simply a matter of making (int)value convert any valid JSON number into an int - as long it fits within the range of an int, and ditto for all the other numeric types. This can be assisted with a helper class, JsonNumber, which is based on the following observations.
The only fly in the ointment is floating-point types. The floating point value 1.1f actually equals 1.10000002384185791015625, but users would be unhappy if JsonNumber(1.1f) != 1.1f or if JsonNumber(1.1f).ToString() == "1.10000002384185791015625". It should, however, be the case that JsonNumber("1.10000002384185791015625") == 1.1f. I think the answer is to say that comparisons with and storage of floating-point types specifically will be fuzzy. What this means is that:
Given a JsonNumber struct, we can define JsonValue as:
Now we can 1) extract a type if we know what it is (regardless of how it's internally stored), and 2) find the type and the value. Notably, JsonValue does not store Guids or Foos. There should be no JsonValue<Guid> because JSON doesn't have a Guid type. There should be no JsonValue<T> at all. JSON only has four types, null, Boolean, number, and string (or three nullable types, to look at it another way), and all values are resolved to those basic JSON types at the time of construction. For backwards compatibility, we'll keep the convention that null JSON values are simply null JsonValues. (And this way, it'd actually be reliable. With your current code, you can have non-null JsonValues that represent the null value, via Create<T>() if T serializes to "null".)
I want to copy a JsonValue from one place to another.
I see no way to do this except by removing the Parent property. It doesn't seem all that useful to me, anyway - there is no corresponding property name or array index - and this lets us easily copy JsonNodes from one place to another without restriction. If the user creates a cycle, that's his problem. But in case we can't remove the Parent property, having JsonValue defined as above lets us easily implement a Clone() operation.
I want to compare JsonValues for equality.
This also becomes easy to support if JsonValue is defined as above:
API Usage
See use cases in the Background and Motivation section.
Alternative Designs
For greater backwards compatibility, just add JsonNumber, JsonValue.Type, JsonValue.AsBoolean(), JsonValue.AsNumber(), and JsonValue.AsString() and leave everything else the same. (int)JsonValue.Create(5L) will still fail, but (int)JsonValue.Create(5L).AsNumber() will succeed.
Risks
The text was updated successfully, but these errors were encountered: