From 81cea84a37a973dbbf2f164731706a88bf1a3ecd Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Thu, 10 Mar 2022 15:00:40 +0100 Subject: [PATCH 1/7] Fetch supported matrix versions from server in initialization --- crates/matrix-sdk/src/client.rs | 23 +++++++++++++++++------ crates/matrix-sdk/src/http_client.rs | 15 +++++++++------ 2 files changed, 26 insertions(+), 12 deletions(-) diff --git a/crates/matrix-sdk/src/client.rs b/crates/matrix-sdk/src/client.rs index 42450ed08..5c0b69905 100644 --- a/crates/matrix-sdk/src/client.rs +++ b/crates/matrix-sdk/src/client.rs @@ -118,6 +118,8 @@ pub(crate) struct ClientInner { http_client: HttpClient, /// User session data. base_client: BaseClient, + /// The Matrix versions the server supports (well-known ones only) + server_versions: Arc<[MatrixVersion]>, /// Locks making sure we only have one group session sharing request in /// flight per room. #[cfg(feature = "encryption")] @@ -189,10 +191,17 @@ impl Client { let http_client = HttpClient::new(client, homeserver.clone(), session, config.request_config); + let server_versions = http_client + .send(get_supported_versions::Request::new(), None, vec![MatrixVersion::V1_0].into()) + .await? + .known_versions() + .collect(); + let inner = Arc::new(ClientInner { homeserver, http_client, base_client, + server_versions, #[cfg(feature = "encryption")] group_session_locks: Default::default(), #[cfg(feature = "encryption")] @@ -751,15 +760,13 @@ impl Client { .try_into_http_request::>( homeserver.as_str(), SendAccessToken::None, - // FIXME: Use versions reported by server - &[MatrixVersion::V1_0], + &self.inner.server_versions, ) } else { sso_login::v3::Request::new(redirect_url).try_into_http_request::>( homeserver.as_str(), SendAccessToken::None, - // FIXME: Use versions reported by server - &[MatrixVersion::V1_0], + &self.inner.server_versions, ) }; @@ -1542,7 +1549,11 @@ impl Client { }); let request_config = self.inner.http_client.request_config.timeout(timeout); - Ok(self.inner.http_client.upload(request, Some(request_config)).await?) + Ok(self + .inner + .http_client + .upload(request, Some(request_config), self.inner.server_versions.clone()) + .await?) } /// Send an arbitrary request to the server, without updating client state. @@ -1594,7 +1605,7 @@ impl Client { Request: OutgoingRequest + Debug, HttpError: From>, { - self.inner.http_client.send(request, config).await + self.inner.http_client.send(request, config, self.inner.server_versions.clone()).await } /// Get information of all our own devices. diff --git a/crates/matrix-sdk/src/http_client.rs b/crates/matrix-sdk/src/http_client.rs index 50e510a1e..4105d3cf6 100644 --- a/crates/matrix-sdk/src/http_client.rs +++ b/crates/matrix-sdk/src/http_client.rs @@ -115,6 +115,7 @@ impl HttpClient { request: Request, session: Arc>>, config: Option, + server_versions: Arc<[MatrixVersion]>, ) -> Result, HttpError> { let config = match config { Some(config) => config, @@ -150,8 +151,7 @@ impl HttpClient { request.try_into_http_request::( &self.homeserver.read().await.to_string(), send_access_token, - // FIXME: Use versions reported by server - &[MatrixVersion::V1_0], + &server_versions, )? } else { let (send_access_token, user_id) = { @@ -166,8 +166,7 @@ impl HttpClient { &self.homeserver.read().await.to_string(), send_access_token, &user_id, - // FIXME: Use versions reported by server - &[MatrixVersion::V1_0], + &server_versions, )? }; @@ -179,8 +178,10 @@ impl HttpClient { &self, request: create_content::v3::Request<'_>, config: Option, + server_versions: Arc<[MatrixVersion]>, ) -> Result { - let response = self.send_request(request, self.session.clone(), config).await?; + let response = + self.send_request(request, self.session.clone(), config, server_versions).await?; Ok(create_content::v3::Response::try_from_http_response(response)?) } @@ -188,12 +189,14 @@ impl HttpClient { &self, request: Request, config: Option, + server_versions: Arc<[MatrixVersion]>, ) -> Result where Request: OutgoingRequest + Debug, HttpError: From>, { - let response = self.send_request(request, self.session.clone(), config).await?; + let response = + self.send_request(request, self.session.clone(), config, server_versions).await?; trace!("Got response: {:?}", response); From b1f2adf9c0be15305f51e9639d2df3363dd18dca Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Thu, 10 Mar 2022 15:01:10 +0100 Subject: [PATCH 2/7] Ensure random request with Request: Send gets a Send future as well --- crates/matrix-sdk/src/client.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/matrix-sdk/src/client.rs b/crates/matrix-sdk/src/client.rs index 5c0b69905..1b04d6875 100644 --- a/crates/matrix-sdk/src/client.rs +++ b/crates/matrix-sdk/src/client.rs @@ -1406,6 +1406,7 @@ impl Client { /// client.public_rooms(limit, since, server).await; /// # }); /// ``` + #[cfg_attr(not(target_arch = "wasm32"), deny(clippy::future_not_send))] pub async fn public_rooms( &self, limit: Option, From 65e7688a3fc58781081475be6e5c1a3efa7252fc Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Thu, 10 Mar 2022 16:05:51 +0100 Subject: [PATCH 3/7] Don't require discovery to be mocked in tests --- crates/matrix-sdk/src/client.rs | 46 +++++++++++++++++++------- crates/matrix-sdk/src/config/client.rs | 11 ++++++ 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/crates/matrix-sdk/src/client.rs b/crates/matrix-sdk/src/client.rs index 1b04d6875..84a07895a 100644 --- a/crates/matrix-sdk/src/client.rs +++ b/crates/matrix-sdk/src/client.rs @@ -191,11 +191,18 @@ impl Client { let http_client = HttpClient::new(client, homeserver.clone(), session, config.request_config); - let server_versions = http_client - .send(get_supported_versions::Request::new(), None, vec![MatrixVersion::V1_0].into()) - .await? - .known_versions() - .collect(); + let server_versions = match config.server_versions { + Some(vs) => vs, + None => http_client + .send( + get_supported_versions::Request::new(), + None, + vec![MatrixVersion::V1_0].into(), + ) + .await? + .known_versions() + .collect(), + }; let inner = Arc::new(ClientInner { homeserver, @@ -2365,6 +2372,7 @@ pub(crate) mod test { uiaa::{self, UiaaResponse}, }, error::{FromHttpResponseError, ServerError}, + MatrixVersion, }, assign, device_id, directory::Filter, @@ -2397,8 +2405,11 @@ pub(crate) mod test { device_id: device_id!("DEVICEID").to_owned(), }; let homeserver = url::Url::parse(&mockito::server_url()).unwrap(); - let config = - ClientConfig::new().await.unwrap().request_config(RequestConfig::new().disable_retry()); + let config = ClientConfig::new() + .await + .unwrap() + .request_config(RequestConfig::new().disable_retry()) + .server_versions([MatrixVersion::V1_0]); let client = Client::new_with_config(homeserver, config).await.unwrap(); client.restore_login(session).await.unwrap(); @@ -2464,7 +2475,8 @@ pub(crate) mod test { async fn login() { let homeserver = Url::from_str(&mockito::server_url()).unwrap(); - let client = Client::new(homeserver.clone()).await.unwrap(); + let config = ClientConfig::new().await.unwrap().server_versions([MatrixVersion::V1_0]); + let client = Client::new_with_config(homeserver.clone(), config).await.unwrap(); let _m_types = mock("GET", "/_matrix/client/r0/login") .with_status(200) @@ -2496,7 +2508,11 @@ pub(crate) mod test { #[async_test] async fn login_with_discovery() { let homeserver = Url::from_str(&mockito::server_url()).unwrap(); - let config = ClientConfig::new().await.unwrap().use_discovery_response(); + let config = ClientConfig::new() + .await + .unwrap() + .use_discovery_response() + .server_versions([MatrixVersion::V1_0]); let client = Client::new_with_config(homeserver, config).await.unwrap(); @@ -2516,7 +2532,11 @@ pub(crate) mod test { #[async_test] async fn login_no_discovery() { let homeserver = Url::from_str(&mockito::server_url()).unwrap(); - let config = ClientConfig::new().await.unwrap().use_discovery_response(); + let config = ClientConfig::new() + .await + .unwrap() + .use_discovery_response() + .server_versions([MatrixVersion::V1_0]); let client = Client::new_with_config(homeserver.clone(), config).await.unwrap(); @@ -2542,7 +2562,8 @@ pub(crate) mod test { .create(); let homeserver = Url::from_str(&mockito::server_url()).unwrap(); - let client = Client::new(homeserver).await.unwrap(); + let config = ClientConfig::new().await.unwrap().server_versions([MatrixVersion::V1_0]); + let client = Client::new_with_config(homeserver, config).await.unwrap(); let idp = crate::client::get_login_types::v3::IdentityProvider::new( "some-id".to_owned(), "idp-name".to_owned(), @@ -2579,7 +2600,8 @@ pub(crate) mod test { async fn login_with_sso_token() { let homeserver = Url::from_str(&mockito::server_url()).unwrap(); - let client = Client::new(homeserver).await.unwrap(); + let config = ClientConfig::new().await.unwrap().server_versions([MatrixVersion::V1_0]); + let client = Client::new_with_config(homeserver, config).await.unwrap(); let _m = mock("GET", "/_matrix/client/r0/login") .with_status(200) diff --git a/crates/matrix-sdk/src/config/client.rs b/crates/matrix-sdk/src/config/client.rs index 8639c7953..8b7e81552 100644 --- a/crates/matrix-sdk/src/config/client.rs +++ b/crates/matrix-sdk/src/config/client.rs @@ -21,6 +21,7 @@ use std::{ use http::header::InvalidHeaderValue; use matrix_sdk_base::{BaseClientConfig, StateStore}; +use ruma::api::MatrixVersion; use crate::{config::RequestConfig, HttpSend, Result}; @@ -77,6 +78,7 @@ pub struct ClientConfig { pub(crate) client: Option>, pub(crate) appservice_mode: bool, pub(crate) use_discovery_response: bool, + pub(crate) server_versions: Option>, } #[cfg(not(tarpaulin_include))] @@ -279,4 +281,13 @@ impl ClientConfig { self.use_discovery_response = true; self } + + #[cfg(test)] + pub(crate) fn server_versions( + mut self, + server_versions: impl IntoIterator, + ) -> Self { + self.server_versions = Some(server_versions.into_iter().collect()); + self + } } From 872c35efd2908ccd9dff8a271bd53125eac85e6b Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Thu, 10 Mar 2022 16:52:01 +0100 Subject: [PATCH 4/7] Fix a typo --- crates/matrix-sdk-test-macros/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk-test-macros/src/lib.rs b/crates/matrix-sdk-test-macros/src/lib.rs index 493d72c54..7e87424b2 100644 --- a/crates/matrix-sdk-test-macros/src/lib.rs +++ b/crates/matrix-sdk-test-macros/src/lib.rs @@ -3,7 +3,7 @@ use quote::{format_ident, quote, ToTokens}; use syn::{self, parse_macro_input}; /// Attribute to use `wasm_bindgen_test` for wasm32 targets and `tokio::test` -/// for everything else with async-support and custom result-tyupes +/// for everything else with async-support and custom result-types #[proc_macro_attribute] pub fn async_test(_attr: TokenStream, item: TokenStream) -> TokenStream { let fun = parse_macro_input!(item as syn::ItemFn); From 7b2dfa39cff443b1dddad0a3e85ef5c1c80d644b Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Mon, 14 Mar 2022 12:57:58 +0100 Subject: [PATCH 5/7] Remove Client::get_supported_versions This is now always done as part of constructing the Client, unless the user actively opted out. --- crates/matrix-sdk/src/client/mod.rs | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index 4f0405f5c..904d4f947 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -44,7 +44,6 @@ use ruma::{ capabilities::{get_capabilities, Capabilities}, device::{delete_devices, get_devices}, directory::{get_public_rooms, get_public_rooms_filtered}, - discover::get_supported_versions, filter::{create_filter::v3::Request as FilterUploadRequest, FilterDefinition}, media::{create_content, get_content, get_content_thumbnail}, membership::{join_room_by_id, join_room_by_id_or_alias}, @@ -210,29 +209,6 @@ impl Client { *homeserver = homeserver_url; } - /// Get the versions supported by the homeserver. - /// - /// This method should be used to check that a server is a valid Matrix - /// homeserver. - /// - /// # Example - /// ```no_run - /// # use futures::executor::block_on; - /// # block_on(async { - /// use matrix_sdk::{Client}; - /// use url::Url; - /// - /// let homeserver = Url::parse("http://example.com")?; - /// let client = Client::new(homeserver).await?; - /// - /// // Check that it is a valid homeserver. - /// client.get_supported_versions().await?; - /// # Result::<_, anyhow::Error>::Ok(()) }); - /// ``` - pub async fn get_supported_versions(&self) -> HttpResult { - self.send(get_supported_versions::Request::new(), Some(RequestConfig::short_retry())).await - } - /// Get the capabilities of the homeserver. /// /// This method should be used to check what features are supported by the From 8df7e1cc8ba5904ff339ac9dfb1cdf415a5b6289 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Mon, 14 Mar 2022 15:08:48 +0100 Subject: [PATCH 6/7] Fix hanging tests --- crates/matrix-sdk/src/client/builder.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/client/builder.rs b/crates/matrix-sdk/src/client/builder.rs index 2f5f9a7ee..6e08ed3f4 100644 --- a/crates/matrix-sdk/src/client/builder.rs +++ b/crates/matrix-sdk/src/client/builder.rs @@ -326,7 +326,7 @@ impl ClientBuilder { None => http_client .send( get_supported_versions::Request::new(), - None, + Some(RequestConfig::short_retry()), [MatrixVersion::V1_0].into_iter().collect(), ) .await? From 15df8fef955990589cc9a457b5c532f7838fb48c Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Mon, 14 Mar 2022 15:08:54 +0100 Subject: [PATCH 7/7] Fix broken tests --- crates/matrix-sdk/src/client/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index 904d4f947..e8a161bf4 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -349,7 +349,7 @@ impl Client { /// # block_on(async { /// # let client = matrix_sdk::Client::builder() /// # .homeserver_url(homeserver) - /// # .check_supported_versions(false) + /// # .server_versions([ruma::api::MatrixVersion::V1_0]) /// # .build() /// # .await /// # .unwrap(); @@ -468,7 +468,7 @@ impl Client { /// # block_on(async { /// # let client = matrix_sdk::Client::builder() /// # .homeserver_url(homeserver) - /// # .check_supported_versions(false) + /// # .server_versions([ruma::api::MatrixVersion::V1_0]) /// # .build() /// # .await /// # .unwrap();