Implement file sorting and enhance upload handling with fixes#2
Merged
Conversation
… formatting feat: enhance upload handling with zero-byte support and refactor parsing logic
Reviewer's GuideRefactors login handling and shared request utilities, adds safer DEK rewrap logic during password changes, introduces worker-backed and virtualized file table sorting with better handling of unknown file sizes, and tightens upload protocol handling and versioning. Sequence diagram for worker-backed file sortingsequenceDiagram
actor User
participant FilesPage as +page.svelte
participant SortWorkerClient as sort-worker-client.ts
participant FileSortWorker as file-sort.worker.ts
participant Sorting as sorting.ts
User->>FilesPage: loadDirectory / setSort
FilesPage->>FilesPage: applyDirectoryEntries
alt shouldDeferFileSort == false
FilesPage->>Sorting: sortFileEntries
Sorting-->>FilesPage: SortedFileEntries
FilesPage->>FilesPage: update folders, documents
else shouldDeferFileSort == true
FilesPage->>FilesPage: queueDirectorySort
FilesPage->>SortWorkerClient: sortFileEntriesAsync
alt Worker unsupported or threshold not met
SortWorkerClient->>Sorting: sortFileEntries
Sorting-->>SortWorkerClient: SortedFileEntries
SortWorkerClient-->>FilesPage: SortedFileEntries
else Worker available
SortWorkerClient->>SortWorkerClient: getSortWorker
SortWorkerClient->>FileSortWorker: postMessage SortRequest
FileSortWorker->>Sorting: sortFileEntries
Sorting-->>FileSortWorker: SortedFileEntries
FileSortWorker-->>SortWorkerClient: SortResponse
SortWorkerClient-->>FilesPage: SortedFileEntries
end
FilesPage->>FilesPage: update folders, documents (if requestId matches)
end
Sequence diagram for DEK rewrap during password changesequenceDiagram
actor User
participant ChangePassword as change_password
participant State as AppHandleState
participant Auth as get_connection_auth
participant Transfer as rewrap_and_upload_preference_dek
participant Server as set_passwd
User->>ChangePassword: change_password(old_password, new_password)
ChangePassword->>State: read dek
alt dek exists
ChangePassword->>Auth: get_connection_auth
alt auth matches username and token != pending_2fa
ChangePassword->>Transfer: rewrap_and_upload_preference_dek(new_password)
Transfer-->>ChangePassword: encrypted
ChangePassword->>ChangePassword: prepared_dek_rewrap = Some(dek, encrypted,...)
else
ChangePassword->>ChangePassword: prepared_dek_rewrap = None
end
else dek missing
ChangePassword->>ChangePassword: prepared_dek_rewrap = None
end
ChangePassword->>Server: set_passwd
Server-->>ChangePassword: Response
alt response.code != 200 and prepared_dek_rewrap
ChangePassword->>Transfer: rewrap_and_upload_preference_dek(old_password)
alt rollback ok
Transfer-->>ChangePassword: encrypted_old
ChangePassword->>State: remember_server_preference_dek(encrypted_old)
ChangePassword-->>User: Err(response)
else rollback failed
Transfer-->>ChangePassword: Err
ChangePassword-->>User: Err(response + rollback error)
end
else response.code != 200 and no prepared_dek_rewrap
ChangePassword-->>User: Err(response)
else response.code == 200 and prepared_dek_rewrap
ChangePassword->>State: store dek
ChangePassword->>State: remember_server_preference_dek(encrypted)
ChangePassword-->>User: Ok
else response.code == 200 and no prepared_dek_rewrap
ChangePassword-->>User: Ok
end
Flow diagram for upload ready signal parsingflowchart TD
A["parse_ready_signal(ready_str, file_size)"] --> B{"ready_str == stop?"}
B -- Yes --> C{"file_size == 0?"}
C -- Yes --> D["return Ok(None)<br/>(complete zero-byte upload)"]
C -- No --> E["return Err(server sent stop<br/>for a non-empty upload)"]
B -- No --> F["chunk_size = ready_str.strip_prefix('ready ')<br/>.split_whitespace().next()<br/>.parse().unwrap_or(8192)"]
F --> G["return Ok(Some(chunk_size))"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The new
send_typed_action_requesthelper is currently private toshared_helpers.rs, butbrowsing.rscalls it directly; you’ll need to make it visible (e.g.pub(crate)/pub(super)or re-expose it) to avoid a compile error. - In
FileTable.svelte,.file-table-scroll-viewporthasoverflow: hiddenwhen not virtualized, which can prevent vertical scrolling in constrained layouts; consider preservingoverflow-y: auto(or matching the previous behavior) for non-virtualized lists as well.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `send_typed_action_request` helper is currently private to `shared_helpers.rs`, but `browsing.rs` calls it directly; you’ll need to make it visible (e.g. `pub(crate)` / `pub(super)` or re-expose it) to avoid a compile error.
- In `FileTable.svelte`, `.file-table-scroll-viewport` has `overflow: hidden` when not virtualized, which can prevent vertical scrolling in constrained layouts; consider preserving `overflow-y: auto` (or matching the previous behavior) for non-virtualized lists as well.
## Individual Comments
### Comment 1
<location path="crates/transfer/src/upload.rs" line_range="105-108" />
<code_context>
- .and_then(|s| s.split_whitespace().next())
- .and_then(|n| n.parse().ok())
- .unwrap_or(8192);
+ let Some(chunk_size) = parse_ready_signal(&ready_str, file_size)? else {
+ on_progress(0, file_size);
+ return Ok(());
+ };
</code_context>
<issue_to_address>
**suggestion:** Zero-byte uploads report progress as (0, 0) instead of a completed state, which may be surprising for callers.
When `parse_ready_signal` returns `Ok(None)` (zero-byte upload), `send` invokes `on_progress(0, file_size)` and returns `Ok(())`. With `file_size == 0`, the callback always sees `(0, 0)`. If callers treat completion as `(total, total)`, this may not be recognized as finished.
Consider either:
- Invoking `on_progress(file_size, file_size)` specifically for zero-byte completion, or
- Clearly documenting `(0, 0)` as the "completed empty upload" state and ensuring callers handle it accordingly.
```suggestion
let Some(chunk_size) = parse_ready_signal(&ready_str, file_size)? else {
// Zero-byte upload: treat as completed and report (total, total)
on_progress(file_size, file_size);
return Ok(());
};
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+105
to
+108
| let Some(chunk_size) = parse_ready_signal(&ready_str, file_size)? else { | ||
| on_progress(0, file_size); | ||
| return Ok(()); | ||
| }; |
There was a problem hiding this comment.
suggestion: Zero-byte uploads report progress as (0, 0) instead of a completed state, which may be surprising for callers.
When parse_ready_signal returns Ok(None) (zero-byte upload), send invokes on_progress(0, file_size) and returns Ok(()). With file_size == 0, the callback always sees (0, 0). If callers treat completion as (total, total), this may not be recognized as finished.
Consider either:
- Invoking
on_progress(file_size, file_size)specifically for zero-byte completion, or - Clearly documenting
(0, 0)as the "completed empty upload" state and ensuring callers handle it accordingly.
Suggested change
| let Some(chunk_size) = parse_ready_signal(&ready_str, file_size)? else { | |
| on_progress(0, file_size); | |
| return Ok(()); | |
| }; | |
| let Some(chunk_size) = parse_ready_signal(&ready_str, file_size)? else { | |
| // Zero-byte upload: treat as completed and report (total, total) | |
| on_progress(file_size, file_size); | |
| return Ok(()); | |
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Introduce performant, worker-backed file sorting and virtualization for large directories while improving auth handling, upload protocol robustness, and document size semantics.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests: