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

feat(send queue): send attachments with the send queue #4195

Merged
merged 12 commits into from
Nov 6, 2024

Conversation

bnjbvr
Copy link
Member

@bnjbvr bnjbvr commented Oct 30, 2024

Fixes #1732. This adds support for sending medias via the send queue, which gives us local echoes for free.

The process looks like this:

  • put the medias (thumbnail and file) into the media cache (with temporary MXC IDs)
  • send a local echo for the media event that references the local media cache
  • if thumbnail
    • create a request to upload the thumbnail
    • create a dependent request to upload the file, depending on the thumbnail being uploaded
    • create a dependent request to send the media event, depending on the file upload
  • otherwise
    • create a request to upload the file
    • create a dependent request to send the media event
  • once the medias have been uploaded, send a local echo edit for the media event, that fixes the MXC IDs to the final ones

The send queue dependency system is made such that, a dependent request depends on at most one other request. The design chosen in this PR is that dependencies are kept linear, each request depends on the previous one to be finished. As such, it means that when uploading a thumbnail, we need to remember the thumbnail MXC ID when uploading the file, because the file request will pass that information (when it's completed) to the media event.


This can not be reviewed commit by commit. Instead, I'd recommend looking at the commit which improves the module doc comment first: 1d0ba02

And then review the rest of the changes; we can also have a call to do the review together.

Sits on top of #4199 and #4200.

@bnjbvr bnjbvr changed the title Bnjbvr/refactor send queue storage feat(send queue): send attachments with the send queue Oct 30, 2024
Copy link

codecov bot commented Oct 30, 2024

Codecov Report

Attention: Patch coverage is 78.32370% with 75 lines in your changes missing coverage. Please review.

Project coverage is 84.82%. Comparing base (0942dab) to head (d1c34b5).
Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
crates/matrix-sdk/src/send_queue.rs 79.14% 34 Missing ⚠️
crates/matrix-sdk/src/send_queue/upload.rs 81.63% 27 Missing ⚠️
crates/matrix-sdk-ui/src/timeline/futures.rs 0.00% 10 Missing ⚠️
...rates/matrix-sdk-ui/src/timeline/controller/mod.rs 0.00% 2 Missing ⚠️
crates/matrix-sdk/src/media.rs 88.88% 1 Missing ⚠️
crates/matrix-sdk/src/room/mod.rs 75.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4195      +/-   ##
==========================================
- Coverage   84.87%   84.82%   -0.06%     
==========================================
  Files         272      273       +1     
  Lines       29251    29519     +268     
==========================================
+ Hits        24828    25040     +212     
- Misses       4423     4479      +56     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bnjbvr bnjbvr force-pushed the bnjbvr/refactor-send-queue-storage branch 2 times, most recently from 19e5936 to 3cbf449 Compare October 31, 2024 17:00
@bnjbvr bnjbvr force-pushed the bnjbvr/refactor-send-queue-storage branch 6 times, most recently from cfb6039 to 51d85b6 Compare November 5, 2024 16:37
@bnjbvr bnjbvr marked this pull request as ready for review November 5, 2024 16:37
@bnjbvr bnjbvr requested a review from a team as a code owner November 5, 2024 16:37
@bnjbvr bnjbvr requested review from poljar and Hywan and removed request for a team and poljar November 5, 2024 16:37
Copy link
Member

@Hywan Hywan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a pair-review via Element Call. I'm approving this PR, it's an excellent code. @bnjbvr has taken notes for things that are left to do, but nothing blocking the merge.

Impressive PR. Well done!

@bnjbvr bnjbvr force-pushed the bnjbvr/refactor-send-queue-storage branch from 13d5de4 to 08ba679 Compare November 6, 2024 13:23
@bnjbvr bnjbvr force-pushed the bnjbvr/refactor-send-queue-storage branch from 38d9612 to d1c34b5 Compare November 6, 2024 14:19
@bnjbvr bnjbvr enabled auto-merge (rebase) November 6, 2024 14:26
@bnjbvr bnjbvr merged commit 8d07f36 into main Nov 6, 2024
40 checks passed
@bnjbvr bnjbvr deleted the bnjbvr/refactor-send-queue-storage branch November 6, 2024 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Media+Timeline] Local echo support for media attachments
2 participants