refactor(test): avoid proliferation of builder submethods in the MockClientBuilder

Instead of having one static method duplicating an underlying
`ClientBuilder` method, we can pass the builder directly to a closure,
that will replace it. Call sites are a bit more verbose, but that would
avoid having to add duplicate `MockClientBuilder` methods for each
`ClientBuilder` method.

Author: bnjbvr

benchmarks/benches/event_cache.rs

@@ -86,12 +86,14 @@ fn handle_room_updates(c: &mut Criterion) {
                                 // Setup code.
                                 let client = server
                                     .client_builder()
-                                    .store_config(
-                                        StoreConfig::new(
-                                            "cross-process-store-locks-holder-name".to_owned(),
+                                    .on_builder(|builder| {
+                                        builder.store_config(
+                                            StoreConfig::new(
+                                                "cross-process-store-locks-holder-name".to_owned(),
+                                            )
+                                            .event_cache_store(store.clone()),
                                         )
-                                        .event_cache_store(store.clone()),
-                                    )
+                                    })
                                     .build()
                                     .await;
 

crates/matrix-sdk/src/authentication/oauth/cross_process.rs

@@ -269,7 +269,11 @@ mod tests {
         // Create a client that will use sqlite databases.
 
         let tmp_dir = tempfile::tempdir()?;
-        let client = MockClientBuilder::new(None).sqlite_store(&tmp_dir).unlogged().build().await;
+        let client = MockClientBuilder::new(None)
+            .on_builder(|builder| builder.sqlite_store(&tmp_dir, None))
+            .unlogged()
+            .build()
+            .await;
 
         let tokens = mock_session_tokens_with_refresh();
 
@@ -315,8 +319,12 @@ mod tests {
         server.mock_who_am_i().ok().expect(1).named("whoami").mount().await;
 
         let tmp_dir = tempfile::tempdir()?;
-        let client =
-            server.client_builder().sqlite_store(&tmp_dir).registered_with_oauth().build().await;
+        let client = server
+            .client_builder()
+            .on_builder(|builder| builder.sqlite_store(&tmp_dir, None))
+            .registered_with_oauth()
+            .build()
+            .await;
         let oauth = client.oauth();
 
         // Enable cross-process lock.
@@ -365,7 +373,12 @@ mod tests {
         oauth_server.mock_token().ok().expect(1).named("token").mount().await;
 
         let tmp_dir = tempfile::tempdir()?;
-        let client = server.client_builder().sqlite_store(&tmp_dir).unlogged().build().await;
+        let client = server
+            .client_builder()
+            .on_builder(|builder| builder.sqlite_store(&tmp_dir, None))
+            .unlogged()
+            .build()
+            .await;
         let oauth = client.oauth();
 
         let next_tokens = mock_session_tokens_with_refresh();
@@ -414,7 +427,12 @@ mod tests {
 
         // Create the first client.
         let tmp_dir = tempfile::tempdir()?;
-        let client = server.client_builder().sqlite_store(&tmp_dir).unlogged().build().await;
+        let client = server
+            .client_builder()
+            .on_builder(|builder| builder.sqlite_store(&tmp_dir, None))
+            .unlogged()
+            .build()
+            .await;
 
         let oauth = client.oauth();
         oauth.enable_cross_process_refresh_lock("client1".to_owned()).await?;
@@ -425,15 +443,24 @@ mod tests {
 
         // Create a second client, without restoring it, to test that a token update
         // before restoration doesn't cause new issues.
-        let unrestored_client =
-            server.client_builder().sqlite_store(&tmp_dir).unlogged().build().await;
+        let unrestored_client = server
+            .client_builder()
+            .on_builder(|builder| builder.sqlite_store(&tmp_dir, None))
+            .unlogged()
+            .build()
+            .await;
         let unrestored_oauth = unrestored_client.oauth();
         unrestored_oauth.enable_cross_process_refresh_lock("unrestored_client".to_owned()).await?;
 
         {
             // Create a third client that will run a refresh while the others two are doing
             // nothing.
-            let client3 = server.client_builder().sqlite_store(&tmp_dir).unlogged().build().await;
+            let client3 = server
+                .client_builder()
+                .on_builder(|builder| builder.sqlite_store(&tmp_dir, None))
+                .unlogged()
+                .build()
+                .await;
 
             let oauth3 = client3.oauth();
             oauth3.enable_cross_process_refresh_lock("client3".to_owned()).await?;
@@ -541,7 +568,12 @@ mod tests {
         oauth_server.mock_revocation().ok().expect(1).named("revocation").mount().await;
 
         let tmp_dir = tempfile::tempdir()?;
-        let client = server.client_builder().sqlite_store(&tmp_dir).unlogged().build().await;
+        let client = server
+            .client_builder()
+            .on_builder(|builder| builder.sqlite_store(&tmp_dir, None))
+            .unlogged()
+            .build()
+            .await;
         let oauth = client.oauth().insecure_rewrite_https_to_http();
 
         // Enable cross-process lock.

crates/matrix-sdk/src/client/mod.rs

@@ -3069,7 +3069,7 @@ pub(crate) mod tests {
         let server = MatrixMockServer::new().await;
         let client = server
             .client_builder()
-            .request_config(RequestConfig::new().retry_limit(3))
+            .on_builder(|builder| builder.request_config(RequestConfig::new().retry_limit(3)))
             .build()
             .await;
 
@@ -3087,7 +3087,9 @@ pub(crate) mod tests {
         let server = MatrixMockServer::new().await;
         let client = server
             .client_builder()
-            .request_config(RequestConfig::new().max_retry_time(retry_timeout))
+            .on_builder(|builder| {
+                builder.request_config(RequestConfig::new().max_retry_time(retry_timeout))
+            })
             .build()
             .await;
 
@@ -3101,8 +3103,11 @@ pub(crate) mod tests {
     #[async_test]
     async fn test_short_retry_initial_http_requests() {
         let server = MatrixMockServer::new().await;
-        let client =
-            server.client_builder().request_config(RequestConfig::short_retry()).build().await;
+        let client = server
+            .client_builder()
+            .on_builder(|builder| builder.request_config(RequestConfig::short_retry()))
+            .build()
+            .await;
 
         server.mock_login().error500().expect(3..).mount().await;
 
@@ -3318,10 +3323,12 @@ pub(crate) mod tests {
         let client = server
             .client_builder()
             .no_server_versions()
-            .store_config(
-                StoreConfig::new("cross-process-store-locks-holder-name".to_owned())
-                    .state_store(memory_store.clone()),
-            )
+            .on_builder(|builder| {
+                builder.store_config(
+                    StoreConfig::new("cross-process-store-locks-holder-name".to_owned())
+                        .state_store(memory_store.clone()),
+                )
+            })
             .build()
             .await;
 
@@ -3372,10 +3379,12 @@ pub(crate) mod tests {
         let client = server
             .client_builder()
             .no_server_versions()
-            .store_config(
-                StoreConfig::new("cross-process-store-locks-holder-name".to_owned())
-                    .state_store(memory_store.clone()),
-            )
+            .on_builder(|builder| {
+                builder.store_config(
+                    StoreConfig::new("cross-process-store-locks-holder-name".to_owned())
+                        .state_store(memory_store.clone()),
+                )
+            })
             .build()
             .await;
 
@@ -3390,10 +3399,12 @@ pub(crate) mod tests {
         let client = server
             .client_builder()
             .no_server_versions()
-            .store_config(
-                StoreConfig::new("cross-process-store-locks-holder-name".to_owned())
-                    .state_store(memory_store.clone()),
-            )
+            .on_builder(|builder| {
+                builder.store_config(
+                    StoreConfig::new("cross-process-store-locks-holder-name".to_owned())
+                        .state_store(memory_store.clone()),
+                )
+            })
             .build()
             .await;
 
@@ -3424,8 +3435,10 @@ pub(crate) mod tests {
     #[async_test]
     async fn test_no_network_doesnt_cause_infinite_retries() {
         // We want infinite retries for transient errors.
-        let client =
-            MockClientBuilder::new(None).request_config(RequestConfig::new()).build().await;
+        let client = MockClientBuilder::new(None)
+            .on_builder(|builder| builder.request_config(RequestConfig::new()))
+            .build()
+            .await;
 
         // We don't define a mock server on purpose here, so that the error is really a
         // network error.

crates/matrix-sdk/src/event_cache/room/mod.rs

@@ -2221,9 +2221,12 @@ mod timed_tests {
         let event_cache_store = Arc::new(MemoryStore::new());
 
         let client = MockClientBuilder::new(None)
-            .store_config(
-                StoreConfig::new("hodlor".to_owned()).event_cache_store(event_cache_store.clone()),
-            )
+            .on_builder(|builder| {
+                builder.store_config(
+                    StoreConfig::new("hodlor".to_owned())
+                        .event_cache_store(event_cache_store.clone()),
+                )
+            })
             .build()
             .await;
 
@@ -2295,9 +2298,12 @@ mod timed_tests {
         let event_cache_store = Arc::new(MemoryStore::new());
 
         let client = MockClientBuilder::new(None)
-            .store_config(
-                StoreConfig::new("hodlor".to_owned()).event_cache_store(event_cache_store.clone()),
-            )
+            .on_builder(|builder| {
+                builder.store_config(
+                    StoreConfig::new("hodlor".to_owned())
+                        .event_cache_store(event_cache_store.clone()),
+                )
+            })
             .build()
             .await;
 
@@ -2434,9 +2440,12 @@ mod timed_tests {
             .unwrap();
 
         let client = MockClientBuilder::new(None)
-            .store_config(
-                StoreConfig::new("hodlor".to_owned()).event_cache_store(event_cache_store.clone()),
-            )
+            .on_builder(|builder| {
+                builder.store_config(
+                    StoreConfig::new("hodlor".to_owned())
+                        .event_cache_store(event_cache_store.clone()),
+                )
+            })
             .build()
             .await;
 
@@ -2585,9 +2594,12 @@ mod timed_tests {
             .unwrap();
 
         let client = MockClientBuilder::new(None)
-            .store_config(
-                StoreConfig::new("hodlor".to_owned()).event_cache_store(event_cache_store.clone()),
-            )
+            .on_builder(|builder| {
+                builder.store_config(
+                    StoreConfig::new("hodlor".to_owned())
+                        .event_cache_store(event_cache_store.clone()),
+                )
+            })
             .build()
             .await;
 
@@ -2706,9 +2718,12 @@ mod timed_tests {
             .unwrap();
 
         let client = MockClientBuilder::new(None)
-            .store_config(
-                StoreConfig::new("holder".to_owned()).event_cache_store(event_cache_store.clone()),
-            )
+            .on_builder(|builder| {
+                builder.store_config(
+                    StoreConfig::new("holder".to_owned())
+                        .event_cache_store(event_cache_store.clone()),
+                )
+            })
             .build()
             .await;
 

crates/matrix-sdk/src/test_utils/client.rs

@@ -14,10 +14,7 @@
 
 //! Augmented [`ClientBuilder`] that can set up an already logged-in user.
 
-use matrix_sdk_base::{
-    store::{RoomLoadSettings, StoreConfig},
-    SessionMeta,
-};
+use matrix_sdk_base::{store::RoomLoadSettings, SessionMeta};
 use ruma::{api::MatrixVersion, owned_device_id, owned_user_id, OwnedDeviceId, OwnedUserId};
 
 use crate::{
@@ -39,7 +36,7 @@ impl MockClientBuilder {
     /// default).
     ///
     /// If no homeserver is provided, `http://localhost` is used as a homeserver.
-    pub(crate) fn new(homeserver: Option<&str>) -> Self {
+    pub fn new(homeserver: Option<&str>) -> Self {
         let homeserver = homeserver.unwrap_or("http://localhost");
 
         let default_builder = Client::builder()
@@ -63,23 +60,6 @@ impl MockClientBuilder {
         self
     }
 
-    /// Enable the share history on invite feature for the Client.
-    #[cfg(feature = "e2e-encryption")]
-    pub fn enable_share_history_on_invite(mut self) -> Self {
-        self.builder = self.builder.with_enable_share_history_on_invite(true);
-        self
-    }
-
-    /// Use the given encryption settings with the test client.
-    #[cfg(feature = "e2e-encryption")]
-    pub fn with_encryption_settings(
-        mut self,
-        settings: crate::encryption::EncryptionSettings,
-    ) -> Self {
-        self.builder = self.builder.with_encryption_settings(settings);
-        self
-    }
-
     /// Set the cached server versions in the client.
     pub fn server_versions(mut self, versions: Vec<MatrixVersion>) -> Self {
         self.server_versions = ServerVersions::Custom(versions);
@@ -121,30 +101,25 @@ impl MockClientBuilder {
         self
     }
 
-    /// Override the default [`RequestConfig`] for the underlying
-    /// [`ClientBuilder`].
-    pub fn request_config(mut self, request_config: RequestConfig) -> Self {
-        self.builder = self.builder.request_config(request_config);
-        self
-    }
-
-    /// Provides another [`StoreConfig`] for the underlying [`ClientBuilder`].
-    pub fn store_config(mut self, store_config: StoreConfig) -> Self {
-        self.builder = self.builder.store_config(store_config);
-        self
-    }
-
-    /// Use an SQLite store at the given path for the underlying
-    /// [`ClientBuilder`].
-    #[cfg(feature = "sqlite")]
-    pub fn sqlite_store(mut self, path: impl AsRef<std::path::Path>) -> Self {
-        self.builder = self.builder.sqlite_store(path, None);
-        self
-    }
-
-    /// Handle refreshing access tokens automatically.
-    pub fn handle_refresh_tokens(mut self) -> Self {
-        self.builder = self.builder.handle_refresh_tokens();
+    /// Apply changes to the underlying [`ClientBuilder`].
+    ///
+    /// ```
+    /// # tokio_test::block_on(async {
+    /// use matrix_sdk::test_utils::client::MockClientBuilder;
+    ///
+    /// MockClientBuilder::new(None)
+    ///     .on_builder(|builder| {
+    ///         // Here it's possible to modify the underlying `ClientBuilder`.
+    ///         builder
+    ///             .handle_refresh_tokens()
+    ///             .cross_process_store_locks_holder_name("hodor".to_owned())
+    ///     })
+    ///     .build()
+    ///     .await;
+    /// # anyhow::Ok(()) });
+    /// ```
+    pub fn on_builder<F: FnOnce(ClientBuilder) -> ClientBuilder>(mut self, f: F) -> Self {
+        self.builder = f(self.builder);
         self
     }
 
@@ -157,6 +132,7 @@ impl MockClientBuilder {
         }
 
         let client = builder.build().await.expect("building client failed");
+
         self.auth_state.maybe_restore_client(&client).await;
 
         client