From 8021d7238916f66356776554892f7543d913abbe Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 13 Jun 2024 12:00:22 +0200 Subject: [PATCH 1/2] feat(ui): Fetch the latest event from `Room` instead of `SlidingSyncRoom` in `room_list::Room`. This patch updates `matrix_sdk_ui::room_list::Room::latest_event` to fetch the latest event from `matrix_sdk::Room` instead of `matrix_sdk::sliding_sync::SlidingSyncRoom`. It removes one dependency to `SlidingSyncRoom` and it simplifies the code. --- .../src/room_list_service/room.rs | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/crates/matrix-sdk-ui/src/room_list_service/room.rs b/crates/matrix-sdk-ui/src/room_list_service/room.rs index 94b7159fe..0a1648ccc 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/room.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/room.rs @@ -22,7 +22,7 @@ use ruma::{api::client::sync::sync_events::v4::RoomSubscription, events::StateEv use super::Error; use crate::{ - timeline::{EventTimelineItem, SlidingSyncRoomExt, TimelineBuilder}, + timeline::{EventTimelineItem, TimelineBuilder}, Timeline, }; @@ -154,6 +154,9 @@ impl Room { /// contains a local event. Otherwise, it comes from the cache. This method /// does not fetch any events or calculate anything — if it's not /// already available, we return `None`. + /// + /// Reminder: this method also returns `None` is the latest event is not + /// suitable for use in a message preview. pub async fn latest_event(&self) -> Option { // Look for a local event in the `Timeline`. // @@ -161,15 +164,22 @@ impl Room { if let Some(timeline) = self.inner.timeline.get() { // If it contains a `latest_event`… if let Some(timeline_last_event) = timeline.latest_event().await { - // If it's a local echo… - if timeline_last_event.is_local_echo() { - return Some(timeline_last_event); - } + // If it's a local event or a remote event, we return it. + return Some(timeline_last_event); } } // Otherwise, fallback to the classical path. - self.inner.sliding_sync_room.latest_timeline_item().await + if let Some(latest_event) = self.inner.room.latest_event() { + EventTimelineItem::from_latest_event( + self.inner.room.client(), + self.inner.room.room_id(), + latest_event, + ) + .await + } else { + None + } } /// Create a new [`TimelineBuilder`] with the default configuration. From 6d1289482ac3589c088ec89f8ee2d542b5382297 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 13 Jun 2024 12:02:10 +0200 Subject: [PATCH 2/2] chore(ui): Remove `SlidingSyncRoomExt`. This patch removes the `matrix_sdk_ui::timeline::SlidingSyncRoomExt` trait as it's no longer necessary since `room_list::Room::latest_event` no longer uses `SlidingSyncRoom` to fetch the latest event. --- .../src/room_list_service/room.rs | 10 +- crates/matrix-sdk-ui/src/timeline/mod.rs | 2 - .../src/timeline/sliding_sync_ext.rs | 140 ------------------ crates/matrix-sdk/src/sliding_sync/README.md | 4 - 4 files changed, 6 insertions(+), 150 deletions(-) delete mode 100644 crates/matrix-sdk-ui/src/timeline/sliding_sync_ext.rs diff --git a/crates/matrix-sdk-ui/src/room_list_service/room.rs b/crates/matrix-sdk-ui/src/room_list_service/room.rs index 0a1648ccc..9d16ba818 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/room.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/room.rs @@ -150,10 +150,12 @@ impl Room { /// Get the latest event in the timeline. /// - /// The latest event comes first from the `Timeline` if it exists and if it - /// contains a local event. Otherwise, it comes from the cache. This method - /// does not fetch any events or calculate anything — if it's not - /// already available, we return `None`. + /// The latest event comes first from the `Timeline`, it can be a local or a + /// remote event. Note that the `Timeline` can have more information esp. if + /// it has run a backpagination for example. Otherwise if the `Timeline` + /// doesn't have any latest event, it comes from the cache. This method + /// does not fetch any events or calculate anything — if it's not already + /// available, we return `None`. /// /// Reminder: this method also returns `None` is the latest event is not /// suitable for use in a message preview. diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index 3f30474f7..1c5768776 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -78,7 +78,6 @@ mod pagination; mod polls; mod reactions; mod read_receipts; -mod sliding_sync_ext; #[cfg(test)] mod tests; #[cfg(feature = "e2e-encryption")] @@ -102,7 +101,6 @@ pub use self::{ pagination::LiveBackPaginationStatus, polls::PollResult, reactions::ReactionSenderData, - sliding_sync_ext::SlidingSyncRoomExt, traits::RoomExt, virtual_item::VirtualTimelineItem, }; diff --git a/crates/matrix-sdk-ui/src/timeline/sliding_sync_ext.rs b/crates/matrix-sdk-ui/src/timeline/sliding_sync_ext.rs deleted file mode 100644 index 336910df2..000000000 --- a/crates/matrix-sdk-ui/src/timeline/sliding_sync_ext.rs +++ /dev/null @@ -1,140 +0,0 @@ -// Copyright 2023 The Matrix.org Foundation C.I.C. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -use async_trait::async_trait; -use matrix_sdk::SlidingSyncRoom; -use tracing::instrument; - -use super::EventTimelineItem; - -#[async_trait] -pub trait SlidingSyncRoomExt { - /// Get the latest timeline item of this room, if it is already cached. - /// - /// Use `Timeline::latest_event` instead if you already have a timeline for - /// this `SlidingSyncRoom`. - async fn latest_timeline_item(&self) -> Option; -} - -#[async_trait] -impl SlidingSyncRoomExt for SlidingSyncRoom { - /// Get a timeline item representing the latest event in this room. - /// This method wraps latest_event, converting the event into an - /// EventTimelineItem. - #[instrument(skip_all)] - async fn latest_timeline_item(&self) -> Option { - let latest_event = - self.client().get_room(self.room_id()).and_then(|room| room.latest_event())?; - - EventTimelineItem::from_latest_event(self.client(), self.room_id(), latest_event).await - } -} - -#[cfg(test)] -mod tests { - use assert_matches::assert_matches; - use matrix_sdk::{test_utils::logged_in_client, Client, SlidingSyncRoom}; - use matrix_sdk_base::deserialized_responses::SyncTimelineEvent; - use matrix_sdk_test::async_test; - use ruma::{ - api::client::sync::sync_events::v4, - events::room::message::{MessageFormat, MessageType}, - room_id, - serde::Raw, - user_id, RoomId, UInt, UserId, - }; - use serde_json::json; - - use crate::timeline::{SlidingSyncRoomExt, TimelineDetails}; - - #[async_test] - async fn test_initially_latest_message_event_is_none() { - // Given a room with no latest event - let room_id = room_id!("!r:x.uk").to_owned(); - let client = logged_in_client(None).await; - let room = SlidingSyncRoom::new(client, room_id, None, Vec::new()); - - // When we ask for the latest event, it is None - assert!(room.latest_timeline_item().await.is_none()); - } - - #[async_test] - async fn test_latest_message_event_is_wrapped_as_a_timeline_item() { - // Given a room exists, and an event came in through a sync - let room_id = room_id!("!r:x.uk"); - let user_id = user_id!("@s:o.uk"); - let client = logged_in_client(None).await; - let event = message_event(room_id, user_id, "**My msg**", "My msg", 122343); - process_event_via_sync_test_helper(room_id, event, &client).await; - - // When we ask for the latest event in the room - let room = SlidingSyncRoom::new(client.clone(), room_id.to_owned(), None, Vec::new()); - let actual = room.latest_timeline_item().await.unwrap(); - - // Then it is wrapped as an EventTimelineItem - assert_eq!(actual.sender, user_id); - assert_matches!(actual.sender_profile, TimelineDetails::Unavailable); - assert_eq!(actual.timestamp.0, UInt::new(122343).unwrap()); - if let MessageType::Text(txt) = actual.content.as_message().unwrap().msgtype() { - assert_eq!(txt.body, "**My msg**"); - let formatted = txt.formatted.as_ref().unwrap(); - assert_eq!(formatted.format, MessageFormat::Html); - assert_eq!(formatted.body, "My msg"); - } else { - panic!("Unexpected message type"); - } - } - - async fn process_event_via_sync_test_helper( - room_id: &RoomId, - event: SyncTimelineEvent, - client: &Client, - ) { - let mut room = v4::SlidingSyncRoom::new(); - room.timeline.push(event.event); - - let mut response = v4::Response::new("6".to_owned()); - response.rooms.insert(room_id.to_owned(), room); - - client.process_sliding_sync_test_helper(&response).await.unwrap(); - } - - fn message_event( - room_id: &RoomId, - user_id: &UserId, - body: &str, - formatted_body: &str, - ts: u64, - ) -> SyncTimelineEvent { - SyncTimelineEvent::new( - Raw::from_json_string( - json!({ - "event_id": "$eventid6", - "sender": user_id, - "origin_server_ts": ts, - "type": "m.room.message", - "room_id": room_id.to_string(), - "content": { - "body": body, - "format": "org.matrix.custom.html", - "formatted_body": formatted_body, - "msgtype": "m.text" - }, - }) - .to_string(), - ) - .unwrap(), - ) - } -} diff --git a/crates/matrix-sdk/src/sliding_sync/README.md b/crates/matrix-sdk/src/sliding_sync/README.md index ce0aec946..7775d92ed 100644 --- a/crates/matrix-sdk/src/sliding_sync/README.md +++ b/crates/matrix-sdk/src/sliding_sync/README.md @@ -343,10 +343,6 @@ copies accordingly. Because of where the loop sits in the stack, that can be a bit tedious though, so lists and rooms have an additional way of subscribing to updates via [`eyeball`]. -The `Timeline` one can receive per room by calling `.timeline()` (from -`matrix_sdk_ui::timeline::SlidingSyncRoomExt`) will be populated with the -currently cached timeline events. - ## Caching All room data, for filled but also _invalidated_ rooms, including the entire