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

Expose a TypedResults method to push a partial response #55753

Open
ranma42 opened this issue May 16, 2024 · 2 comments
Open

Expose a TypedResults method to push a partial response #55753

ranma42 opened this issue May 16, 2024 · 2 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc

Comments

@ranma42
Copy link
Contributor

ranma42 commented May 16, 2024

Background and Motivation

In a project I am developing I have the need to accept ranged requests and provide the client with partial responses (206).
The data for these responses is managed in a chunk-based store which is somewhat awkward to wrap in a Stream.
For full responses, the PushStreamHttpResult is ideal, but it currently does not support partial requests.
It is quite straightforward to implement an IResult that does what I need, but it looks like I would need to re-implement HttpResultsHelper.WriteResultAsFileCore as it is an internal method.

Proposed API

namespace Microsoft.AspNetCore.Http;

public static class TypedResults
{
+    /// <summary>
+    /// Allows writing directly to the response body.
+    /// <para>
+    /// This supports range requests (<see cref="StatusCodes.Status206PartialContent"/> or
+    /// <see cref="StatusCodes.Status416RangeNotSatisfiable"/> if the range is not satisfiable).
+    /// </para>
+    /// </summary>
+    /// <param name="callback">The callback that allows users to write directly to the HTTP response.</param>
+    /// <param name="fileLength">The total length of the file.</param>
+    /// <param name="contentType">The <c>Content-Type</c> of the response. Defaults to <c>application/octet-stream</c>.</param>
+    /// <param name="fileDownloadName">The the file name to be used in the <c>Content-Disposition</c> header.</param>
+    /// <param name="lastModified">The <see cref="DateTimeOffset"/> of when the file was last modified.
+    /// Used to configure the <c>Last-Modified</c> response header and perform conditional range requests.</param>
+    /// <param name="entityTag">The <see cref="EntityTagHeaderValue"/> to be configure the <c>ETag</c> response header
+    /// and perform conditional requests.</param>
+    /// <param name="enableRangeProcessing">Set to <c>true</c> to enable range requests processing.</param>
+    /// <returns>The created <see cref="PushStreamHttpResult"/> for the response.</returns>
+    public static PushStreamHttpResult Stream(
+        Func<RangeItemHeaderValue?, HttpContext, Task> callback,
+        long? fileLength = null,
+        string? contentType = null,
+        string? fileDownloadName = null,
+        DateTimeOffset? lastModified = null,
+        EntityTagHeaderValue? entityTag = null,
+        bool enableRangeProcessing = false);

Usage Examples

// respond with a stream representing the selected part of a resource
// this avoids moving around any useless data
app.MapGet("/example1/{key}", async (string key, IStore store) =>
    TypedResults.Stream(
        async (range, httpContext) => {
            using var stream = range is null
                ? await store.GetContent(key)
                : await store.GetPartialContent(key, range);

            await StreamCopyOperation.CopyToAsync(stream, httpContext.Response.Body, count: null, bufferSize: 64 * 1024, cancel: httpContext.RequestAborted);
        },
        fileLength: await store.GetSize(key)
    );

// respond with chunks representing the selected part of a resource
// the chunks might be "wasting" (in this case they have a fixed size),
// but they might come from a local memory cache
app.MapGet("/example2/{key}", async (string key, IChunkStore store) => {
    var size = await store.GetSize(key);
    return TypedResults.Stream(
        async (range, httpContext) =>
        {
            var chunkSize = store.GetChunkSize();
            var to = (range?.To + 1) ?? size;
            var start = range?.From ?? (size - to);
            var end = range?.From is null ? size : to;

            while (start < end)
            {
                var chunk = await store.GetChunk(start / chunkSize);
                var slice = chunk.AsMemory().Slice(
                    (int)(start % chunkSize),
                    (int)Math.Min(chunkSize, end - start)
                );
                await httpContext.Response.Body.WriteAsync(slice);
                start += slice.Length;
            }
        },
        fileLength: size,
    )
});

Alternative Designs

The proposed API exposes the range information as RangeItemHeaderValue?.
There are several alternatives to represent the range; this type was proposed based on what is used internally/emitted by HttpResultsHelper.WriteResultAsFileCore.

The proposed API returns a PushStreamHttpResult, under the assumption that the same class can be used for both methods implementing responses-pushing-to-body.
This would involve changes to PushStreamHttpResult; another approach would be to write a separate IResult implementation in Microsoft.AspNetCore.Http.HttpResults.

As mentioned in the background, it is possible to develop an independent IResult implementation; this requires duplicating the functionality implemented in HttpResultsHelper.WriteResultAsFileCore.

Another option is to wrap this as a Stream and use the TypedResults.Stream(Stream stream, ...) overload, but this is inconvenient for two reasons:

  • this makes push-based operations harder to express (which I believe is the reason for the existence of TypedResults.Stream(Func<Stream, Task> stream, ...))
  • the async creation

See https://gist.github.com/ranma42/a528555972f16c17ed1b840bfe7fbf5c for an example of the stream hack.

Risks

The API should be an extension of the existing API and AFAICT involves no breaking change.

I have experimented locally with an implementation that extends PushStreamHttpResult; in order to reuse the same class, some minor changes are needed that should not result in performance regressions.
A straightforward implementation would simply replace the callback with a Func<RangeItemHeaderValue?, HttpContext, Task> and wrap the Func<Stream, Task> streamWriterCallback as (range, httpContext) => streamWriterCallback(httpContext.Response.Body).

@ranma42 ranma42 added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label May 16, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label May 16, 2024
@ranma42
Copy link
Contributor Author

ranma42 commented May 18, 2024

I pushed my experiment as https://github.com/ranma42/aspnetcore/tree/partial-push-stream

@davidfowl davidfowl added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions labels May 18, 2024
@ranma42
Copy link
Contributor Author

ranma42 commented Jun 7, 2024

I see that the range has probably been intentionally omitted. #39383 (comment) mentions that it could be implemented manually, but I did not find any straightforward way to do that (without re-implementing the whole range computation machinery). Maybe it would be an interesting example to add in the docs.

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-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc
Projects
None yet
Development

No branches or pull requests

3 participants
@davidfowl @ranma42 and others