Skip to content

feat(send_queue): Implement sending of MSC4274 galleries #4977

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.

Sign up for GitHub

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

Merged
bnjbvr merged 25 commits into from
May 27, 2025
Merged

feat(send_queue): Implement sending of MSC4274 galleries #4977

Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
0c3161b
feat(send_queue): Implement sending of MSC4274 galleries
Johennes Apr 25, 2025
d749ebe
Merge branch 'main' into johannes/msc4274-step2
Johennes May 23, 2025
9a62767
Use an OwnedTransactionId to avoid cloning
Johennes May 23, 2025
58626d2
Add missing trailing periods in comments
Johennes May 23, 2025
932dbf9
Move client and cache store outside of loop
Johennes May 23, 2025
c6731ca
Prevent sending empty galleries
Johennes May 23, 2025
47fe934
Remove superfluous type annotations
Johennes May 23, 2025
49c9576
Pre-allocate capacity for item vectors
Johennes May 23, 2025
421f5bd
Extract macro for MessageType / GalleryItemType
Johennes May 23, 2025
b5f4828
Remove superfluous self
Johennes May 23, 2025
7149b73
Rename make_message_event to make_media_event
Johennes May 23, 2025
0e58e50
Bundle items with GalleryConfig
Johennes May 23, 2025
fc4aa7e
Avoid type annotation and pre-allocate vector
Johennes May 23, 2025
e47706e
Avoid unnecessary type annotation
Johennes May 23, 2025
4028bb4
Use let / else to reduce indentation
Johennes May 23, 2025
3acc0e8
Relocate and rephrase comment for FinishGalleryItemInfo
Johennes May 23, 2025
7e7a38c
Extract shared function to update cache keys
Johennes May 23, 2025
aaa392c
Extract method to push thumbnail & media requests
Johennes May 26, 2025
26601a4
Move pushing the dependent media request out of the if / else
Johennes May 26, 2025
b5b6723
Avoid unwrapping last_upload_file_txn
Johennes May 26, 2025
9a491b6
Extract method for caching
Johennes May 26, 2025
6b2b3f0
Merge branch 'main' into johannes/msc4274-step2
Johennes May 26, 2025
946e05a
Rename config parameter to gallery
Johennes May 27, 2025
662d6c7
Remove superfluous blank line
Johennes May 27, 2025
f235e56
Reduce indentation by returning early
Johennes May 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 50 additions & 68 deletions crates/matrix-sdk/src/send_queue/upload.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,79 +105,61 @@ fn update_gallery_event_after_upload(
echo: &mut RoomMessageEventContent,
sent: HashMap<String, AccumulatedSentMediaInfo>,
) {
match &mut echo.msgtype {
MessageType::Gallery(gallery) => {
// Some variants look really similar below, but the `event` and `info` are all
// different types…
for itemtype in gallery.itemtypes.iter_mut() {
match itemtype {
GalleryItemType::Audio(event) => match sent.get(&event.source.unique_key()) {
Some(sent) => event.source = sent.file.clone(),
None => error!(
"key for item {:?} does not exist on gallery event",
&event.source
),
},
GalleryItemType::File(event) => match sent.get(&event.source.unique_key()) {
Some(sent) => {
event.source = sent.file.clone();
if let Some(info) = event.info.as_mut() {
info.thumbnail_source = sent.thumbnail.clone();
}
}
None => error!(
"key for item {:?} does not exist on gallery event",
&event.source
),
},
GalleryItemType::Image(event) => match sent.get(&event.source.unique_key()) {
Some(sent) => {
event.source = sent.file.clone();
if let Some(info) = event.info.as_mut() {
info.thumbnail_source = sent.thumbnail.clone();
}
}
None => error!(
"key for item {:?} does not exist on gallery event",
&event.source
),
},
GalleryItemType::Video(event) => match sent.get(&event.source.unique_key()) {
Some(sent) => {
event.source = sent.file.clone();
if let Some(info) = event.info.as_mut() {
info.thumbnail_source = sent.thumbnail.clone();
}
}
None => error!(
"key for item {:?} does not exist on gallery event",
&event.source
),
},
let MessageType::Gallery(gallery) = &mut echo.msgtype else {
// All `GalleryItemType` created by `Room::make_gallery_item_type` should be
// handled here. The only way to end up here is that a item type has
// been tampered with in the database.
error!("Invalid gallery item types in database");
// Only crash debug builds.
debug_assert!(false, "invalid item type in database {:?}", echo.msgtype());
return;
};

_ => {
// All `GalleryItemType` created by `Room::make_gallery_item_type` should be
// handled here. The only way to end up here is that a item type has
// been tampered with in the database.
error!("Invalid gallery item types in database");
// Only crash debug builds.
debug_assert!(
false,
"invalid gallery item type in database {:?}",
itemtype
);
// Some variants look really similar below, but the `event` and `info` are all
// different types…
for itemtype in gallery.itemtypes.iter_mut() {
match itemtype {
GalleryItemType::Audio(event) => match sent.get(&event.source.unique_key()) {
Comment on lines +124 to +125
Copy link
Copy Markdown
Contributor

@bnjbvr bnjbvr May 27, 2025

Choose a reason for hiding this comment

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

Can we invert the position of the match sent.get(...) and match itemtype? Then, we don't need to repeat it in each match arm.

Copy link
Copy Markdown
Contributor Author

@Johennes Johennes May 27, 2025

Choose a reason for hiding this comment

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

Hm, I think this doesn't work because event used in the inner match is only set through the outer match? We could maybe add an fn source(&self): Option<MediaSource> on GalleryItemType. That would probably have to go into Ruma though?

Copy link
Copy Markdown
Contributor

@bnjbvr bnjbvr May 27, 2025

Choose a reason for hiding this comment

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

Ah you're right; let's defer!

Some(sent) => event.source = sent.file.clone(),
None => error!("key for item {:?} does not exist on gallery event", &event.source),
},
GalleryItemType::File(event) => match sent.get(&event.source.unique_key()) {
Some(sent) => {
event.source = sent.file.clone();
if let Some(info) = event.info.as_mut() {
info.thumbnail_source = sent.thumbnail.clone();
}
}
None => error!("key for item {:?} does not exist on gallery event", &event.source),
},
GalleryItemType::Image(event) => match sent.get(&event.source.unique_key()) {
Some(sent) => {
event.source = sent.file.clone();
if let Some(info) = event.info.as_mut() {
info.thumbnail_source = sent.thumbnail.clone();
}
}
None => error!("key for item {:?} does not exist on gallery event", &event.source),
},
GalleryItemType::Video(event) => match sent.get(&event.source.unique_key()) {
Some(sent) => {
event.source = sent.file.clone();
if let Some(info) = event.info.as_mut() {
info.thumbnail_source = sent.thumbnail.clone();
}
}
None => error!("key for item {:?} does not exist on gallery event", &event.source),
},

_ => {
// All `GalleryItemType` created by `Room::make_gallery_item_type` should be
// handled here. The only way to end up here is that a item type has
// been tampered with in the database.
error!("Invalid gallery item types in database");
// Only crash debug builds.
debug_assert!(false, "invalid gallery item type in database {:?}", itemtype);
}
}
_ => {
// All `GalleryItemType` created by `Room::make_gallery_item_type` should be
// handled here. The only way to end up here is that a item type has
// been tampered with in the database.
error!("Invalid gallery item types in database");
// Only crash debug builds.
debug_assert!(false, "invalid item type in database {:?}", echo.msgtype());
}
}
}

Expand Down