improves efficiency parsing Response

[egbertn]

Suggestion: Refactor to Use JsonDocument.ParseAsync for Improved Efficiency

Description

This Pull Request refactors instances of JsonDocument.Parse to JsonDocument.ParseAsync across the project. The new implementation uses await response.Content.ReadAsStreamAsync(Context.RequestAborted) and JsonDocument.ParseAsync to parse JSON content. This change improves performance and memory efficiency by leveraging asynchronous stream reading and parsing.

Reason for Change

Current Implementation

The current implementation reads the entire JSON content as a string before parsing it:

using var payload = JsonDocument.Parse(await response.Content.ReadAsStringAsync(Context.RequestAborted));

Proposed Implementation

The proposed implementation reads the JSON content as a stream and parses it asynchronously:

using var stream = await response.Content.ReadAsStreamAsync(Context.RequestAborted);
using var payload = await JsonDocument.ParseAsync(stream, cancellationToken: Context.RequestAborted);

Benefits

Performance: Asynchronous operations allow for better use of resources and improve the responsiveness of the application.
Memory Efficiency: Reading the content as a stream avoids the overhead of creating an intermediate string representation of the entire response content, reducing memory usage.

Impact

Impact

This refactor is applied across the project, ensuring that all instances where JsonDocument.Parse was used are updated to JsonDocument.ParseAsync. The change should be backward compatible and only improves the performance and efficiency of the existing code.

Example:

before

using var payload = JsonDocument.Parse(await response.Content.ReadAsStringAsync(Context.RequestAborted));

after:

using var stream = await response.Content.ReadAsStreamAsync(Context.RequestAborted);
using var payload = await JsonDocument.ParseAsync(stream, cancellationToken: Context.RequestAborted);

[kevinchalet]

The change should be backward compatible and only improves the performance and efficiency of the existing code.

Actually, it's a bit more complicated than that: JsonDocument.ParseAsync() always expects UTF-8-encoded streams, which isn't something we validate/guarantee in the aspnet-contrib providers (the MSFT-owned providers have the same issue).

Most - sane - implementations return ASCII/UTF-8 responses, but it's not mandatory and OAuth 2.0 servers are free to use any encoding they want for their userinfo implementation (including unsafe encodings like UTF-7 🤣)

JsonDocument.Parse() + ReadAsStringAsync() doesn't have this problem as ReadAsStringAsync() always uses the returned Content-Type header to determine how the response was encoded and returns a proper UTF-16-encoded string JsonDocument.Parse() can directly use.

So, it's possible we broke a provider with that change: to be safe, I'd suggest reverting it and if we really want to improve performance, use System.Net.Http.Json's ReadFromJsonAsync<JsonElement>() helper, that automatically transcodes the response when it's not ASCII/UTF-8 before deserializing the JSON payload. It's what OpenIddict uses: https://github.com/openiddict/openiddict-core/blob/fed1ad3d91055213a25876695700029f34589700/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.cs#L667-L670

[egbertn]

The change should be backward compatible and only improves the performance and efficiency of the existing code.

Actually, it's a bit more complicated than that: JsonDocument.ParseAsync() always expects UTF-8-encoded streams, which isn't something we validate/guarantee in the aspnet-contrib providers (the MSFT-owned providers have the same issue).

Most - sane - implementations return ASCII/UTF-8 responses, but it's not mandatory and OAuth 2.0 servers are free to use any encoding they want for their userinfo implementation (including unsafe encodings like UTF-7 🤣)

JsonDocument.Parse() + ReadAsStringAsync() doesn't have this problem as ReadAsStringAsync() always uses the returned Content-Type header to determine how the response was encoded and returns a proper UTF-16-encoded string JsonDocument.Parse() can directly use.

So, it's possible we broke a provider with that change: to be safe, I'd suggest reverting it and if we really want to improve performance, use System.Net.Http.Json's ReadFromJsonAsync<JsonElement>() helper, that automatically transcodes the response when it's not ASCII/UTF-8 before deserializing the JSON payload. It's what OpenIddict uses: https://github.com/openiddict/openiddict-core/blob/fed1ad3d91055213a25876695700029f34589700/src/OpenIddict.Client.SystemNetHttp/OpenIddictClientSystemNetHttpHandlers.cs#L667-L670

Correct. I did not realize that. Sorry. The closest working solution -if- accepted would be instead of the two lines

using var payload = await response.Content.ReadFromJsonAsync(AdobeIOResponseContext.Default.JsonDocument, Context.RequestAborted) ?? throw new HttpRequestException("Invalid Response Received"); ;

This however, is a refactor requiring quite more work to get the compiler happy.

[martincostello]

If we were to switch to using ReadFromJsonAsync<T>(), we'd need to introduce shared source to remove the need for dozens of copies of the JsonSerializationContext for the JSON source generator, and I'm not convinced that's worth the additional maintenance burden.