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]: Streaming APIs for the System.Net.Http.Json extensions #87577

Closed
IEvangelist opened this issue Jun 14, 2023 · 8 comments · Fixed by #89258
Closed

[API Proposal]: Streaming APIs for the System.Net.Http.Json extensions #87577

IEvangelist opened this issue Jun 14, 2023 · 8 comments · Fixed by #89258
Assignees
Labels
api-approved API was approved in API review, it can be implemented area-System.Text.Json
Milestone

Comments

@IEvangelist
Copy link
Member

IEvangelist commented Jun 14, 2023

Background and motivation

As of .NET 8 preview 5 (and starting with .NET 5), there are various extension methods within the System.Net.Http.Json namespace (specifically the System.Net.Http.Json NuGet package 📦) that expose JSON-centric functionality to conveniently encapsulate common workflows. Such as, but not limited to extending the HttpClient:

  • GetFromJsonAsync
  • PostAsJsonAsync
  • PutAsJsonAsync
  • PatchAsJsonAsync
  • DeleteFromJsonAsync

All of these APIs are Task-like returning, i.e., either Task<TValue> or ValueTask<TValue> However, there are no extension methods for some of the streaming APIs, that return IAsyncEnumerable<TValue>. While this is obviously possible without the use of in-built extension methods, it would be really convenient for the runtime to include said extensions.

Ideally, we could encapsulate all of the logic required to efficiently delegate JSON-streaming calls to the JsonSerializer.DeserializeAsyncEnumerable functionality.

API Proposal

This would be scoped to GET on the HttpClient.

namespace System.Net.Http.Json;

public static partial class HttpClientJsonExtensions
{
    public static IAsyncEnumerable<TValue?> GetFromJsonAsAsyncEnumerable<TValue>(
        this HttpClient client,
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri,
        JsonSerializerOptions? options,
        CancellationToken cancellationToken = default) { }

    public static IAsyncEnumerable<TValue?> GetFromJsonAsAsyncEnumerable<TValue>(
        this HttpClient client,
        Uri? requestUri,
        JsonSerializerOptions? options,
        CancellationToken cancellationToken = default) { }

    public static IAsyncEnumerable<TValue?> GetFromJsonAsAsyncEnumerable<TValue>(
        this HttpClient client,
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri,
        JsonTypeInfo<TValue> jsonTypeInfo,
        CancellationToken cancellationToken = default) { }

    public static IAsyncEnumerable<TValue?> GetFromJsonAsAsyncEnumerable<TValue>(
        this HttpClient client,
        Uri? requestUri,
        JsonTypeInfo<TValue> jsonTypeInfo,
        CancellationToken cancellationToken = default) { }

    public static IAsyncEnumerable<TValue?> GetFromJsonAsAsyncEnumerable<TValue>(
        this HttpClient client,
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri,
        CancellationToken cancellationToken = default) { }

    public static IAsyncEnumerable<TValue?> GetFromJsonAsAsyncEnumerable<TValue>(
        this HttpClient client,
        Uri? requestUri,
        CancellationToken cancellationToken = default) { }
}

And the HttpContent:

public static partial class HttpContentJsonExtensions
{
    public static IAsyncEnumerable<T?> ReadFromJsonAsAsyncEnumerable<T>(
        this HttpContent content, 
        JsonSerializerOptions? options, 
        CancellationToken cancellationToken = default) { }

    public static IAsyncEnumerable<T?> ReadFromJsonAsAsyncEnumerable<T>(
        this HttpContent content, 
        CancellationToken cancellationToken = default) { }

    public static IAsyncEnumerable<T?> ReadFromJsonAsAsyncEnumerable<T>(
        this HttpContent content,
        JsonTypeInfo<T> jsonTypeInfo,
        CancellationToken cancellationToken = default) { }
}
Potential Implementation This would essentially encapsulate the following logic:
public static class HttpRequestJsonExtensions
{
    public static async IAsyncEnumerable<TValue?> GetFromJsonAsAsyncEnumerable<TValue>(
        this HttpClient client, 
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri, 
        JsonSerializerOptions? options, 
        [EnumeratorCancellation] CancellationToken cancellationToken = default)
    {
        ArgumentNullException.ThrowIfNull(client);

        TimeSpan timeout = client.Timeout;

        // Create the CTS before the initial SendAsync so that the SendAsync counts against the timeout.
        CancellationTokenSource? linkedCTS = null;
        if (timeout != Timeout.InfiniteTimeSpan)
        {
            linkedCTS = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
            linkedCTS.CancelAfter(timeout);
        }

        Task<HttpResponseMessage> responseTask;
        try
        {
            // Intentionally using cancellationToken instead of the linked one 
            // here as HttpClient will enforce the Timeout on its own for this part
            responseTask = client.GetAsync(
                CreateUri(requestUri), HttpCompletionOption.ResponseHeadersRead, cancellationToken);
        }
        catch
        {
            linkedCTS?.Dispose();
            throw;
        }

        using HttpResponseMessage response = await responseTask.ConfigureAwait(false);
        response.EnsureSuccessStatusCode();

        using Stream contentStream =
            await response.Content
                .ReadAsStreamAsync(cancellationToken)
                .ConfigureAwait(false);

        await foreach (TValue? value in JsonSerializer.DeserializeAsyncEnumerable<TValue>(
            contentStream, options, cancellationToken).ConfigureAwait(false))
        {
            yield return value;
        }
    }

    private static Uri? CreateUri(string? uri) =>
       string.IsNullOrEmpty(uri) ? null : new Uri(uri, UriKind.RelativeOrAbsolute);
}

I'd love to be the one to implement this, if it's approved.

API Usage

// Imagine an ASP.NET Core Minimal API that returns an IAsyncEnumerable<T> where T is a `TimeSeries`.
public record class TimeSeries(string Name, DateTime dateTime, decimal Value);

// Imagine that this is the consuming code.
HttpClient client = new();
await foreach (var timeSeries in client.GetFromJsonAsAsyncEnumerable<TimeSeries>("api/streaming/endpoint"))
{
    // Use timeSeries values
}

Alternative Designs

No response

Risks

No response

@IEvangelist IEvangelist added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jun 14, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Jun 14, 2023
@ghost
Copy link

ghost commented Jun 14, 2023

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

Issue Details

Background and motivation

As of .NET 8 preview 5, there are various extension methods within the System.Net.Http.Json namespace (specifically the System.Net.Http.Json NuGet package 📦) that expose JSON-centric functionality to conveniently encapsulate common workflows. Such as, but not limited to extending the HttpClient:

  • GetFromJsonAsync
  • PostAsJsonAsync
  • PutAsJsonAsync
  • PatchAsJsonAsync
  • DeleteFromJsonAsync

All of these APIs are Task-like returning, i.e.; either Task<T> or ValueTask<T> However, there are no extension methods for some of the streaming APIs, IAsyncEnumerable<T>.

API Proposal

namespace System.Net.Http.Json;

public static partial class HttpClientJsonExtensions
{
    static IAsyncEnumerable<TValue> GetFromJsonStreamAsync<TValue>(
        this HttpClient client,
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri,
        JsonSerializerOptions? options,
        CancellationToken cancellationToken = default) { }
}

API Usage

// Imagine an ASP.NET Core Minimal API that returns an IAsyncEnumerable<T> where T is a `TimeSeries`.
public record class TimeSeries(string Name, DateTime dateTime, decimal Value);

// Imagine that this is the consuming code.
HttpClient client = new();
await foreach (var timeSeries in client.GetFromJsonStreamAsync<TimeSeries>("api/streaming/endpoint"))
{
    // Use timeSeries values
}

Alternative Designs

No response

Risks

No response

Author: IEvangelist
Assignees: -
Labels:

api-suggestion, area-System.Net.Http

Milestone: -

@ghost
Copy link

ghost commented Jun 15, 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

Background and motivation

As of .NET 8 preview 5, there are various extension methods within the System.Net.Http.Json namespace (specifically the System.Net.Http.Json NuGet package 📦) that expose JSON-centric functionality to conveniently encapsulate common workflows. Such as, but not limited to extending the HttpClient:

  • GetFromJsonAsync
  • PostAsJsonAsync
  • PutAsJsonAsync
  • PatchAsJsonAsync
  • DeleteFromJsonAsync

All of these APIs are Task-like returning, i.e.; either Task<T> or ValueTask<T> However, there are no extension methods for some of the streaming APIs, IAsyncEnumerable<T>.

API Proposal

namespace System.Net.Http.Json;

public static partial class HttpClientJsonExtensions
{
    static IAsyncEnumerable<TValue> GetFromJsonStreamAsync<TValue>(
        this HttpClient client,
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri,
        JsonSerializerOptions? options,
        CancellationToken cancellationToken = default) { }
}

This would essentially encapsulate the following logic:

public static class HttpRequestJsonExtensions
{
    public static async IAsyncEnumerable<TValue?> GetFromJsonStreamAsync<TValue>(
        this HttpClient client, 
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri, 
        JsonSerializerOptions? options, 
        [EnumeratorCancellation] CancellationToken cancellationToken = default)
    {
        ArgumentNullException.ThrowIfNull(client);

        TimeSpan timeout = client.Timeout;

        // Create the CTS before the initial SendAsync so that the SendAsync counts against the timeout.
        CancellationTokenSource? linkedCTS = null;
        if (timeout != Timeout.InfiniteTimeSpan)
        {
            linkedCTS = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
            linkedCTS.CancelAfter(timeout);
        }

        // We call SendAsync outside of the async Core method to propagate exception even without awaiting the returned task.
        Task<HttpResponseMessage> responseTask;
        try
        {
            // Intentionally using cancellationToken instead of the linked one here as HttpClient will enforce the Timeout on its own for this part
            responseTask = client.GetAsync(CreateUri(requestUri), HttpCompletionOption.ResponseHeadersRead, cancellationToken);
        }
        catch
        {
            linkedCTS?.Dispose();
            throw;
        }

        using HttpResponseMessage response = await responseTask.ConfigureAwait(false);
        response.EnsureSuccessStatusCode();

        using Stream contentStream =
            await response.Content
                .ReadAsStreamAsync(cancellationToken)
                .ConfigureAwait(false);

        await foreach (TValue? value in JsonSerializer.DeserializeAsyncEnumerable<TValue>(contentStream, options, cancellationToken).ConfigureAwait(false))
        {
            yield return value;
        }
    }

    private static Uri? CreateUri(string? uri) =>
       string.IsNullOrEmpty(uri) ? null : new Uri(uri, UriKind.RelativeOrAbsolute);
}

I'd love to be the one to implement this, if it's approved.

API Usage

// Imagine an ASP.NET Core Minimal API that returns an IAsyncEnumerable<T> where T is a `TimeSeries`.
public record class TimeSeries(string Name, DateTime dateTime, decimal Value);

// Imagine that this is the consuming code.
HttpClient client = new();
await foreach (var timeSeries in client.GetFromJsonStreamAsync<TimeSeries>("api/streaming/endpoint"))
{
    // Use timeSeries values
}

Alternative Designs

No response

Risks

No response

Author: IEvangelist
Assignees: -
Labels:

api-suggestion, area-System.Text.Json, untriaged

Milestone: -

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Jun 15, 2023

I think it's something we could consider for GET and maybe POST, but not PUT, PATCH or DELETE since these typically don't produce responses that could be streamed. We should also include an overload accepting JsonTypeInfo for AOT apps:

public static partial class HttpClientJsonExtensions
{
    public static IAsyncEnumerable<TValue> GetFromJsonStreamAsync<TValue>(
        this HttpClient client,
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri,
        JsonSerializerOptions? options,
        CancellationToken cancellationToken = default) { }

    public static IAsyncEnumerable<TValue> GetFromJsonStreamAsync<TValue>(
        this HttpClient client,
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri,
        JsonTypeInfo<TValue> jsonTypeInfo,
        CancellationToken cancellationToken = default) { }
}

@eiriktsarpalis eiriktsarpalis removed the untriaged New issue has not been triaged by the area owner label Jun 15, 2023
@eiriktsarpalis eiriktsarpalis added this to the Future milestone Jun 15, 2023
@IEvangelist
Copy link
Member Author

Per request by @eiriktsarpalis, here is a POC branch, and the commit with the proposed additions: main...IEvangelist:runtime:streaming-json-http-apis.

@eiriktsarpalis
Copy link
Member

Looks good overall, couple of remarks:

  1. Looking at the existing GetFromJsonAsync overloads it seems we also need a JsonSerializerContext overload for consistency?
  2. Following naming convention from the JsonSerializer class, I think the new method group should be called GetFromJsonAsAsyncEnumerable.

@IEvangelist
Copy link
Member Author

I've updated my POC branch, and the API proposal description with the suggested names. As for the JsonSerializerContext overloads, those are specifically on the object? overloads which don't feel appropriate here.

@eiriktsarpalis eiriktsarpalis added the api-ready-for-review API is ready for review, it is NOT ready for implementation label Jun 15, 2023
@eiriktsarpalis eiriktsarpalis modified the milestones: Future, 8.0.0 Jun 15, 2023
@eiriktsarpalis eiriktsarpalis removed the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jun 15, 2023
@IEvangelist IEvangelist added blocking Marks issues that we want to fast track in order to unblock other important work and removed blocking Marks issues that we want to fast track in order to unblock other important work labels Jul 11, 2023
@dersia
Copy link

dersia commented Jul 13, 2023

I think it's something we could consider for GET and maybe POST, but not PUT, PATCH or DELETE since these typically don't produce responses that could be streamed. We should also include an overload accepting JsonTypeInfo for AOT apps:

public static partial class HttpClientJsonExtensions
{
    public static IAsyncEnumerable<TValue> GetFromJsonStreamAsync<TValue>(
        this HttpClient client,
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri,
        JsonSerializerOptions? options,
        CancellationToken cancellationToken = default) { }

    public static IAsyncEnumerable<TValue> GetFromJsonStreamAsync<TValue>(
        this HttpClient client,
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri,
        JsonTypeInfo<TValue> jsonTypeInfo,
        CancellationToken cancellationToken = default) { }
}

Although I agree, that PUT, PATCH and DELETE shouldn't produce output, I have seen too many projects where those endpoints still create stream able output. So I would suggest to add those for completion.

@terrajobst
Copy link
Member

terrajobst commented Jul 18, 2023

Video

  • Makes sense as proposed
    • We believe the other verbs POST, PUT, DELETE are less useful. Users can dispatch to the extension methods on the HttpContent.
namespace System.Net.Http.Json;

public static partial class HttpClientJsonExtensions
{
    public static IAsyncEnumerable<TValue?> GetFromJsonAsAsyncEnumerable<TValue>(
        this HttpClient client,
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri,
        JsonSerializerOptions? options,
        CancellationToken cancellationToken = default);

    public static IAsyncEnumerable<TValue?> GetFromJsonAsAsyncEnumerable<TValue>(
        this HttpClient client,
        Uri? requestUri,
        JsonSerializerOptions? options,
        CancellationToken cancellationToken = default);

    public static IAsyncEnumerable<TValue?> GetFromJsonAsAsyncEnumerable<TValue>(
        this HttpClient client,
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri,
        JsonTypeInfo<TValue> jsonTypeInfo,
        CancellationToken cancellationToken = default);

    public static IAsyncEnumerable<TValue?> GetFromJsonAsAsyncEnumerable<TValue>(
        this HttpClient client,
        Uri? requestUri,
        JsonTypeInfo<TValue> jsonTypeInfo,
        CancellationToken cancellationToken = default);

    public static IAsyncEnumerable<TValue?> GetFromJsonAsAsyncEnumerable<TValue>(
        this HttpClient client,
        [StringSyntax(StringSyntaxAttribute.Uri)] string? requestUri,
        CancellationToken cancellationToken = default);

    public static IAsyncEnumerable<TValue?> GetFromJsonAsAsyncEnumerable<TValue>(
        this HttpClient client,
        Uri? requestUri,
        CancellationToken cancellationToken = default);
}
public static partial class HttpContentJsonExtensions
{
    public static IAsyncEnumerable<T?> ReadFromJsonAsAsyncEnumerable<T>(
        this HttpContent content, 
        JsonSerializerOptions? options, 
        CancellationToken cancellationToken = default);

    public static IAsyncEnumerable<T?> ReadFromJsonAsAsyncEnumerable<T>(
        this HttpContent content, 
        CancellationToken cancellationToken = default);

    public static IAsyncEnumerable<T?> ReadFromJsonAsAsyncEnumerable<T>(
        this HttpContent content,
        JsonTypeInfo<T> jsonTypeInfo,
        CancellationToken cancellationToken = default);
}

@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 Jul 18, 2023
IEvangelist added a commit to IEvangelist/runtime that referenced this issue Jul 20, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jul 20, 2023
eiriktsarpalis added a commit that referenced this issue Jul 24, 2023
#89258)

* Contributes to #87577

* More updates

* Added unit tests, generated ref, and minor clean up

* Added missing triple slash comments

* Update src/libraries/System.Net.Http.Json/tests/FunctionalTests/HttpClientJsonExtensionsTests.cs

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>

* Correct the preprocessor directives, and delegate to JsonTypeInfo overload - per peer feedback.

* Refactor for deferred execution, remove helper methods since they're no longer needed

* Add test to ensure deferred execution semantics, updates from Miha.

* Apply suggestions from code review

Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>

* Update src/libraries/System.Net.Http.Json/src/System/Net/Http/Json/HttpClientJsonExtensions.Get.AsyncEnumerable.cs

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>

* Update test per Miha's feedback

* A few more updates from peer review, Eirik's nits.

* No need for the length limit read stream.

* Add limit per invocation, not element. Share length limit read stream logic.

---------

Co-authored-by: Eirik Tsarpalis <eirik.tsarpalis@gmail.com>
Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 24, 2023
@ghost ghost locked as resolved and limited conversation to collaborators Aug 23, 2023
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants