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

WriteFileHttpResponse updates #4446

Merged
merged 3 commits into from
Sep 16, 2023
Merged

Conversation

sfmskywalker
Copy link
Member

@sfmskywalker sfmskywalker commented Sep 16, 2023

  • Support for large zip files by using temp files.
  • Buffered response via FileStreamResult
  • FileStreamResult supports byte range download resumption

=== auto-pr-body ===

Summary
This pull request addresses a variety of refactoring changes, such as moving SendFileStream out of the WriteFileHttpResponse class and into a helper class as well as abstracting common code between SendSingleFileAsync and SendMultipleFileAsync into a new method. Additional modifications to the CreateContentDisposition, WriteResponseAsync, and SendMultipleFilesAsync methods are also included.

List of Changes

  • Added a private method called GetContentType that takes parameters ActivityExecutionContext and string filename and returns a string.
  • Modified existing CreateContentDisposition method to account for the new GetContentType method.
  • Added WriteResponseAsync that takes parameter httpContext instead of httpContext.Response.
  • Removed using System.Net.Http.Headers
  • Added using Microsoft.AspNetCore.Mvc and using Microsoft.AspNetCore.Routing
  • Updated WriteResponseAsync to set status code to HttpStatusCode.OK
  • Updated SendDownloadablesAsync parameter from HttpResponse to HttpContext
  • Updated SendSingleFileAsync parameter from HttpResponse to HttpContext
  • Updated SendMultipleFilesAsync parameter from HttpResponse to HttpContext
  • Added a logging service to SendMultipleFilesAsync
  • Updated SendSingleFileAsync to handle two scenarios for filename and content type
  • Updated SendMultipleFilesAsync to use a temporary file to stream the zip archive back to the client

Refactoring Target

  • Move SendFileStream out of the WriteFileHttpResponse class and into a helper class.
  • Abstract the common code between SendSingleFileAsync and SendMultipleFileAsync into a new method.
  • Move functionality out of SendMultipleFilesAsync that is not related to producing a zip archive (e.g., ILogger).
  • Consider abstracting the GetContentType method into a dedicated class and/or move to a service layer.
  • To ensure better performance, modify the WriteResponseAsync method to accept parameters as needed, and to use an interface to allow for a cleaner and more abstract data access pattern.

@sfmskywalker sfmskywalker added the elsa 3 This issue is specific to Elsa 3 label Sep 16, 2023
@sfmskywalker sfmskywalker merged commit 66a8ad0 into v3 Sep 16, 2023
2 checks passed
@sfmskywalker sfmskywalker deleted the v3-write-file-http-response-buffered branch September 16, 2023 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
elsa 3 This issue is specific to Elsa 3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant