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

[API Proposal]: add overloads to support IAsyncEnumerable with source-gen #59268

Closed
Tracked by #63762
steveharter opened this issue Sep 17, 2021 · 6 comments · Fixed by #68985
Closed
Tracked by #63762

[API Proposal]: add overloads to support IAsyncEnumerable with source-gen #59268

steveharter opened this issue Sep 17, 2021 · 6 comments · Fixed by #68985
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Json blocking Marks issues that we want to fast track in order to unblock other important work Cost:S Work that requires one engineer up to 1 week Priority:2 Work that is important, but not critical for the release source-generator Indicates an issue with a source generator feature
Milestone

Comments

@steveharter
Copy link
Member

steveharter commented Sep 17, 2021

Background and motivation

Add overloads to support IAsyncEnumerable with source-gen. See #58221 for more information.

Once implemented, source-gen scenarios using should no longer throw NotSupportedException when using IAsyncEnumerable<T>.

API Proposal

namespace System.Text.Json
{
     public static class JsonSerializer
     {
         public static IAsyncEnumerable<TValue?> DeserializeAsyncEnumerable<TValue>(Stream utf8Json, JsonSerializerOptions options, CancellationToken cancellationToken = default);
+        public static IAsyncEnumerable<TValue?> DeserializeAsyncEnumerable<TValue>(Stream utf8Json, JsonTypeInfo<TValue> jsonTypeInfo, CancellationToken cancellationToken = default);
     }
}

namespace System.Text.Json.Serialization.Metadata
{
    [EditorBrowsable(EditorBrowsableState.Never)]
    public static class JsonMetadataServices
    {
        public static JsonTypeInfo<TCollection> CreateIEnumerableInfo<TCollection, TElement>(JsonSerializerOptions options, JsonCollectionInfoValues<TCollection> collectionInfo) where TCollection : IEnumerable<TElement> {}
+       public static JsonTypeInfo<TAsyncEnumerable> CreateIAsyncEnumerableInfo<TAsyncEnumerable, TElement>(JsonSerializerOptions options, JsonCollectionInfoValues<TAsyncEnumerable> collectionInfo) where TAsyncEnumerable : IAsyncEnumerable<TElement> {}
    }
}

API Usage

JsonSerializer.Serialize(new MyPoco { Data = ... }, MyContext.MyPoco); // Currently not supported

[JsonSerializable(typeof(typeof(MyPoco))]
public class MyContext : JsonSerializerContext {}

public class MyPoco
{
    public IAsyncEnumerable<int> Data { get; set; }
}

Risks

No response

@steveharter steveharter added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json labels Sep 17, 2021
@steveharter steveharter added this to the 7.0.0 milestone Sep 17, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Sep 17, 2021
@ghost
Copy link

ghost commented Sep 17, 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

Add overloads to support IAsyncEnumerable with source-gen. See #58221 for more information.

Once implemented, source-gen scenarios using should no longer throw NotSupportedException when using IAsyncEnumerable<T>.

API Proposal

namespace System.Text.Json
{
     public static class JsonSerializer
     {
+        public static System.Collections.Generic.IAsyncEnumerable<TValue?> DeserializeAsyncEnumerable<TValue>(System.IO.Stream utf8Json, System.Text.Json.Serialization.Metadata.JsonTypeInfo<TValue> jsonTypeInfo, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken));

     }
}

namespace System.Text.Json.Serialization.Metadata
{
    public static class JsonMetadataServices
    {
+        public static System.Text.Json.Serialization.JsonConverter<T?> GetAsyncEnumerableConverter<T>(System.Text.Json.Serialization.Metadata.JsonTypeInfo<T> underlyingTypeInfo);
    }
}

API Usage

// fast-path serialization will generate calls to serialize. For example on a property on a Foo POCO class:
private static void FooSerialize(global::System.Text.Json.Utf8JsonWriter writer, Foo value)
{
    ...
    writer.WritePropertyName(PropName_MyAsyncEnumerable);
    global::System.Text.Json.JsonSerializer.Serialize(writer, value.MyAsyncEnumerable, MySerializationContext.Default.MyAsyncEnumerable);
    ...
}

// and manual calls to deserialize are supported:
System.IO.Stream utf8Json = ...;
IAsyncEnumerable<MyAsyncEnumerable> value = JsonSerializer.DeserializeAsyncEnumerable(stream, MySerializationContext.Default.MyAsyncEnumerable);

Risks

No response

Author: steveharter
Assignees: -
Labels:

api-suggestion, area-System.Text.Json

Milestone: 7.0.0

@eiriktsarpalis eiriktsarpalis removed the untriaged New issue has not been triaged by the area owner label Sep 17, 2021
@layomia layomia added the source-generator Indicates an issue with a source generator feature label Sep 22, 2021
@eiriktsarpalis eiriktsarpalis added Cost:S Work that requires one engineer up to 1 week help wanted [up-for-grabs] Good issue for external contributors Priority:2 Work that is important, but not critical for the release labels Feb 7, 2022
@jeffhandley
Copy link
Member

@pranavkm / @davidfowl -- Would ASP.NET use this in .NET 7 if we included it in one of the later previews? If not, we're leaning toward punting this to Future.

@eiriktsarpalis
Copy link
Member

Exposing top-level methods for IAsyncEnumerable serialization is one thing, but for completeness we also need to consider exposing the IAsyncEnumerable converter in the JsonMetadataServices namespace to ensure parity in scenaria like the ones described in #58221

@eiriktsarpalis eiriktsarpalis removed the help wanted [up-for-grabs] Good issue for external contributors label Apr 20, 2022
@eiriktsarpalis
Copy link
Member

fast-path serialization will generate calls to serialize. For example on a property on a Foo POCO class

FWIW I don't think we would be able to support fast-path serialization for IAsyncEnumerable easily. It would require exposing a SerializeHandler delegate that supports asynchrony, e.g. something like Func<Utf8JsonWriter, TAsyncEnumerable, Task>, but I'm not sure we'd be able to plug this in easily into our existing serialization infrastructure.

@eiriktsarpalis eiriktsarpalis self-assigned this Apr 20, 2022
@eiriktsarpalis
Copy link
Member

I've updated the OP, believe we can take this to API review now.

@eiriktsarpalis eiriktsarpalis added api-ready-for-review API is ready for review, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Apr 20, 2022
@terrajobst
Copy link
Member

terrajobst commented May 5, 2022

Video

Looks good as proposed

namespace System.Text.Json
{
     public partial static class JsonSerializer
     {
        // Existing
        // public static IAsyncEnumerable<TValue?> DeserializeAsyncEnumerable<TValue>(Stream utf8Json, JsonSerializerOptions options, CancellationToken cancellationToken = default);
        public static IAsyncEnumerable<TValue?> DeserializeAsyncEnumerable<TValue>(Stream utf8Json, JsonTypeInfo<TValue> jsonTypeInfo, CancellationToken cancellationToken = default);
     }
}

namespace System.Text.Json.Serialization.Metadata
{
    [EditorBrowsable(EditorBrowsableState.Never)]
    public static class JsonMetadataServices
    {
        // Existing
        // public static JsonTypeInfo<TCollection> CreateIEnumerableInfo<TCollection, TElement>(JsonSerializerOptions options, JsonCollectionInfoValues<TCollection> collectionInfo) where TCollection : IEnumerable<TElement>;
        public static JsonTypeInfo<TAsyncEnumerable> CreateIAsyncEnumerableInfo<TAsyncEnumerable, TElement>(JsonSerializerOptions options, JsonCollectionInfoValues<TAsyncEnumerable> collectionInfo) where TAsyncEnumerable : IAsyncEnumerable<TElement>;
    }
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels May 5, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label May 6, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label May 7, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Jun 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Json blocking Marks issues that we want to fast track in order to unblock other important work Cost:S Work that requires one engineer up to 1 week Priority:2 Work that is important, but not critical for the release source-generator Indicates an issue with a source generator feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants