perf: increase performance for folder and multiple file downloads#1403
perf: increase performance for folder and multiple file downloads#1403JammingBen merged 2 commits intomainfrom
Conversation
99e3acf to
4f6ba3f
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR optimizes file download performance by passing signed URLs directly to the browser instead of manually creating downloadable blobs. This eliminates unnecessary HEAD requests for single file downloads and improves performance, especially for large files and folders.
Key Changes:
- Replaces blob-based download mechanism with direct URL passing to the browser
- Deprecates
isUrlSigningEnabledandsignUrlTimeoutparameters as URL signing is now always supported - Updates test files to remove assertions for deprecated
HEADrequest patterns
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web-pkg/src/services/archiver.ts | Replaces fetch/blob download logic with direct URL handling using window.open() or triggerDownloadWithFilename() |
| packages/web-client/src/webdav/getFileUrl.ts | Deprecates URL signing parameters, removes conditional signing logic, and simplifies URL generation |
| packages/web-pkg/src/helpers/download.ts | Adds default empty string parameter to filename for optional usage |
| packages/web-pkg/src/composables/download/useDownloadFile.ts | Removes doHeadRequest flag and isUrlSigningEnabled parameter from download calls |
| packages/web-pkg/src/composables/appDefaults/useAppFileHandling.ts | Removes unused isUrlSigningEnabled parameter from file URL generation |
| packages/web-pkg/src/composables/piniaStores/capabilities.ts | Adds deprecation notice to supportUrlSigning capability |
| packages/web-pkg/tests/unit/services/archiver.spec.ts | Updates test to verify window.open() instead of URL.createObjectURL() |
| tests/e2e/support/objects/app-files/resource/actions.ts | Removes HEAD request waiting and response collection for version downloads |
| tests/e2e/support/objects/app-files/resource/index.ts | Simplifies return type by removing response array from download method |
| tests/e2e/support/objects/account/actions.ts | Removes HEAD request waiting from GDPR export download |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
ee09285 to
562a14d
Compare
Instead of manually creating a blob which can be downloaded by the browser, we now pass the signed URL directly to the browser which gives it a huge performance boost, especially with large downloads.
* deprecates `isUrlSigningEnabled` since it's always supported by the browser * deprecates `signUrlTimeout` since it has now effect anyways
562a14d to
7a88abf
Compare
|
Looks good to me, but can we think of any issue with that approach. e.G. ttl of the links (data stream gets interrupted while downloading a big file or slow network) and other crazy stuff ? spawning @kulmann |
Hm yeah, but that would be problematic with either approach 🤔 IMO it should be more robust by utilizing the browser's functions instead of doing things via custom JS. |
kulmann
left a comment
There was a problem hiding this comment.
Ooof size is large 😅 Thanks for fixing this!
ttl of the links is long enough IMO. |
Instead of manually creating a blob which can be downloaded by the browser, we now pass the signed URL directly to the browser which gives it a huge performance boost, especially with large downloads. Also saves up one unnecessary
HEADrequest when downloading single files.In addition to that:
isUrlSigningEnabledsince it's always supported by the serversignUrlTimeoutsince it has now effect anyways