Skip to content

Commit f9e5851

Browse files
committed
change(ffi): stop retrieving room list last messages from through the timeline
As per the plan defined in #4718: ``` the room_list_service::room::RoomInner shouldn't make use of its inner timeline; it's only used in a direct getter, or to compute the latest room event, but it's not working as intended, since local echoes aren't properly displayed in the room list. This non-working feature can be removed, in favor of #4112 ```
1 parent ed08fc4 commit f9e5851

File tree

9 files changed

+33
-179
lines changed

9 files changed

+33
-179
lines changed

Cargo.lock

Lines changed: 0 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

bindings/matrix-sdk-ffi/src/room.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,10 +86,6 @@ impl Room {
8686
pub(crate) fn new(inner: SdkRoom, utd_hook: Option<Arc<UtdHookManager>>) -> Self {
8787
Room { inner, timeline: Default::default(), utd_hook }
8888
}
89-
90-
pub(crate) fn with_timeline(inner: SdkRoom, timeline: TimelineLock) -> Self {
91-
Room { inner, timeline, utd_hook: None }
92-
}
9389
}
9490

9591
#[matrix_sdk_ffi_macros::export]

bindings/matrix-sdk-ffi/src/room_list.rs

Lines changed: 8 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,22 @@ use std::{fmt::Debug, mem::MaybeUninit, ptr::addr_of_mut, sync::Arc, time::Durat
44

55
use async_compat::get_runtime_handle;
66
use eyeball_im::VectorDiff;
7-
use futures_util::{pin_mut, StreamExt, TryFutureExt};
7+
use futures_util::{pin_mut, StreamExt};
88
use matrix_sdk::ruma::{
99
api::client::sync::sync_events::UnreadNotificationsCount as RumaUnreadNotificationsCount,
1010
RoomId,
1111
};
12-
use matrix_sdk_ui::{
13-
room_list_service::filters::{
14-
new_filter_all, new_filter_any, new_filter_category, new_filter_favourite,
15-
new_filter_fuzzy_match_room_name, new_filter_invite, new_filter_joined,
16-
new_filter_non_left, new_filter_none, new_filter_normalized_match_room_name,
17-
new_filter_unread, BoxedFilterFn, RoomCategory,
18-
},
19-
timeline::default_event_filter,
12+
use matrix_sdk_ui::room_list_service::filters::{
13+
new_filter_all, new_filter_any, new_filter_category, new_filter_favourite,
14+
new_filter_fuzzy_match_room_name, new_filter_invite, new_filter_joined, new_filter_non_left,
15+
new_filter_none, new_filter_normalized_match_room_name, new_filter_unread, BoxedFilterFn,
16+
RoomCategory,
2017
};
2118
use ruma::{OwnedRoomOrAliasId, OwnedServerName, ServerName};
22-
use tokio::sync::RwLock;
2319

2420
use crate::{
25-
error::ClientError,
26-
room::{Membership, Room},
27-
room_info::RoomInfo,
28-
room_preview::RoomPreview,
29-
timeline::{configuration::TimelineEventTypeFilter, EventTimelineItem, Timeline},
30-
utils::AsyncRuntimeDropped,
31-
TaskHandle,
21+
error::ClientError, room::Membership, room_info::RoomInfo, room_preview::RoomPreview,
22+
timeline::EventTimelineItem, utils::AsyncRuntimeDropped, TaskHandle,
3223
};
3324

3425
#[derive(Debug, thiserror::Error, uniffi::Error)]
@@ -593,67 +584,6 @@ impl RoomListItem {
593584
Ok(Arc::new(RoomPreview::new(AsyncRuntimeDropped::new(client), room_preview)))
594585
}
595586

596-
/// Build a full `Room` FFI object, filling its associated timeline.
597-
///
598-
/// An error will be returned if the room is a state different than joined
599-
/// or if its internal timeline hasn't been initialized.
600-
fn full_room(&self) -> Result<Arc<Room>, RoomListError> {
601-
if !matches!(self.membership(), Membership::Joined) {
602-
return Err(RoomListError::IncorrectRoomMembership {
603-
expected: vec![Membership::Joined],
604-
actual: self.membership(),
605-
});
606-
}
607-
608-
if let Some(timeline) = self.inner.timeline() {
609-
Ok(Arc::new(Room::with_timeline(
610-
self.inner.inner_room().clone(),
611-
Arc::new(RwLock::new(Some(Timeline::from_arc(timeline)))),
612-
)))
613-
} else {
614-
Err(RoomListError::TimelineNotInitialized {
615-
room_name: self.inner.inner_room().room_id().to_string(),
616-
})
617-
}
618-
}
619-
620-
/// Checks whether the Room's timeline has been initialized before.
621-
fn is_timeline_initialized(&self) -> bool {
622-
self.inner.is_timeline_initialized()
623-
}
624-
625-
/// Initializes the timeline for this room using the provided parameters.
626-
///
627-
/// * `event_type_filter` - An optional [`TimelineEventTypeFilter`] to be
628-
/// used to filter timeline events besides the default timeline filter. If
629-
/// `None` is passed, only the default timeline filter will be used.
630-
/// * `internal_id_prefix` - An optional String that will be prepended to
631-
/// all the timeline item's internal IDs, making it possible to
632-
/// distinguish different timeline instances from each other.
633-
async fn init_timeline(
634-
&self,
635-
event_type_filter: Option<Arc<TimelineEventTypeFilter>>,
636-
internal_id_prefix: Option<String>,
637-
) -> Result<(), RoomListError> {
638-
let mut timeline_builder = self
639-
.inner
640-
.default_room_timeline_builder()
641-
.map_err(|err| RoomListError::InitializingTimeline { error: err.to_string() })?;
642-
643-
if let Some(event_type_filter) = event_type_filter {
644-
timeline_builder = timeline_builder.event_filter(move |event, room_version_id| {
645-
// Always perform the default filter first
646-
default_event_filter(event, room_version_id) && event_type_filter.filter(event)
647-
});
648-
}
649-
650-
if let Some(internal_id_prefix) = internal_id_prefix {
651-
timeline_builder = timeline_builder.with_internal_id_prefix(internal_id_prefix);
652-
}
653-
654-
self.inner.init_timeline_with_builder(timeline_builder).map_err(RoomListError::from).await
655-
}
656-
657587
/// Checks whether the room is encrypted or not.
658588
///
659589
/// **Note**: this info may not be reliable if you don't set up

bindings/matrix-sdk-ffi/src/timeline/mod.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,6 @@ impl Timeline {
9999
Arc::new(Self { inner })
100100
}
101101

102-
pub(crate) fn from_arc(inner: Arc<matrix_sdk_ui::timeline::Timeline>) -> Arc<Self> {
103-
// SAFETY: repr(transparent) means transmuting the arc this way is allowed
104-
unsafe { Arc::from_raw(Arc::into_raw(inner) as _) }
105-
}
106-
107102
fn send_attachment(
108103
self: Arc<Self>,
109104
params: UploadParameters,

crates/matrix-sdk-ui/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ unstable-msc3956 = ["ruma/unstable-msc3956"]
2525
[dependencies]
2626
as_variant = { workspace = true }
2727
async_cell = "0.2.2"
28-
async-once-cell = "0.5.4"
2928
async-rx = { workspace = true }
3029
async-stream = { workspace = true }
3130
bitflags = { workspace = true }

crates/matrix-sdk-ui/src/room_list_service/room.rs

Lines changed: 5 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,10 @@
1717
use core::fmt;
1818
use std::{ops::Deref, sync::Arc};
1919

20-
use async_once_cell::OnceCell as AsyncOnceCell;
2120
use ruma::RoomId;
2221

2322
use super::Error;
24-
use crate::{
25-
timeline::{EventTimelineItem, TimelineBuilder},
26-
Timeline,
27-
};
23+
use crate::timeline::{EventTimelineItem, TimelineBuilder};
2824

2925
/// A room in the room list.
3026
///
@@ -43,9 +39,6 @@ impl fmt::Debug for Room {
4339
struct RoomInner {
4440
/// The underlying client room.
4541
room: matrix_sdk::Room,
46-
47-
/// The timeline of the room.
48-
timeline: AsyncOnceCell<Arc<Timeline>>,
4942
}
5043

5144
impl Deref for Room {
@@ -59,7 +52,7 @@ impl Deref for Room {
5952
impl Room {
6053
/// Create a new `Room`.
6154
pub(super) fn new(room: matrix_sdk::Room) -> Self {
62-
Self { inner: Arc::new(RoomInner { room, timeline: AsyncOnceCell::new() }) }
55+
Self { inner: Arc::new(RoomInner { room }) }
6356
}
6457

6558
/// Get the room ID.
@@ -77,56 +70,13 @@ impl Room {
7770
&self.inner.room
7871
}
7972

80-
/// Get the timeline of the room if one exists.
81-
pub fn timeline(&self) -> Option<Arc<Timeline>> {
82-
self.inner.timeline.get().cloned()
83-
}
84-
85-
/// Get whether the timeline has been already initialised or not.
86-
pub fn is_timeline_initialized(&self) -> bool {
87-
self.inner.timeline.get().is_some()
88-
}
89-
90-
/// Initialize the timeline of the room with an event type filter so only
91-
/// some events are returned. If a previous timeline exists, it'll
92-
/// return an error. Otherwise, a Timeline will be returned.
93-
pub async fn init_timeline_with_builder(&self, builder: TimelineBuilder) -> Result<(), Error> {
94-
if self.inner.timeline.get().is_some() {
95-
Err(Error::TimelineAlreadyExists(self.inner.room.room_id().to_owned()))
96-
} else {
97-
self.inner
98-
.timeline
99-
.get_or_try_init(async { Ok(Arc::new(builder.build().await?)) })
100-
.await
101-
.map_err(Error::InitializingTimeline)?;
102-
Ok(())
103-
}
104-
}
105-
106-
/// Get the latest event in the timeline.
107-
///
108-
/// The latest event comes first from the `Timeline`, it can be a local or a
109-
/// remote event. Note that the `Timeline` can have more information esp. if
110-
/// it has run a backpagination for example. Otherwise if the `Timeline`
111-
/// doesn't have any latest event, it comes from the cache. This method
112-
/// does not fetch any events or calculate anything — if it's not already
113-
/// available, we return `None`.
73+
/// Get the room's latest event from the cache. This method does not fetch
74+
/// any events or calculate anything — if it's not already available, we
75+
/// return `None`.
11476
///
11577
/// Reminder: this method also returns `None` is the latest event is not
11678
/// suitable for use in a message preview.
11779
pub async fn latest_event(&self) -> Option<EventTimelineItem> {
118-
// Look for a local event in the `Timeline`.
119-
//
120-
// First off, let's see if a `Timeline` exists…
121-
if let Some(timeline) = self.inner.timeline.get() {
122-
// If it contains a `latest_event`…
123-
if let Some(timeline_last_event) = timeline.latest_event().await {
124-
// If it's a local event or a remote event, we return it.
125-
return Some(timeline_last_event);
126-
}
127-
}
128-
129-
// Otherwise, fallback to the classical path.
13080
if let Some(latest_event) = self.inner.room.latest_event() {
13181
EventTimelineItem::from_latest_event(
13282
self.inner.room.client(),

crates/matrix-sdk-ui/tests/integration/room_list_service.rs

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2464,8 +2464,7 @@ async fn test_room_timeline() -> Result<(), Error> {
24642464
mock_encryption_state(&server, false).await;
24652465

24662466
let room = room_list.room(room_id)?;
2467-
room.init_timeline_with_builder(room.default_room_timeline_builder().unwrap()).await?;
2468-
let timeline = room.timeline().unwrap();
2467+
let timeline = room.default_room_timeline_builder().unwrap().build().await?;
24692468

24702469
let (previous_timeline_items, mut timeline_items_stream) = timeline.subscribe().await;
24712470

@@ -2566,7 +2565,7 @@ async fn test_room_latest_event() -> Result<(), Error> {
25662565
};
25672566

25682567
let room = room_list.room(room_id)?;
2569-
room.init_timeline_with_builder(room.default_room_timeline_builder().unwrap()).await?;
2568+
let timeline = room.default_room_timeline_builder().unwrap().build().await?;
25702569

25712570
// The latest event does not exist.
25722571
assert!(room.latest_event().await.is_none());
@@ -2617,9 +2616,6 @@ async fn test_room_latest_event() -> Result<(), Error> {
26172616
assert!(latest_event.is_local_echo().not());
26182617
assert_eq!(latest_event.event_id(), Some(event_id!("$x1:bar.org")));
26192618

2620-
// Now let's compare with the result of the `Timeline`.
2621-
let timeline = room.timeline().unwrap();
2622-
26232619
// The latest event matches the latest event of the `Timeline`.
26242620
assert_matches!(
26252621
timeline.latest_event().await,
@@ -2643,15 +2639,6 @@ async fn test_room_latest_event() -> Result<(), Error> {
26432639
}
26442640
);
26452641

2646-
// The latest event is a local event.
2647-
assert_matches!(
2648-
room.latest_event().await,
2649-
Some(event) => {
2650-
assert!(event.is_local_echo());
2651-
assert_eq!(event.event_id(), None);
2652-
}
2653-
);
2654-
26552642
Ok(())
26562643
}
26572644

@@ -2893,10 +2880,7 @@ async fn test_multiple_timeline_init() {
28932880
// Get a RoomListService::Room, initialize the timeline, start a pagination.
28942881
let room = room_list.room(room_id).unwrap();
28952882

2896-
let builder = room.default_room_timeline_builder().unwrap();
2897-
room.init_timeline_with_builder(builder).await.unwrap();
2898-
2899-
let timeline = room.timeline().unwrap();
2883+
let timeline = room.default_room_timeline_builder().unwrap().build().await.unwrap();
29002884

29012885
spawn(async move { timeline.paginate_backwards(20).await })
29022886
};
@@ -2909,6 +2893,5 @@ async fn test_multiple_timeline_init() {
29092893
task.abort();
29102894

29112895
// A new timeline for the same room can still be constructed.
2912-
let builder = room.default_room_timeline_builder().unwrap();
2913-
room.init_timeline_with_builder(builder).await.unwrap();
2896+
room.default_room_timeline_builder().unwrap().build().await.unwrap();
29142897
}

labs/multiverse/src/main.rs

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -280,14 +280,16 @@ impl App {
280280
}
281281
};
282282

283-
if let Err(err) = ui_room.init_timeline_with_builder(builder).await {
284-
error!("error when creating default timeline: {err}");
285-
continue;
286-
}
283+
let timeline = match builder.build().await {
284+
Ok(timeline) => timeline,
285+
Err(err) => {
286+
error!("error when creating default timeline: {err}");
287+
continue;
288+
}
289+
};
287290

288291
// Save the timeline in the cache.
289-
let sdk_timeline = ui_room.timeline().unwrap();
290-
let (items, stream) = sdk_timeline.subscribe().await;
292+
let (items, stream) = timeline.subscribe().await;
291293
let items = Arc::new(Mutex::new(items));
292294

293295
// Spawn a timeline task that will listen to all the timeline item changes.
@@ -306,7 +308,7 @@ impl App {
306308

307309
new_timelines.push((
308310
ui_room.room_id().to_owned(),
309-
Timeline { timeline: sdk_timeline, items, task: timeline_task },
311+
Timeline { timeline: Arc::new(timeline), items, task: timeline_task },
310312
));
311313

312314
// Save the room list service room in the cache.

labs/multiverse/src/widgets/room_view/mod.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use matrix_sdk::{
1212
},
1313
RoomState,
1414
};
15+
use matrix_sdk_ui::timeline::TimelineBuilder;
1516
use ratatui::{prelude::*, widgets::*};
1617
use tokio::{spawn, task::JoinHandle};
1718

@@ -328,7 +329,12 @@ impl RoomView {
328329
};
329330

330331
// Mark as read!
331-
match room.timeline().unwrap().mark_as_read(ReceiptType::Read).await {
332+
let Ok(timeline) = TimelineBuilder::new(&room).build().await else {
333+
self.status_handle.set_message("failed creating timeline".to_owned());
334+
return;
335+
};
336+
337+
match timeline.mark_as_read(ReceiptType::Read).await {
332338
Ok(did) => {
333339
self.status_handle.set_message(format!(
334340
"did {}send a read receipt!",

0 commit comments

Comments
 (0)