Skip to content

Commit

Permalink
feat(ui): RoomListService::subscribe_to_rooms no longer has a `sett…
Browse files Browse the repository at this point in the history 
…ings` argument.

This patch removes the `settings` argument of
`RoomListService::subscribe_to_rooms`. The settings were mostly composed
of:

* `required_state`: now shared with `all_rooms`, so that we are
  sure they are synced; except that `m.room.create` is added for
  subscriptions.
* `timeline_limit`: now defaults to 20.

This patch thus creates the `DEFAULT_REQUIRED_STATE` and
`DEFAULT_ROOM_SUBSCRIPTION_TIMELINE_LIMIT` constants.

Finally, this patch updates the tests, and updates all usages of
`subscribe_to_rooms`.
  • Loading branch information
@Hywan
Hywan committed Oct 22, 2024
1 parent 2779bed commit bab2cc7
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 117 deletions.
36 changes: 2 additions & 34 deletions bindings/matrix-sdk-ffi/src/room_list.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,22 +135,15 @@ impl RoomListService {
})))
}

fn subscribe_to_rooms(
&self,
room_ids: Vec<String>,
settings: Option<RoomSubscription>,
) -> Result<(), RoomListError> {
fn subscribe_to_rooms(&self, room_ids: Vec<String>) -> Result<(), RoomListError> {
let room_ids = room_ids
.into_iter()
.map(|room_id| {
RoomId::parse(&room_id).map_err(|_| RoomListError::InvalidRoomId { error: room_id })
})
.collect::<Result<Vec<_>, _>>()?;

self.inner.subscribe_to_rooms(
&room_ids.iter().map(AsRef::as_ref).collect::<Vec<_>>(),
settings.map(Into::into),
);
self.inner.subscribe_to_rooms(&room_ids.iter().map(AsRef::as_ref).collect::<Vec<_>>());

Ok(())
}
Expand Down Expand Up @@ -680,31 +673,6 @@ impl RoomListItem {
}
}

#[derive(uniffi::Record)]
pub struct RequiredState {
pub key: String,
pub value: String,
}

#[derive(uniffi::Record)]
pub struct RoomSubscription {
pub required_state: Option<Vec<RequiredState>>,
pub timeline_limit: u32,
pub include_heroes: Option<bool>,
}

impl From<RoomSubscription> for http::request::RoomSubscription {
fn from(val: RoomSubscription) -> Self {
assign!(http::request::RoomSubscription::default(), {
required_state: val.required_state.map(|r|
r.into_iter().map(|s| (s.key.into(), s.value)).collect()
).unwrap_or_default(),
timeline_limit: val.timeline_limit.into(),
include_heroes: val.include_heroes,
})
}
}

#[derive(uniffi::Object)]
pub struct UnreadNotificationsCount {
highlight_count: u32,
Expand Down
63 changes: 34 additions & 29 deletions crates/matrix-sdk-ui/src/room_list_service/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ mod room_list;
pub mod sorters;
mod state;

use std::{sync::Arc, time::Duration};
use std::{iter::once, sync::Arc, time::Duration};

use async_stream::stream;
use eyeball::Subscriber;
Expand All @@ -69,14 +69,30 @@ use matrix_sdk::{
use matrix_sdk_base::sliding_sync::http;
pub use room::*;
pub use room_list::*;
use ruma::{assign, directory::RoomTypeFilter, events::StateEventType, OwnedRoomId, RoomId};
use ruma::{assign, directory::RoomTypeFilter, events::StateEventType, OwnedRoomId, RoomId, UInt};
pub use state::*;
use thiserror::Error;
use tokio::time::timeout;
use tracing::debug;

use crate::timeline;

/// The default `required_state` constant value for sliding sync lists and
/// sliding sync room subscriptions.
const DEFAULT_REQUIRED_STATE: &[(StateEventType, &str)] = &[
(StateEventType::RoomName, ""),
(StateEventType::RoomEncryption, ""),
(StateEventType::RoomMember, "$LAZY"),
(StateEventType::RoomMember, "$ME"),
(StateEventType::RoomTopic, ""),
(StateEventType::RoomCanonicalAlias, ""),
(StateEventType::RoomPowerLevels, ""),
(StateEventType::CallMember, ""),
];

/// The default `timeline_limit` value when used with room subscriptions.
const DEFAULT_ROOM_SUBSCRIPTION_TIMELINE_LIMIT: u32 = 20;

/// The [`RoomListService`] type. See the module's documentation to learn more.
#[derive(Debug)]
pub struct RoomListService {
Expand Down Expand Up @@ -145,17 +161,12 @@ impl RoomListService {
.add_range(ALL_ROOMS_DEFAULT_SELECTIVE_RANGE),
)
.timeline_limit(1)
.required_state(vec![
(StateEventType::RoomName, "".to_owned()),
(StateEventType::RoomEncryption, "".to_owned()),
(StateEventType::RoomMember, "$LAZY".to_owned()),
(StateEventType::RoomMember, "$ME".to_owned()),
(StateEventType::RoomTopic, "".to_owned()),
(StateEventType::RoomCanonicalAlias, "".to_owned()),
(StateEventType::RoomPowerLevels, "".to_owned()),
(StateEventType::RoomPinnedEvents, "".to_owned()),
(StateEventType::CallMember, "".to_owned()),
])
.required_state(
DEFAULT_REQUIRED_STATE
.iter()
.map(|(state_event, value)| (state_event.clone(), (*value).to_owned()))
.collect(),
)
.include_heroes(Some(true))
.filters(Some(assign!(http::request::ListFilters::default(), {
// As defined in the [SlidingSync MSC](https://github.com/matrix-org/matrix-spec-proposals/blob/9450ced7fb9cf5ea9077d029b3adf36aebfa8709/proposals/3575-sync.md?plain=1#L444)
Expand Down Expand Up @@ -382,22 +393,16 @@ impl RoomListService {
///
/// It means that all events from these rooms will be received every time,
/// no matter how the `RoomList` is configured.
pub fn subscribe_to_rooms(
&self,
room_ids: &[&RoomId],
settings: Option<http::request::RoomSubscription>,
) {
let mut settings = settings.unwrap_or_default();

// Make sure to always include the room creation event in the required state
// events, to know what the room version is.
if !settings
.required_state
.iter()
.any(|(event_type, _state_key)| *event_type == StateEventType::RoomCreate)
{
settings.required_state.push((StateEventType::RoomCreate, "".to_owned()));
}
pub fn subscribe_to_rooms(&self, room_ids: &[&RoomId]) {
let settings = assign!(http::request::RoomSubscription::default(), {
required_state: DEFAULT_REQUIRED_STATE.iter().map(|(state_event, value)| {
(state_event.clone(), (*value).to_owned())
})
.chain(once((StateEventType::RoomCreate, "".to_owned())))
.chain(once((StateEventType::RoomPinnedEvents, "".to_owned())))
.collect(),
timeline_limit: UInt::from(DEFAULT_ROOM_SUBSCRIPTION_TIMELINE_LIMIT),
});

let cancel_in_flight_request = match self.state_machine.get() {
State::Init | State::Recovering | State::Error { .. } | State::Terminated { .. } => {
Expand Down
68 changes: 20 additions & 48 deletions crates/matrix-sdk-ui/tests/integration/room_list_service.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,7 @@ use assert_matches::assert_matches;
use eyeball_im::VectorDiff;
use futures_util::{pin_mut, FutureExt, StreamExt};
use matrix_sdk::{test_utils::logged_in_client_with_server, Client};
use matrix_sdk_base::{
sliding_sync::http::request::RoomSubscription, sync::UnreadNotificationsCount,
};
use matrix_sdk_base::sync::UnreadNotificationsCount;
use matrix_sdk_test::{async_test, mocks::mock_encryption_state};
use matrix_sdk_ui::{
room_list_service::{
Expand All @@ -20,10 +18,8 @@ use matrix_sdk_ui::{
RoomListService,
};
use ruma::{
api::client::room::create_room::v3::Request as CreateRoomRequest,
assign, event_id,
events::{room::message::RoomMessageEventContent, StateEventType},
mxc_uri, room_id, uint,
api::client::room::create_room::v3::Request as CreateRoomRequest, event_id,
events::room::message::RoomMessageEventContent, mxc_uri, room_id,
};
use serde_json::json;
use stream_assert::{assert_next_matches, assert_pending};
Expand Down Expand Up @@ -334,7 +330,6 @@ async fn test_sync_all_states() -> Result<(), Error> {
["m.room.topic", ""],
["m.room.canonical_alias", ""],
["m.room.power_levels", ""],
["m.room.pinned_events", ""],
["org.matrix.msc3401.call.member", ""],
],
"include_heroes": true,
Expand Down Expand Up @@ -2083,18 +2078,7 @@ async fn test_room_subscription() -> Result<(), Error> {

// Subscribe.

room_list.subscribe_to_rooms(
&[room_id_1],
Some(assign!(RoomSubscription::default(), {
required_state: vec![
(StateEventType::RoomName, "".to_owned()),
(StateEventType::RoomTopic, "".to_owned()),
(StateEventType::RoomAvatar, "".to_owned()),
(StateEventType::RoomCanonicalAlias, "".to_owned()),
],
timeline_limit: uint!(30),
})),
);
room_list.subscribe_to_rooms(&[room_id_1]);

sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
Expand All @@ -2109,12 +2093,17 @@ async fn test_room_subscription() -> Result<(), Error> {
room_id_1: {
"required_state": [
["m.room.name", ""],
["m.room.encryption", ""],
["m.room.member", "$LAZY"],
["m.room.member", "$ME"],
["m.room.topic", ""],
["m.room.avatar", ""],
["m.room.canonical_alias", ""],
["m.room.power_levels", ""],
["m.room.pinned_events", ""],
["org.matrix.msc3401.call.member", ""],
["m.room.create", ""],
],
"timeline_limit": 30,
"timeline_limit": 20,
},
},
},
Expand All @@ -2127,18 +2116,7 @@ async fn test_room_subscription() -> Result<(), Error> {

// Subscribe to another room.

room_list.subscribe_to_rooms(
&[room_id_2],
Some(assign!(RoomSubscription::default(), {
required_state: vec![
(StateEventType::RoomName, "".to_owned()),
(StateEventType::RoomTopic, "".to_owned()),
(StateEventType::RoomAvatar, "".to_owned()),
(StateEventType::RoomCanonicalAlias, "".to_owned()),
],
timeline_limit: uint!(30),
})),
);
room_list.subscribe_to_rooms(&[room_id_2]);

sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
Expand All @@ -2153,12 +2131,17 @@ async fn test_room_subscription() -> Result<(), Error> {
room_id_2: {
"required_state": [
["m.room.name", ""],
["m.room.encryption", ""],
["m.room.member", "$LAZY"],
["m.room.member", "$ME"],
["m.room.topic", ""],
["m.room.avatar", ""],
["m.room.canonical_alias", ""],
["m.room.power_levels", ""],
["m.room.pinned_events", ""],
["org.matrix.msc3401.call.member", ""],
["m.room.create", ""],
],
"timeline_limit": 30,
"timeline_limit": 20,
},
},
},
Expand All @@ -2171,18 +2154,7 @@ async fn test_room_subscription() -> Result<(), Error> {

// Subscribe to an already subscribed room. Nothing happens.

room_list.subscribe_to_rooms(
&[room_id_1],
Some(assign!(RoomSubscription::default(), {
required_state: vec![
(StateEventType::RoomName, "".to_owned()),
(StateEventType::RoomTopic, "".to_owned()),
(StateEventType::RoomAvatar, "".to_owned()),
(StateEventType::RoomCanonicalAlias, "".to_owned()),
],
timeline_limit: uint!(30),
})),
);
room_list.subscribe_to_rooms(&[room_id_1]);

sync_then_assert_request_and_fake_response! {
[server, room_list, sync]
Expand Down
8 changes: 2 additions & 6 deletions labs/multiverse/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,8 @@ use matrix_sdk::{
ruma::{
api::client::receipt::create_receipt::v3::ReceiptType,
events::room::message::{MessageType, RoomMessageEventContent},
uint, MilliSecondsSinceUnixEpoch, OwnedRoomId, RoomId,
MilliSecondsSinceUnixEpoch, OwnedRoomId, RoomId,
},
sliding_sync::http::request::RoomSubscription,
AuthSession, Client, ServerName, SqliteCryptoStore, SqliteStateStore,
};
use matrix_sdk_ui::{
Expand Down Expand Up @@ -445,10 +444,7 @@ impl App {
.get_selected_room_id(Some(selected))
.and_then(|room_id| self.ui_rooms.lock().unwrap().get(&room_id).cloned())
{
let mut sub = RoomSubscription::default();
sub.timeline_limit = uint!(30);

self.sync_service.room_list_service().subscribe_to_rooms(&[room.room_id()], Some(sub));
self.sync_service.room_list_service().subscribe_to_rooms(&[room.room_id()]);
self.current_room_subscription = Some(room);
}
}
Expand Down

0 comments on commit bab2cc7

Please sign in to comment.