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
Add Results.Stream overload that takes a Func<Stream, Task> #39383
Comments
Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:
|
- Func<Stream, Task> streamFactory
+Func<Stream, Task> writeHttpResponseStreamAsync, or something like that. I think we'd spoken about this API when reviewing the |
Thanks for contacting us. We're moving this issue to the |
API review: We decided to add a few more overloads during review. public class Results
{
+ public IResult Stream(
+ Func<Stream, Task> streamWriterCallback,
+ string? contentType = null,
+ string? fileDownloadName = null,
+ DateTimeOffset? lastModified = null,
+ EntityTagHeaderValue? entityTag = null,
+ bool enableRangeProcessing = false);
+ public IResult Stream(
+ PipeReader pipeReader,
+ string? contentType = null,
+ string? fileDownloadName = null,
+ DateTimeOffset? lastModified = null,
+ EntityTagHeaderValue? entityTag = null,
+ bool enableRangeProcessing = false);
+ public IResult Stream(
+ Func<PipeWriter, Task> pipeWriterCallback,
+ string? contentType = null,
+ string? fileDownloadName = null,
+ DateTimeOffset? lastModified = null,
+ EntityTagHeaderValue? entityTag = null,
+ bool enableRangeProcessing = false);
+ public IResult Bytes(
+ ReadOnlyMemory<byte> byteBuffer,
+ string? contentType = null,
+ string? fileDownloadName = null,
+ bool enableRangeProcessing = false,
+ DateTimeOffset? lastModified = null,
+ EntityTagHeaderValue? entityTag = null);
} public class ControllerBase
{
+ public PushFileStreamResult File(
+ Func<Stream, Task> streamWriterCallback,
+ string? contentType = null,
+ string? fileDownloadName = null,
+ DateTimeOffset? lastModified = null,
+ EntityTagHeaderValue? entityTag = null,
+ bool enableRangeProcessing = false);
// This needs to add a remark that the byte buffer
// should not use a pooled source since it's lifetime
// isn't tied to the action result execution.
// Accessing FileContentResult.FileContents should copy the results from the memory
+ public FileContentResult File(
+ ReadOnlyMemory<byte> fileContents,
+ string? contentType = null,
+ string? fileDownloadName = null,
+ bool enableRangeProcessing = false,
+ DateTimeOffset? lastModified = null,
+ EntityTagHeaderValue? entityTag = null);
+ public class PushFileStreamResult : FileResult
+ {
+ public Func<Stream, Task> StreamWriterCallback { get; set; }
+ }
}
We should also add PipeReader / PipeWriter overloads for controllers. @rafikiassumani-msft could we have someone propose the API for this and bring it to review? |
I just tried to implement this and the overloads conflicts because of how we use optional parameters... We might need to rename these or find a clever workaround. |
Actually this is fine, it's just the API analyzer complaining. |
OK having implemented, this I think we need to support an overload of the callbacks that support range processing: public class Results
{
// Range processing overloads
+ public IResult Stream(
+ Func<Stream, long?, long, Task> streamWriterCallback,
+ string? contentType = null,
+ string? fileDownloadName = null,
+ DateTimeOffset? lastModified = null,
+ EntityTagHeaderValue? entityTag = null);
+ public IResult Stream(
+ Func<PipeWriter, long?, long, Task> pipeWriterCallback,
+ string? contentType = null,
+ string? fileDownloadName = null,
+ DateTimeOffset? lastModified = null,
+ EntityTagHeaderValue? entityTag = null);
// Non-Range processing overloads
+ public IResult Stream(
+ Func<Stream, Task> streamWriterCallback,
+ string? contentType = null,
+ string? fileDownloadName = null,
+ DateTimeOffset? lastModified = null,
+ EntityTagHeaderValue? entityTag = null);
+ public IResult Stream(
+ Func<PipeWriter, Task> pipeWriterCallback,
+ string? contentType = null,
+ string? fileDownloadName = null,
+ DateTimeOffset? lastModified = null,
+ EntityTagHeaderValue? entityTag = null);
} |
@pranavkm brought up the fact that we might want a custom delegate type so we can document the arguments of the range processing overloads. The downside is that we need 2 types (one for pipelines and one for streams): Here's a strawman: public delegate Task WriteRangeStreamCallback(Stream stream, long? start, long length);
public delegate Task WriteRangePipeWriterCallback(PipeWriter pipeWriter, long? start, long length); I'm not yet convinced... |
Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:
|
We made a couple of decisions:
|
Triage: We will keep this issue open to give us time to add these results to Web APIs in MVC. |
It looks like the new Can we expect it for .NET 7 ? If not, can we expect it for .NET 8 ? |
|
Since it has both @davidfowl you said
So is it OK to ignore RS0027?
Wouldn't it be wiser to go the same route as all the other File(Func<Stream, Task> streamWriterCallback, string contentType);
File(Func<Stream, Task> streamWriterCallback, string contentType, bool enableRangeProcessing);
File(Func<Stream, Task> streamWriterCallback, string contentType, string? fileDownloadName);
File(Func<Stream, Task> streamWriterCallback, string contentType, string? fileDownloadName, bool enableRangeProcessing);
File(Func<Stream, Task> streamWriterCallback, string contentType, string? fileDownloadName, DateTimeOffset? lastModified, EntityTagHeaderValue entityTag);
File(Func<Stream, Task> streamWriterCallback, string contentType, string? fileDownloadName, DateTimeOffset? lastModified, EntityTagHeaderValue entityTag, bool enableRangeProcessing);
File(Func<Stream, Task> streamWriterCallback, string contentType, DateTimeOffset? lastModified, EntityTagHeaderValue entityTag);
File(Func<Stream, Task> streamWriterCallback, string contentType, DateTimeOffset? lastModified, EntityTagHeaderValue entityTag, bool enableRangeProcessing); |
Edit: I guess one could capture the |
Well, .NET 8 RC1 was released yesterday so I guess this will be postponed to at least .NET 9 or is there still a chance to get it merged for the .NET 8 general availability (GA) release? |
I see that the I'd be willing to help but I'd prefer to get answers to my questions first before diving into the implementation. |
Details about the new process can be found here. |
Thanks @martincostello for pointing out the right document. So if I understand correctly, it's up to @davidfowl (the engineer assigned to this issue) to give directions on how to move forward, right? |
Background and Motivation
There are scenarios where APIs want to work directly on the response stream without buffering. Today the Stream overloads assume you have a stream that you have produced that is then copied to the underlying response stream, instead, we want to provide a result type that gives you access to the underlying response stream.
Example:
https://github.com/khalidabuhakmeh/dotnet-dramameter/blob/main/DramaMeter/Program.cs#L37-L43
Another is writing a blob storage results to the http response.
Proposed API
Usage Examples
Risks
None
The text was updated successfully, but these errors were encountered: