feat: Get rid of "common extensions"

Common extensions are confusing, and they've included `e2ee` and `to-device` by default. This is not a sane default anymore,
now that there's the concept of `EncryptionSync`: it's either we have the encryption sync that enables e2ee and to-device +
a room list sync that doesn't, OR we have a single room list that has both.

Room List was misconfigured to always use `e2ee` and `to-device`, which was incorrect when it's running with the `EncryptionSync`
in the background. This is now removed, and properly tested.
This commit is contained in:
Benjamin Bouvier
2023-06-29 18:04:13 +02:00
parent 48a6bffc1e
commit 77290ff2c1
6 changed files with 47 additions and 46 deletions

View File

@@ -813,12 +813,6 @@ impl SlidingSyncBuilder {
Ok(Arc::new(builder))
}
pub fn with_common_extensions(self: Arc<Self>) -> Arc<Self> {
let mut builder = unwrap_or_clone_arc(self);
builder.inner = builder.inner.with_common_extensions();
Arc::new(builder)
}
pub fn without_e2ee_extension(self: Arc<Self>) -> Arc<Self> {
let mut builder = unwrap_or_clone_arc(self);
builder.inner = builder.inner.without_e2ee_extension();

View File

@@ -80,7 +80,9 @@ use matrix_sdk::{
pub use room::*;
pub use room_list::*;
use ruma::{
api::client::sync::sync_events::v4::{E2EEConfig, SyncRequestListFilters, ToDeviceConfig},
api::client::sync::sync_events::v4::{
AccountDataConfig, E2EEConfig, SyncRequestListFilters, ToDeviceConfig,
},
assign,
events::{StateEventType, TimelineEventType},
OwnedRoomId, RoomId,
@@ -131,8 +133,12 @@ impl RoomListService {
}
async fn new_internal(client: Client, with_encryption: bool) -> Result<Self, Error> {
let mut builder =
client.sliding_sync("room-list").map_err(Error::SlidingSync)?.with_common_extensions();
let mut builder = client
.sliding_sync("room-list")
.map_err(Error::SlidingSync)?
.with_account_data_extension(
assign!(AccountDataConfig::default(), { enabled: Some(true) }),
);
if with_encryption {
builder = builder
@@ -459,4 +465,21 @@ mod tests {
Ok(())
}
#[async_test]
async fn test_no_to_device_and_e2ee_if_not_explicitly_set() -> Result<(), Error> {
let (client, _) = new_client().await;
let no_encryption = RoomListService::new(client.clone()).await?;
let extensions = no_encryption.sliding_sync.extensions_config();
assert_eq!(extensions.e2ee.enabled, None);
assert_eq!(extensions.to_device.enabled, None);
let with_encryption = RoomListService::new_with_encryption(client).await?;
let extensions = with_encryption.sliding_sync.extensions_config();
assert_eq!(extensions.e2ee.enabled, Some(true));
assert_eq!(extensions.to_device.enabled, Some(true));
Ok(())
}
}

View File

@@ -192,9 +192,9 @@ any implementation as they please. Handling of the data of the e2ee,
to-device and typing-extensions takes place transparently within the SDK.
By default [`SlidingSync`][] doesn't activate _any_ extensions to save on
bandwidth, but we generally recommend to use the [`with_common_extensions`
when building sliding sync](`SlidingSyncBuilder::with_common_extensions`) to
active e2ee, to-device-messages and account-data-extensions.
bandwidth, but we generally recommend to use the `with_XXX_extensions` family
of methods when building sliding sync to enable e2ee, to-device-messages and
account-data-extensions.
## Timeline events
@@ -419,7 +419,13 @@ let active_list_name = "active-list".to_owned();
let sliding_sync_builder = client
.sliding_sync("main-sync")?
.sliding_sync_proxy(Url::parse("http://sliding-sync.example.org")?) // our proxy server
.with_common_extensions(); // we want the e2ee and to-device enabled, please
.with_account_data_extension(
assign!(v4::AccountDataConfig::default(), { enabled: Some(true) }),
) // we enable the account-data extension
.with_e2ee_extension(assign!(v4::E2EEConfig::default(), { enabled: Some(true) })) // and the e2ee extension
.with_to_device_extension(
assign!(v4::ToDeviceConfig::default(), { enabled: Some(true) }),
); // and the to-device extension
let full_sync_list = SlidingSyncList::builder(&full_sync_list_name)
.sync_mode(SlidingSyncMode::Growing { batch_size: 50, maximum_number_of_rooms_to_fetch: Some(500) }) // sync up by growing the window

View File

@@ -98,37 +98,6 @@ impl SlidingSyncBuilder {
Ok(self.add_list(list))
}
/// Activate e2ee, to-device-message and account data extensions if not yet
/// configured.
///
/// Will leave any extension configuration found untouched, so the order
/// does not matter.
pub fn with_common_extensions(mut self) -> Self {
{
let cfg = self.extensions.get_or_insert_with(Default::default);
if cfg.to_device.enabled.is_none() {
cfg.to_device.enabled = Some(true);
}
if cfg.e2ee.enabled.is_none() {
cfg.e2ee.enabled = Some(true);
}
if cfg.account_data.enabled.is_none() {
cfg.account_data.enabled = Some(true);
}
// TODO: Re-enable once a `lists` and `rooms` will support `*`. See
// https://github.com/matrix-org/matrix-rust-sdk/issues/2037 for more info.
/*
if cfg.receipts.enabled.is_none() {
cfg.receipts.enabled = Some(true);
}
*/
}
self
}
/// Activate e2ee, to-device-message, account data, typing and receipt
/// extensions if not yet configured.
///

View File

@@ -700,6 +700,16 @@ impl SlidingSync {
pub async fn force_cache_to_storage(&self, to_device_token: Option<String>) -> Result<()> {
self.cache_to_storage(to_device_token).await
}
/// Read the static extension configuration for this Sliding Sync.
///
/// Note: this is not the next content of the sticky parameters, but rightly
/// the static configuration that was set during creation of this
/// Sliding Sync.
pub fn extensions_config(&self) -> ExtensionsConfig {
let sticky = self.inner.sticky.read().unwrap();
sticky.data().extensions.clone()
}
}
#[derive(Debug)]

View File

@@ -14,8 +14,7 @@ async fn setup(
let client = get_client_for_user(name, use_sqlite_store).await?;
let sliding_sync_builder = client
.sliding_sync("test-slidingsync")?
.sliding_sync_proxy(sliding_sync_proxy_url.parse()?)
.with_common_extensions();
.sliding_sync_proxy(sliding_sync_proxy_url.parse()?);
Ok((client, sliding_sync_builder))
}