Skip to content

Commit

Permalink
sdk: Ignore the sliding sync proxy value when using SSS.
Browse files Browse the repository at this point in the history
  • Loading branch information
pixlwave committed Jul 22, 2024
1 parent f37799d commit 7920ea0
Showing 1 changed file with 69 additions and 22 deletions.
91 changes: 69 additions & 22 deletions crates/matrix-sdk/src/client/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ use ruma::{
};
use thiserror::Error;
use tokio::sync::{broadcast, Mutex, OnceCell};
use tracing::{debug, field::debug, instrument, Span};
use tracing::{debug, field::debug, info, instrument, Span};
use url::Url;

use super::{Client, ClientInner};
Expand Down Expand Up @@ -450,6 +450,7 @@ 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),

Expand All @@ -471,19 +472,31 @@ impl ClientBuilder {
let mut sliding_sync_proxy =
self.sliding_sync_proxy.as_ref().map(|url| Url::parse(url)).transpose()?;

#[allow(unused_variables)]
if let Some(well_known) = well_known {
#[cfg(feature = "experimental-sliding-sync")]
if sliding_sync_proxy.is_none() {
sliding_sync_proxy =
well_known.sliding_sync_proxy.and_then(|p| Url::parse(&p.url).ok())
#[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.
info!("Simplified MSC3575 is enabled, ignoring any sliding sync proxy.");
sliding_sync_proxy = None;
} else {
// Otherwise, if a proxy wasn't set, use the one discovered from the well-known.
if let Some(well_known) = well_known {
if sliding_sync_proxy.is_none() {
sliding_sync_proxy =
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);
if self.requires_sliding_sync {
if self.is_simplified_sliding_sync_enabled {
// TODO: The server doesn't yet expose the availability of SSS.
} else {
if sliding_sync_proxy.is_none() {
return Err(ClientBuildError::SlidingSyncNotAvailable);
}
}
}

let homeserver = Url::parse(&homeserver)?;
Expand Down Expand Up @@ -879,7 +892,7 @@ pub(crate) mod tests {
#[async_test]
async fn test_discovery_invalid_server() {
// Given a new client builder.
let mut builder = ClientBuilder::new();
let mut builder = make_ffi_builder();

// When building a client with an invalid server name.
builder = builder.server_name_or_homeserver_url("⚠️ This won't work 🚫");
Expand All @@ -892,7 +905,7 @@ pub(crate) mod tests {
#[async_test]
async fn test_discovery_no_server() {
// Given a new client builder.
let mut builder = ClientBuilder::new();
let mut builder = make_ffi_builder();

// When building a client with a valid server name that doesn't exist.
builder = builder.server_name_or_homeserver_url("localhost:3456");
Expand All @@ -908,7 +921,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 = ClientBuilder::new();
let mut builder = make_ffi_builder();

// When building a client with the server's URL.
builder = builder.server_name_or_homeserver_url(server.uri());
Expand All @@ -922,7 +935,7 @@ pub(crate) mod tests {
async fn test_discovery_direct_legacy() {
// Given a homeserver without a well-known file.
let homeserver = make_mock_homeserver().await;
let mut builder = ClientBuilder::new();
let mut builder = make_ffi_builder();

// When building a client with the server's URL.
builder = builder.server_name_or_homeserver_url(homeserver.uri());
Expand All @@ -938,7 +951,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 = ClientBuilder::new();
let mut builder = make_ffi_builder();
#[cfg(feature = "experimental-sliding-sync")]
{
builder = builder.sliding_sync_proxy("https://localhost:1234");
Expand All @@ -958,7 +971,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 = ClientBuilder::new();
let mut builder = make_ffi_builder();

let well_known = make_well_known_json(&homeserver.uri(), None);
let bad_json = well_known.to_string().replace(',', "");
Expand All @@ -985,7 +998,7 @@ pub(crate) mod tests {
// doesn't support sliding sync.
let server = MockServer::start().await;
let homeserver = make_mock_homeserver().await;
let mut builder = ClientBuilder::new();
let mut builder = make_ffi_builder();

Mock::given(method("GET"))
.and(path("/.well-known/matrix/client"))
Expand All @@ -1011,7 +1024,7 @@ pub(crate) mod tests {
// sliding sync proxy.
let server = MockServer::start().await;
let homeserver = make_mock_homeserver().await;
let mut builder = ClientBuilder::new();
let mut builder = make_ffi_builder();

Mock::given(method("GET"))
.and(path("/.well-known/matrix/client"))
Expand All @@ -1038,7 +1051,7 @@ pub(crate) mod tests {
// sliding sync proxy.
let server = MockServer::start().await;
let homeserver = make_mock_homeserver().await;
let mut builder = ClientBuilder::new();
let mut builder = make_ffi_builder();

Mock::given(method("GET"))
.and(path("/.well-known/matrix/client"))
Expand All @@ -1061,6 +1074,33 @@ pub(crate) mod tests {
assert_eq!(client.sliding_sync_proxy(), Some("https://localhost:9012".parse().unwrap()));
}

#[async_test]
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 = make_ffi_builder();

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.simplified_sliding_sync(true);
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.
#[cfg(feature = "experimental-sliding-sync")]
assert!(_client.sliding_sync_proxy().is_none());
}

/* Requires sliding sync */

#[async_test]
Expand All @@ -1070,7 +1110,7 @@ pub(crate) mod tests {
// doesn't support sliding sync.
let server = MockServer::start().await;
let homeserver = make_mock_homeserver().await;
let mut builder = ClientBuilder::new();
let mut builder = make_ffi_builder();

Mock::given(method("GET"))
.and(path("/.well-known/matrix/client"))
Expand All @@ -1096,7 +1136,7 @@ pub(crate) mod tests {
// sliding sync proxy.
let server = MockServer::start().await;
let homeserver = make_mock_homeserver().await;
let mut builder = ClientBuilder::new();
let mut builder = make_ffi_builder();

Mock::given(method("GET"))
.and(path("/.well-known/matrix/client"))
Expand All @@ -1121,7 +1161,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 = ClientBuilder::new();
let mut builder = make_ffi_builder();
builder = builder.sliding_sync_proxy("https://localhost:1234");

// When building a client that requires sliding sync with the server's URL.
Expand Down Expand Up @@ -1174,4 +1214,11 @@ pub(crate) mod tests {
object
})
}

/// The FFI's build has SSS disabled by default, but the SDK has it enabled
/// by default. The tests are aligned with the FFI for now (until we remove
/// regular sliding sync).
fn make_ffi_builder() -> ClientBuilder {
ClientBuilder::new().simplified_sliding_sync(false)
}
}

0 comments on commit 7920ea0

Please sign in to comment.