From b91730436da03f796797fa94a7581611420a89fa Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 23 Aug 2024 15:23:14 +0200 Subject: [PATCH] feat: Use `sliding_sync::Version` everywhere. This patch replaces all the API using simplified sliding sync, or sliding sync proxy, by a unified `sliding_sync::Version` type! This patch disables auto-discovery for the moment. It will be re-enable with the next patches. --- bindings/matrix-sdk-ffi/src/authentication.rs | 11 +- bindings/matrix-sdk-ffi/src/client.rs | 72 ++++++---- bindings/matrix-sdk-ffi/src/client_builder.rs | 51 +++----- .../src/room_list_service/mod.rs | 15 ++- crates/matrix-sdk/src/client/builder.rs | 123 ++++++------------ crates/matrix-sdk/src/client/mod.rs | 46 ++----- crates/matrix-sdk/src/sliding_sync/README.md | 8 +- crates/matrix-sdk/src/sliding_sync/builder.rs | 29 ++--- crates/matrix-sdk/src/sliding_sync/client.rs | 10 +- crates/matrix-sdk/src/sliding_sync/error.rs | 5 + crates/matrix-sdk/src/sliding_sync/mod.rs | 72 +++++----- .../tests/integration/encryption/backups.rs | 1 - .../src/helpers.rs | 6 +- .../src/tests/sliding_sync/room.rs | 5 +- 14 files changed, 209 insertions(+), 245 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/authentication.rs b/bindings/matrix-sdk-ffi/src/authentication.rs index ca694102815..37aaff4e8e2 100644 --- a/bindings/matrix-sdk-ffi/src/authentication.rs +++ b/bindings/matrix-sdk-ffi/src/authentication.rs @@ -19,12 +19,12 @@ use matrix_sdk::{ }; use url::Url; -use crate::client::Client; +use crate::client::{Client, SlidingSyncVersion}; #[derive(uniffi::Object)] pub struct HomeserverLoginDetails { pub(crate) url: String, - pub(crate) sliding_sync_proxy: Option, + pub(crate) sliding_sync_version: SlidingSyncVersion, pub(crate) supports_oidc_login: bool, pub(crate) supports_password_login: bool, } @@ -36,10 +36,9 @@ impl HomeserverLoginDetails { self.url.clone() } - /// The URL of the discovered or manually set sliding sync proxy, - /// if any. - pub fn sliding_sync_proxy(&self) -> Option { - self.sliding_sync_proxy.clone() + /// The sliding sync version. + pub fn sliding_sync_version(&self) -> SlidingSyncVersion { + self.sliding_sync_version.clone() } /// Whether the current homeserver supports login using OIDC. diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index 7d71473a0d4..8a37679f00b 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -39,6 +39,7 @@ use matrix_sdk::{ serde::Raw, EventEncryptionAlgorithm, RoomId, TransactionId, UInt, UserId, }, + sliding_sync::Version as SdkSlidingSyncVersion, AuthApi, AuthSession, Client as MatrixClient, SessionChange, SessionTokens, }; use matrix_sdk_ui::notification_client::{ @@ -261,11 +262,11 @@ impl Client { pub async fn homeserver_login_details(&self) -> Arc { let supports_oidc_login = self.inner.oidc().fetch_authentication_issuer().await.is_ok(); let supports_password_login = self.supports_password_login().await.ok().unwrap_or(false); - let sliding_sync_proxy = self.sliding_sync_proxy().map(|proxy_url| proxy_url.to_string()); + let sliding_sync_version = self.sliding_sync_version(); Arc::new(HomeserverLoginDetails { url: self.homeserver(), - sliding_sync_proxy, + sliding_sync_version, supports_oidc_login, supports_password_login, }) @@ -383,19 +384,11 @@ impl Client { /// Restores the client from a `Session`. pub async fn restore_session(&self, session: Session) -> Result<(), ClientError> { - let sliding_sync_proxy = session.sliding_sync_proxy.clone(); + let sliding_sync_version = session.sliding_sync_version.clone(); let auth_session: AuthSession = session.try_into()?; self.restore_session_inner(auth_session).await?; - - if !self.inner.is_simplified_sliding_sync_enabled() { - if let Some(sliding_sync_proxy) = sliding_sync_proxy { - let sliding_sync_proxy = Url::parse(&sliding_sync_proxy) - .map_err(|error| ClientError::Generic { msg: error.to_string() })?; - - self.inner.set_sliding_sync_proxy(Some(sliding_sync_proxy)); - } - } + self.inner.set_sliding_sync_version(sliding_sync_version.try_into()?); Ok(()) } @@ -467,11 +460,9 @@ impl Client { Ok(()) } - /// The sliding sync proxy of the homeserver. It is either set automatically - /// during discovery or manually via `set_sliding_sync_proxy` or `None` - /// when not configured. - pub fn sliding_sync_proxy(&self) -> Option { - self.inner.sliding_sync_proxy() + /// The sliding sync version. + pub fn sliding_sync_version(&self) -> SlidingSyncVersion { + self.inner.sliding_sync_version().into() } /// Whether or not the client's homeserver supports the password login flow. @@ -1083,9 +1074,9 @@ impl Client { let auth_api = client.auth_api().context("Missing authentication API")?; let homeserver_url = client.homeserver().into(); - let sliding_sync_proxy = client.sliding_sync_proxy().map(|url| url.to_string()); + let sliding_sync_version = client.sliding_sync_version(); - Session::new(auth_api, homeserver_url, sliding_sync_proxy) + Session::new(auth_api, homeserver_url, sliding_sync_version.into()) } fn save_session( @@ -1312,15 +1303,15 @@ pub struct Session { /// Additional data for this session if OpenID Connect was used for /// authentication. pub oidc_data: Option, - /// The URL for the sliding sync proxy used for this session. - pub sliding_sync_proxy: Option, + /// The sliding sync version used for this session. + pub sliding_sync_version: SlidingSyncVersion, } impl Session { fn new( auth_api: AuthApi, homeserver_url: String, - sliding_sync_proxy: Option, + sliding_sync_version: SlidingSyncVersion, ) -> Result { match auth_api { // Build the session from the regular Matrix Auth Session. @@ -1338,7 +1329,7 @@ impl Session { device_id: device_id.to_string(), homeserver_url, oidc_data: None, - sliding_sync_proxy, + sliding_sync_version, }) } // Build the session from the OIDC UserSession. @@ -1375,7 +1366,7 @@ impl Session { device_id: device_id.to_string(), homeserver_url, oidc_data, - sliding_sync_proxy, + sliding_sync_version, }) } _ => Err(anyhow!("Unknown authentication API").into()), @@ -1393,7 +1384,7 @@ impl TryFrom for AuthSession { device_id, homeserver_url: _, oidc_data, - sliding_sync_proxy: _, + sliding_sync_version: _, } = value; if let Some(oidc_data) = oidc_data { @@ -1581,3 +1572,34 @@ impl MediaFileHandle { ) } } + +#[derive(Clone, uniffi::Enum)] +pub enum SlidingSyncVersion { + None, + Proxy { url: String }, + Native, +} + +impl From for SlidingSyncVersion { + fn from(value: SdkSlidingSyncVersion) -> Self { + match value { + SdkSlidingSyncVersion::None => Self::None, + SdkSlidingSyncVersion::Proxy { url } => Self::Proxy { url: url.to_string() }, + SdkSlidingSyncVersion::Native => Self::Native, + } + } +} + +impl TryFrom for SdkSlidingSyncVersion { + type Error = ClientError; + + fn try_from(value: SlidingSyncVersion) -> Result { + Ok(match value { + SlidingSyncVersion::None => Self::None, + SlidingSyncVersion::Proxy { url } => Self::Proxy { + url: Url::parse(&url).map_err(|e| ClientError::Generic { msg: e.to_string() })?, + }, + SlidingSyncVersion::Native => Self::Native, + }) + } +} diff --git a/bindings/matrix-sdk-ffi/src/client_builder.rs b/bindings/matrix-sdk-ffi/src/client_builder.rs index 94a182589cd..d52a9e4591a 100644 --- a/bindings/matrix-sdk-ffi/src/client_builder.rs +++ b/bindings/matrix-sdk-ffi/src/client_builder.rs @@ -15,12 +15,16 @@ use matrix_sdk::{ }; use ruma::api::error::{DeserializationError, FromHttpResponseError}; use tracing::{debug, error}; +use url::Url; use zeroize::Zeroizing; use super::{client::Client, RUNTIME}; use crate::{ - authentication::OidcConfiguration, client::ClientSessionDelegate, error::ClientError, - helpers::unwrap_or_clone_arc, task_handle::TaskHandle, + authentication::OidcConfiguration, + client::{ClientSessionDelegate, SlidingSyncVersion}, + error::ClientError, + helpers::unwrap_or_clone_arc, + task_handle::TaskHandle, }; /// A list of bytes containing a certificate in DER or PEM form. @@ -251,9 +255,7 @@ pub struct ClientBuilder { homeserver_cfg: Option, passphrase: Zeroizing>, user_agent: Option, - requires_sliding_sync: bool, - sliding_sync_proxy: Option, - is_simplified_sliding_sync_enabled: bool, + sliding_sync_version: SlidingSyncVersion, proxy: Option, disable_ssl_verification: bool, disable_automatic_token_refresh: bool, @@ -276,10 +278,7 @@ impl ClientBuilder { homeserver_cfg: None, passphrase: Zeroizing::new(None), user_agent: None, - requires_sliding_sync: false, - sliding_sync_proxy: None, - // By default, Simplified MSC3575 is turned off. - is_simplified_sliding_sync_enabled: false, + sliding_sync_version: SlidingSyncVersion::None, proxy: None, disable_ssl_verification: false, disable_automatic_token_refresh: false, @@ -365,21 +364,9 @@ impl ClientBuilder { Arc::new(builder) } - pub fn requires_sliding_sync(self: Arc) -> Arc { + pub fn sliding_sync_version(self: Arc, version: SlidingSyncVersion) -> Arc { let mut builder = unwrap_or_clone_arc(self); - builder.requires_sliding_sync = true; - Arc::new(builder) - } - - pub fn sliding_sync_proxy(self: Arc, sliding_sync_proxy: Option) -> Arc { - let mut builder = unwrap_or_clone_arc(self); - builder.sliding_sync_proxy = sliding_sync_proxy; - Arc::new(builder) - } - - pub fn simplified_sliding_sync(self: Arc, enable: bool) -> Arc { - let mut builder = unwrap_or_clone_arc(self); - builder.is_simplified_sliding_sync_enabled = enable; + builder.sliding_sync_version = version; Arc::new(builder) } @@ -550,15 +537,15 @@ impl ClientBuilder { .with_encryption_settings(builder.encryption_settings) .with_room_key_recipient_strategy(builder.room_key_recipient_strategy); - if let Some(sliding_sync_proxy) = builder.sliding_sync_proxy { - inner_builder = inner_builder.sliding_sync_proxy(sliding_sync_proxy); - } - - inner_builder = - inner_builder.simplified_sliding_sync(builder.is_simplified_sliding_sync_enabled); - - if builder.requires_sliding_sync { - inner_builder = inner_builder.requires_sliding_sync(); + match builder.sliding_sync_version { + SlidingSyncVersion::None => {} + SlidingSyncVersion::Proxy { url } => { + inner_builder = inner_builder.sliding_sync_proxy( + Url::parse(&url) + .map_err(|e| ClientBuildError::Generic { message: e.to_string() })?, + ) + } + SlidingSyncVersion::Native => inner_builder = inner_builder.sliding_sync_native(), } if let Some(config) = builder.request_config { diff --git a/crates/matrix-sdk-ui/src/room_list_service/mod.rs b/crates/matrix-sdk-ui/src/room_list_service/mod.rs index d45445b06ac..4f07e64352d 100644 --- a/crates/matrix-sdk-ui/src/room_list_service/mod.rs +++ b/crates/matrix-sdk-ui/src/room_list_service/mod.rs @@ -455,11 +455,13 @@ pub enum SyncIndicator { mod tests { use std::future::ready; + use assert_matches::assert_matches; use futures_util::{pin_mut, StreamExt}; use matrix_sdk::{ config::RequestConfig, matrix_auth::{MatrixSession, MatrixSessionTokens}, reqwest::Url, + sliding_sync::Version as SlidingSyncVersion, Client, SlidingSyncMode, }; use matrix_sdk_base::SessionMeta; @@ -513,17 +515,20 @@ mod tests { { let room_list = RoomListService::new(client.clone()).await?; - - assert!(room_list.sliding_sync().sliding_sync_proxy().is_none()); + assert_matches!(room_list.sliding_sync().version(), SlidingSyncVersion::Native); } { let url = Url::parse("https://foo.matrix/").unwrap(); - client.set_sliding_sync_proxy(Some(url.clone())); + client.set_sliding_sync_version(SlidingSyncVersion::Proxy { url: url.clone() }); let room_list = RoomListService::new(client.clone()).await?; - - assert_eq!(room_list.sliding_sync().sliding_sync_proxy(), Some(url)); + assert_matches!( + room_list.sliding_sync().version(), + SlidingSyncVersion::Proxy { url: given_url } => { + assert_eq!(&url, given_url); + } + ); } Ok(()) diff --git a/crates/matrix-sdk/src/client/builder.rs b/crates/matrix-sdk/src/client/builder.rs index 7c8aab0e936..98ea47ddd15 100644 --- a/crates/matrix-sdk/src/client/builder.rs +++ b/crates/matrix-sdk/src/client/builder.rs @@ -38,6 +38,8 @@ use crate::encryption::EncryptionSettings; use crate::http_client::HttpSettings; #[cfg(feature = "experimental-oidc")] use crate::oidc::OidcCtx; +#[cfg(feature = "experimental-sliding-sync")] +use crate::sliding_sync::Version as SlidingSyncVersion; use crate::{ authentication::AuthCtx, client::ClientServerCapabilities, config::RequestConfig, error::RumaApiError, http_client::HttpClient, send_queue::SendQueueData, HttpError, @@ -87,11 +89,7 @@ use crate::{ pub struct ClientBuilder { homeserver_cfg: Option, #[cfg(feature = "experimental-sliding-sync")] - requires_sliding_sync: bool, - #[cfg(feature = "experimental-sliding-sync")] - sliding_sync_proxy: Option, - #[cfg(feature = "experimental-sliding-sync")] - is_simplified_sliding_sync_enabled: bool, + sliding_sync_version: SlidingSyncVersion, http_cfg: Option, store_config: BuilderStoreConfig, request_config: RequestConfig, @@ -110,12 +108,7 @@ impl ClientBuilder { Self { homeserver_cfg: None, #[cfg(feature = "experimental-sliding-sync")] - requires_sliding_sync: false, - #[cfg(feature = "experimental-sliding-sync")] - sliding_sync_proxy: None, - // Simplified MSC3575 is turned on by default for the SDK. - #[cfg(feature = "experimental-sliding-sync")] - is_simplified_sliding_sync_enabled: true, + sliding_sync_version: SlidingSyncVersion::Native, http_cfg: None, store_config: BuilderStoreConfig::Custom(StoreConfig::default()), request_config: Default::default(), @@ -143,19 +136,7 @@ impl ClientBuilder { self } - /// Ensures that the client is built with support for sliding-sync, either - /// by discovering a proxy through the homeserver's well-known or by - /// providing one through [`Self::sliding_sync_proxy`]. - /// - /// In the future this may also perform a check for native support on the - /// homeserver. - #[cfg(feature = "experimental-sliding-sync")] - pub fn requires_sliding_sync(mut self) -> Self { - self.requires_sliding_sync = true; - self - } - - /// Set the sliding-sync proxy URL to use. + /// Set sliding sync to use the proxy, i.e. MSC3575. /// /// This value is always used no matter if the homeserver URL was defined /// with [`Self::homeserver_url`] or auto-discovered via @@ -163,15 +144,22 @@ impl ClientBuilder { /// [`Self::server_name_or_homeserver_url`] - any proxy discovered via the /// well-known lookup will be ignored. #[cfg(feature = "experimental-sliding-sync")] - pub fn sliding_sync_proxy(mut self, url: impl AsRef) -> Self { - self.sliding_sync_proxy = Some(url.as_ref().to_owned()); + pub fn sliding_sync_proxy(mut self, url: Url) -> Self { + self.sliding_sync_version = SlidingSyncVersion::Proxy { url }; + self + } + + /// Set sliding sync to be native, i.e. Simplified MSC3575. + #[cfg(feature = "experimental-sliding-sync")] + pub fn sliding_sync_native(mut self) -> Self { + self.sliding_sync_version = SlidingSyncVersion::Native; self } - /// Enable or disable Simplified MSC3575. + /// Set sliding sync to a specific version. #[cfg(feature = "experimental-sliding-sync")] - pub fn simplified_sliding_sync(mut self, enable: bool) -> Self { - self.is_simplified_sliding_sync_enabled = enable; + pub fn sliding_sync_version(mut self, version: SlidingSyncVersion) -> Self { + self.sliding_sync_version = version; self } @@ -487,10 +475,7 @@ impl ClientBuilder { #[cfg(feature = "experimental-oidc")] let allow_insecure_oidc = homeserver.starts_with("http://"); - #[cfg(feature = "experimental-sliding-sync")] - let mut sliding_sync_proxy = - self.sliding_sync_proxy.as_ref().map(|url| Url::parse(url)).transpose()?; - + /* #[cfg(feature = "experimental-sliding-sync")] if self.is_simplified_sliding_sync_enabled { // When using Simplified MSC3575, don't use a sliding sync proxy, allow the @@ -504,12 +489,7 @@ impl ClientBuilder { well_known.sliding_sync_proxy.and_then(|p| Url::parse(&p.url).ok()) } } - - #[cfg(feature = "experimental-sliding-sync")] - if self.requires_sliding_sync && sliding_sync_proxy.is_none() { - // In the future we will need to check for native support on the homeserver too. - return Err(ClientBuildError::SlidingSyncNotAvailable); - } + */ let homeserver = Url::parse(&homeserver)?; @@ -537,9 +517,7 @@ impl ClientBuilder { auth_ctx, homeserver, #[cfg(feature = "experimental-sliding-sync")] - sliding_sync_proxy, - #[cfg(feature = "experimental-sliding-sync")] - self.is_simplified_sliding_sync_enabled, + self.sliding_sync_version, http_client, base_client, server_capabilities, @@ -916,7 +894,7 @@ pub(crate) mod tests { #[async_test] async fn test_discovery_invalid_server() { // Given a new client builder. - let mut builder = make_non_sss_client_builder(); + let mut builder = ClientBuilder::new(); // When building a client with an invalid server name. builder = builder.server_name_or_homeserver_url("⚠️ This won't work 🚫"); @@ -929,7 +907,7 @@ pub(crate) mod tests { #[async_test] async fn test_discovery_no_server() { // Given a new client builder. - let mut builder = make_non_sss_client_builder(); + let mut builder = ClientBuilder::new(); // When building a client with a valid server name that doesn't exist. builder = builder.server_name_or_homeserver_url("localhost:3456"); @@ -945,7 +923,7 @@ pub(crate) mod tests { // Given a random web server that isn't a Matrix homeserver or hosting the // well-known file for one. let server = MockServer::start().await; - let mut builder = make_non_sss_client_builder(); + let mut builder = ClientBuilder::new(); // When building a client with the server's URL. builder = builder.server_name_or_homeserver_url(server.uri()); @@ -955,11 +933,12 @@ pub(crate) mod tests { assert_matches!(error, ClientBuildError::AutoDiscovery(FromHttpResponseError::Server(_))); } + /* #[async_test] async fn test_discovery_direct_legacy() { // Given a homeserver without a well-known file. let homeserver = make_mock_homeserver().await; - let mut builder = make_non_sss_client_builder(); + let mut builder = ClientBuilder::new(); // When building a client with the server's URL. builder = builder.server_name_or_homeserver_url(homeserver.uri()); @@ -975,7 +954,7 @@ pub(crate) mod tests { // Given a homeserver without a well-known file and with a custom sliding sync // proxy injected. let homeserver = make_mock_homeserver().await; - let mut builder = make_non_sss_client_builder(); + let mut builder = ClientBuilder::new(); #[cfg(feature = "experimental-sliding-sync")] { builder = builder.sliding_sync_proxy("https://localhost:1234"); @@ -995,7 +974,7 @@ pub(crate) mod tests { // Given a base server with a well-known file that has errors. let server = MockServer::start().await; let homeserver = make_mock_homeserver().await; - let mut builder = make_non_sss_client_builder(); + let mut builder = ClientBuilder::new(); let well_known = make_well_known_json(&homeserver.uri(), None); let bad_json = well_known.to_string().replace(',', ""); @@ -1022,7 +1001,7 @@ pub(crate) mod tests { // doesn't support sliding sync. let server = MockServer::start().await; let homeserver = make_mock_homeserver().await; - let mut builder = make_non_sss_client_builder(); + let mut builder = ClientBuilder::new(); Mock::given(method("GET")) .and(path("/.well-known/matrix/client")) @@ -1048,7 +1027,7 @@ pub(crate) mod tests { // sliding sync proxy. let server = MockServer::start().await; let homeserver = make_mock_homeserver().await; - let mut builder = make_non_sss_client_builder(); + let mut builder = ClientBuilder::new(); Mock::given(method("GET")) .and(path("/.well-known/matrix/client")) @@ -1075,7 +1054,7 @@ pub(crate) mod tests { // sliding sync proxy. let server = MockServer::start().await; let homeserver = make_mock_homeserver().await; - let mut builder = make_non_sss_client_builder(); + let mut builder = ClientBuilder::new(); Mock::given(method("GET")) .and(path("/.well-known/matrix/client")) @@ -1105,7 +1084,7 @@ pub(crate) mod tests { // sliding sync proxy. let server = MockServer::start().await; let homeserver = make_mock_homeserver().await; - let mut builder = make_non_sss_client_builder(); + let mut builder = ClientBuilder::new(); Mock::given(method("GET")) .and(path("/.well-known/matrix/client")) @@ -1117,16 +1096,18 @@ pub(crate) mod tests { .await; // When building a client for simplified sliding sync with the base server. - builder = builder.simplified_sliding_sync(true); + builder = builder.sliding_sync_native(); builder = builder.server_name_or_homeserver_url(server.uri()); let client = builder.build().await.unwrap(); // Then a client should not use the discovered sliding sync proxy. assert!(client.sliding_sync_proxy().is_none()); } + */ /* Requires sliding sync */ + /* #[async_test] #[cfg(feature = "experimental-sliding-sync")] async fn test_requires_sliding_sync_with_legacy_well_known() { @@ -1134,7 +1115,7 @@ pub(crate) mod tests { // doesn't support sliding sync. let server = MockServer::start().await; let homeserver = make_mock_homeserver().await; - let mut builder = make_non_sss_client_builder(); + let mut builder = ClientBuilder::new(); Mock::given(method("GET")) .and(path("/.well-known/matrix/client")) @@ -1160,7 +1141,7 @@ pub(crate) mod tests { // sliding sync proxy. let server = MockServer::start().await; let homeserver = make_mock_homeserver().await; - let mut builder = make_non_sss_client_builder(); + let mut builder = ClientBuilder::new(); Mock::given(method("GET")) .and(path("/.well-known/matrix/client")) @@ -1178,23 +1159,7 @@ pub(crate) mod tests { // Then a client should be built with support for sliding sync. assert_eq!(_client.sliding_sync_proxy(), Some("https://localhost:1234".parse().unwrap())); } - - #[async_test] - #[cfg(feature = "experimental-sliding-sync")] - async fn test_requires_sliding_sync_with_custom_proxy() { - // Given a homeserver without a well-known file and with a custom sliding sync - // proxy injected. - let homeserver = make_mock_homeserver().await; - let mut builder = make_non_sss_client_builder(); - builder = builder.sliding_sync_proxy("https://localhost:1234"); - - // When building a client that requires sliding sync with the server's URL. - builder = builder.requires_sliding_sync().server_name_or_homeserver_url(homeserver.uri()); - let _client = builder.build().await.unwrap(); - - // Then a client should be built with support for sliding sync. - assert_eq!(_client.sliding_sync_proxy(), Some("https://localhost:1234".parse().unwrap())); - } + */ /* Helper functions */ @@ -1238,18 +1203,4 @@ pub(crate) mod tests { object }) } - - /// These tests were built with regular sliding sync in mind so until - /// we remove it and update the tests, this makes a builder with SSS - /// disabled. - fn make_non_sss_client_builder() -> ClientBuilder { - let mut builder = ClientBuilder::new(); - - #[cfg(feature = "experimental-sliding-sync")] - { - builder = builder.simplified_sliding_sync(false); - } - - builder - } } diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index a96f8d072f1..d5bf08ce464 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -77,6 +77,8 @@ use url::Url; use self::futures::SendRequest; #[cfg(feature = "experimental-oidc")] use crate::oidc::Oidc; +#[cfg(feature = "experimental-sliding-sync")] +use crate::sliding_sync::Version as SlidingSyncVersion; use crate::{ authentication::{AuthCtx, AuthData, ReloadSessionCallback, SaveSessionCallback}, config::RequestConfig, @@ -229,15 +231,8 @@ pub(crate) struct ClientInner { /// The URL of the homeserver to connect to. homeserver: StdRwLock, - /// The sliding sync proxy that is trusted by the homeserver. #[cfg(feature = "experimental-sliding-sync")] - sliding_sync_proxy: StdRwLock>, - - /// Whether Simplified MSC3575 is used or not. - /// - /// This value must not be changed during the lifetime of the `Client`. - #[cfg(feature = "experimental-sliding-sync")] - is_simplified_sliding_sync_enabled: bool, + sliding_sync_version: StdRwLock, /// The underlying HTTP client. pub(crate) http_client: HttpClient, @@ -311,8 +306,7 @@ impl ClientInner { async fn new( auth_ctx: Arc, homeserver: Url, - #[cfg(feature = "experimental-sliding-sync")] sliding_sync_proxy: Option, - #[cfg(feature = "experimental-sliding-sync")] is_simplified_sliding_sync_enabled: bool, + #[cfg(feature = "experimental-sliding-sync")] sliding_sync_version: SlidingSyncVersion, http_client: HttpClient, base_client: BaseClient, server_capabilities: ClientServerCapabilities, @@ -325,9 +319,7 @@ impl ClientInner { homeserver: StdRwLock::new(homeserver), auth_ctx, #[cfg(feature = "experimental-sliding-sync")] - sliding_sync_proxy: StdRwLock::new(sliding_sync_proxy), - #[cfg(feature = "experimental-sliding-sync")] - is_simplified_sliding_sync_enabled, + sliding_sync_version: StdRwLock::new(sliding_sync_version), http_client, base_client, locks: Default::default(), @@ -467,24 +459,17 @@ impl Client { self.inner.homeserver.read().unwrap().clone() } - /// The sliding sync proxy that is trusted by the homeserver. + /// Get the sliding version. #[cfg(feature = "experimental-sliding-sync")] - pub fn sliding_sync_proxy(&self) -> Option { - let server = self.inner.sliding_sync_proxy.read().unwrap(); - Some(server.as_ref()?.clone()) + pub fn sliding_sync_version(&self) -> SlidingSyncVersion { + self.inner.sliding_sync_version.read().unwrap().clone() } - /// Force to set the sliding sync proxy URL. + /// Override the sliding version. #[cfg(feature = "experimental-sliding-sync")] - pub fn set_sliding_sync_proxy(&self, sliding_sync_proxy: Option) { - let mut lock = self.inner.sliding_sync_proxy.write().unwrap(); - *lock = sliding_sync_proxy; - } - - /// Check whether Simplified MSC3575 must be used. - #[cfg(feature = "experimental-sliding-sync")] - pub fn is_simplified_sliding_sync_enabled(&self) -> bool { - self.inner.is_simplified_sliding_sync_enabled + pub fn set_sliding_sync_version(&self, version: SlidingSyncVersion) { + let mut lock = self.inner.sliding_sync_version.write().unwrap(); + *lock = version; } /// Get the Matrix user session meta information. @@ -2187,17 +2172,12 @@ impl Client { /// Create a new specialized `Client` that can process notifications. pub async fn notification_client(&self) -> Result { - #[cfg(feature = "experimental-sliding-sync")] - let sliding_sync_proxy = self.inner.sliding_sync_proxy.read().unwrap().clone(); - let client = Client { inner: ClientInner::new( self.inner.auth_ctx.clone(), self.homeserver(), #[cfg(feature = "experimental-sliding-sync")] - sliding_sync_proxy, - #[cfg(feature = "experimental-sliding-sync")] - self.inner.is_simplified_sliding_sync_enabled, + self.sliding_sync_version(), self.inner.http_client.clone(), self.inner.base_client.clone_with_in_memory_state_store(), self.inner.server_capabilities.read().await.clone(), diff --git a/crates/matrix-sdk/src/sliding_sync/README.md b/crates/matrix-sdk/src/sliding_sync/README.md index 504a8168382..423623b465b 100644 --- a/crates/matrix-sdk/src/sliding_sync/README.md +++ b/crates/matrix-sdk/src/sliding_sync/README.md @@ -39,14 +39,14 @@ A unique identifier, less than 16 chars long, is required for each instance of Sliding Sync, and must be provided when getting a builder: ```rust,no_run -# use matrix_sdk::Client; +# use matrix_sdk::{Client, sliding_sync::Version}; # use url::Url; # async { # let homeserver = Url::parse("http://example.com")?; # let client = Client::new(homeserver).await?; let sliding_sync_builder = client .sliding_sync("main-sync")? - .sliding_sync_proxy(Url::parse("http://sliding-sync.example.org")?); + .version(Version::Proxy { url: Url::parse("http://sliding-sync.example.org")? }); # anyhow::Ok(()) # }; @@ -332,7 +332,7 @@ whenever a new set of timeline items is received by the server. # Full example ```rust,no_run -use matrix_sdk::{Client, sliding_sync::{SlidingSyncList, SlidingSyncMode}}; +use matrix_sdk::{Client, sliding_sync::{SlidingSyncList, SlidingSyncMode, Version}}; use matrix_sdk_base::sliding_sync::http; use ruma::{assign, events::StateEventType}; use tracing::{warn, error, info, debug}; @@ -346,7 +346,7 @@ let full_sync_list_name = "full-sync".to_owned(); 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 + .version(Version::Proxy { url: Url::parse("http://sliding-sync.example.org")? }) // our proxy server .with_account_data_extension( assign!(http::request::AccountData::default(), { enabled: Some(true) }), ) // we enable the account-data extension diff --git a/crates/matrix-sdk/src/sliding_sync/builder.rs b/crates/matrix-sdk/src/sliding_sync/builder.rs index a3377a6d64d..ad1d7dac983 100644 --- a/crates/matrix-sdk/src/sliding_sync/builder.rs +++ b/crates/matrix-sdk/src/sliding_sync/builder.rs @@ -9,12 +9,12 @@ use matrix_sdk_base::sliding_sync::http; use matrix_sdk_common::timer; use ruma::OwnedRoomId; use tokio::sync::{broadcast::channel, Mutex as AsyncMutex, RwLock as AsyncRwLock}; -use url::Url; use super::{ cache::{format_storage_key_prefix, restore_sliding_sync_state}, sticky_parameters::SlidingSyncStickyManager, Error, SlidingSync, SlidingSyncInner, SlidingSyncListBuilder, SlidingSyncPositionMarkers, + Version, }; use crate::{sliding_sync::SlidingSyncStickyParameters, Client, Result}; @@ -26,7 +26,7 @@ use crate::{sliding_sync::SlidingSyncStickyParameters, Client, Result}; pub struct SlidingSyncBuilder { id: String, storage_key: String, - sliding_sync_proxy: Option, + version: Option, client: Client, lists: Vec, extensions: Option, @@ -48,7 +48,7 @@ impl SlidingSyncBuilder { Ok(Self { id, storage_key, - sliding_sync_proxy: None, + version: None, client, lists: Vec::new(), extensions: None, @@ -61,14 +61,9 @@ impl SlidingSyncBuilder { } } - /// Set the sliding sync proxy URL. - /// - /// Note you might not need that in general, since the client uses the - /// `.well-known` endpoint to automatically find the sliding sync proxy - /// URL. This method should only be called if the proxy is at a - /// different URL than the one publicized in the `.well-known` endpoint. - pub fn sliding_sync_proxy(mut self, value: Url) -> Self { - self.sliding_sync_proxy = Some(value); + /// Set a specific version that will override the one from the [`Client`]. + pub fn version(mut self, version: Version) -> Self { + self.version = Some(version); self } @@ -235,6 +230,12 @@ impl SlidingSyncBuilder { pub async fn build(self) -> Result { let client = self.client; + let version = self.version.unwrap_or_else(|| client.sliding_sync_version()); + + if matches!(version, Version::None) { + return Err(crate::error::Error::SlidingSync(Error::VersionIsMissing)); + } + let (internal_channel_sender, _internal_channel_receiver) = channel(8); let mut lists = BTreeMap::new(); @@ -268,13 +269,9 @@ impl SlidingSyncBuilder { let rooms = AsyncRwLock::new(rooms); let lists = AsyncRwLock::new(lists); - // Use the configured sliding sync proxy, or if not set, try to use the one - // auto-discovered by the client, if any. - let sliding_sync_proxy = self.sliding_sync_proxy.or_else(|| client.sliding_sync_proxy()); - Ok(SlidingSync::new(SlidingSyncInner { id: self.id, - sliding_sync_proxy, + version, client, storage_key: self.storage_key, diff --git a/crates/matrix-sdk/src/sliding_sync/client.rs b/crates/matrix-sdk/src/sliding_sync/client.rs index e6fc89c2b07..2a609c03c6e 100644 --- a/crates/matrix-sdk/src/sliding_sync/client.rs +++ b/crates/matrix-sdk/src/sliding_sync/client.rs @@ -62,7 +62,7 @@ impl Client { ) -> Result { let response = self .base_client() - .process_sliding_sync(response, &(), self.is_simplified_sliding_sync_enabled()) + .process_sliding_sync(response, &(), self.sliding_sync_version().is_native()) .await?; tracing::debug!("done processing on base_client"); @@ -119,14 +119,18 @@ impl<'a> SlidingSyncResponseProcessor<'a> { Ok(()) } - pub async fn handle_room_response(&mut self, response: &http::Response) -> Result<()> { + pub async fn handle_room_response( + &mut self, + response: &http::Response, + from_simplified_sliding_sync: bool, + ) -> Result<()> { self.response = Some( self.client .base_client() .process_sliding_sync( response, &SlidingSyncPreviousEventsProvider(self.rooms), - self.client.is_simplified_sliding_sync_enabled(), + from_simplified_sliding_sync, ) .await?, ); diff --git a/crates/matrix-sdk/src/sliding_sync/error.rs b/crates/matrix-sdk/src/sliding_sync/error.rs index 15ebaf9e911..10706d860fd 100644 --- a/crates/matrix-sdk/src/sliding_sync/error.rs +++ b/crates/matrix-sdk/src/sliding_sync/error.rs @@ -7,6 +7,11 @@ use tokio::task::JoinError; #[derive(Error, Debug)] #[non_exhaustive] pub enum Error { + /// Sliding sync has been configured with a missing version. See + /// [`crate::sliding_sync::Version`]. + #[error("Sliding sync version is missing")] + VersionIsMissing, + /// The response we've received from the server can't be parsed or doesn't /// match up with the current expectations on the client side. A /// `sync`-restart might be required. diff --git a/crates/matrix-sdk/src/sliding_sync/mod.rs b/crates/matrix-sdk/src/sliding_sync/mod.rs index 1975ed3ab2c..6661a7de29c 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -33,6 +33,7 @@ use std::{ }; use async_stream::stream; +pub use client::Version; use futures_core::stream::Stream; pub use matrix_sdk_base::sliding_sync::http; use matrix_sdk_common::timer; @@ -46,7 +47,6 @@ use tokio::{ sync::{broadcast::Sender, Mutex as AsyncMutex, OwnedMutexGuard, RwLock as AsyncRwLock}, }; use tracing::{debug, error, info, instrument, trace, warn, Instrument, Span}; -use url::Url; #[cfg(feature = "e2e-encryption")] use self::utils::JoinHandleExt as _; @@ -74,8 +74,7 @@ pub(super) struct SlidingSyncInner { /// Used to distinguish different connections to the sliding sync proxy. id: String, - /// Customize the sliding sync proxy URL. - sliding_sync_proxy: Option, + version: Version, /// The HTTP Matrix client. client: Client, @@ -264,7 +263,7 @@ impl SlidingSync { // Compute `limited` for the SS proxy only, if we're interested in a room list // query. - if !self.inner.client.is_simplified_sliding_sync_enabled() && must_process_rooms_response { + if !self.inner.version.is_native() && must_process_rooms_response { let known_rooms = self.inner.rooms.read().await; compute_limited(&known_rooms, &mut sliding_sync_response.rooms); } @@ -297,7 +296,9 @@ impl SlidingSync { // // NOTE: SS proxy workaround. if must_process_rooms_response { - response_processor.handle_room_response(&sliding_sync_response).await?; + response_processor + .handle_room_response(&sliding_sync_response, self.inner.version.is_native()) + .await?; } response_processor.process_and_take_response().await? @@ -537,7 +538,7 @@ impl SlidingSync { // Prepare the request. let request = self.inner.client.send(request, Some(request_config)).with_homeserver_override( - self.inner.sliding_sync_proxy.as_ref().map(ToString::to_string), + self.inner.version.overriding_url().map(ToString::to_string), ); // Send the request and get a response with end-to-end encryption support. @@ -670,7 +671,7 @@ impl SlidingSync { // because it's the future standard. If // `Client::is_simplified_sliding_sync_enabled` is turned off, the // Simplified MSC3575 `Request` must be transformed into a MSC3575 `Request`. - if !self.inner.client.is_simplified_sliding_sync_enabled() { + if !self.inner.version.is_native() { self.send_sync_request( Into::::into(request), request_config, @@ -837,9 +838,9 @@ impl SlidingSync { position_lock.pos = Some(new_pos); } - /// Get the URL to Sliding Sync. - pub fn sliding_sync_proxy(&self) -> Option { - self.inner.sliding_sync_proxy.clone() + /// Get the sliding sync version used by this instance. + pub fn version(&self) -> &Version { + &self.inner.version } /// Read the static extension configuration for this Sliding Sync. @@ -1053,7 +1054,6 @@ fn compute_limited( #[cfg(test)] #[allow(clippy::dbg_macro)] mod tests { - use std::{ collections::BTreeMap, future::ready, @@ -1079,7 +1079,7 @@ mod tests { compute_limited, http, sticky_parameters::{LazyTransactionId, SlidingSyncStickyManager}, FrozenSlidingSync, SlidingSync, SlidingSyncList, SlidingSyncListBuilder, SlidingSyncMode, - SlidingSyncRoom, SlidingSyncStickyParameters, + SlidingSyncRoom, SlidingSyncStickyParameters, Version, }; use crate::{ sliding_sync::cache::restore_sliding_sync_state, test_utils::logged_in_client, Result, @@ -1993,42 +1993,56 @@ mod tests { } #[async_test] - async fn test_sliding_sync_proxy_url() -> Result<()> { + async fn test_sliding_sync_version() -> Result<()> { let server = MockServer::start().await; let client = logged_in_client(Some(server.uri())).await; + // By default, sliding sync inherits its version from the client, which is + // `Native`. { - // A server that doesn't expose a sliding sync proxy gets and transmits none, by - // default. - let sync = client.sliding_sync("no-proxy")?.build().await?; + let sync = client.sliding_sync("default")?.build().await?; - assert!(sync.sliding_sync_proxy().is_none()); + assert_matches!(sync.version(), Version::Native); } + // Sliding can override the configuration from the client. { - // The sliding sync builder can be used to customize a proxy, though. let url = Url::parse("https://bar.matrix/").unwrap(); - let sync = - client.sliding_sync("own-proxy")?.sliding_sync_proxy(url.clone()).build().await?; - assert_eq!(sync.sliding_sync_proxy(), Some(url)); + let sync = client + .sliding_sync("own-proxy")? + .version(Version::Proxy { url: url.clone() }) + .build() + .await?; + + assert_matches!( + sync.version(), + Version::Proxy { url: given_url } => { + assert_eq!(&url, given_url); + } + ); } - // Set the client's proxy, that will be inherited by sliding sync. + // Sliding sync inherits from the client… let url = Url::parse("https://foo.matrix/").unwrap(); - client.set_sliding_sync_proxy(Some(url.clone())); + client.set_sliding_sync_version(Version::Proxy { url: url.clone() }); { // The sliding sync inherits the client's sliding sync proxy URL. let sync = client.sliding_sync("client-proxy")?.build().await?; - assert_eq!(sync.sliding_sync_proxy(), Some(url)); + + assert_matches!( + sync.version(), + Version::Proxy { url: given_url } => { + assert_eq!(&url, given_url); + } + ); } { - // …unless we override it. - let url = Url::parse("https://bar.matrix/").unwrap(); - let sync = - client.sliding_sync("own-proxy")?.sliding_sync_proxy(url.clone()).build().await?; - assert_eq!(sync.sliding_sync_proxy(), Some(url)); + // …unless we override it afterwards. + let sync = client.sliding_sync("own-proxy")?.version(Version::Native).build().await?; + + assert_matches!(sync.version(), Version::Native); } Ok(()) diff --git a/crates/matrix-sdk/tests/integration/encryption/backups.rs b/crates/matrix-sdk/tests/integration/encryption/backups.rs index a037ccf095f..a813aad37b6 100644 --- a/crates/matrix-sdk/tests/integration/encryption/backups.rs +++ b/crates/matrix-sdk/tests/integration/encryption/backups.rs @@ -725,7 +725,6 @@ async fn incremental_upload_of_keys_sliding_sync() -> Result<()> { let server = wiremock::MockServer::start().await; let builder = Client::builder() .homeserver_url(server.uri()) - .sliding_sync_proxy(server.uri()) .server_versions([ruma::api::MatrixVersion::V1_0]); let client = diff --git a/testing/matrix-sdk-integration-testing/src/helpers.rs b/testing/matrix-sdk-integration-testing/src/helpers.rs index c6b095b8b46..3b729097f5b 100644 --- a/testing/matrix-sdk-integration-testing/src/helpers.rs +++ b/testing/matrix-sdk-integration-testing/src/helpers.rs @@ -19,6 +19,7 @@ use matrix_sdk::{ }; use once_cell::sync::Lazy; use rand::Rng as _; +use reqwest::Url; use tempfile::{tempdir, TempDir}; use tokio::{sync::Mutex, time::sleep}; @@ -82,10 +83,11 @@ impl TestClientBuilder { let mut client_builder = Client::builder() .user_agent("matrix-sdk-integration-tests") .homeserver_url(homeserver_url) - .sliding_sync_proxy(sliding_sync_proxy_url) // Disable Simplified MSC3575 for the integration tests as, at the time of writing // (2024-07-15), we use a Synapse version that doesn't support Simplified MSC3575. - .simplified_sliding_sync(false) + .sliding_sync_proxy( + Url::parse(&sliding_sync_proxy_url).expect("Sliding sync proxy URL is invalid"), + ) .with_encryption_settings(self.encryption_settings) .request_config(RequestConfig::short_retry()); diff --git a/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs b/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs index 8cf82bbdd3a..1966f90765c 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/sliding_sync/room.rs @@ -268,13 +268,12 @@ async fn test_joined_user_can_create_push_context_with_room_list_service() -> Re // And a new device for Alice that uses sliding sync, let hs = alice.homeserver(); - let sliding_sync_url = alice.sliding_sync_proxy(); + let sliding_sync_version = alice.sliding_sync_version(); let alice_id = alice.user_id().unwrap().localpart().to_owned(); let alice = Client::builder() .homeserver_url(hs) - .simplified_sliding_sync(false) - .sliding_sync_proxy(sliding_sync_url.unwrap()) + .sliding_sync_version(sliding_sync_version) .build() .await .unwrap();