Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/System.Text.Json/docs/writable_json_dom_spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ Mailbox.SendAllEmployeesData(employees.AsJsonElement());
## Open questions
* Do we want to add recursive equals on `JsonArray` and `JsonObject`?
* Do we want to make `JsonNode` derived types implement `IComparable` (which ones)?
* Do we want `JsonObject` to implement `IDictionary` and `JsonArray` to implement `IList` (currently JsonArray does, but JsonObject not)?
* Would escaped characters be supported for creating `JsonNumber` from string?
* Is the API for `JsonNode` and `JsonElement` interactions sufficient?
* Do we want to support duplicate and order preservation/control when adding/removing values in `JsonArray`/`JsonObject`?
Expand All @@ -260,6 +261,8 @@ Mailbox.SendAllEmployeesData(employees.AsJsonElement());
* Do we want to support creating `JsonNumber` from `BigInterger` without changing it to string?
* Should `ToString` on `JsonBoolean` and `JsonString` return the .NET or JSON representation?
* Do we want to keep implicit cast operators (even though for `JsonNumber` it would mean throwing in some cases, which is against FDG)?
* Do we want overloads that take a `StringComparison` enum for methods retrieving properties? It would make API easier to use where case is not important.
* Where do we want to enable user to set handling properties manner?

## Useful links

Expand Down
59 changes: 59 additions & 0 deletions src/System.Text.Json/ref/System.Text.Json.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@

namespace System.Text.Json
{
public enum DuplicatePropertyNameHandling
{
Replace = 0,
Ignore = 1,
Error = 2,
}
public sealed partial class JsonBoolean : System.Text.Json.JsonNode, System.IEquatable<System.Text.Json.JsonBoolean>
{
public JsonBoolean() { }
Expand Down Expand Up @@ -260,6 +266,56 @@ public void SetUInt64(ulong value) { }
[System.CLSCompliantAttribute(false)]
public bool TryGetUInt64(out ulong value) { throw null; }
}
public sealed partial class JsonObject : System.Text.Json.JsonNode, System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, System.Text.Json.JsonNode>>, System.Collections.IEnumerable
Comment thread
kasiabulat marked this conversation as resolved.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO JsonObject should implement IDictionary<string, JsonNode> and JsonArray should implement IList<JsonNode>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JsonArray is planned to implement IList, but we decided not to make JsonObject implement IDictionary, because it would be then possible to pass it for methods that accepts a dictionary, which we didn't find useful and desired. @bartonjs ?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kasiabulat are there any methods from IDictionary that we would be missing if we wanted to implement that? Are there any Sort methods in IDictionary?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are no extension methods accepting IDictionary / IList instead of IEnumerable. LINQ queries work faster with IList and that's why making JsonArray implement it seemed valuable. No queries work faster with IDictionary - that's also why we thought implementing it by JsonObject would be unnecessary.

To implement IDictionary we are lacking:

  • IsReadOnly
  • Keys (but we have PropertyNames),
  • Values (but we have PropertyValues)
  • Clear()
  • CopyTo(T[], Int32)
  • Contains(T)
  • ContainsKey(TKey) (but we have ContainsProperty)
  • TryGetValue(TKey, TValue) (but we have (Try)GetPropertyValue).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my point was that unless there were compelling extension methods or other specific use cases that it probably didn't need to implement the IDictionary interface. e.g. that one should be able to say "It implements IDictionary and IReadOnlyDictionary because [reason]" where "[reason]" is something more concrete than "because it's logically a dictionary" or "because it already had all the right methods".

Same result, different presentation: If no one will ever use the fact that it's an I(ReadOnly)Dictionary then it didn't need to assert the interface.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amoungst other benefits, IDictionary allows a collection initializer to be used:

var o = new JsonObject
{
    ["Name"] = "James",
    ["Radness"] = 98,
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amoungst other benefits, IDictionary allows a collection initializer to be used:

That doesn't require IDictionary. Just IEnumerable + suitable Add methods, which it already has 😄

{
public JsonObject(System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, System.Text.Json.JsonNode>> jsonProperties, System.Text.Json.DuplicatePropertyNameHandling duplicatePropertyNameHandling = System.Text.Json.DuplicatePropertyNameHandling.Replace) { }
public JsonObject(System.Text.Json.DuplicatePropertyNameHandling duplicatePropertyNameHandling = System.Text.Json.DuplicatePropertyNameHandling.Replace) { }
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't have expected DuplicatePropertyNameHandling to show up within JsonObject's API surface, but rather within the options passed in to the type that creates JsonObjects from some json payload (say JsonDocumentOptions).

Do we think there will be a need for this setting when creating a JsonObject itself via the ctor? What scenario would it be used for?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

People who are manually creating a JsonObject in code can choose whether to use o.Add("name", "value") or o["name"] = "value". That choice is basically the equivalent of duplicating property handling.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So actually, my Add behaves as specified in DuplicatePropertyNameHandling, to be consistent. So while indexer will always replace values, Add will either throw / replace or ignore (defaultly would behave just as indexer, but I can change the default behavior to exceptions).

Maybe we could have those options both in JsonObject and passed to parsing method? I think people might want to specify the behavior they want also when creating objects from C# - what this API is designed for?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think people might want to specify the behavior they want also when creating objects from C# - what this API is designed for?

Can you give an example/sample where you think it would be useful to have DuplicatePropertyNameHandling when creating JsonObject?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For example something containing interactions with user?

var properties = new JsonObject(DuplicatePropertyNameHandling.Error);

while(PropertiesReady())
{
    string userProvidedPropertyName = Console.ReadLine();
    string userProvidedPropertyValue = Console.ReadLine();
    try
    {
        properties.Add(userProvidedPropertyName, userProvidedPropertyValue);
    }
    catch (ArgumentException)
    {
        Console.WriteLine(string.Format("Property name `{0}` already exists. Please provide properties with unique names.", userProvidedPropertyName));
    }
}
Mailbox.SendData(properties.AsJsonElement());

public System.Text.Json.JsonNode this[string propertyName] { get { throw null; } set { } }
public System.Collections.Generic.ICollection<string> PropertyNames { get { throw null; } }
public System.Collections.Generic.ICollection<System.Text.Json.JsonNode> PropertyValues { get { throw null; } }
public void Add(System.Collections.Generic.KeyValuePair<string, System.Text.Json.JsonNode> jsonProperty) { }
public void Add(string propertyName, bool propertyValue) { }
Comment thread
kasiabulat marked this conversation as resolved.
public void Add(string propertyName, byte propertyValue) { }
public void Add(string propertyName, System.DateTime propertyValue) { }
public void Add(string propertyName, System.DateTimeOffset propertyValue) { }
public void Add(string propertyName, decimal propertyValue) { }
public void Add(string propertyName, double propertyValue) { }
public void Add(string propertyName, System.Guid propertyValue) { }
public void Add(string propertyName, short propertyValue) { }
public void Add(string propertyName, int propertyValue) { }
public void Add(string propertyName, long propertyValue) { }
public void Add(string propertyName, System.ReadOnlySpan<char> propertyValue) { }
[System.CLSCompliantAttribute(false)]
public void Add(string propertyName, sbyte propertyValue) { }
public void Add(string propertyName, float propertyValue) { }
public void Add(string propertyName, string propertyValue) { }
public void Add(string propertyName, System.Text.Json.JsonNode propertyValue) { }
[System.CLSCompliantAttribute(false)]
public void Add(string propertyName, ushort propertyValue) { }
[System.CLSCompliantAttribute(false)]
public void Add(string propertyName, uint propertyValue) { }
[System.CLSCompliantAttribute(false)]
public void Add(string propertyName, ulong propertyValue) { }
public void AddRange(System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, System.Text.Json.JsonNode>> jsonProperties) { }
public bool ContainsProperty(string propertyName) { throw null; }
Comment thread
kasiabulat marked this conversation as resolved.
public System.Collections.Generic.IEnumerator<System.Collections.Generic.KeyValuePair<string, System.Text.Json.JsonNode>> GetEnumerator() { throw null; }
public System.Text.Json.JsonObject GetJsonObjectPropertyValue(string propertyName) { throw null; }
public System.Text.Json.JsonNode GetPropertyValue(string propertyName) { throw null; }
public bool Remove(string propertyName) { throw null; }
System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; }
public bool TryGetJsonObjectPropertyValue(string propertyName, out System.Text.Json.JsonObject jsonObject) { throw null; }
public bool TryGetPropertyValue(string propertyName, out System.Text.Json.JsonNode jsonNode) { throw null; }
}
public partial struct JsonObjectEnumerator : System.Collections.Generic.IEnumerator<System.Collections.Generic.KeyValuePair<string, System.Text.Json.JsonNode>>, System.Collections.IEnumerator, System.IDisposable
{
private object _dummy;
public JsonObjectEnumerator(System.Text.Json.JsonObject jsonObject) { throw null; }
public System.Collections.Generic.KeyValuePair<string, System.Text.Json.JsonNode> Current { get { throw null; } }
object System.Collections.IEnumerator.Current { get { throw null; } }
public void Dispose() { }
public bool MoveNext() { throw null; }
public void Reset() { }
}
public readonly partial struct JsonProperty
{
private readonly object _dummy;
Expand Down Expand Up @@ -324,6 +380,9 @@ public JsonSerializerOptions() { }
public sealed partial class JsonString : System.Text.Json.JsonNode, System.IEquatable<System.Text.Json.JsonString>
{
public JsonString() { }
public JsonString(System.DateTime value) { }
public JsonString(System.DateTimeOffset value) { }
public JsonString(System.Guid value) { }
public JsonString(System.ReadOnlySpan<char> value) { }
public JsonString(string value) { }
public string Value { get { throw null; } set { } }
Expand Down
14 changes: 13 additions & 1 deletion src/System.Text.Json/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -429,4 +429,16 @@
<data name="EmptyStringToInitializeNumber" xml:space="preserve">
<value>Expected a number, but instead got empty string.</value>
</data>
</root>
<data name="JsonObjectDuplicateKey" xml:space="preserve">
<value>Property with name '{0}' already exists.</value>
</data>
<data name="PropertyNotFound" xml:space="preserve">
<value>Property with name '{0}' not found.</value>
</data>
<data name="PropertyTypeMismatch" xml:space="preserve">
<value>Property with name '{0}' has a different type than expected.</value>
</data>
<data name="InvalidDuplicatePropertyNameHandling" xml:space="preserve">
<value>The DuplicatePropertyNameHandling enum must be set to one of the supported values.</value>
</data>
</root>
5 changes: 4 additions & 1 deletion src/System.Text.Json/src/System.Text.Json.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,12 @@
<Reference Include="System.Threading.Tasks.Extensions" />
</ItemGroup>
<ItemGroup>
<Compile Include="System\Text\Json\Node\DuplicatePropertyNameHandling.cs" />
<Compile Include="System\Text\Json\Node\JsonBoolean.cs" />
<Compile Include="System\Text\Json\Node\JsonNode.cs" />
<Compile Include="System\Text\Json\Node\JsonNumber.cs" />
<Compile Include="System\Text\Json\Node\JsonNumber.cs" />
<Compile Include="System\Text\Json\Node\JsonObject.cs" />
<Compile Include="System\Text\Json\Node\JsonObjectEnumerator.cs" />
Comment thread
kasiabulat marked this conversation as resolved.
<Compile Include="System\Text\Json\Node\JsonString.cs" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
namespace System.Text.Json
{
/// <summary>
/// Specifies how duplicate property names are handled when added to JSON object.
/// </summary>
public enum DuplicatePropertyNameHandling
{
/// <summary>
/// Replace the existing value when there is a duplicate property. The value of the last property in the JSON object will be used.
/// </summary>
Replace,
/// <summary>
/// Ignore the new value when there is a duplicate property. The value of the first property in the JSON object will be used.
/// </summary>
Ignore,
/// <summary>
/// Throw an <exception cref="ArgumentException"/> when a duplicate property is encountered.
/// </summary>
Error,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
namespace System.Text.Json
{
/// <summary>
/// Represents a boolean JSON value.
/// Represents a mutable boolean JSON value.
/// </summary>
public sealed class JsonBoolean : JsonNode, IEquatable<JsonBoolean>
{
Expand Down
2 changes: 1 addition & 1 deletion src/System.Text.Json/src/System/Text/Json/Node/JsonNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
namespace System.Text.Json
{
/// <summary>
/// The base class that represents a single node within a JSON document.
/// The base class that represents a single node within a mutable JSON document.
/// </summary>
public abstract class JsonNode
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
namespace System.Text.Json
{
/// <summary>
/// Represents a numeric JSON value.
/// Represents a mutable numeric JSON value.
/// </summary>
public sealed class JsonNumber : JsonNode, IEquatable<JsonNumber>
{
Expand Down
Loading