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

Developers should have access to System.Text.Json's default internal converters #63791

Open
4 tasks
Tracked by #77018
eiriktsarpalis opened this issue Jan 14, 2022 · 8 comments
Open
4 tasks
Tracked by #77018
Labels
area-System.Text.Json Team:Libraries User Story A single user-facing feature. Can be grouped under an epic.
Milestone

Comments

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Jan 14, 2022

Background and Motivation

Currently, the default json converters used for objects and collection types are internal to System.Text.Json. The library uses hardcoded conventions to decide what default converters to apply to each type: for instance classes that implement IEnumerable<T> are automatically treated as collections, and classes that implement IAsyncEnumerable<T> can only be handled by the async-only IAsyncEnumerable converter. As of today, System.Text.Json offers no mechanism for users to override this convention, and we have received a number of questions from the community about this issue:

Currently the best available workaround is for users to either reimplement the default converters, or wrap/cast the values into a type that produces the desired serialization contract. At the same time, we offer no mechanism for users to tweak the serialization behaviour of the default converters other than using attribute annotations (to be ameliorated once #63686 has been implemented). Related issues:

Proposal

This story proposes making the default internal converters public, so that users are free to opt in to specific converter strategies for their types. Consider the following example:

public class MyClass : IEnumerable<int>, IAsyncEnumerable<int>
{
}

By default, members of type MyClass are serialized as IAsyncEnumerables. Users can still force IEnumerable semantics if they cast the instance to IEnumerable<int>, however there is currently no way to serialize the type as an object. By exposing the default converter types, we can decorate our type definition with attributes:

[JsonConverter(typeof(JsonObjectConverter))] // serialize as object
// [JsonConverter(typeof(JsonCollectionConverter))] // serialize as collection
// [JsonConverter(typeof(JsonAsyncEnumerableConverter)] // serialize as IAsyncEnumerable
public class MyClass : IEnumerable<int>, IAsyncEnumerable<int>
{
}

Moreover, we should consider exposing parameterization on the converters so that users can configure the contract (for example what #38514 is asking for).

NB we should update the source generator so that relevant JsonPropertyAttribute annotations are recognized and trigger relevant metadata generation, cf. #82001 (comment)

Progress

  • Prototyping work
  • API proposal
  • Implementation & tests
  • Documentation
@eiriktsarpalis eiriktsarpalis added area-System.Text.Json User Story A single user-facing feature. Can be grouped under an epic. Priority:2 Work that is important, but not critical for the release Cost:S Work that requires one engineer up to 1 week Team:Libraries labels Jan 14, 2022
@eiriktsarpalis eiriktsarpalis added this to the 7.0.0 milestone Jan 14, 2022
@ghost
Copy link

ghost commented Jan 14, 2022

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

Issue Details

Background and Motivation

Currently, the default json converters used for objects and collection types are internal to System.Text.Json. The library uses hardcoded conventions to decide what default converters to apply to each type: for instance classes that implement IEnumerable<T> are automatically treated as collections, and classes that implement IAsyncEnumerable<T> can only be handled by the async-only IAsyncEnumerable converter. As of today, System.Text.Json offers no mechanism for users to override this convention, and we have received a number of questions from the community about this issue:

Currently the best available workaround is for users to either reimplement the default converters, or wrap/cast the values into a type that produces the desired serialization contract. At the same time, we offer no mechanism for users to tweak the serialization behaviour of the default converters other than using attribute annotations (to be ameliorated once #63686 has been implemented). Related issues:

Proposal

This story proposes making the default internal converters public, so that users are free to opt in to specific converter strategies for their types. Consider the following example:

public class MyClass : IEnumerable<int>, IAsyncEnumerable<int>
{
}

By default, members of type MyClass are serialized as IAsyncEnumerables. Users can still force IEnumerable semantics if they cast the instance to IEnumerable<int>, however there is currently no way to serialize the type as an object. By exposing the default converter types, we can decorate our type definition with attributes:

[JsonConverter(typeof(JsonObjectConverter))] // serialize as object
// [JsonConverter(typeof(JsonCollectionConverter))] // serialize as collection
// [JsonConverter(typeof(JsonAsyncEnumerableConverter)] // serialize as IAsyncEnumerable
public class MyClass : IEnumerable<int>, IAsyncEnumerable<int>
{
}

Moreover, we should consider exposing parameterization on the converters so that users can configure the contract (for example what #38514 is asking for).

Progress

  • Prototyping work
  • API proposal
  • Implementation & tests
  • Documentation
Author: eiriktsarpalis
Assignees: -
Labels:

area-System.Text.Json, User Story, Priority:2, Cost:S, Team:Libraries

Milestone: 7.0.0

@asik
Copy link

asik commented Mar 31, 2022

Related to #67361 , for me the ideal scenario would be that JsonConverter sees that it's an IEnumerable<T>, sees that it has a constructor that takes in an IEnumerable<T>, and uses that constructor.

@eiriktsarpalis
Copy link
Member Author

Related to #67361 , for me the ideal scenario would be that JsonConverter sees that it's an IEnumerable<T>, sees that it has a constructor that takes in an IEnumerable<T>, and uses that constructor.

Yes, that might be a good convention to honor. Similarly for dictionary types that accept IEnumerable<KeyValuePair<TKey, TValue>> parameters.

@eiriktsarpalis
Copy link
Member Author

We likely won't have time to work on this before .NET 7 is done, moving to 8.0.0

@eiriktsarpalis eiriktsarpalis removed Priority:2 Work that is important, but not critical for the release Cost:S Work that requires one engineer up to 1 week labels Jan 23, 2023
@eiriktsarpalis
Copy link
Member Author

eiriktsarpalis commented Jan 23, 2023

Moving to Future as we won't be able to work on this for 8.0

@MooVC
Copy link

MooVC commented Jan 25, 2023

What is the suggested workaround in this case? I've a type that wraps an array (private field) that implements IEnumerable of that type. I would like it to be serialised as an object, using a public constructor that accepts an array. The type is intended to be immutable, so it would be counterintuitive to allow for collection overrides e.g. Add.

@eiriktsarpalis
Copy link
Member Author

The only workaround for now is authoring a custom converter for the type, unfortunately.

@paul-michalik
Copy link

paul-michalik commented Feb 28, 2023

What is the suggested workaround in this case? I've a type that wraps an array (private field) that implements IEnumerable of that type. I would like it to be serialised as an object, using a public constructor that accepts an array. The type is intended to be immutable, so it would be counterintuitive to allow for collection overrides e.g. Add.

I am currently solving this by deriving such types from List<T>. This provides the expected behavior (transparent serialization and deserialization as collection of T) but of course adds some risk if the collection should not be manipulated as, well List<T>. I use this for internal types only, so it is mostly harmless.

I still do not understand what the problem is with implementing a default deserialization converter for types implementing e.g. ICollection<T>: the documentation says it all, this is exactly how it should behave:

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Text.Json Team:Libraries User Story A single user-facing feature. Can be grouped under an epic.
Projects
None yet
Development

No branches or pull requests

5 participants