-
Notifications
You must be signed in to change notification settings - Fork 408
feat(send_queue): send redactions via the send queue #6250
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
base: main
Are you sure you want to change the base?
Changes from 9 commits
c288212
ba8f1bb
9fbe8d3
40f42ff
08e764b
d34a773
1d21e85
9719663
75896b2
de1fae3
5007d2f
112ea05
44bc666
4560ac8
b2007a6
a25d0ed
8e60190
6a4a0af
78c4707
fc3fa36
78eb03d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1268,6 +1268,23 @@ impl<P: RoomDataProvider> TimelineController<P> { | |
| LocalEchoContent::React { key, send_handle, applies_to } => { | ||
| self.handle_local_reaction(key, send_handle, applies_to).await; | ||
| } | ||
|
|
||
| LocalEchoContent::Redaction { redacts, send_error, .. } => { | ||
| self.handle_local_redaction(echo.transaction_id.clone(), redacts).await; | ||
|
|
||
| if let Some(send_error) = send_error { | ||
| self.update_event_send_state( | ||
| &echo.transaction_id, | ||
| EventSendState::SendingFailed { | ||
| error: Arc::new(matrix_sdk::Error::SendQueueWedgeError(Box::new( | ||
| send_error, | ||
| ))), | ||
| is_recoverable: false, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't this bool be based on a field of
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, I think not. This is actually copied from the |
||
| }, | ||
| ) | ||
| .await; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -1308,6 +1325,34 @@ impl<P: RoomDataProvider> TimelineController<P> { | |
| tr.commit(); | ||
| } | ||
|
|
||
| /// Applies a local echo of a redaction. | ||
| pub(super) async fn handle_local_redaction( | ||
| &self, | ||
| txn_id: OwnedTransactionId, | ||
| redacts: OwnedEventId, | ||
| ) { | ||
| let mut state = self.state.write().await; | ||
| let mut tr = state.transaction(); | ||
|
|
||
| let target = TimelineEventItemId::EventId(redacts); | ||
|
|
||
| let aggregation = Aggregation::new( | ||
| TimelineEventItemId::TransactionId(txn_id), | ||
| AggregationKind::Redaction { is_local: true }, | ||
| ); | ||
|
|
||
| tr.meta.aggregations.add(target.clone(), aggregation.clone()); | ||
| find_item_and_apply_aggregation( | ||
| &tr.meta.aggregations, | ||
| &mut tr.items, | ||
| &target, | ||
| aggregation, | ||
| &tr.meta.room_version_rules, | ||
| ); | ||
|
|
||
| tr.commit(); | ||
| } | ||
|
|
||
| /// Handle a single room send queue update. | ||
| pub(crate) async fn handle_room_send_queue_update(&self, update: RoomSendQueueUpdate) { | ||
| match update { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,8 +78,13 @@ pub struct EventTimelineItem { | |
| pub(super) forwarder_profile: Option<TimelineDetails<Profile>>, | ||
| /// The timestamp of the event. | ||
| pub(super) timestamp: MilliSecondsSinceUnixEpoch, | ||
| /// The content of the event. | ||
| /// The content of the event. Might be redacted if a redaction for this | ||
| /// event is currently being sent or has been received from the server. | ||
| pub(super) content: TimelineItemContent, | ||
| /// If a redaction for this event is currently being sent but the server | ||
| /// hasn't yet acknowledged it via its remote echo, the original content | ||
| /// before redaction. Otherwise, None. | ||
| pub(super) unredacted_content: Option<TimelineItemContent>, | ||
|
Comment on lines
+85
to
+89
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I realize I forgot to ask the previous time, but could we store the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, I'm probably entirely misunderstanding but does
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or wait, you mean move Sorry, I'm most likely just being dense. 🙈
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, sorry, my comment was about moving the unredacted content field inside
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, I see. I gave it a shot in 4560ac8 but I'm not sure how well it turned out. A part of the change bled through to the FFI layer and it looks like the recursive type might cause some more issues there since neither |
||
| /// The kind of event timeline item, local or remote. | ||
| pub(super) kind: EventTimelineItemKind, | ||
| /// Whether or not the event belongs to an encrypted room. | ||
|
|
@@ -135,6 +140,7 @@ impl EventTimelineItem { | |
| forwarder_profile, | ||
| timestamp, | ||
| content, | ||
| unredacted_content: None, | ||
| kind, | ||
| is_room_encrypted, | ||
| } | ||
|
|
@@ -478,7 +484,7 @@ impl EventTimelineItem { | |
| } | ||
|
|
||
| /// Create a clone of the current item, with content that's been redacted. | ||
| pub(super) fn redact(&self, rules: &RedactionRules) -> Self { | ||
| pub(super) fn redact(&self, rules: &RedactionRules, is_local: bool) -> Self { | ||
| let content = self.content.redact(rules); | ||
| let kind = match &self.kind { | ||
| EventTimelineItemKind::Local(l) => EventTimelineItemKind::Local(l.clone()), | ||
|
|
@@ -491,6 +497,29 @@ impl EventTimelineItem { | |
| forwarder_profile: self.forwarder_profile.clone(), | ||
| timestamp: self.timestamp, | ||
| content, | ||
| unredacted_content: is_local.then(|| self.content.clone()), | ||
| kind, | ||
| is_room_encrypted: self.is_room_encrypted, | ||
| } | ||
| } | ||
|
|
||
| /// Create a clone of the current item, with content restored from the | ||
| /// item's unredacted_content field (if it was previously set by a call to | ||
| /// the `redact(...)` method). | ||
| pub(super) fn unredact(&self) -> Self { | ||
Johennes marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| let Some(content) = &self.unredacted_content else { return self.clone() }; | ||
| let kind = match &self.kind { | ||
| EventTimelineItemKind::Local(l) => EventTimelineItemKind::Local(l.clone()), | ||
| EventTimelineItemKind::Remote(r) => EventTimelineItemKind::Remote(r.redact()), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a comment that we can't unredact a remote item, that's why we're calling
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm, actually I think you've found a bug here. I just copied this from the
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ouch. Makes sense to add such fields. Maybe we can group all the |
||
| }; | ||
| Self { | ||
| sender: self.sender.clone(), | ||
| sender_profile: self.sender_profile.clone(), | ||
| forwarder: self.forwarder.clone(), | ||
| forwarder_profile: self.forwarder_profile.clone(), | ||
| timestamp: self.timestamp, | ||
| content: content.clone(), | ||
| unredacted_content: None, | ||
| kind, | ||
| is_room_encrypted: self.is_room_encrypted, | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.