From f2ca0697af073026d4b15000c49cc98f137fd7a0 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Mon, 19 May 2025 09:00:22 +0300 Subject: [PATCH] chore(ui): remove the timeline's builder method and make the builder's constructor public --- benchmarks/benches/room_bench.rs | 7 +++-- benchmarks/benches/timeline.rs | 4 +-- bindings/matrix-sdk-ffi/src/room.rs | 4 +-- .../src/room_list_service/room.rs | 2 +- crates/matrix-sdk-ui/src/timeline/builder.rs | 2 +- crates/matrix-sdk-ui/src/timeline/mod.rs | 5 ---- crates/matrix-sdk-ui/src/timeline/traits.rs | 2 +- .../tests/integration/timeline/focus_event.rs | 10 +++---- .../tests/integration/timeline/mod.rs | 4 +-- .../integration/timeline/pinned_event.rs | 29 +++++++++---------- .../integration/timeline/sliding_sync.rs | 7 ++--- .../tests/integration/timeline/thread.rs | 6 ++-- 12 files changed, 37 insertions(+), 45 deletions(-) diff --git a/benchmarks/benches/room_bench.rs b/benchmarks/benches/room_bench.rs index 06d9aa95d..dec60110e 100644 --- a/benchmarks/benches/room_bench.rs +++ b/benchmarks/benches/room_bench.rs @@ -7,7 +7,10 @@ use matrix_sdk_base::{ }; use matrix_sdk_sqlite::SqliteStateStore; use matrix_sdk_test::{event_factory::EventFactory, JoinedRoomBuilder, StateTestEvent}; -use matrix_sdk_ui::{timeline::TimelineFocus, Timeline}; +use matrix_sdk_ui::{ + timeline::{TimelineBuilder, TimelineFocus}, + Timeline, +}; use ruma::{ api::client::membership::get_member_events, device_id, @@ -182,7 +185,7 @@ pub fn load_pinned_events_benchmark(c: &mut Criterion) { .await .unwrap(); - let timeline = Timeline::builder(&room) + let timeline = TimelineBuilder::new(&room) .with_focus(TimelineFocus::PinnedEvents { max_events_to_load: 100, max_concurrent_requests: 10, diff --git a/benchmarks/benches/timeline.rs b/benchmarks/benches/timeline.rs index d0215839f..f8a53066c 100644 --- a/benchmarks/benches/timeline.rs +++ b/benchmarks/benches/timeline.rs @@ -1,7 +1,7 @@ use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion, Throughput}; use matrix_sdk::test_utils::mocks::MatrixMockServer; use matrix_sdk_test::{event_factory::EventFactory, JoinedRoomBuilder, StateTestEvent}; -use matrix_sdk_ui::Timeline; +use matrix_sdk_ui::{timeline::TimelineBuilder, Timeline}; use ruma::{ events::room::message::RoomMessageEventContentWithoutRelation, owned_room_id, owned_user_id, EventId, @@ -102,7 +102,7 @@ pub fn create_timeline_with_initial_events(c: &mut Criterion) { BenchmarkId::new("create_timeline_with_initial_events", format!("{NUM_EVENTS} events")), |b| { b.to_async(&runtime).iter(|| async { - let timeline = Timeline::builder(&room) + let timeline = TimelineBuilder::new(&room) .track_read_marker_and_receipts() .build() .await diff --git a/bindings/matrix-sdk-ffi/src/room.rs b/bindings/matrix-sdk-ffi/src/room.rs index 6c2071eb5..a18815a2a 100644 --- a/bindings/matrix-sdk-ffi/src/room.rs +++ b/bindings/matrix-sdk-ffi/src/room.rs @@ -12,7 +12,7 @@ use matrix_sdk::{ ComposerDraft as SdkComposerDraft, ComposerDraftType as SdkComposerDraftType, EncryptionState, RoomHero as SdkRoomHero, RoomMemberships, RoomState, }; -use matrix_sdk_ui::timeline::{default_event_filter, RoomExt}; +use matrix_sdk_ui::timeline::{default_event_filter, RoomExt, TimelineBuilder}; use mime::Mime; use ruma::{ assign, @@ -189,7 +189,7 @@ impl Room { &self, configuration: TimelineConfiguration, ) -> Result, ClientError> { - let mut builder = matrix_sdk_ui::timeline::Timeline::builder(&self.inner); + let mut builder = matrix_sdk_ui::timeline::TimelineBuilder::new(&self.inner); builder = builder .with_focus(configuration.focus.try_into()?) 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 504f27684..95759883a 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/room.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/room.rs @@ -144,6 +144,6 @@ impl Room { /// If the room was synced before some initial events will be added to the /// [`TimelineBuilder`]. pub fn default_room_timeline_builder(&self) -> Result { - Ok(Timeline::builder(&self.inner.room).track_read_marker_and_receipts()) + Ok(TimelineBuilder::new(&self.inner.room).track_read_marker_and_receipts()) } } diff --git a/crates/matrix-sdk-ui/src/timeline/builder.rs b/crates/matrix-sdk-ui/src/timeline/builder.rs index cd2fa32d6..9acda5402 100644 --- a/crates/matrix-sdk-ui/src/timeline/builder.rs +++ b/crates/matrix-sdk-ui/src/timeline/builder.rs @@ -57,7 +57,7 @@ pub struct TimelineBuilder { } impl TimelineBuilder { - pub(super) fn new(room: &Room) -> Self { + pub fn new(room: &Room) -> Self { Self { room: room.clone(), settings: TimelineSettings::default(), diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index 69341a56a..1e79247ef 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -148,11 +148,6 @@ pub enum DateDividerMode { } impl Timeline { - /// Create a new [`TimelineBuilder`] for the given room. - pub fn builder(room: &Room) -> TimelineBuilder { - TimelineBuilder::new(room) - } - /// Returns the room for this timeline. pub fn room(&self) -> &Room { self.controller.room() diff --git a/crates/matrix-sdk-ui/src/timeline/traits.rs b/crates/matrix-sdk-ui/src/timeline/traits.rs index 4dcff54a4..0249086b6 100644 --- a/crates/matrix-sdk-ui/src/timeline/traits.rs +++ b/crates/matrix-sdk-ui/src/timeline/traits.rs @@ -68,7 +68,7 @@ impl RoomExt for Room { } fn timeline_builder(&self) -> TimelineBuilder { - Timeline::builder(self).track_read_marker_and_receipts() + TimelineBuilder::new(self).track_read_marker_and_receipts() } } diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/focus_event.rs b/crates/matrix-sdk-ui/tests/integration/timeline/focus_event.rs index 65e9f374b..a674a3ea7 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/focus_event.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/focus_event.rs @@ -24,7 +24,7 @@ use matrix_sdk_test::{ async_test, event_factory::EventFactory, mocks::mock_encryption_state, JoinedRoomBuilder, SyncResponseBuilder, ALICE, BOB, }; -use matrix_sdk_ui::{timeline::TimelineFocus, Timeline}; +use matrix_sdk_ui::timeline::{TimelineBuilder, TimelineFocus}; use ruma::{event_id, events::room::message::RoomMessageEventContent, room_id}; use stream_assert::assert_pending; use tokio::time::sleep; @@ -70,7 +70,7 @@ async fn test_new_focused() { mock_encryption_state(&server, false).await; let room = client.get_room(room_id).unwrap(); - let timeline = Timeline::builder(&room) + let timeline = TimelineBuilder::new(&room) .with_focus(TimelineFocus::Event { target: target_event.to_owned(), num_context_events: 20, @@ -218,7 +218,7 @@ async fn test_live_aggregations_are_reflected_on_focused_timelines() { mock_encryption_state(&server, false).await; let room = client.get_room(room_id).unwrap(); - let timeline = Timeline::builder(&room) + let timeline = TimelineBuilder::new(&room) .with_focus(TimelineFocus::Event { target: target_event.to_owned(), num_context_events: 20, @@ -300,7 +300,7 @@ async fn test_focused_timeline_local_echoes() { mock_encryption_state(&server, false).await; let room = client.get_room(room_id).unwrap(); - let timeline = Timeline::builder(&room) + let timeline = TimelineBuilder::new(&room) .with_focus(TimelineFocus::Event { target: target_event.to_owned(), num_context_events: 20, @@ -379,7 +379,7 @@ async fn test_focused_timeline_doesnt_show_local_echoes() { mock_encryption_state(&server, false).await; let room = client.get_room(room_id).unwrap(); - let timeline = Timeline::builder(&room) + let timeline = TimelineBuilder::new(&room) .with_focus(TimelineFocus::Event { target: target_event.to_owned(), num_context_events: 20, diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs b/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs index 47275ac15..8ae263687 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/mod.rs @@ -29,7 +29,7 @@ use matrix_sdk_test::{ use matrix_sdk_ui::{ timeline::{ AnyOtherFullStateEventContent, Error, EventSendState, RedactError, RoomExt, - TimelineEventItemId, TimelineItemContent, VirtualTimelineItem, + TimelineBuilder, TimelineEventItemId, TimelineItemContent, VirtualTimelineItem, }, Timeline, }; @@ -668,7 +668,7 @@ async fn test_timeline_without_encryption_can_update() { // Previously this would have panicked. // We're creating a timeline without read receipts tracking to check only the // encryption changes. - let timeline = Timeline::builder(&room).build().await.unwrap(); + let timeline = TimelineBuilder::new(&room).build().await.unwrap(); let (items, mut stream) = timeline.subscribe().await; assert_eq!(items.len(), 2); diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/pinned_event.rs b/crates/matrix-sdk-ui/tests/integration/timeline/pinned_event.rs index c2f40c7db..477151705 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/pinned_event.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/pinned_event.rs @@ -16,10 +16,7 @@ use matrix_sdk_test::{ async_test, event_factory::EventFactory, JoinedRoomBuilder, StateTestEvent, SyncResponseBuilder, BOB, }; -use matrix_sdk_ui::{ - timeline::{RoomExt, TimelineFocus}, - Timeline, -}; +use matrix_sdk_ui::timeline::{RoomExt, TimelineBuilder, TimelineFocus}; use ruma::{ assign, event_id, events::{ @@ -70,7 +67,7 @@ async fn test_new_pinned_events_are_not_added_on_sync() { .await .expect("Room should be synced"); let timeline = - Timeline::builder(&room).with_focus(pinned_events_focus(100)).build().await.unwrap(); + TimelineBuilder::new(&room).with_focus(pinned_events_focus(100)).build().await.unwrap(); assert!( timeline.live_back_pagination_status().await.is_none(), @@ -133,7 +130,7 @@ async fn test_new_pinned_event_ids_reload_the_timeline() { .expect("Room should be synced"); let timeline = - Timeline::builder(&room).with_focus(pinned_events_focus(100)).build().await.unwrap(); + TimelineBuilder::new(&room).with_focus(pinned_events_focus(100)).build().await.unwrap(); assert!( timeline.live_back_pagination_status().await.is_none(), @@ -208,7 +205,7 @@ async fn test_max_events_to_load_is_honored() { .await .expect("Sync failed"); - let ret = Timeline::builder(&room).with_focus(pinned_events_focus(1)).build().await; + let ret = TimelineBuilder::new(&room).with_focus(pinned_events_focus(1)).build().await; // We're only taking the last event id, `$2`, and it's not available so the // timeline fails to initialise. @@ -245,7 +242,7 @@ async fn test_cached_events_are_kept_for_different_room_instances() { let (room_cache, _drop_handles) = room.event_cache().await.unwrap(); let timeline = - Timeline::builder(&room).with_focus(pinned_events_focus(2)).build().await.unwrap(); + TimelineBuilder::new(&room).with_focus(pinned_events_focus(2)).build().await.unwrap(); assert!( timeline.live_back_pagination_status().await.is_none(), @@ -274,7 +271,7 @@ async fn test_cached_events_are_kept_for_different_room_instances() { // And a new timeline one let timeline = - Timeline::builder(&room).with_focus(pinned_events_focus(2)).build().await.unwrap(); + TimelineBuilder::new(&room).with_focus(pinned_events_focus(2)).build().await.unwrap(); let (items, _) = timeline.subscribe().await; assert!(!items.is_empty()); // These events came from the cache @@ -299,7 +296,7 @@ async fn test_pinned_timeline_with_pinned_event_ids_and_empty_result_fails() { .mock_and_sync(&client, &server) .await .expect("Sync failed"); - let ret = Timeline::builder(&room).with_focus(pinned_events_focus(1)).build().await; + let ret = TimelineBuilder::new(&room).with_focus(pinned_events_focus(1)).build().await; // The timeline couldn't load any events so it fails to initialise assert!(ret.is_err()); @@ -318,7 +315,7 @@ async fn test_pinned_timeline_with_no_pinned_event_ids_is_just_empty() { .await .expect("Sync failed"); let timeline = - Timeline::builder(&room).with_focus(pinned_events_focus(1)).build().await.unwrap(); + TimelineBuilder::new(&room).with_focus(pinned_events_focus(1)).build().await.unwrap(); // The timeline couldn't load any events, but it expected none, so it just // returns an empty list @@ -346,7 +343,7 @@ async fn test_pinned_timeline_with_no_pinned_events_and_an_utd_on_sync_is_just_e mock_events_endpoint(&server, room_id, vec![utd_event]).await; let timeline = - Timeline::builder(&room).with_focus(pinned_events_focus(1)).build().await.unwrap(); + TimelineBuilder::new(&room).with_focus(pinned_events_focus(1)).build().await.unwrap(); // The timeline couldn't load any events, but it expected none, so it just // returns an empty list @@ -369,7 +366,7 @@ async fn test_pinned_timeline_with_no_pinned_events_on_pagination_is_just_empty( .await .expect("Sync failed"); let pinned_timeline = - Timeline::builder(&room).with_focus(pinned_events_focus(1)).build().await.unwrap(); + TimelineBuilder::new(&room).with_focus(pinned_events_focus(1)).build().await.unwrap(); // The timeline couldn't load any events, but it expected none, so it just // returns an empty list @@ -430,7 +427,7 @@ async fn test_pinned_timeline_with_pinned_utd_on_sync_contains_it() { mock_events_endpoint(&server, room_id, vec![utd_event]).await; let timeline = - Timeline::builder(&room).with_focus(pinned_events_focus(1)).build().await.unwrap(); + TimelineBuilder::new(&room).with_focus(pinned_events_focus(1)).build().await.unwrap(); // The timeline loaded with just a day divider and the pinned UTD let (items, _) = timeline.subscribe().await; @@ -463,7 +460,7 @@ async fn test_edited_events_are_reflected_in_sync() { .await .expect("Sync failed"); let timeline = - Timeline::builder(&room).with_focus(pinned_events_focus(100)).build().await.unwrap(); + TimelineBuilder::new(&room).with_focus(pinned_events_focus(100)).build().await.unwrap(); assert!( timeline.live_back_pagination_status().await.is_none(), @@ -535,7 +532,7 @@ async fn test_redacted_events_are_reflected_in_sync() { .await .expect("Sync failed"); let timeline = - Timeline::builder(&room).with_focus(pinned_events_focus(100)).build().await.unwrap(); + TimelineBuilder::new(&room).with_focus(pinned_events_focus(100)).build().await.unwrap(); assert!( timeline.live_back_pagination_status().await.is_none(), diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs b/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs index 1fa987131..bcd7d9242 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/sliding_sync.rs @@ -24,10 +24,7 @@ use matrix_sdk::{ SlidingSyncListBuilder, SlidingSyncMode, UpdateSummary, }; use matrix_sdk_test::{async_test, mocks::mock_encryption_state}; -use matrix_sdk_ui::{ - timeline::{TimelineItem, TimelineItemKind}, - Timeline, -}; +use matrix_sdk_ui::timeline::{TimelineBuilder, TimelineItem, TimelineItemKind}; use ruma::{room_id, user_id, RoomId}; use serde_json::json; use wiremock::{http::Method, Match, Mock, MockServer, Request, ResponseTemplate}; @@ -404,7 +401,7 @@ async fn timeline_test_helper( anyhow::anyhow!("Room {room_id} not found in client. Can't provide a timeline for it") })?; - let timeline = Timeline::builder(&sdk_room).track_read_marker_and_receipts().build().await?; + let timeline = TimelineBuilder::new(&sdk_room).track_read_marker_and_receipts().build().await?; Ok(timeline.subscribe().await) } diff --git a/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs b/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs index 6e0eaf613..98a27963c 100644 --- a/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs +++ b/crates/matrix-sdk-ui/tests/integration/timeline/thread.rs @@ -17,7 +17,7 @@ use eyeball_im::VectorDiff; use futures_util::StreamExt as _; use matrix_sdk::test_utils::mocks::{MatrixMockServer, RoomRelationsResponseTemplate}; use matrix_sdk_test::{async_test, event_factory::EventFactory}; -use matrix_sdk_ui::{timeline::TimelineFocus, Timeline}; +use matrix_sdk_ui::timeline::{TimelineBuilder, TimelineFocus}; use ruma::{event_id, events::AnyTimelineEvent, owned_event_id, room_id, serde::Raw, user_id}; use stream_assert::assert_pending; @@ -55,7 +55,7 @@ async fn test_new_thread() { let room = server.sync_joined_room(&client, room_id).await; - let timeline = Timeline::builder(&room) + let timeline = TimelineBuilder::new(&room) .with_focus(TimelineFocus::Thread { root_event_id: thread_root_event_id, num_events: 1 }) .build() .await @@ -121,7 +121,7 @@ async fn test_thread_backpagination() { let room = server.sync_joined_room(&client, room_id).await; - let timeline = Timeline::builder(&room) + let timeline = TimelineBuilder::new(&room) .with_focus(TimelineFocus::Thread { root_event_id: thread_root_event_id, num_events: 1 }) .build() .await