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

Support top-level populate methods for JSON (patch) #84018

Open
krwq opened this issue Mar 28, 2023 · 1 comment
Open

Support top-level populate methods for JSON (patch) #84018

krwq opened this issue Mar 28, 2023 · 1 comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json
Milestone

Comments

@krwq
Copy link
Member

krwq commented Mar 28, 2023

#78556 (PR: #83669) is adding support for populating properties. The only missing element is top-level methods allowing scenarios similar to Patch.

Hypothetical usage:

Person p = new()
{
  Name = "John Doe",
  Age = 42,
};

JsonSerializer.PopulateObject("""{"Age": 47}""", typeof(Person), p); // order of args TBD
// p.Name => "John Doe"
// p.Age => 47

class Person
{
  public string Name { get; set; }
  public int Age { get; set; }
}

Example APIs could look like these:

public static partial class JsonSerializer
{
    public static void PopulateObject(System.IO.Stream utf8Json, System.Type type, object target, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
    public static void PopulateObject(System.ReadOnlySpan<byte> utf8Json, System.Type type, object target, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
    public static void PopulateObject([System.Diagnostics.CodeAnalysis.StringSyntaxAttribute("Json")] System.ReadOnlySpan<char> json, System.Type type, object target, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
    public static void PopulateObject([System.Diagnostics.CodeAnalysis.StringSyntaxAttribute("Json")] string json, System.Type type, object target, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
    public static void PopulateObject(ref System.Text.Json.Utf8JsonReader reader, System.Type type, object target, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
    
    public static System.Threading.Tasks.ValueTask PopulateObjectAsync(System.IO.Stream utf8Json, System.Type type, object target, System.Text.Json.JsonSerializerOptions? options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
    public static System.Threading.Tasks.ValueTask PopulateObjectAsync<TValue>(System.IO.Stream utf8Json, TValue target, System.Text.Json.JsonSerializerOptions? options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) where TValue : class { throw null; }
    public static void PopulateObject<TValue>(System.IO.Stream utf8Json, TValue target, System.Text.Json.JsonSerializerOptions? options = null) where TValue : class { throw null; }
    public static void PopulateObject<TValue>(System.ReadOnlySpan<byte> utf8Json, TValue target, System.Text.Json.JsonSerializerOptions? options = null) where TValue : class { throw null; }
    public static void PopulateObject<TValue>([System.Diagnostics.CodeAnalysis.StringSyntaxAttribute("Json")] System.ReadOnlySpan<char> json, TValue target, System.Text.Json.JsonSerializerOptions? options = null) where TValue : class { throw null; }
    public static void PopulateObject<TValue>([System.Diagnostics.CodeAnalysis.StringSyntaxAttribute("Json")] string json, TValue target, System.Text.Json.JsonSerializerOptions? options = null) where TValue : class { throw null; }
    
    public static void PopulateObject<TValue>(ref System.Text.Json.Utf8JsonReader reader, TValue target, System.Text.Json.JsonSerializerOptions? options = null) where TValue : class { throw null; }
    
    // TODO: add also JsonTypeInfo<T> and JsonSerializerContext overloads
}

Open questions

  • structs (if so then we either need object + Type or ref)
  • Do we need/want all overloads? Which one is most generic? (reader?) Which one is most popular (string?) Maybe just these two?

Cost

Better implementations would be to make converters implement Populate directly, I already have a prototype of that here: #79659 - there is still some work left but theoretically should be functional (prototype was started before my PR for properties and needs rebase and integration and bug fixes likely - current version should pass all or most tests).
The simple/hacky implementation could be done if we added 1-2 overloads (i.e. async+ Utf8JsonReader and sync string overloads which take object and type) we should satisfy most scenarios without spending super long on that. Hacky implementation could just create top level property info and pretend root level is actually property. It would be an allocation or two but we don't need to do refactoring (to be fair that refactor is due and will be needed for converters work so I'm fine with any decision we make). The biggest cost will still be testing all overloads and blocking all improper configurations and testing through all possible converters supporting populate (if we restrict refactoring we technically can skip that part as it's already tested with properties).

One factor to mention here is that refactoring is needed if we are to create extension for converters (there are several open issues related to that). I've stubbed out deserialization part of that in the prototype PR. It should make the other work cheaper. Serialization bit needs more thinking before it can be productized but if we start using it internally we should be able to flush out most of the issues.

Alternative APIs:

Known workarounds:
#78556 (comment) - it's far from perfect but allows to unblock if really needed.

cc: @BrunoBlanes @johncrim @dersia @VincentH-Net @eiriktsarpalis @layomia @tarekgh @gregsdennis

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Mar 28, 2023
@ghost
Copy link

ghost commented Mar 28, 2023

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

#78556 is adding support for populating properties. The only missing element is top-level methods allowing scenarios similar to Patch.

Hypothetical usage:

Person p = new()
{
  Name = "John Doe",
  Age = 42,
};

JsonSerializer.PopulateObject("""{"Age": 47}""", typeof(Person), p); // order of args TBD
// p.Age => 47

class Person
{
  public string Name { get; set; }
  public int Age { get; set; }
}

Example APIs could look like these:

public static partial class JsonSerializer
{
    public static void PopulateObject(System.IO.Stream utf8Json, System.Type type, object target, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
    public static void PopulateObject(System.ReadOnlySpan<byte> utf8Json, System.Type type, object target, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
    public static void PopulateObject([System.Diagnostics.CodeAnalysis.StringSyntaxAttribute("Json")] System.ReadOnlySpan<char> json, System.Type type, object target, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
    public static void PopulateObject([System.Diagnostics.CodeAnalysis.StringSyntaxAttribute("Json")] string json, System.Type type, object target, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
    public static void PopulateObject(ref System.Text.Json.Utf8JsonReader reader, System.Type type, object target, System.Text.Json.JsonSerializerOptions? options = null) { throw null; }
    
    public static System.Threading.Tasks.ValueTask PopulateObjectAsync(System.IO.Stream utf8Json, System.Type type, object target, System.Text.Json.JsonSerializerOptions? options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
    public static System.Threading.Tasks.ValueTask PopulateObjectAsync<TValue>(System.IO.Stream utf8Json, TValue target, System.Text.Json.JsonSerializerOptions? options = null, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) where TValue : class { throw null; }
    public static void PopulateObject<TValue>(System.IO.Stream utf8Json, TValue target, System.Text.Json.JsonSerializerOptions? options = null) where TValue : class { throw null; }
    public static void PopulateObject<TValue>(System.ReadOnlySpan<byte> utf8Json, TValue target, System.Text.Json.JsonSerializerOptions? options = null) where TValue : class { throw null; }
    public static void PopulateObject<TValue>([System.Diagnostics.CodeAnalysis.StringSyntaxAttribute("Json")] System.ReadOnlySpan<char> json, TValue target, System.Text.Json.JsonSerializerOptions? options = null) where TValue : class { throw null; }
    public static void PopulateObject<TValue>([System.Diagnostics.CodeAnalysis.StringSyntaxAttribute("Json")] string json, TValue target, System.Text.Json.JsonSerializerOptions? options = null) where TValue : class { throw null; }
    
    public static void PopulateObject<TValue>(ref System.Text.Json.Utf8JsonReader reader, TValue target, System.Text.Json.JsonSerializerOptions? options = null) where TValue : class { throw null; }
    
    // TODO: add also JsonTypeInfo<T> and JsonSerializerContext overloads
}

Open questions:

  • structs (if so then we either need object + Type or ref)
  • Do we need/want all overloads? Which one is most generic? (reader?) Which one is most popular (string?) Maybe just these two?

Alternative APIs:

Known workarounds:
#78556 (comment) - it's far from perfect but allows to unblock if really needed.

cc: @BrunoBlanes @johncrim @dersia @VincentH-Net @eiriktsarpalis @layomia @tarekgh @gregsdennis

Author: krwq
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@krwq krwq added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Mar 28, 2023
@krwq krwq added this to the Future milestone May 30, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json
Projects
None yet
Development

No branches or pull requests

1 participant