refactor(sdk): Remove SlidingSyncRoomInner::client.

This patch removes `SlidingSyncRoomInner::client` because, first
off, it's not `Send`, and second, it's useless. Nobody uses it, it's
basically dead code… annoying dead code… bad dead code!
This commit is contained in:
Ivan Enderlin
2025-01-08 15:10:33 +01:00
parent 6b2233f8c4
commit 5675ac7f46
6 changed files with 56 additions and 94 deletions

View File

@@ -20,8 +20,8 @@ use assert_matches2::assert_let;
use eyeball_im::{Vector, VectorDiff};
use futures_util::{pin_mut, FutureExt, Stream, StreamExt};
use matrix_sdk::{
test_utils::logged_in_client_with_server, SlidingSync, SlidingSyncList, SlidingSyncListBuilder,
SlidingSyncMode, UpdateSummary,
test_utils::{logged_in_client, logged_in_client_with_server},
Client, SlidingSync, SlidingSyncList, SlidingSyncListBuilder, SlidingSyncMode, UpdateSummary,
};
use matrix_sdk_test::{async_test, mocks::mock_encryption_state};
use matrix_sdk_ui::{
@@ -223,7 +223,9 @@ macro_rules! assert_timeline_stream {
pub(crate) use assert_timeline_stream;
async fn new_sliding_sync(lists: Vec<SlidingSyncListBuilder>) -> Result<(MockServer, SlidingSync)> {
async fn new_sliding_sync(
lists: Vec<SlidingSyncListBuilder>,
) -> Result<(Client, MockServer, SlidingSync)> {
let (client, server) = logged_in_client_with_server().await;
let mut sliding_sync_builder = client.sliding_sync("integration-test")?;
@@ -234,7 +236,7 @@ async fn new_sliding_sync(lists: Vec<SlidingSyncListBuilder>) -> Result<(MockSer
let sliding_sync = sliding_sync_builder.build().await?;
Ok((server, sliding_sync))
Ok((client, server, sliding_sync))
}
async fn create_one_room(
@@ -268,13 +270,13 @@ async fn create_one_room(
}
async fn timeline_test_helper(
client: &Client,
sliding_sync: &SlidingSync,
room_id: &RoomId,
) -> Result<(Vector<Arc<TimelineItem>>, impl Stream<Item = VectorDiff<Arc<TimelineItem>>>)> {
let sliding_sync_room = sliding_sync.get_room(room_id).await.unwrap();
let room_id = sliding_sync_room.room_id();
let client = sliding_sync_room.client();
let sdk_room = client.get_room(room_id).ok_or_else(|| {
anyhow::anyhow!("Room {room_id} not found in client. Can't provide a timeline for it")
})?;
@@ -295,7 +297,7 @@ impl Match for SlidingSyncMatcher {
#[async_test]
async fn test_timeline_basic() -> Result<()> {
let (server, sliding_sync) = new_sliding_sync(vec![SlidingSyncList::builder("foo")
let (client, server, sliding_sync) = new_sliding_sync(vec![SlidingSyncList::builder("foo")
.sync_mode(SlidingSyncMode::new_selective().add_range(0..=10))])
.await?;
@@ -309,7 +311,7 @@ async fn test_timeline_basic() -> Result<()> {
mock_encryption_state(&server, false).await;
let (timeline_items, mut timeline_stream) =
timeline_test_helper(&sliding_sync, room_id).await?;
timeline_test_helper(&client, &sliding_sync, room_id).await?;
assert!(timeline_items.is_empty());
// Receiving a bunch of events.
@@ -344,7 +346,7 @@ async fn test_timeline_basic() -> Result<()> {
#[async_test]
async fn test_timeline_duplicated_events() -> Result<()> {
let (server, sliding_sync) = new_sliding_sync(vec![SlidingSyncList::builder("foo")
let (client, server, sliding_sync) = new_sliding_sync(vec![SlidingSyncList::builder("foo")
.sync_mode(SlidingSyncMode::new_selective().add_range(0..=10))])
.await?;
@@ -357,7 +359,7 @@ async fn test_timeline_duplicated_events() -> Result<()> {
mock_encryption_state(&server, false).await;
let (_, mut timeline_stream) = timeline_test_helper(&sliding_sync, room_id).await?;
let (_, mut timeline_stream) = timeline_test_helper(&client, &sliding_sync, room_id).await?;
// Receiving events.
{
@@ -422,7 +424,7 @@ async fn test_timeline_duplicated_events() -> Result<()> {
#[async_test]
async fn test_timeline_read_receipts_are_updated_live() -> Result<()> {
let (server, sliding_sync) = new_sliding_sync(vec![SlidingSyncList::builder("foo")
let (client, server, sliding_sync) = new_sliding_sync(vec![SlidingSyncList::builder("foo")
.sync_mode(SlidingSyncMode::new_selective().add_range(0..=10))])
.await?;
@@ -436,7 +438,7 @@ async fn test_timeline_read_receipts_are_updated_live() -> Result<()> {
mock_encryption_state(&server, false).await;
let (timeline_items, mut timeline_stream) =
timeline_test_helper(&sliding_sync, room_id).await?;
timeline_test_helper(&client, &sliding_sync, room_id).await?;
assert!(timeline_items.is_empty());
// Receiving initial events.

View File

@@ -248,10 +248,7 @@ pub(super) async fn restore_sliding_sync_state(
restored_fields.rooms = frozen_rooms
.into_iter()
.map(|frozen_room| {
(
frozen_room.room_id.clone(),
SlidingSyncRoom::from_frozen(frozen_room, client.clone()),
)
(frozen_room.room_id.clone(), SlidingSyncRoom::from_frozen(frozen_room))
})
.collect();
}
@@ -355,11 +352,11 @@ mod tests {
rooms.insert(
room_id1.clone(),
SlidingSyncRoom::new(client.clone(), room_id1.clone(), None, Vec::new()),
SlidingSyncRoom::new(room_id1.clone(), None, Vec::new()),
);
rooms.insert(
room_id2.clone(),
SlidingSyncRoom::new(client.clone(), room_id2.clone(), None, Vec::new()),
SlidingSyncRoom::new(room_id2.clone(), None, Vec::new()),
);
}

View File

@@ -1,7 +1,7 @@
//! Sliding Sync errors.
use matrix_sdk_common::executor::JoinError;
use thiserror::Error;
use tokio::task::JoinError;
/// Internal representation of errors in Sliding Sync.
#[derive(Error, Debug)]

View File

@@ -359,7 +359,6 @@ impl SlidingSync {
rooms_map.insert(
room_id.clone(),
SlidingSyncRoom::new(
self.inner.client.clone(),
room_id.clone(),
room_data.prev_batch,
timeline,
@@ -2269,9 +2268,6 @@ mod tests {
#[async_test]
async fn test_limited_flag_computation() {
let server = MockServer::start().await;
let client = logged_in_client(Some(server.uri())).await;
let make_event = |event_id: &str| -> SyncTimelineEvent {
SyncTimelineEvent::new(
Raw::from_json_string(
@@ -2313,7 +2309,6 @@ mod tests {
// it's not marked as initial in the response.
not_initial.to_owned(),
SlidingSyncRoom::new(
client.clone(),
no_overlap.to_owned(),
None,
vec![event_a.clone(), event_b.clone()],
@@ -2323,7 +2318,6 @@ mod tests {
// This has no events overlapping with the response timeline, hence limited.
no_overlap.to_owned(),
SlidingSyncRoom::new(
client.clone(),
no_overlap.to_owned(),
None,
vec![event_a.clone(), event_b.clone()],
@@ -2333,7 +2327,6 @@ mod tests {
// This has event_c in common with the response timeline.
partial_overlap.to_owned(),
SlidingSyncRoom::new(
client.clone(),
partial_overlap.to_owned(),
None,
vec![event_a.clone(), event_b.clone(), event_c.clone()],
@@ -2343,7 +2336,6 @@ mod tests {
// This has all events in common with the response timeline.
complete_overlap.to_owned(),
SlidingSyncRoom::new(
client.clone(),
partial_overlap.to_owned(),
None,
vec![event_c.clone(), event_d.clone()],
@@ -2354,7 +2346,6 @@ mod tests {
// limited.
no_remote_events.to_owned(),
SlidingSyncRoom::new(
client.clone(),
no_remote_events.to_owned(),
None,
vec![event_c.clone(), event_d.clone()],
@@ -2364,14 +2355,13 @@ mod tests {
// We don't have events for this room locally, and even if the remote room contains
// some events, it's not a limited sync.
no_local_events.to_owned(),
SlidingSyncRoom::new(client.clone(), no_local_events.to_owned(), None, vec![]),
SlidingSyncRoom::new(no_local_events.to_owned(), None, vec![]),
),
(
// Already limited, but would be marked limited if the flag wasn't ignored (same as
// partial overlap).
already_limited.to_owned(),
SlidingSyncRoom::new(
client,
already_limited.to_owned(),
None,
vec![event_a, event_b, event_c.clone()],

View File

@@ -8,8 +8,6 @@ use matrix_sdk_base::{deserialized_responses::SyncTimelineEvent, sliding_sync::h
use ruma::{OwnedRoomId, RoomId};
use serde::{Deserialize, Serialize};
use crate::Client;
/// The state of a [`SlidingSyncRoom`].
#[derive(Copy, Clone, Debug, Default, PartialEq)]
pub enum SlidingSyncRoomState {
@@ -40,14 +38,12 @@ pub struct SlidingSyncRoom {
impl SlidingSyncRoom {
/// Create a new `SlidingSyncRoom`.
pub fn new(
client: Client,
room_id: OwnedRoomId,
prev_batch: Option<String>,
timeline: Vec<SyncTimelineEvent>,
) -> Self {
Self {
inner: Arc::new(SlidingSyncRoomInner {
client,
room_id,
state: RwLock::new(SlidingSyncRoomState::NotLoaded),
prev_batch: RwLock::new(prev_batch),
@@ -74,11 +70,6 @@ impl SlidingSyncRoom {
self.inner.timeline_queue.read().unwrap().clone()
}
/// Get a clone of the associated client.
pub fn client(&self) -> Client {
self.inner.client.clone()
}
pub(super) fn update(
&mut self,
room_data: http::response::Room,
@@ -129,12 +120,11 @@ impl SlidingSyncRoom {
*state = SlidingSyncRoomState::Loaded;
}
pub(super) fn from_frozen(frozen_room: FrozenSlidingSyncRoom, client: Client) -> Self {
pub(super) fn from_frozen(frozen_room: FrozenSlidingSyncRoom) -> Self {
let FrozenSlidingSyncRoom { room_id, prev_batch, timeline_queue } = frozen_room;
Self {
inner: Arc::new(SlidingSyncRoomInner {
client,
room_id,
prev_batch: RwLock::new(prev_batch),
state: RwLock::new(SlidingSyncRoomState::Preloaded),
@@ -157,9 +147,6 @@ impl SlidingSyncRoom {
#[derive(Debug)]
struct SlidingSyncRoomInner {
/// The client, used to fetch [`Room`][crate::Room].
client: Client,
/// The room ID.
room_id: OwnedRoomId,
@@ -232,13 +219,9 @@ mod tests {
use matrix_sdk_test::async_test;
use ruma::{events::room::message::RoomMessageEventContent, room_id, serde::Raw, RoomId};
use serde_json::json;
use wiremock::MockServer;
use super::{http, NUMBER_OF_TIMELINE_EVENTS_TO_KEEP_FOR_THE_CACHE};
use crate::{
sliding_sync::{FrozenSlidingSyncRoom, SlidingSyncRoom, SlidingSyncRoomState},
test_utils::logged_in_client,
};
use crate::sliding_sync::{FrozenSlidingSyncRoom, SlidingSyncRoom, SlidingSyncRoomState};
macro_rules! room_response {
( $( $json:tt )+ ) => {
@@ -248,24 +231,21 @@ mod tests {
};
}
async fn new_room(room_id: &RoomId, inner: http::response::Room) -> SlidingSyncRoom {
new_room_with_timeline(room_id, inner, vec![]).await
fn new_room(room_id: &RoomId, inner: http::response::Room) -> SlidingSyncRoom {
new_room_with_timeline(room_id, inner, vec![])
}
async fn new_room_with_timeline(
fn new_room_with_timeline(
room_id: &RoomId,
inner: http::response::Room,
timeline: Vec<SyncTimelineEvent>,
) -> SlidingSyncRoom {
let server = MockServer::start().await;
let client = logged_in_client(Some(server.uri())).await;
SlidingSyncRoom::new(client, room_id.to_owned(), inner.prev_batch, timeline)
SlidingSyncRoom::new(room_id.to_owned(), inner.prev_batch, timeline)
}
#[async_test]
async fn test_state_from_not_loaded() {
let mut room = new_room(room_id!("!foo:bar.org"), room_response!({})).await;
let mut room = new_room(room_id!("!foo:bar.org"), room_response!({}));
assert_eq!(room.state(), SlidingSyncRoomState::NotLoaded);
@@ -277,7 +257,7 @@ mod tests {
#[async_test]
async fn test_state_from_preloaded() {
let mut room = new_room(room_id!("!foo:bar.org"), room_response!({})).await;
let mut room = new_room(room_id!("!foo:bar.org"), room_response!({}));
room.set_state(SlidingSyncRoomState::Preloaded);
@@ -290,7 +270,7 @@ mod tests {
#[async_test]
async fn test_room_room_id() {
let room_id = room_id!("!foo:bar.org");
let room = new_room(room_id, room_response!({})).await;
let room = new_room(room_id, room_response!({}));
assert_eq!(room.room_id(), room_id);
}
@@ -299,7 +279,7 @@ mod tests {
async fn test_prev_batch() {
// Default value.
{
let room = new_room(room_id!("!foo:bar.org"), room_response!({})).await;
let room = new_room(room_id!("!foo:bar.org"), room_response!({}));
assert_eq!(room.prev_batch(), None);
}
@@ -307,15 +287,14 @@ mod tests {
// Some value when initializing.
{
let room =
new_room(room_id!("!foo:bar.org"), room_response!({"prev_batch": "t111_222_333"}))
.await;
new_room(room_id!("!foo:bar.org"), room_response!({"prev_batch": "t111_222_333"}));
assert_eq!(room.prev_batch(), Some("t111_222_333".to_owned()));
}
// Some value when updating.
{
let mut room = new_room(room_id!("!foo:bar.org"), room_response!({})).await;
let mut room = new_room(room_id!("!foo:bar.org"), room_response!({}));
assert_eq!(room.prev_batch(), None);
@@ -329,7 +308,7 @@ mod tests {
#[async_test]
async fn test_timeline_queue_initially_empty() {
let room = new_room(room_id!("!foo:bar.org"), room_response!({})).await;
let room = new_room(room_id!("!foo:bar.org"), room_response!({}));
assert!(room.timeline_queue().is_empty());
}
@@ -368,8 +347,8 @@ mod tests {
};
}
#[async_test]
async fn test_timeline_queue_initially_not_empty() {
#[test]
fn test_timeline_queue_initially_not_empty() {
let room = new_room_with_timeline(
room_id!("!foo:bar.org"),
room_response!({}),
@@ -377,8 +356,7 @@ mod tests {
timeline_event!(from "@alice:baz.org" with id "$x0:baz.org" at 0: "message 0"),
timeline_event!(from "@alice:baz.org" with id "$x1:baz.org" at 1: "message 1"),
],
)
.await;
);
{
let timeline_queue = room.timeline_queue();
@@ -394,8 +372,8 @@ mod tests {
}
}
#[async_test]
async fn test_timeline_queue_update_with_empty_timeline() {
#[test]
fn test_timeline_queue_update_with_empty_timeline() {
let mut room = new_room_with_timeline(
room_id!("!foo:bar.org"),
room_response!({}),
@@ -403,8 +381,7 @@ mod tests {
timeline_event!(from "@alice:baz.org" with id "$x0:baz.org" at 0: "message 0"),
timeline_event!(from "@alice:baz.org" with id "$x1:baz.org" at 1: "message 1"),
],
)
.await;
);
{
let timeline_queue = room.timeline_queue();
@@ -436,8 +413,8 @@ mod tests {
}
}
#[async_test]
async fn test_timeline_queue_update_with_empty_timeline_and_with_limited() {
#[test]
fn test_timeline_queue_update_with_empty_timeline_and_with_limited() {
let mut room = new_room_with_timeline(
room_id!("!foo:bar.org"),
room_response!({}),
@@ -445,8 +422,7 @@ mod tests {
timeline_event!(from "@alice:baz.org" with id "$x0:baz.org" at 0: "message 0"),
timeline_event!(from "@alice:baz.org" with id "$x1:baz.org" at 1: "message 1"),
],
)
.await;
);
{
let timeline_queue = room.timeline_queue();
@@ -477,8 +453,8 @@ mod tests {
}
}
#[async_test]
async fn test_timeline_queue_update_from_preloaded() {
#[test]
fn test_timeline_queue_update_from_preloaded() {
let mut room = new_room_with_timeline(
room_id!("!foo:bar.org"),
room_response!({}),
@@ -486,8 +462,7 @@ mod tests {
timeline_event!(from "@alice:baz.org" with id "$x0:baz.org" at 0: "message 0"),
timeline_event!(from "@alice:baz.org" with id "$x1:baz.org" at 1: "message 1"),
],
)
.await;
);
room.set_state(SlidingSyncRoomState::Preloaded);
@@ -527,8 +502,8 @@ mod tests {
}
}
#[async_test]
async fn test_timeline_queue_update_from_not_loaded() {
#[test]
fn test_timeline_queue_update_from_not_loaded() {
let mut room = new_room_with_timeline(
room_id!("!foo:bar.org"),
room_response!({}),
@@ -536,8 +511,7 @@ mod tests {
timeline_event!(from "@alice:baz.org" with id "$x0:baz.org" at 0: "message 0"),
timeline_event!(from "@alice:baz.org" with id "$x1:baz.org" at 1: "message 1"),
],
)
.await;
);
{
let timeline_queue = room.timeline_queue();
@@ -577,8 +551,8 @@ mod tests {
}
}
#[async_test]
async fn test_timeline_queue_update_from_not_loaded_with_limited() {
#[test]
fn test_timeline_queue_update_from_not_loaded_with_limited() {
let mut room = new_room_with_timeline(
room_id!("!foo:bar.org"),
room_response!({}),
@@ -586,8 +560,7 @@ mod tests {
timeline_event!(from "@alice:baz.org" with id "$x0:baz.org" at 0: "message 0"),
timeline_event!(from "@alice:baz.org" with id "$x1:baz.org" at 1: "message 1"),
],
)
.await;
);
{
let timeline_queue = room.timeline_queue();
@@ -678,8 +651,8 @@ mod tests {
assert_eq!(deserialized.room_id, frozen_room.room_id);
}
#[async_test]
async fn test_frozen_sliding_sync_room_has_a_capped_version_of_the_timeline() {
#[test]
fn test_frozen_sliding_sync_room_has_a_capped_version_of_the_timeline() {
// Just below the limit.
{
let max = NUMBER_OF_TIMELINE_EVENTS_TO_KEEP_FOR_THE_CACHE - 1;
@@ -705,8 +678,7 @@ mod tests {
room_id!("!foo:bar.org"),
room_response!({}),
timeline_events,
)
.await;
);
let frozen_room = FrozenSlidingSyncRoom::from(&room);
assert_eq!(frozen_room.timeline_queue.len(), max + 1);
@@ -743,8 +715,7 @@ mod tests {
room_id!("!foo:bar.org"),
room_response!({}),
timeline_events,
)
.await;
);
let frozen_room = FrozenSlidingSyncRoom::from(&room);
assert_eq!(

View File

@@ -1,5 +1,7 @@
//! Moaaar features for Sliding Sync.
#![cfg(feature = "e2e-encryption")]
use std::{
future::Future,
pin::Pin,
@@ -23,7 +25,7 @@ impl<T> Drop for AbortOnDrop<T> {
}
}
impl<T> Future for AbortOnDrop<T> {
impl<T: 'static> Future for AbortOnDrop<T> {
type Output = Result<T, JoinError>;
fn poll(mut self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Self::Output> {