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 6bca1ab
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 107 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
61 changes: 32 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,31 @@ 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::RoomPinnedEvents, ""),
(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 +162,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 +394,13 @@ 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()))).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
57 changes: 17 additions & 40 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 @@ -2083,18 +2083,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 +2098,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 +2121,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 +2136,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 +2159,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
5 changes: 1 addition & 4 deletions labs/multiverse/src/main.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -445,10 +445,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 6bca1ab

Please sign in to comment.