From f0100f5a82baaab8f28b3567a15a79735936862d Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 23 Aug 2024 15:22:53 +0200 Subject: [PATCH 01/11] feat(sdk): Add `sliding_sync::Version`. --- crates/matrix-sdk/src/sliding_sync/client.rs | 32 ++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/crates/matrix-sdk/src/sliding_sync/client.rs b/crates/matrix-sdk/src/sliding_sync/client.rs index 6734b2cece5..e6fc89c2b07 100644 --- a/crates/matrix-sdk/src/sliding_sync/client.rs +++ b/crates/matrix-sdk/src/sliding_sync/client.rs @@ -4,10 +4,42 @@ use imbl::Vector; use matrix_sdk_base::{sliding_sync::http, sync::SyncResponse, PreviousEventsProvider}; use ruma::{events::AnyToDeviceEvent, serde::Raw, OwnedRoomId}; use tracing::error; +use url::Url; use super::{SlidingSync, SlidingSyncBuilder}; use crate::{Client, Result, SlidingSyncRoom}; +/// A sliding sync version. +#[derive(Clone, Debug)] +pub enum Version { + /// No version. Useful to represent that sliding sync is disable for + /// example, and that the version is unknown. + None, + + /// Use the version of the sliding sync proxy, i.e. MSC3575. + Proxy { + /// URL to the proxy. + url: Url, + }, + + /// Use the version of the sliding sync implementation inside Synapse, i.e. + /// Simplified MSC3575. + Native, +} + +impl Version { + pub(crate) fn is_native(&self) -> bool { + matches!(self, Self::Native) + } + + pub(crate) fn overriding_url(&self) -> Option<&Url> { + match self { + Self::Proxy { url } => Some(url), + _ => None, + } + } +} + impl Client { /// Create a [`SlidingSyncBuilder`] tied to this client, with the given /// identifier. From b91730436da03f796797fa94a7581611420a89fa Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Fri, 23 Aug 2024 15:23:14 +0200 Subject: [PATCH 02/11] 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(); From a5e6bd03231e72fd03ba6a392495a16cf3372b33 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 26 Aug 2024 15:57:58 +0200 Subject: [PATCH 03/11] feat(sdk): `ClientBuilder` extracts `get_supported_versions`. This patch changes the type of `discover_homeserver_from_server_name_or_url`. It now returns a `Url` instead of a `String` for the homeserver URL. It also returns an `Option` in addition to the other values. The change from `String` to `Url` is necessary to avoid a double-parsing. It was parsed in `build()` but previously in `discover_homeserver_from_server_name_or_url` in the last branch. The addition of `get_supported_versions::Response` is necessary for the next patch. It's going to be helpful to auto-discover sliding sync in Synapse. The change happens here because `get_supported_versions::Response` is already received in `discover_homeserver_from_server_name_or_url`. This patch makes it easy to re-use it so that the request is sent only once. This patch therefore changes `check_is_homeserver` a little bit to become `get_supported_versions`, and inlines its previous call inside `discover_homeserver_from_server_name_or_url`. --- crates/matrix-sdk/src/client/builder.rs | 45 +++++++++++++------------ 1 file changed, 24 insertions(+), 21 deletions(-) diff --git a/crates/matrix-sdk/src/client/builder.rs b/crates/matrix-sdk/src/client/builder.rs index 98ea47ddd15..4cc6bbc3c31 100644 --- a/crates/matrix-sdk/src/client/builder.rs +++ b/crates/matrix-sdk/src/client/builder.rs @@ -458,12 +458,13 @@ impl ClientBuilder { let http_client = HttpClient::new(inner_http_client.clone(), self.request_config); #[allow(unused_variables)] - let (homeserver, well_known) = match homeserver_cfg { - HomeserverConfig::Url(url) => (url, None), + let (homeserver, well_known, versions) = match homeserver_cfg { + HomeserverConfig::Url(url) => (Url::parse(&url)?, None, None), HomeserverConfig::ServerName { server: server_name, protocol } => { let well_known = discover_homeserver(server_name, protocol, &http_client).await?; - (well_known.homeserver.base_url.clone(), Some(well_known)) + + (Url::parse(&well_known.homeserver.base_url)?, Some(well_known), None) } HomeserverConfig::ServerNameOrUrl(server_name_or_url) => { @@ -472,9 +473,6 @@ impl ClientBuilder { } }; - #[cfg(feature = "experimental-oidc")] - let allow_insecure_oidc = homeserver.starts_with("http://"); - /* #[cfg(feature = "experimental-sliding-sync")] if self.is_simplified_sliding_sync_enabled { @@ -491,7 +489,9 @@ impl ClientBuilder { } */ - let homeserver = Url::parse(&homeserver)?; + + #[cfg(feature = "experimental-oidc")] + let allow_insecure_oidc = homeserver.scheme() == "http"; let auth_ctx = Arc::new(AuthCtx { handle_refresh_tokens: self.handle_refresh_tokens, @@ -541,7 +541,10 @@ impl ClientBuilder { async fn discover_homeserver_from_server_name_or_url( mut server_name_or_url: String, http_client: &HttpClient, -) -> Result<(String, Option), ClientBuildError> { +) -> Result< + (Url, Option, Option), + ClientBuildError, +> { let mut discovery_error: Option = None; // Attempt discovery as a server name first. @@ -556,7 +559,7 @@ async fn discover_homeserver_from_server_name_or_url( match discover_homeserver(server_name.clone(), protocol, http_client).await { Ok(well_known) => { - return Ok((well_known.homeserver.base_url.clone(), Some(well_known))); + return Ok((Url::parse(&well_known.homeserver.base_url)?, Some(well_known), None)); } Err(e) => { debug!(error = %e, "Well-known discovery failed."); @@ -575,8 +578,13 @@ async fn discover_homeserver_from_server_name_or_url( // trying a homeserver URL. if let Ok(homeserver_url) = Url::parse(&server_name_or_url) { // Make sure the URL is definitely for a homeserver. - if check_is_homeserver(&homeserver_url, http_client).await { - return Ok((homeserver_url.to_string(), None)); + match get_supported_versions(&homeserver_url, http_client).await { + Ok(_) => { + return Ok((homeserver_url, None, None)); + } + Err(e) => { + debug!(error = %e, "Checking supported versions failed."); + } } } @@ -626,9 +634,11 @@ async fn discover_homeserver( Ok(well_known) } -/// Checks if the given URL represents a valid homeserver. -async fn check_is_homeserver(homeserver_url: &Url, http_client: &HttpClient) -> bool { - match http_client +async fn get_supported_versions( + homeserver_url: &Url, + http_client: &HttpClient, +) -> Result { + http_client .send( get_supported_versions::Request::new(), Some(RequestConfig::short_retry()), @@ -638,13 +648,6 @@ async fn check_is_homeserver(homeserver_url: &Url, http_client: &HttpClient) -> Default::default(), ) .await - { - Ok(_) => true, - Err(e) => { - debug!(error = %e, "Checking supported versions failed."); - false - } - } } #[allow(clippy::unused_async)] // False positive when building with !sqlite & !indexeddb From 3793c06739e6ddcabe9b9f0293205067aa3b1c62 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 26 Aug 2024 16:10:23 +0200 Subject: [PATCH 04/11] feat(sdk): Add `sliding_sync::VersionBuilder`. This patch adds a builder for `sliding_sync::Version`. It is a similar enum except that it has `DiscoverProxy` and `DiscoverNative` to automatically configure `Version::Proxy` or `Version::Native`. --- crates/matrix-sdk/src/sliding_sync/client.rs | 221 ++++++++++++++++++- crates/matrix-sdk/src/sliding_sync/error.rs | 5 + crates/matrix-sdk/src/sliding_sync/mod.rs | 2 +- 3 files changed, 225 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/client.rs b/crates/matrix-sdk/src/sliding_sync/client.rs index 2a609c03c6e..b63cdbcb800 100644 --- a/crates/matrix-sdk/src/sliding_sync/client.rs +++ b/crates/matrix-sdk/src/sliding_sync/client.rs @@ -2,11 +2,16 @@ use std::collections::BTreeMap; use imbl::Vector; use matrix_sdk_base::{sliding_sync::http, sync::SyncResponse, PreviousEventsProvider}; -use ruma::{events::AnyToDeviceEvent, serde::Raw, OwnedRoomId}; +use ruma::{ + api::client::discovery::{discover_homeserver, get_supported_versions}, + events::AnyToDeviceEvent, + serde::Raw, + OwnedRoomId, +}; use tracing::error; use url::Url; -use super::{SlidingSync, SlidingSyncBuilder}; +use super::{Error, SlidingSync, SlidingSyncBuilder}; use crate::{Client, Result, SlidingSyncRoom}; /// A sliding sync version. @@ -40,6 +45,97 @@ impl Version { } } +/// A builder for [`Version`]. +#[derive(Clone, Debug)] +pub enum VersionBuilder { + /// Build a [`Version::None`]. + None, + + /// Build a [`Version::Proxy`]. + Proxy { + /// Coerced URL to the proxy. + url: Url, + }, + + /// Build a [`Version::Native`]. + Native, + + /// Build a [`Version::Proxy`] by auto-discovering it. + /// + /// It is available if the server enables it via `.well-known`. + DiscoverProxy, + + /// Build a [`Version::Native`] by auto-discovering it. + /// + /// It is available if the server enables it via `/versions`. + DiscoverNative, +} + +impl VersionBuilder { + pub(crate) fn needs_get_supported_versions(&self) -> bool { + matches!(self, Self::DiscoverNative) + } + + /// Build a [`Version`]. + /// + /// It can fail if auto-discovering fails, e.g. if `.well-known` + /// or `/versions` do contain invalid data. + pub fn build( + self, + well_known: Option<&discover_homeserver::Response>, + versions: Option<&get_supported_versions::Response>, + ) -> Result { + Ok(match self { + Self::None => Version::None, + + Self::Proxy { url } => Version::Proxy { url }, + + Self::Native => Version::Native, + + Self::DiscoverProxy => { + let Some(well_known) = well_known else { + return Err(Error::VersionCannotBeDiscovered( + "`.well-known` is `None`".to_owned(), + )); + }; + + let Some(info) = &well_known.sliding_sync_proxy else { + return Err(Error::VersionCannotBeDiscovered( + "`.well-known` does not contain a `sliding_sync_proxy` entry".to_owned(), + )); + }; + + let url = Url::parse(&info.url).map_err(|e| { + Error::VersionCannotBeDiscovered(format!( + "`.well-known` contains an invalid `sliding_sync_proxy` entry ({e})" + )) + })?; + + Version::Proxy { url } + } + + Self::DiscoverNative => { + let Some(versions) = versions else { + return Err(Error::VersionCannotBeDiscovered( + "`/versions` is `None`".to_owned(), + )); + }; + + match versions.unstable_features.get("org.matrix.simplified_msc3575") { + Some(value) if *value => Version::Native, + _ => { + return Err(Error::VersionCannotBeDiscovered( + "`/versions` does not contain `org.matrix.simplified_msc3575` in its \ + `unstable_features`" + .to_owned(), + )) + } + } + } + }) + } +} + impl Client { /// Create a [`SlidingSyncBuilder`] tied to this client, with the given /// identifier. @@ -178,16 +274,137 @@ async fn update_in_memory_caches(client: &Client, response: &SyncResponse) -> Re mod tests { use std::collections::BTreeMap; + use assert_matches::assert_matches; use matrix_sdk_base::notification_settings::RoomNotificationMode; use matrix_sdk_test::async_test; use ruma::{assign, room_id, serde::Raw}; use serde_json::json; + use url::Url; use crate::{ error::Result, sliding_sync::http, test_utils::logged_in_client_with_server, SlidingSyncList, SlidingSyncMode, }; + use super::{discover_homeserver, get_supported_versions, Error, Version, VersionBuilder}; + + #[test] + fn test_version_builder_none() { + assert_matches!(VersionBuilder::None.build(None, None), Ok(Version::None)); + } + + #[test] + fn test_version_builder_proxy() { + let expected_url = Url::parse("https://matrix.org:1234").unwrap(); + + assert_matches!( + VersionBuilder::Proxy { url: expected_url.clone() }.build(None, None), + Ok(Version::Proxy { url }) => { + assert_eq!(url, expected_url); + } + ); + } + + #[test] + fn test_version_builder_native() { + assert_matches!(VersionBuilder::Native.build(None, None), Ok(Version::Native)); + } + + #[test] + fn test_version_builder_discover_proxy() { + let expected_url = Url::parse("https://matrix.org:1234").unwrap(); + let mut response = discover_homeserver::Response::new( + discover_homeserver::HomeserverInfo::new("matrix.org".to_owned()), + ); + response.sliding_sync_proxy = + Some(discover_homeserver::SlidingSyncProxyInfo::new(expected_url.to_string())); + + assert_matches!( + VersionBuilder::DiscoverProxy.build(Some(&response), None), + Ok(Version::Proxy { url }) => { + assert_eq!(url, expected_url); + } + ); + } + + #[test] + fn test_version_builder_discover_proxy_no_well_known() { + assert_matches!( + VersionBuilder::DiscoverProxy.build(None, None), + Err(Error::VersionCannotBeDiscovered(msg)) => { + assert_eq!(msg, "`.well-known` is `None`"); + } + ); + } + + #[test] + fn test_version_builder_discover_proxy_no_sliding_sync_proxy_in_well_known() { + let mut response = discover_homeserver::Response::new( + discover_homeserver::HomeserverInfo::new("matrix.org".to_owned()), + ); + response.sliding_sync_proxy = None; // already `None` but the test is clearer now. + + assert_matches!( + VersionBuilder::DiscoverProxy.build(Some(&response), None), + Err(Error::VersionCannotBeDiscovered(msg)) => { + assert_eq!(msg, "`.well-known` does not contain a `sliding_sync_proxy` entry"); + } + ); + } + + #[test] + fn test_version_builder_discover_proxy_invalid_sliding_sync_proxy_in_well_known() { + let mut response = discover_homeserver::Response::new( + discover_homeserver::HomeserverInfo::new("matrix.org".to_owned()), + ); + response.sliding_sync_proxy = + Some(discover_homeserver::SlidingSyncProxyInfo::new("💥".to_owned())); + + assert_matches!( + VersionBuilder::DiscoverProxy.build(Some(&response), None), + Err(Error::VersionCannotBeDiscovered(msg)) => { + assert_eq!(msg, "`.well-known` contains an invalid `sliding_sync_proxy` entry (relative URL without a base)"); + } + ); + } + + #[test] + fn test_version_builder_discover_native() { + let mut response = get_supported_versions::Response::new(vec![]); + response.unstable_features = [("org.matrix.simplified_msc3575".to_owned(), true)].into(); + + assert_matches!( + VersionBuilder::DiscoverNative.build(None, Some(&response)), + Ok(Version::Native) + ); + } + + #[test] + fn test_version_builder_discover_native_no_supported_versions() { + assert_matches!( + VersionBuilder::DiscoverNative.build(None, None), + Err(Error::VersionCannotBeDiscovered(msg)) => { + assert_eq!(msg, "`/versions` is `None`".to_owned()); + } + ); + } + + #[test] + fn test_version_builder_discover_native_unstable_features_is_disabled() { + let mut response = get_supported_versions::Response::new(vec![]); + response.unstable_features = [("org.matrix.simplified_msc3575".to_owned(), false)].into(); + + assert_matches!( + VersionBuilder::DiscoverNative.build(None, Some(&response)), + Err(Error::VersionCannotBeDiscovered(msg)) => { + assert_eq!( + msg, + "`/versions` does not contain `org.matrix.simplified_msc3575` in its `unstable_features`".to_owned() + ); + } + ); + } + #[async_test] async fn test_cache_user_defined_notification_mode() -> Result<()> { let (client, _server) = logged_in_client_with_server().await; diff --git a/crates/matrix-sdk/src/sliding_sync/error.rs b/crates/matrix-sdk/src/sliding_sync/error.rs index 10706d860fd..b76b3ffab5f 100644 --- a/crates/matrix-sdk/src/sliding_sync/error.rs +++ b/crates/matrix-sdk/src/sliding_sync/error.rs @@ -12,6 +12,11 @@ pub enum Error { #[error("Sliding sync version is missing")] VersionIsMissing, + /// Sliding sync version has been configured to be discovered, + /// but it has failed. See [`crate::sliding_sync::Version`]. + #[error("Sliding sync version cannot be discovered: {0}")] + VersionCannotBeDiscovered(String), + /// 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 6661a7de29c..216fe2a5a30 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -33,7 +33,7 @@ use std::{ }; use async_stream::stream; -pub use client::Version; +pub use client::{Version, VersionBuilder}; use futures_core::stream::Stream; pub use matrix_sdk_base::sliding_sync::http; use matrix_sdk_common::timer; From ea8e885f19c7a0a2b2aaecd583226e7fdf67bcaa Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 26 Aug 2024 16:18:38 +0200 Subject: [PATCH 05/11] feat: Use the new `sliding_sync::VersionBuilder`. --- bindings/matrix-sdk-ffi/src/client_builder.rs | 75 ++++++++++++------- crates/matrix-sdk/src/client/builder.rs | 74 ++++++++---------- .../src/helpers.rs | 8 +- 3 files changed, 84 insertions(+), 73 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/client_builder.rs b/bindings/matrix-sdk-ffi/src/client_builder.rs index d52a9e4591a..1386ec121b9 100644 --- a/bindings/matrix-sdk-ffi/src/client_builder.rs +++ b/bindings/matrix-sdk-ffi/src/client_builder.rs @@ -10,6 +10,9 @@ use matrix_sdk::{ encryption::{BackupDownloadStrategy, EncryptionSettings}, reqwest::Certificate, ruma::{ServerName, UserId}, + sliding_sync::{ + Error as MatrixSlidingSyncError, VersionBuilder as MatrixSlidingSyncVersionBuilder, + }, Client as MatrixClient, ClientBuildError as MatrixClientBuildError, HttpError, IdParseError, RumaApiError, }; @@ -20,11 +23,8 @@ use zeroize::Zeroizing; use super::{client::Client, RUNTIME}; use crate::{ - authentication::OidcConfiguration, - client::{ClientSessionDelegate, SlidingSyncVersion}, - error::ClientError, - helpers::unwrap_or_clone_arc, - task_handle::TaskHandle, + authentication::OidcConfiguration, client::ClientSessionDelegate, error::ClientError, + helpers::unwrap_or_clone_arc, task_handle::TaskHandle, }; /// A list of bytes containing a certificate in DER or PEM form. @@ -82,7 +82,7 @@ pub enum HumanQrLoginError { Declined, #[error("An unknown error has happened.")] Unknown, - #[error("The homeserver doesn't provide a sliding sync proxy in its configuration.")] + #[error("The homeserver doesn't provide sliding sync in its configuration.")] SlidingSyncNotAvailable, #[error("Unable to use OIDC as the supplied client metadata is invalid.")] OidcMetadataInvalid, @@ -194,12 +194,10 @@ pub enum ClientBuildError { WellKnownLookupFailed(RumaApiError), #[error(transparent)] WellKnownDeserializationError(DeserializationError), - #[error("The homeserver doesn't provide a trusted sliding sync proxy in its well-known configuration.")] - SlidingSyncNotAvailable, - + #[error(transparent)] + SlidingSync(MatrixSlidingSyncError), #[error(transparent)] Sdk(MatrixClientBuildError), - #[error("Failed to build the client: {message}")] Generic { message: String }, } @@ -215,10 +213,7 @@ impl From for ClientBuildError { MatrixClientBuildError::AutoDiscovery(FromHttpResponseError::Deserialization(e)) => { ClientBuildError::WellKnownDeserializationError(e) } - MatrixClientBuildError::SlidingSyncNotAvailable => { - ClientBuildError::SlidingSyncNotAvailable - } - + MatrixClientBuildError::SlidingSync(e) => ClientBuildError::SlidingSync(e), _ => ClientBuildError::Sdk(e), } } @@ -255,7 +250,7 @@ pub struct ClientBuilder { homeserver_cfg: Option, passphrase: Zeroizing>, user_agent: Option, - sliding_sync_version: SlidingSyncVersion, + sliding_sync_version_builder: SlidingSyncVersionBuilder, proxy: Option, disable_ssl_verification: bool, disable_automatic_token_refresh: bool, @@ -278,7 +273,7 @@ impl ClientBuilder { homeserver_cfg: None, passphrase: Zeroizing::new(None), user_agent: None, - sliding_sync_version: SlidingSyncVersion::None, + sliding_sync_version_builder: SlidingSyncVersionBuilder::None, proxy: None, disable_ssl_verification: false, disable_automatic_token_refresh: false, @@ -364,9 +359,12 @@ impl ClientBuilder { Arc::new(builder) } - pub fn sliding_sync_version(self: Arc, version: SlidingSyncVersion) -> Arc { + pub fn sliding_sync_version_builder( + self: Arc, + version_builder: SlidingSyncVersionBuilder, + ) -> Arc { let mut builder = unwrap_or_clone_arc(self); - builder.sliding_sync_version = version; + builder.sliding_sync_version_builder = version_builder; Arc::new(builder) } @@ -537,15 +535,31 @@ impl ClientBuilder { .with_encryption_settings(builder.encryption_settings) .with_room_key_recipient_strategy(builder.room_key_recipient_strategy); - 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() })?, + match builder.sliding_sync_version_builder { + SlidingSyncVersionBuilder::None => { + inner_builder = inner_builder + .sliding_sync_version_builder(MatrixSlidingSyncVersionBuilder::None) + } + SlidingSyncVersionBuilder::Proxy { url } => { + inner_builder = inner_builder.sliding_sync_version_builder( + MatrixSlidingSyncVersionBuilder::Proxy { + url: Url::parse(&url) + .map_err(|e| ClientBuildError::Generic { message: e.to_string() })?, + }, ) } - SlidingSyncVersion::Native => inner_builder = inner_builder.sliding_sync_native(), + SlidingSyncVersionBuilder::Native => { + inner_builder = inner_builder + .sliding_sync_version_builder(MatrixSlidingSyncVersionBuilder::Native) + } + SlidingSyncVersionBuilder::DiscoverProxy => { + inner_builder = inner_builder + .sliding_sync_version_builder(MatrixSlidingSyncVersionBuilder::DiscoverProxy) + } + SlidingSyncVersionBuilder::DiscoverNative => { + inner_builder = inner_builder + .sliding_sync_version_builder(MatrixSlidingSyncVersionBuilder::DiscoverNative) + } } if let Some(config) = builder.request_config { @@ -606,7 +620,7 @@ impl ClientBuilder { let builder = self.server_name_or_homeserver_url(server_name.to_owned()); let client = builder.build().await.map_err(|e| match e { - ClientBuildError::SlidingSyncNotAvailable => HumanQrLoginError::SlidingSyncNotAvailable, + ClientBuildError::SlidingSync(_) => HumanQrLoginError::SlidingSyncNotAvailable, _ => { error!("Couldn't build the client {e:?}"); HumanQrLoginError::Unknown @@ -647,3 +661,12 @@ pub struct RequestConfig { /// Base delay between retries. retry_timeout: Option, } + +#[derive(Clone, uniffi::Enum)] +pub enum SlidingSyncVersionBuilder { + None, + Proxy { url: String }, + Native, + DiscoverProxy, + DiscoverNative, +} diff --git a/crates/matrix-sdk/src/client/builder.rs b/crates/matrix-sdk/src/client/builder.rs index 4cc6bbc3c31..4bf7c0b12cb 100644 --- a/crates/matrix-sdk/src/client/builder.rs +++ b/crates/matrix-sdk/src/client/builder.rs @@ -39,7 +39,7 @@ 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::sliding_sync::VersionBuilder as SlidingSyncVersionBuilder; use crate::{ authentication::AuthCtx, client::ClientServerCapabilities, config::RequestConfig, error::RumaApiError, http_client::HttpClient, send_queue::SendQueueData, HttpError, @@ -89,7 +89,7 @@ use crate::{ pub struct ClientBuilder { homeserver_cfg: Option, #[cfg(feature = "experimental-sliding-sync")] - sliding_sync_version: SlidingSyncVersion, + sliding_sync_version_builder: SlidingSyncVersionBuilder, http_cfg: Option, store_config: BuilderStoreConfig, request_config: RequestConfig, @@ -108,7 +108,7 @@ impl ClientBuilder { Self { homeserver_cfg: None, #[cfg(feature = "experimental-sliding-sync")] - sliding_sync_version: SlidingSyncVersion::Native, + sliding_sync_version_builder: SlidingSyncVersionBuilder::Native, http_cfg: None, store_config: BuilderStoreConfig::Custom(StoreConfig::default()), request_config: Default::default(), @@ -136,30 +136,13 @@ impl ClientBuilder { self } - /// 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 - /// [`Self::server_name`], [`Self::insecure_server_name_no_tls`], or - /// [`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: 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 - } - /// Set sliding sync to a specific version. #[cfg(feature = "experimental-sliding-sync")] - pub fn sliding_sync_version(mut self, version: SlidingSyncVersion) -> Self { - self.sliding_sync_version = version; + pub fn sliding_sync_version_builder( + mut self, + version_builder: SlidingSyncVersionBuilder, + ) -> Self { + self.sliding_sync_version_builder = version_builder; self } @@ -458,7 +441,7 @@ impl ClientBuilder { let http_client = HttpClient::new(inner_http_client.clone(), self.request_config); #[allow(unused_variables)] - let (homeserver, well_known, versions) = match homeserver_cfg { + let (homeserver, well_known, supported_versions) = match homeserver_cfg { HomeserverConfig::Url(url) => (Url::parse(&url)?, None, None), HomeserverConfig::ServerName { server: server_name, protocol } => { @@ -473,22 +456,24 @@ impl ClientBuilder { } }; - /* #[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 - // requests to be sent directly to the homeserver. - tracing::info!("Simplified MSC3575 is enabled, ignoring any sliding sync proxy."); - sliding_sync_proxy = None; - } else if let Some(well_known) = well_known { - // Otherwise, if a proxy wasn't set, use the one discovered from the well-known. - if sliding_sync_proxy.is_none() { - sliding_sync_proxy = - well_known.sliding_sync_proxy.and_then(|p| Url::parse(&p.url).ok()) - } - } - */ + let sliding_sync_version = { + let supported_versions = match supported_versions { + Some(versions) => Some(versions), + None if self.sliding_sync_version_builder.needs_get_supported_versions() => { + Some(get_supported_versions(&homeserver, &http_client).await?) + } + None => None, + }; + + let version = self + .sliding_sync_version_builder + .build(well_known.as_ref(), supported_versions.as_ref())?; + tracing::info!(?version, "selected sliding sync version"); + + version + }; #[cfg(feature = "experimental-oidc")] let allow_insecure_oidc = homeserver.scheme() == "http"; @@ -517,7 +502,7 @@ impl ClientBuilder { auth_ctx, homeserver, #[cfg(feature = "experimental-sliding-sync")] - self.sliding_sync_version, + sliding_sync_version, http_client, base_client, server_capabilities, @@ -819,9 +804,10 @@ pub enum ClientBuildError { #[error("Error looking up the .well-known endpoint on auto-discovery")] AutoDiscovery(FromHttpResponseError), - /// The builder requires support for sliding sync but it isn't available. - #[error("The homeserver doesn't support sliding sync and a custom proxy wasn't configured.")] - SlidingSyncNotAvailable, + /// Error from sliding sync. + #[cfg(feature = "experimental-sliding-sync")] + #[error(transparent)] + SlidingSync(#[from] crate::sliding_sync::Error), /// An error encountered when trying to parse the homeserver url. #[error(transparent)] diff --git a/testing/matrix-sdk-integration-testing/src/helpers.rs b/testing/matrix-sdk-integration-testing/src/helpers.rs index 3b729097f5b..f18542fff4d 100644 --- a/testing/matrix-sdk-integration-testing/src/helpers.rs +++ b/testing/matrix-sdk-integration-testing/src/helpers.rs @@ -15,6 +15,7 @@ use matrix_sdk::{ api::client::{account::register::v3::Request as RegistrationRequest, uiaa}, RoomId, }, + sliding_sync::VersionBuilder, Client, ClientBuilder, Room, }; use once_cell::sync::Lazy; @@ -85,9 +86,10 @@ impl TestClientBuilder { .homeserver_url(homeserver_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. - .sliding_sync_proxy( - Url::parse(&sliding_sync_proxy_url).expect("Sliding sync proxy URL is invalid"), - ) + .sliding_sync_version_builder(VersionBuilder::Proxy { + url: Url::parse(&sliding_sync_proxy_url) + .expect("Sliding sync proxy URL is invalid"), + }) .with_encryption_settings(self.encryption_settings) .request_config(RequestConfig::short_retry()); From 89907899fe8f53acb9aabb77acd7b5f678c64eb6 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 26 Aug 2024 17:08:08 +0200 Subject: [PATCH 06/11] test(sdk): Restore disabled tests. --- crates/matrix-sdk/src/client/builder.rs | 144 ++++++++---------------- 1 file changed, 44 insertions(+), 100 deletions(-) diff --git a/crates/matrix-sdk/src/client/builder.rs b/crates/matrix-sdk/src/client/builder.rs index 4bf7c0b12cb..60e93bc22a2 100644 --- a/crates/matrix-sdk/src/client/builder.rs +++ b/crates/matrix-sdk/src/client/builder.rs @@ -853,6 +853,8 @@ pub(crate) mod tests { }; use super::*; + #[cfg(feature = "experimental-sliding-sync")] + use crate::sliding_sync::Version as SlidingSyncVersion; #[test] fn test_sanitize_server_name() { @@ -922,7 +924,6 @@ 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. @@ -933,9 +934,9 @@ pub(crate) mod tests { builder = builder.server_name_or_homeserver_url(homeserver.uri()); let _client = builder.build().await.unwrap(); - // Then a client should be built without support for sliding sync or OIDC. + // Then a client should be built with native support for sliding sync. #[cfg(feature = "experimental-sliding-sync")] - assert!(_client.sliding_sync_proxy().is_none()); + assert!(_client.sliding_sync_version().is_native()); } #[async_test] @@ -945,9 +946,14 @@ pub(crate) mod tests { let homeserver = make_mock_homeserver().await; let mut builder = ClientBuilder::new(); #[cfg(feature = "experimental-sliding-sync")] - { - builder = builder.sliding_sync_proxy("https://localhost:1234"); - } + let url = { + let url = Url::parse("https://localhost:1234").unwrap(); + builder = builder.sliding_sync_version_builder(SlidingSyncVersionBuilder::Proxy { + url: url.clone(), + }); + + url + }; // When building a client with the server's URL. builder = builder.server_name_or_homeserver_url(homeserver.uri()); @@ -955,7 +961,12 @@ pub(crate) mod tests { // Then a client should be built with support for sliding sync. #[cfg(feature = "experimental-sliding-sync")] - assert_eq!(_client.sliding_sync_proxy(), Some("https://localhost:1234".parse().unwrap())); + assert_matches!( + _client.sliding_sync_version(), + SlidingSyncVersion::Proxy { url: given_url } => { + assert_eq!(given_url, url); + } + ); } #[async_test] @@ -1005,9 +1016,10 @@ pub(crate) mod tests { builder = builder.server_name_or_homeserver_url(server.uri()); let _client = builder.build().await.unwrap(); - // Then a client should be built without support for sliding sync or OIDC. + // Then a client should be built with native support for sliding sync. + // It's native support because it's the default. Nothing is checked here. #[cfg(feature = "experimental-sliding-sync")] - assert!(_client.sliding_sync_proxy().is_none()); + assert!(_client.sliding_sync_version().is_native()); } #[async_test] @@ -1027,13 +1039,21 @@ pub(crate) mod tests { .mount(&server) .await; - // When building a client with the base server. - builder = builder.server_name_or_homeserver_url(server.uri()); + // When building a client with the base server, with sliding sync to + // auto-discover the proxy. + builder = builder + .server_name_or_homeserver_url(server.uri()) + .sliding_sync_version_builder(SlidingSyncVersionBuilder::DiscoverProxy); let _client = builder.build().await.unwrap(); // Then a client should be built with support for sliding sync. #[cfg(feature = "experimental-sliding-sync")] - assert_eq!(_client.sliding_sync_proxy(), Some("https://localhost:1234".parse().unwrap())); + assert_matches!( + _client.sliding_sync_version(), + SlidingSyncVersion::Proxy { url } => { + assert_eq!(url, Url::parse("https://localhost:1234").unwrap()); + } + ); } #[async_test] @@ -1056,99 +1076,23 @@ pub(crate) mod tests { // When building a client with the base server and a custom sliding sync proxy // set. - builder = builder.sliding_sync_proxy("https://localhost:9012"); - builder = builder.server_name_or_homeserver_url(server.uri()); - let client = builder.build().await.unwrap(); + let url = Url::parse("https://localhost:9012").unwrap(); - // Then a client should be built and configured with the custom sliding sync - // proxy. - #[cfg(feature = "experimental-sliding-sync")] - assert_eq!(client.sliding_sync_proxy(), Some("https://localhost:9012".parse().unwrap())); - } + builder = builder + .sliding_sync_version_builder(SlidingSyncVersionBuilder::Proxy { url: url.clone() }) + .server_name_or_homeserver_url(server.uri()); - #[async_test] - #[cfg(feature = "experimental-sliding-sync")] - async fn test_discovery_well_known_with_simplified_sliding_sync() { - // Given a base server with a well-known file that points to a homeserver with a - // sliding sync proxy. - let server = MockServer::start().await; - let homeserver = make_mock_homeserver().await; - let mut builder = ClientBuilder::new(); - - Mock::given(method("GET")) - .and(path("/.well-known/matrix/client")) - .respond_with(ResponseTemplate::new(200).set_body_json(make_well_known_json( - &homeserver.uri(), - Some("https://localhost:1234"), - ))) - .mount(&server) - .await; - - // When building a client for simplified sliding sync with the base server. - 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() { - // Given a base server with a well-known file that points to a homeserver that - // doesn't support sliding sync. - let server = MockServer::start().await; - let homeserver = make_mock_homeserver().await; - let mut builder = ClientBuilder::new(); - - Mock::given(method("GET")) - .and(path("/.well-known/matrix/client")) - .respond_with( - ResponseTemplate::new(200) - .set_body_json(make_well_known_json(&homeserver.uri(), None)), - ) - .mount(&server) - .await; - - // When building a client that requires sliding sync with the base server. - builder = builder.requires_sliding_sync().server_name_or_homeserver_url(server.uri()); - let error = builder.build().await.unwrap_err(); - - // Then the operation should fail due to the lack of sliding sync support. - assert_matches!(error, ClientBuildError::SlidingSyncNotAvailable); - } - - #[async_test] - #[cfg(feature = "experimental-sliding-sync")] - async fn test_requires_sliding_sync_with_well_known() { - // Given a base server with a well-known file that points to a homeserver with a - // sliding sync proxy. - let server = MockServer::start().await; - let homeserver = make_mock_homeserver().await; - let mut builder = ClientBuilder::new(); - - Mock::given(method("GET")) - .and(path("/.well-known/matrix/client")) - .respond_with(ResponseTemplate::new(200).set_body_json(make_well_known_json( - &homeserver.uri(), - Some("https://localhost:1234"), - ))) - .mount(&server) - .await; - - // When building a client that requires sliding sync with the base server. - builder = builder.requires_sliding_sync().server_name_or_homeserver_url(server.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())); + // Then a client should be built and configured with the custom sliding sync + // proxy. + assert_matches!( + client.sliding_sync_version(), + SlidingSyncVersion::Proxy { url: given_url } => { + assert_eq!(url, given_url); + } + ); } - */ /* Helper functions */ From 4b2464b075453afc08346657b7762906fe257288 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 26 Aug 2024 17:20:39 +0200 Subject: [PATCH 07/11] test(sdk): Add 2 more tests for auto-discovery of sliding sync. --- crates/matrix-sdk/src/client/builder.rs | 55 +++++++++++++++++++ crates/matrix-sdk/src/sliding_sync/client.rs | 3 +- .../src/test_json/api_responses.rs | 7 ++- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/crates/matrix-sdk/src/client/builder.rs b/crates/matrix-sdk/src/client/builder.rs index 60e93bc22a2..e895f9ccc36 100644 --- a/crates/matrix-sdk/src/client/builder.rs +++ b/crates/matrix-sdk/src/client/builder.rs @@ -1023,6 +1023,7 @@ pub(crate) mod tests { } #[async_test] + #[cfg(feature = "experimental-sliding-sync")] async fn test_discovery_well_known_with_sliding_sync() { // Given a base server with a well-known file that points to a homeserver with a // sliding sync proxy. @@ -1094,6 +1095,60 @@ pub(crate) mod tests { ); } + #[async_test] + #[cfg(feature = "experimental-sliding-sync")] + async fn test_sliding_sync_discover_proxy() { + // Given a homeserver with a `.well-known` file. + let homeserver = make_mock_homeserver().await; + let mut builder = ClientBuilder::new(); + + let expected_url = Url::parse("https://localhost:1234").unwrap(); + + Mock::given(method("GET")) + .and(path("/.well-known/matrix/client")) + .respond_with(ResponseTemplate::new(200).set_body_json(make_well_known_json( + &homeserver.uri(), + Some(expected_url.as_str()), + ))) + .mount(&homeserver) + .await; + + // When building the client with sliding sync to auto-discover the + // proxy version. + builder = builder + .server_name_or_homeserver_url(homeserver.uri()) + .sliding_sync_version_builder(SlidingSyncVersionBuilder::DiscoverProxy); + + let client = builder.build().await.unwrap(); + + // Then, sliding sync has the correct proxy URL. + assert_matches!( + client.sliding_sync_version(), + SlidingSyncVersion::Proxy { url } => { + assert_eq!(url, expected_url); + } + ); + } + + #[async_test] + #[cfg(feature = "experimental-sliding-sync")] + async fn test_sliding_sync_discover_native() { + // Given a homeserver with a `/versions` file. + let homeserver = make_mock_homeserver().await; + let mut builder = ClientBuilder::new(); + + // When building the client with sliding sync to auto-discover the + // native version. + builder = builder + .server_name_or_homeserver_url(homeserver.uri()) + .sliding_sync_version_builder(SlidingSyncVersionBuilder::DiscoverNative); + + let client = builder.build().await.unwrap(); + + // Then, sliding sync has the correct native version. + assert_matches!(client.sliding_sync_version(), SlidingSyncVersion::Native); + } + /* Helper functions */ async fn make_mock_homeserver() -> MockServer { diff --git a/crates/matrix-sdk/src/sliding_sync/client.rs b/crates/matrix-sdk/src/sliding_sync/client.rs index b63cdbcb800..febcbd84c01 100644 --- a/crates/matrix-sdk/src/sliding_sync/client.rs +++ b/crates/matrix-sdk/src/sliding_sync/client.rs @@ -281,13 +281,12 @@ mod tests { use serde_json::json; use url::Url; + use super::{discover_homeserver, get_supported_versions, Error, Version, VersionBuilder}; use crate::{ error::Result, sliding_sync::http, test_utils::logged_in_client_with_server, SlidingSyncList, SlidingSyncMode, }; - use super::{discover_homeserver, get_supported_versions, Error, Version, VersionBuilder}; - #[test] fn test_version_builder_none() { assert_matches!(VersionBuilder::None.build(None, None), Ok(Version::None)); diff --git a/testing/matrix-sdk-test/src/test_json/api_responses.rs b/testing/matrix-sdk-test/src/test_json/api_responses.rs index cc0298ea234..166ea25a2ed 100644 --- a/testing/matrix-sdk-test/src/test_json/api_responses.rs +++ b/testing/matrix-sdk-test/src/test_json/api_responses.rs @@ -346,9 +346,10 @@ pub static VERSIONS: Lazy = Lazy::new(|| { "r0.6.0" ], "unstable_features": { - "org.matrix.label_based_filtering":true, - "org.matrix.e2e_cross_signing":true, - "org.matrix.msc4028":true + "org.matrix.label_based_filtering": true, + "org.matrix.e2e_cross_signing": true, + "org.matrix.msc4028": true, + "org.matrix.simplified_msc3575": true, } }) }); From fa4ddde726f400742e64682f803b7107d25c047b Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 26 Aug 2024 17:23:09 +0200 Subject: [PATCH 08/11] test: Disable Complement Crypto for a short period. --- .github/workflows/bindings_ci.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/bindings_ci.yml b/.github/workflows/bindings_ci.yml index 28a630b6c84..babd5419e72 100644 --- a/.github/workflows/bindings_ci.yml +++ b/.github/workflows/bindings_ci.yml @@ -177,12 +177,12 @@ jobs: - name: Build Framework run: target/debug/xtask swift build-framework --target=aarch64-apple-ios - complement-crypto: - name: "Run Complement Crypto tests" - uses: matrix-org/complement-crypto/.github/workflows/single_sdk_tests.yml@main - with: - use_rust_sdk: "." # use local checkout - use_complement_crypto: "MATCHING_BRANCH" + #complement-crypto: + # name: "Run Complement Crypto tests" + # uses: matrix-org/complement-crypto/.github/workflows/single_sdk_tests.yml@main + # with: + # use_rust_sdk: "." # use local checkout + # use_complement_crypto: "MATCHING_BRANCH" test-crypto-apple-framework-generation: name: Generate Crypto FFI Apple XCFramework From c4a6ee2f1d5d30e5cf39e1155df1894372ada8f9 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 27 Aug 2024 14:19:23 +0200 Subject: [PATCH 09/11] feat(sdk): Add `Client::available_sliding_sync_versions`. Previous patches have unified all sliding sync versions behind a single type: `Version`. More recent previous patches have introduced `VersionBuilder` so that a `ClientBuilder` can use them to coerce or find the best `Version` possible. This patch implements a last missing piece: `Client::available_sliding_sync_versions` will report all available `Version`s at a given time. This is useful when a `Client` is already built, and a session has been opened/a user is logged, but someone has to take the decision whether it's useful to switch to another sliding sync version or not. --- crates/matrix-sdk/src/sliding_sync/client.rs | 108 ++++++++++++++++++- 1 file changed, 106 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk/src/sliding_sync/client.rs b/crates/matrix-sdk/src/sliding_sync/client.rs index febcbd84c01..32a125c86c1 100644 --- a/crates/matrix-sdk/src/sliding_sync/client.rs +++ b/crates/matrix-sdk/src/sliding_sync/client.rs @@ -3,7 +3,10 @@ use std::collections::BTreeMap; use imbl::Vector; use matrix_sdk_base::{sliding_sync::http, sync::SyncResponse, PreviousEventsProvider}; use ruma::{ - api::client::discovery::{discover_homeserver, get_supported_versions}, + api::{ + client::discovery::{discover_homeserver, get_supported_versions}, + MatrixVersion, + }, events::AnyToDeviceEvent, serde::Raw, OwnedRoomId, @@ -12,7 +15,7 @@ use tracing::error; use url::Url; use super::{Error, SlidingSync, SlidingSyncBuilder}; -use crate::{Client, Result, SlidingSyncRoom}; +use crate::{config::RequestConfig, Client, Result, SlidingSyncRoom}; /// A sliding sync version. #[derive(Clone, Debug)] @@ -137,6 +140,42 @@ impl VersionBuilder { } impl Client { + /// Find all sliding sync versions that are available. + /// + /// Be careful: This method may hit the store and will send new requests for + /// each call. It can be costly to call it repeatedly. + /// + /// If `.well-known` or `/versions` is unreachable, it will simply move + /// potential sliding sync versions aside. No error will be reported. + pub async fn available_sliding_sync_versions(&self) -> Vec { + let well_known = self + .inner + .http_client + .send( + discover_homeserver::Request::new(), + Some(RequestConfig::short_retry()), + self.homeserver().to_string(), + None, + &[MatrixVersion::V1_0], + Default::default(), + ) + .await + .ok(); + let supported_versions = self.unstable_features().await.ok().map(|unstable_features| { + let mut response = get_supported_versions::Response::new(vec![]); + response.unstable_features = unstable_features; + + response + }); + + [VersionBuilder::DiscoverNative, VersionBuilder::DiscoverProxy] + .into_iter() + .filter_map(|version_builder| { + version_builder.build(well_known.as_ref(), supported_versions.as_ref()).ok() + }) + .collect() + } + /// Create a [`SlidingSyncBuilder`] tied to this client, with the given /// identifier. /// @@ -280,6 +319,10 @@ mod tests { use ruma::{assign, room_id, serde::Raw}; use serde_json::json; use url::Url; + use wiremock::{ + matchers::{method, path}, + Mock, ResponseTemplate, + }; use super::{discover_homeserver, get_supported_versions, Error, Version, VersionBuilder}; use crate::{ @@ -404,6 +447,67 @@ mod tests { ); } + #[async_test] + async fn test_available_sliding_sync_versions_none() { + let (client, _server) = logged_in_client_with_server().await; + let available_versions = client.available_sliding_sync_versions().await; + + // `.well-known` and `/versions` aren't available. It's impossible to find any + // versions. + assert!(available_versions.is_empty()); + } + + #[async_test] + async fn test_available_sliding_sync_versions_proxy() { + let (client, server) = logged_in_client_with_server().await; + + Mock::given(method("GET")) + .and(path("/.well-known/matrix/client")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "m.homeserver": { + "base_url": "https://matrix.org", + }, + "org.matrix.msc3575.proxy": { + "url": "https://proxy.matrix.org", + }, + }))) + .mount(&server) + .await; + + let available_versions = client.available_sliding_sync_versions().await; + + // `.well-known` is available. + assert_eq!(available_versions.len(), 1); + assert_matches!( + &available_versions[0], + Version::Proxy { url } => { + assert_eq!(url, &Url::parse("https://proxy.matrix.org").unwrap()); + } + ); + } + + #[async_test] + async fn test_available_sliding_sync_versions_native() { + let (client, server) = logged_in_client_with_server().await; + + Mock::given(method("GET")) + .and(path("/_matrix/client/versions")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "versions": [], + "unstable_features": { + "org.matrix.simplified_msc3575": true, + }, + }))) + .mount(&server) + .await; + + let available_versions = client.available_sliding_sync_versions().await; + + // `/versions` is available. + assert_eq!(available_versions.len(), 1); + assert_matches!(available_versions[0], Version::Native); + } + #[async_test] async fn test_cache_user_defined_notification_mode() -> Result<()> { let (client, _server) = logged_in_client_with_server().await; From 54b066e52d75f369bf25dde430bf40af30020026 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 27 Aug 2024 14:24:58 +0200 Subject: [PATCH 10/11] feat(ffi): Add `Client::available_sliding_sync_versions`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch adds bindings to `Client::available_sliding_sync_versions` to `matrix-sdk-ffi`. This patch also moves `Client::sliding_sync_version` from “private” to “public” FFI API, in thee sense that this method is now exported with UniFFI. --- bindings/matrix-sdk-ffi/src/client.rs | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index 8a37679f00b..e96997319c2 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -460,11 +460,6 @@ impl Client { Ok(()) } - /// 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. pub(crate) async fn supports_password_login(&self) -> anyhow::Result { let login_types = self.inner.matrix_auth().get_login_types().await?; @@ -478,6 +473,22 @@ impl Client { #[uniffi::export(async_runtime = "tokio")] impl Client { + /// The sliding sync version. + pub fn sliding_sync_version(&self) -> SlidingSyncVersion { + self.inner.sliding_sync_version().into() + } + + /// Find all sliding sync versions that are available. + /// + /// Be careful: This method may hit the store and will send new requests for + /// each call. It can be costly to call it repeatedly. + /// + /// If `.well-known` or `/versions` is unreachable, it will simply move + /// potential sliding sync versions aside. No error will be reported. + pub async fn available_sliding_sync_versions(&self) -> Vec { + self.inner.available_sliding_sync_versions().await.into_iter().map(Into::into).collect() + } + pub fn set_delegate( self: Arc, delegate: Option>, From 47f2c6f6860d87a75865295f08f72f92ad204c7b Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 27 Aug 2024 16:43:25 +0200 Subject: [PATCH 11/11] sliding sync version(chore): address review comments --- bindings/matrix-sdk-ffi/src/client_builder.rs | 8 +- crates/matrix-sdk/src/client/builder.rs | 8 +- crates/matrix-sdk/src/client/mod.rs | 4 +- crates/matrix-sdk/src/sliding_sync/client.rs | 93 +++++++++---------- crates/matrix-sdk/src/sliding_sync/error.rs | 5 - crates/matrix-sdk/src/sliding_sync/mod.rs | 6 +- 6 files changed, 63 insertions(+), 61 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/client_builder.rs b/bindings/matrix-sdk-ffi/src/client_builder.rs index 1386ec121b9..52e5aa9d8d5 100644 --- a/bindings/matrix-sdk-ffi/src/client_builder.rs +++ b/bindings/matrix-sdk-ffi/src/client_builder.rs @@ -12,6 +12,7 @@ use matrix_sdk::{ ruma::{ServerName, UserId}, sliding_sync::{ Error as MatrixSlidingSyncError, VersionBuilder as MatrixSlidingSyncVersionBuilder, + VersionBuilderError, }, Client as MatrixClient, ClientBuildError as MatrixClientBuildError, HttpError, IdParseError, RumaApiError, @@ -195,8 +196,11 @@ pub enum ClientBuildError { #[error(transparent)] WellKnownDeserializationError(DeserializationError), #[error(transparent)] + #[allow(dead_code)] // rustc's drunk, this is used SlidingSync(MatrixSlidingSyncError), #[error(transparent)] + SlidingSyncVersion(VersionBuilderError), + #[error(transparent)] Sdk(MatrixClientBuildError), #[error("Failed to build the client: {message}")] Generic { message: String }, @@ -213,7 +217,9 @@ impl From for ClientBuildError { MatrixClientBuildError::AutoDiscovery(FromHttpResponseError::Deserialization(e)) => { ClientBuildError::WellKnownDeserializationError(e) } - MatrixClientBuildError::SlidingSync(e) => ClientBuildError::SlidingSync(e), + MatrixClientBuildError::SlidingSyncVersion(e) => { + ClientBuildError::SlidingSyncVersion(e) + } _ => ClientBuildError::Sdk(e), } } diff --git a/crates/matrix-sdk/src/client/builder.rs b/crates/matrix-sdk/src/client/builder.rs index e895f9ccc36..42461daa9bd 100644 --- a/crates/matrix-sdk/src/client/builder.rs +++ b/crates/matrix-sdk/src/client/builder.rs @@ -564,8 +564,8 @@ async fn discover_homeserver_from_server_name_or_url( if let Ok(homeserver_url) = Url::parse(&server_name_or_url) { // Make sure the URL is definitely for a homeserver. match get_supported_versions(&homeserver_url, http_client).await { - Ok(_) => { - return Ok((homeserver_url, None, None)); + Ok(response) => { + return Ok((homeserver_url, None, Some(response))); } Err(e) => { debug!(error = %e, "Checking supported versions failed."); @@ -804,10 +804,10 @@ pub enum ClientBuildError { #[error("Error looking up the .well-known endpoint on auto-discovery")] AutoDiscovery(FromHttpResponseError), - /// Error from sliding sync. + /// Error when building the sliding sync version. #[cfg(feature = "experimental-sliding-sync")] #[error(transparent)] - SlidingSync(#[from] crate::sliding_sync::Error), + SlidingSyncVersion(#[from] crate::sliding_sync::VersionBuilderError), /// An error encountered when trying to parse the homeserver url. #[error(transparent)] diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index d5bf08ce464..1c0dcd0b857 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -459,13 +459,13 @@ impl Client { self.inner.homeserver.read().unwrap().clone() } - /// Get the sliding version. + /// Get the sliding sync version. #[cfg(feature = "experimental-sliding-sync")] pub fn sliding_sync_version(&self) -> SlidingSyncVersion { self.inner.sliding_sync_version.read().unwrap().clone() } - /// Override the sliding version. + /// Override the sliding sync version. #[cfg(feature = "experimental-sliding-sync")] pub fn set_sliding_sync_version(&self, version: SlidingSyncVersion) { let mut lock = self.inner.sliding_sync_version.write().unwrap(); diff --git a/crates/matrix-sdk/src/sliding_sync/client.rs b/crates/matrix-sdk/src/sliding_sync/client.rs index 32a125c86c1..f0f836c1e1c 100644 --- a/crates/matrix-sdk/src/sliding_sync/client.rs +++ b/crates/matrix-sdk/src/sliding_sync/client.rs @@ -1,5 +1,6 @@ use std::collections::BTreeMap; +use as_variant::as_variant; use imbl::Vector; use matrix_sdk_base::{sliding_sync::http, sync::SyncResponse, PreviousEventsProvider}; use ruma::{ @@ -14,13 +15,13 @@ use ruma::{ use tracing::error; use url::Url; -use super::{Error, SlidingSync, SlidingSyncBuilder}; +use super::{SlidingSync, SlidingSyncBuilder}; use crate::{config::RequestConfig, Client, Result, SlidingSyncRoom}; /// A sliding sync version. #[derive(Clone, Debug)] pub enum Version { - /// No version. Useful to represent that sliding sync is disable for + /// No version. Useful to represent that sliding sync is disabled for /// example, and that the version is unknown. None, @@ -41,13 +42,35 @@ impl Version { } pub(crate) fn overriding_url(&self) -> Option<&Url> { - match self { - Self::Proxy { url } => Some(url), - _ => None, - } + as_variant!(self, Self::Proxy { url } => url) } } +/// An error when building a version. +#[derive(thiserror::Error, Debug)] +pub enum VersionBuilderError { + /// The `.well-known` response is not set. + #[error("`.well-known` is not set")] + WellKnownNotSet, + + /// `.well-known` does not contain a `sliding_sync_proxy` entry. + #[error("`.well-known` does not contain a `sliding_sync_proxy` entry")] + NoSlidingSyncInWellKnown, + + /// The `sliding_sync_proxy` URL in .well-known` is not valid ({0}). + #[error("the `sliding_sync_proxy` URL in .well-known` is not valid ({0})")] + UnparsableSlidingSyncUrl(url::ParseError), + + /// The `/versions` response is not set. + #[error("The `/versions` response is not set")] + MissingVersionsResponse, + + /// `/versions` does not contain `org.matrix.simplified_msc3575` in its + /// `unstable_features`, or it's not set to true. + #[error("`/versions` does not contain `org.matrix.simplified_msc3575` in its `unstable_features`, or it's not set to true.")] + NativeVersionIsUnset, +} + /// A builder for [`Version`]. #[derive(Clone, Debug)] pub enum VersionBuilder { @@ -87,7 +110,7 @@ impl VersionBuilder { self, well_known: Option<&discover_homeserver::Response>, versions: Option<&get_supported_versions::Response>, - ) -> Result { + ) -> Result { Ok(match self { Self::None => Version::None, @@ -97,42 +120,27 @@ impl VersionBuilder { Self::DiscoverProxy => { let Some(well_known) = well_known else { - return Err(Error::VersionCannotBeDiscovered( - "`.well-known` is `None`".to_owned(), - )); + return Err(VersionBuilderError::WellKnownNotSet); }; let Some(info) = &well_known.sliding_sync_proxy else { - return Err(Error::VersionCannotBeDiscovered( - "`.well-known` does not contain a `sliding_sync_proxy` entry".to_owned(), - )); + return Err(VersionBuilderError::NoSlidingSyncInWellKnown); }; - let url = Url::parse(&info.url).map_err(|e| { - Error::VersionCannotBeDiscovered(format!( - "`.well-known` contains an invalid `sliding_sync_proxy` entry ({e})" - )) - })?; + let url = + Url::parse(&info.url).map_err(VersionBuilderError::UnparsableSlidingSyncUrl)?; Version::Proxy { url } } Self::DiscoverNative => { let Some(versions) = versions else { - return Err(Error::VersionCannotBeDiscovered( - "`/versions` is `None`".to_owned(), - )); + return Err(VersionBuilderError::MissingVersionsResponse); }; match versions.unstable_features.get("org.matrix.simplified_msc3575") { Some(value) if *value => Version::Native, - _ => { - return Err(Error::VersionCannotBeDiscovered( - "`/versions` does not contain `org.matrix.simplified_msc3575` in its \ - `unstable_features`" - .to_owned(), - )) - } + _ => return Err(VersionBuilderError::NativeVersionIsUnset), } } }) @@ -324,9 +332,11 @@ mod tests { Mock, ResponseTemplate, }; - use super::{discover_homeserver, get_supported_versions, Error, Version, VersionBuilder}; + use super::{discover_homeserver, get_supported_versions, Version, VersionBuilder}; use crate::{ - error::Result, sliding_sync::http, test_utils::logged_in_client_with_server, + error::Result, + sliding_sync::{http, VersionBuilderError}, + test_utils::logged_in_client_with_server, SlidingSyncList, SlidingSyncMode, }; @@ -373,9 +383,7 @@ mod tests { fn test_version_builder_discover_proxy_no_well_known() { assert_matches!( VersionBuilder::DiscoverProxy.build(None, None), - Err(Error::VersionCannotBeDiscovered(msg)) => { - assert_eq!(msg, "`.well-known` is `None`"); - } + Err(VersionBuilderError::WellKnownNotSet) ); } @@ -388,9 +396,7 @@ mod tests { assert_matches!( VersionBuilder::DiscoverProxy.build(Some(&response), None), - Err(Error::VersionCannotBeDiscovered(msg)) => { - assert_eq!(msg, "`.well-known` does not contain a `sliding_sync_proxy` entry"); - } + Err(VersionBuilderError::NoSlidingSyncInWellKnown) ); } @@ -404,8 +410,8 @@ mod tests { assert_matches!( VersionBuilder::DiscoverProxy.build(Some(&response), None), - Err(Error::VersionCannotBeDiscovered(msg)) => { - assert_eq!(msg, "`.well-known` contains an invalid `sliding_sync_proxy` entry (relative URL without a base)"); + Err(VersionBuilderError::UnparsableSlidingSyncUrl(err)) => { + assert_eq!(err.to_string(), "relative URL without a base"); } ); } @@ -425,9 +431,7 @@ mod tests { fn test_version_builder_discover_native_no_supported_versions() { assert_matches!( VersionBuilder::DiscoverNative.build(None, None), - Err(Error::VersionCannotBeDiscovered(msg)) => { - assert_eq!(msg, "`/versions` is `None`".to_owned()); - } + Err(VersionBuilderError::MissingVersionsResponse) ); } @@ -438,12 +442,7 @@ mod tests { assert_matches!( VersionBuilder::DiscoverNative.build(None, Some(&response)), - Err(Error::VersionCannotBeDiscovered(msg)) => { - assert_eq!( - msg, - "`/versions` does not contain `org.matrix.simplified_msc3575` in its `unstable_features`".to_owned() - ); - } + Err(VersionBuilderError::NativeVersionIsUnset) ); } diff --git a/crates/matrix-sdk/src/sliding_sync/error.rs b/crates/matrix-sdk/src/sliding_sync/error.rs index b76b3ffab5f..10706d860fd 100644 --- a/crates/matrix-sdk/src/sliding_sync/error.rs +++ b/crates/matrix-sdk/src/sliding_sync/error.rs @@ -12,11 +12,6 @@ pub enum Error { #[error("Sliding sync version is missing")] VersionIsMissing, - /// Sliding sync version has been configured to be discovered, - /// but it has failed. See [`crate::sliding_sync::Version`]. - #[error("Sliding sync version cannot be discovered: {0}")] - VersionCannotBeDiscovered(String), - /// 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 216fe2a5a30..cf25c03d5a2 100644 --- a/crates/matrix-sdk/src/sliding_sync/mod.rs +++ b/crates/matrix-sdk/src/sliding_sync/mod.rs @@ -50,7 +50,7 @@ use tracing::{debug, error, info, instrument, trace, warn, Instrument, Span}; #[cfg(feature = "e2e-encryption")] use self::utils::JoinHandleExt as _; -pub use self::{builder::*, error::*, list::*, room::*}; +pub use self::{builder::*, client::VersionBuilderError, error::*, list::*, room::*}; use self::{ cache::restore_sliding_sync_state, client::SlidingSyncResponseProcessor, @@ -74,6 +74,8 @@ pub(super) struct SlidingSyncInner { /// Used to distinguish different connections to the sliding sync proxy. id: String, + /// Either an overridden sliding sync [`Version`], or one inherited from the + /// client. version: Version, /// The HTTP Matrix client. @@ -2005,7 +2007,7 @@ mod tests { assert_matches!(sync.version(), Version::Native); } - // Sliding can override the configuration from the client. + // Sliding sync can override the configuration from the client. { let url = Url::parse("https://bar.matrix/").unwrap(); let sync = client