refactor(timeline): fuse redact_by_id() within redact()

Changelog: `Timeline::redact_by_id` has been fused into
`Timeline::redact`, which now takes a `TimelineEventItemId` as an
identifier of the item (local or remote) to redact.
This commit is contained in:
Benjamin Bouvier
2024-10-15 16:36:34 +02:00
parent a901506a53
commit c4dd2d192e
4 changed files with 35 additions and 238 deletions

View File

@@ -610,10 +610,11 @@ impl Timeline {
event_or_transaction_id: EventOrTransactionId,
reason: Option<String>,
) -> Result<(), ClientError> {
self.inner
.redact_by_id(&(event_or_transaction_id.try_into()?), reason.as_deref())
.await
.map_err(Into::into)
if !self.inner.redact(&(event_or_transaction_id.try_into()?), reason.as_deref()).await? {
// TODO make it a hard error instead
warn!("Couldn't redact item");
}
Ok(())
}
/// Load the reply details for the given event id.

View File

@@ -18,7 +18,6 @@ use matrix_sdk::{
send_queue::RoomSendQueueError,
HttpError,
};
use ruma::OwnedTransactionId;
use thiserror::Error;
use crate::timeline::{pinned_events_loader::PinnedEventsLoaderError, TimelineEventItemId};
@@ -83,8 +82,8 @@ pub enum Error {
#[derive(Error, Debug)]
pub enum RedactError {
/// Local event to redact wasn't found for transaction id
#[error("Local event to redact wasn't found for transaction {0}")]
LocalEventNotFound(OwnedTransactionId),
#[error("Event to redact wasn't found for item id {0:?}")]
ItemNotFound(TimelineEventItemId),
/// An error happened while attempting to redact an event.
#[error(transparent)]

View File

@@ -54,6 +54,7 @@ use ruma::{
};
use thiserror::Error;
use tracing::{error, instrument, trace, warn};
use util::rfind_event_by_item_id;
use crate::timeline::pinned_events_loader::PinnedEventsRoom;
@@ -602,41 +603,6 @@ impl Timeline {
/// Redact an event given its [`TimelineEventItemId`] and an optional
/// reason.
///
/// See [`Self::redact`] for more info.
pub async fn redact_by_id(
&self,
id: &TimelineEventItemId,
reason: Option<&str>,
) -> Result<(), Error> {
let event_id = match id {
TimelineEventItemId::TransactionId(transaction_id) => {
let Some(item) = self.item_by_transaction_id(transaction_id).await else {
return Err(Error::RedactError(RedactError::LocalEventNotFound(
transaction_id.to_owned(),
)));
};
match item.handle() {
TimelineItemHandle::Local(handle) => {
// If there is a local item that hasn't been sent yet, abort the upload
handle.abort().await.map_err(RoomSendQueueError::StorageError)?;
return Ok(());
}
TimelineItemHandle::Remote(event_id) => event_id.to_owned(),
}
}
TimelineEventItemId::EventId(event_id) => event_id.to_owned(),
};
self.room()
.redact(&event_id, reason, None)
.await
.map_err(|e| Error::RedactError(RedactError::HttpError(e)))?;
Ok(())
}
/// Redact an event.
///
/// # Returns
///
/// - Returns `Ok(true)` if the redact happened.
@@ -646,33 +612,27 @@ impl Timeline {
/// interacting with the sending queue.
pub async fn redact(
&self,
event: &EventTimelineItem,
item_id: &TimelineEventItemId,
reason: Option<&str>,
) -> Result<bool, Error> {
let event_id = match event.identifier() {
TimelineEventItemId::TransactionId(_) => {
// See if we have an up-to-date timeline item with that transaction id.
match event.handle() {
TimelineItemHandle::Remote(event_id) => event_id.to_owned(),
TimelineItemHandle::Local(handle) => {
return Ok(handle
.abort()
.await
.map_err(RoomSendQueueError::StorageError)?);
}
}
}
TimelineEventItemId::EventId(event_id) => event_id,
let items = self.items().await;
let Some((_pos, event)) = rfind_event_by_item_id(&items, item_id) else {
return Err(Error::RedactError(RedactError::ItemNotFound(item_id.clone())));
};
self.room()
.redact(&event_id, reason, None)
.await
.map_err(RedactError::HttpError)
.map_err(Error::RedactError)?;
Ok(true)
match event.handle() {
TimelineItemHandle::Remote(event_id) => {
self.room()
.redact(event_id, reason, None)
.await
.map_err(RedactError::HttpError)
.map_err(Error::RedactError)?;
Ok(true)
}
TimelineItemHandle::Local(handle) => {
Ok(handle.abort().await.map_err(RoomSendQueueError::StorageError)?)
}
}
}
/// Fetch unavailable details about the event with the given ID.

View File

@@ -276,9 +276,8 @@ async fn test_redact_message() {
// Redacting a remote event works.
mock_redaction(event_id!("$42")).mount(&server).await;
let event_id = first.as_event().unwrap();
let did_redact = timeline.redact(event_id, Some("inapprops")).await.unwrap();
let did_redact =
timeline.redact(&first.as_event().unwrap().identifier(), Some("inapprops")).await.unwrap();
assert!(did_redact);
// Redacting a local event works.
@@ -301,7 +300,7 @@ async fn test_redact_message() {
assert_matches!(second.send_state(), Some(EventSendState::SendingFailed { .. }));
// Let's redact the local echo.
let did_redact = timeline.redact(second, None).await.unwrap();
let did_redact = timeline.redact(&second.identifier(), None).await.unwrap();
assert!(did_redact);
// Observe local echo being removed.
@@ -371,151 +370,11 @@ async fn test_redact_local_sent_message() {
mock_redaction(event.event_id().unwrap()).expect(1).mount(&server).await;
// Let's redact the local echo with the remote handle.
timeline.redact(event, None).await.unwrap();
timeline.redact(&event.identifier(), None).await.unwrap();
}
#[async_test]
async fn test_redact_by_id_message() {
let room_id = room_id!("!a98sd12bjh:example.org");
let (client, server) = logged_in_client_with_server().await;
let sync_settings = SyncSettings::new().timeout(Duration::from_millis(3000));
let mut sync_builder = SyncResponseBuilder::new();
sync_builder.add_joined_room(JoinedRoomBuilder::new(room_id));
mock_sync(&server, sync_builder.build_json_sync_response(), None).await;
let _response = client.sync_once(sync_settings.clone()).await.unwrap();
server.reset().await;
mock_encryption_state(&server, false).await;
let room = client.get_room(room_id).unwrap();
let timeline = room.timeline().await.unwrap();
let (_, mut timeline_stream) = timeline.subscribe().await;
let factory = EventFactory::new();
factory.set_next_ts(MilliSecondsSinceUnixEpoch::now().get().into());
sync_builder.add_joined_room(
JoinedRoomBuilder::new(room_id).add_timeline_event(
factory.sender(user_id!("@a:b.com")).text_msg("buy my bitcoins bro"),
),
);
mock_sync(&server, sync_builder.build_json_sync_response(), None).await;
let _response = client.sync_once(sync_settings.clone()).await.unwrap();
server.reset().await;
assert_let!(Some(VectorDiff::PushBack { value: first }) = timeline_stream.next().await);
assert_eq!(
first.as_event().unwrap().content().as_message().unwrap().body(),
"buy my bitcoins bro"
);
assert_let!(Some(VectorDiff::PushFront { value: day_divider }) = timeline_stream.next().await);
assert!(day_divider.is_day_divider());
// Redacting a remote event works.
mock_redaction(event_id!("$42")).mount(&server).await;
let event = first.as_event().unwrap();
timeline.redact_by_id(&event.identifier(), Some("inapprops")).await.unwrap();
// Redacting a local event works.
timeline
.send(RoomMessageEventContent::text_plain("i will disappear soon").into())
.await
.unwrap();
assert_let!(Some(VectorDiff::PushBack { value: second }) = timeline_stream.next().await);
let second = second.as_event().unwrap();
assert_matches!(second.send_state(), Some(EventSendState::NotSentYet));
// We haven't set a route for sending events, so this will fail.
assert_let!(Some(VectorDiff::Set { index, value: second }) = timeline_stream.next().await);
assert_eq!(index, 2);
let second = second.as_event().unwrap();
assert!(second.is_local_echo());
assert_matches!(second.send_state(), Some(EventSendState::SendingFailed { .. }));
// Let's redact the local echo.
timeline.redact_by_id(&second.identifier(), None).await.unwrap();
// Observe local echo being removed.
assert_matches!(timeline_stream.next().await, Some(VectorDiff::Remove { index: 2 }));
}
#[async_test]
async fn test_redact_by_local_sent_message() {
let room_id = room_id!("!a98sd12bjh:example.org");
let (client, server) = logged_in_client_with_server().await;
let sync_settings = SyncSettings::new().timeout(Duration::from_millis(3000));
let mut sync_builder = SyncResponseBuilder::new();
sync_builder.add_joined_room(JoinedRoomBuilder::new(room_id));
mock_sync(&server, sync_builder.build_json_sync_response(), None).await;
let _response = client.sync_once(sync_settings.clone()).await.unwrap();
server.reset().await;
mock_encryption_state(&server, false).await;
let room = client.get_room(room_id).unwrap();
let timeline = room.timeline().await.unwrap();
let (_, mut timeline_stream) = timeline.subscribe().await;
// Mock event sending.
Mock::given(method("PUT"))
.and(path_regex(r"^/_matrix/client/r0/rooms/.*/send/.*"))
.and(header("authorization", "Bearer 1234"))
.respond_with(
ResponseTemplate::new(200)
.set_body_json(json!({ "event_id": "$wWgymRfo7ri1uQx0NXO40vLJ" })),
)
.expect(1)
.mount(&server)
.await;
// Send the event so it's added to the send queue as a local event.
timeline
.send(RoomMessageEventContent::text_plain("i will disappear soon").into())
.await
.unwrap();
// Assert the local event is in the timeline now and is not sent yet.
assert_let_timeout!(Some(VectorDiff::PushBack { value: item }) = timeline_stream.next());
let event = item.as_event().unwrap();
assert!(event.is_local_echo());
assert_matches!(event.send_state(), Some(EventSendState::NotSentYet));
// As well as a day divider.
assert_let_timeout!(
Some(VectorDiff::PushFront { value: day_divider }) = timeline_stream.next()
);
assert!(day_divider.is_day_divider());
// We receive an update in the timeline from the send queue.
assert_let_timeout!(Some(VectorDiff::Set { index, value: item }) = timeline_stream.next());
assert_eq!(index, 1);
// Check the event is sent but still considered local.
let event = item.as_event().unwrap();
assert!(event.is_local_echo());
assert_matches!(event.send_state(), Some(EventSendState::Sent { .. }));
// Mock the redaction response for the event we just sent. Ensure it's called
// once.
mock_redaction(event.event_id().unwrap()).expect(1).mount(&server).await;
// Let's redact the local echo with the remote handle.
timeline.redact_by_id(&event.identifier(), None).await.unwrap();
}
#[async_test]
async fn test_redact_by_id_message_with_no_remote_message_present() {
async fn test_redact_nonexisting_item() {
let room_id = room_id!("!a98sd12bjh:example.org");
let (client, server) = logged_in_client_with_server().await;
@@ -534,36 +393,14 @@ async fn test_redact_by_id_message_with_no_remote_message_present() {
let timeline = room.timeline().await.unwrap();
let error = timeline
.redact_by_id(&TimelineEventItemId::EventId(owned_event_id!("$123:example.com")), None)
.redact(&TimelineEventItemId::EventId(owned_event_id!("$123:example.com")), None)
.await
.err();
assert_matches!(error, Some(Error::RedactError(RedactError::HttpError(_))))
}
assert_matches!(error, Some(Error::RedactError(RedactError::ItemNotFound(_))));
#[async_test]
async fn test_redact_by_id_message_with_no_local_message_present() {
let room_id = room_id!("!a98sd12bjh:example.org");
let (client, server) = logged_in_client_with_server().await;
let sync_settings = SyncSettings::new().timeout(Duration::from_millis(3000));
let mut sync_builder = SyncResponseBuilder::new();
sync_builder.add_joined_room(JoinedRoomBuilder::new(room_id));
mock_sync(&server, sync_builder.build_json_sync_response(), None).await;
let _response = client.sync_once(sync_settings.clone()).await.unwrap();
server.reset().await;
mock_encryption_state(&server, false).await;
let room = client.get_room(room_id).unwrap();
let timeline = room.timeline().await.unwrap();
let error = timeline
.redact_by_id(&TimelineEventItemId::TransactionId("something".into()), None)
.await
.err();
assert_matches!(error, Some(Error::RedactError(RedactError::LocalEventNotFound(_))))
let error =
timeline.redact(&TimelineEventItemId::TransactionId("something".into()), None).await.err();
assert_matches!(error, Some(Error::RedactError(RedactError::ItemNotFound(_))));
}
#[async_test]