From b9bd58ff4dd590cf9b4541e67a45a1cadec7df02 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 6 Nov 2024 15:27:24 +0100 Subject: [PATCH] feat: `Client::cross_process_store_locks_holder_name` is used everywhere. See the Changelog Section to get the details. Changelog: `Client::cross_process_store_locks_holder_name` is used everywhere: - `StoreConfig::new()` now takes a `cross_process_store_locks_holder_name` argument. - `StoreConfig` no longer implements `Default`. - `BaseClient::new()` has been removed. - `BaseClient::clone_with_in_memory_state_store()` now takes a `cross_process_store_locks_holder_name` argument. - `BaseClient` no longer implements `Default`. - `EventCacheStoreLock::new()` no longer takes a `key` argument. - `BuilderStoreConfig` no longer has `cross_process_store_locks_holder_name` field for `Sqlite` and `IndexedDb`. --- benchmarks/benches/room_bench.rs | 5 +- benchmarks/benches/store_bench.rs | 10 ++- crates/matrix-sdk-base/src/client.rs | 44 +++++++------ .../src/event_cache_store/mod.rs | 7 ++- crates/matrix-sdk-base/src/rooms/normal.rs | 14 +++-- crates/matrix-sdk-base/src/store/mod.rs | 30 +++++---- crates/matrix-sdk-base/src/test_utils.rs | 6 +- crates/matrix-sdk/src/client/builder/mod.rs | 63 ++++++++++--------- crates/matrix-sdk/src/client/mod.rs | 17 ++++- crates/matrix-sdk/tests/integration/client.rs | 3 +- .../tests/integration/send_queue.rs | 9 ++- 11 files changed, 124 insertions(+), 84 deletions(-) diff --git a/benchmarks/benches/room_bench.rs b/benchmarks/benches/room_bench.rs index 2283cc955ab..57342857285 100644 --- a/benchmarks/benches/room_bench.rs +++ b/benchmarks/benches/room_bench.rs @@ -74,7 +74,10 @@ pub fn receive_all_members_benchmark(c: &mut Criterion) { .block_on(sqlite_store.save_changes(&changes)) .expect("initial filling of sqlite failed"); - let base_client = BaseClient::with_store_config(StoreConfig::new().state_store(sqlite_store)); + let base_client = BaseClient::with_store_config( + StoreConfig::new("cross-process-store-locks-holder-name".to_owned()) + .state_store(sqlite_store), + ); runtime .block_on(base_client.set_session_meta( diff --git a/benchmarks/benches/store_bench.rs b/benchmarks/benches/store_bench.rs index 4e7232c0735..bc6cd8f28ef 100644 --- a/benchmarks/benches/store_bench.rs +++ b/benchmarks/benches/store_bench.rs @@ -69,7 +69,10 @@ pub fn restore_session(c: &mut Criterion) { b.to_async(&runtime).iter(|| async { let client = Client::builder() .homeserver_url("https://matrix.example.com") - .store_config(StoreConfig::new().state_store(store.clone())) + .store_config( + StoreConfig::new("cross-process-store-locks-holder-name".to_owned()) + .state_store(store.clone()), + ) .build() .await .expect("Can't build client"); @@ -96,7 +99,10 @@ pub fn restore_session(c: &mut Criterion) { b.to_async(&runtime).iter(|| async { let client = Client::builder() .homeserver_url("https://matrix.example.com") - .store_config(StoreConfig::new().state_store(store.clone())) + .store_config( + StoreConfig::new("cross-process-store-locks-holder-name".to_owned()) + .state_store(store.clone()), + ) .build() .await .expect("Can't build client"); diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index 20801ee022f..19e49243818 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -139,11 +139,6 @@ impl fmt::Debug for BaseClient { } impl BaseClient { - /// Create a new default client. - pub fn new() -> Self { - BaseClient::with_store_config(StoreConfig::default()) - } - /// Create a new client. /// /// # Arguments @@ -173,8 +168,12 @@ impl BaseClient { /// Clones the current base client to use the same crypto store but a /// different, in-memory store config, and resets transient state. #[cfg(feature = "e2e-encryption")] - pub async fn clone_with_in_memory_state_store(&self) -> Result { - let config = StoreConfig::new().state_store(MemoryStore::new()); + pub async fn clone_with_in_memory_state_store( + &self, + cross_process_store_locks_holder_name: &str, + ) -> Result { + let config = StoreConfig::new(cross_process_store_locks_holder_name.to_owned()) + .state_store(MemoryStore::new()); let config = config.crypto_store(self.crypto_store.clone()); let copy = Self { @@ -207,8 +206,12 @@ impl BaseClient { /// different, in-memory store config, and resets transient state. #[cfg(not(feature = "e2e-encryption"))] #[allow(clippy::unused_async)] - pub async fn clone_with_in_memory_state_store(&self) -> Result { - let config = StoreConfig::new().state_store(MemoryStore::new()); + pub async fn clone_with_in_memory_state_store( + &self, + cross_process_store_locks_holder: &str, + ) -> Result { + let config = StoreConfig::new(cross_process_store_locks_holder.to_owned()) + .state_store(MemoryStore::new()); Ok(Self::with_store_config(config)) } @@ -1689,12 +1692,6 @@ impl BaseClient { } } -impl Default for BaseClient { - fn default() -> Self { - Self::new() - } -} - fn handle_room_member_event_for_profiles( room_id: &RoomId, event: &SyncStateEvent, @@ -1737,8 +1734,9 @@ mod tests { use super::BaseClient; use crate::{ - store::StateStoreExt, test_utils::logged_in_base_client, DisplayName, RoomState, - SessionMeta, + store::{StateStoreExt, StoreConfig}, + test_utils::logged_in_base_client, + DisplayName, RoomState, SessionMeta, }; #[async_test] @@ -1945,7 +1943,9 @@ mod tests { let user_id = user_id!("@alice:example.org"); let room_id = room_id!("!ithpyNKDtmhneaTQja:example.org"); - let client = BaseClient::new(); + let client = BaseClient::with_store_config(StoreConfig::new( + "cross-process-store-locks-holder-name".to_owned(), + )); client .set_session_meta( SessionMeta { user_id: user_id.to_owned(), device_id: "FOOBAR".into() }, @@ -2003,7 +2003,9 @@ mod tests { let inviter_user_id = user_id!("@bob:example.org"); let room_id = room_id!("!ithpyNKDtmhneaTQja:example.org"); - let client = BaseClient::new(); + let client = BaseClient::with_store_config(StoreConfig::new( + "cross-process-store-locks-holder-name".to_owned(), + )); client .set_session_meta( SessionMeta { user_id: user_id.to_owned(), device_id: "FOOBAR".into() }, @@ -2063,7 +2065,9 @@ mod tests { let inviter_user_id = user_id!("@bob:example.org"); let room_id = room_id!("!ithpyNKDtmhneaTQja:example.org"); - let client = BaseClient::new(); + let client = BaseClient::with_store_config(StoreConfig::new( + "cross-process-store-locks-holder-name".to_owned(), + )); client .set_session_meta( SessionMeta { user_id: user_id.to_owned(), device_id: "FOOBAR".into() }, diff --git a/crates/matrix-sdk-base/src/event_cache_store/mod.rs b/crates/matrix-sdk-base/src/event_cache_store/mod.rs index ab41d06327d..76b7032c195 100644 --- a/crates/matrix-sdk-base/src/event_cache_store/mod.rs +++ b/crates/matrix-sdk-base/src/event_cache_store/mod.rs @@ -60,7 +60,10 @@ impl fmt::Debug for EventCacheStoreLock { impl EventCacheStoreLock { /// Create a new lock around the [`EventCacheStore`]. - pub fn new(store: S, key: String, holder: String) -> Self + /// + /// The `holder` argument represents the holder inside the + /// [`CrossProcessStoreLock::new`]. + pub fn new(store: S, holder: String) -> Self where S: IntoEventCacheStore, { @@ -69,7 +72,7 @@ impl EventCacheStoreLock { Self { cross_process_lock: CrossProcessStoreLock::new( LockableEventCacheStore(store.clone()), - key, + "default".to_owned(), holder, ), store, diff --git a/crates/matrix-sdk-base/src/rooms/normal.rs b/crates/matrix-sdk-base/src/rooms/normal.rs index 7181d4d4824..bd3b1c58633 100644 --- a/crates/matrix-sdk-base/src/rooms/normal.rs +++ b/crates/matrix-sdk-base/src/rooms/normal.rs @@ -1850,7 +1850,7 @@ mod tests { use crate::latest_event::LatestEvent; use crate::{ rooms::RoomNotableTags, - store::{IntoStateStore, MemoryStore, StateChanges, StateStore}, + store::{IntoStateStore, MemoryStore, StateChanges, StateStore, StoreConfig}, BaseClient, DisplayName, MinimalStateEvent, OriginalMinimalStateEvent, RoomInfoNotableUpdateReasons, SessionMeta, }; @@ -2138,7 +2138,9 @@ mod tests { #[async_test] async fn test_is_favourite() { // Given a room, - let client = BaseClient::new(); + let client = BaseClient::with_store_config(StoreConfig::new( + "cross-process-store-locks-holder-name".to_owned(), + )); client .set_session_meta( @@ -2216,7 +2218,9 @@ mod tests { #[async_test] async fn test_is_low_priority() { // Given a room, - let client = BaseClient::new(); + let client = BaseClient::with_store_config(StoreConfig::new( + "cross-process-store-locks-holder-name".to_owned(), + )); client .set_session_meta( @@ -2672,7 +2676,9 @@ mod tests { use crate::{RoomInfoNotableUpdate, RoomInfoNotableUpdateReasons}; // Given a room, - let client = BaseClient::new(); + let client = BaseClient::with_store_config(StoreConfig::new( + "cross-process-store-locks-holder-name".to_owned(), + )); client .set_session_meta( diff --git a/crates/matrix-sdk-base/src/store/mod.rs b/crates/matrix-sdk-base/src/store/mod.rs index b68b668009c..b8989b11800 100644 --- a/crates/matrix-sdk-base/src/store/mod.rs +++ b/crates/matrix-sdk-base/src/store/mod.rs @@ -482,7 +482,8 @@ impl StateChanges { /// ``` /// # use matrix_sdk_base::store::StoreConfig; /// -/// let store_config = StoreConfig::new(); +/// let store_config = +/// StoreConfig::new("cross-process-store-locks-holder-name".to_owned()); /// ``` #[derive(Clone)] pub struct StoreConfig { @@ -490,6 +491,7 @@ pub struct StoreConfig { pub(crate) crypto_store: Arc, pub(crate) state_store: Arc, pub(crate) event_cache_store: event_cache_store::EventCacheStoreLock, + cross_process_store_locks_holder_name: String, } #[cfg(not(tarpaulin_include))] @@ -501,17 +503,20 @@ impl fmt::Debug for StoreConfig { impl StoreConfig { /// Create a new default `StoreConfig`. + /// + /// To learn more about `cross_process_store_locks_holder_name`, please read + /// [`CrossProcessStoreLock::new`](matrix_sdk_common::store_locks::CrossProcessStoreLock::new). #[must_use] - pub fn new() -> Self { + pub fn new(cross_process_store_locks_holder_name: String) -> Self { Self { #[cfg(feature = "e2e-encryption")] crypto_store: matrix_sdk_crypto::store::MemoryStore::new().into_crypto_store(), state_store: Arc::new(MemoryStore::new()), event_cache_store: event_cache_store::EventCacheStoreLock::new( event_cache_store::MemoryStore::new(), - "default-key".to_owned(), - "matrix-sdk-base".to_owned(), + cross_process_store_locks_holder_name.clone(), ), + cross_process_store_locks_holder_name, } } @@ -531,21 +536,14 @@ impl StoreConfig { } /// Set a custom implementation of an `EventCacheStore`. - /// - /// The `key` and `holder` arguments represent the key and holder inside the - /// [`CrossProcessStoreLock::new`][matrix_sdk_common::store_locks::CrossProcessStoreLock::new]. - pub fn event_cache_store(mut self, event_cache_store: S, key: String, holder: String) -> Self + pub fn event_cache_store(mut self, event_cache_store: S) -> Self where S: event_cache_store::IntoEventCacheStore, { - self.event_cache_store = - event_cache_store::EventCacheStoreLock::new(event_cache_store, key, holder); + self.event_cache_store = event_cache_store::EventCacheStoreLock::new( + event_cache_store, + self.cross_process_store_locks_holder_name.clone(), + ); self } } - -impl Default for StoreConfig { - fn default() -> Self { - Self::new() - } -} diff --git a/crates/matrix-sdk-base/src/test_utils.rs b/crates/matrix-sdk-base/src/test_utils.rs index 9ac48491ad8..d64928d3c24 100644 --- a/crates/matrix-sdk-base/src/test_utils.rs +++ b/crates/matrix-sdk-base/src/test_utils.rs @@ -18,12 +18,14 @@ use ruma::{owned_user_id, UserId}; -use crate::{BaseClient, SessionMeta}; +use crate::{store::StoreConfig, BaseClient, SessionMeta}; /// Create a [`BaseClient`] with the given user id, if provided, or an hardcoded /// one otherwise. pub(crate) async fn logged_in_base_client(user_id: Option<&UserId>) -> BaseClient { - let client = BaseClient::new(); + let client = BaseClient::with_store_config(StoreConfig::new( + "cross-process-store-locks-holder-name".to_owned(), + )); let user_id = user_id.map(|user_id| user_id.to_owned()).unwrap_or_else(|| owned_user_id!("@u:e.uk")); client diff --git a/crates/matrix-sdk/src/client/builder/mod.rs b/crates/matrix-sdk/src/client/builder/mod.rs index 37839bc5baf..059cb6616b5 100644 --- a/crates/matrix-sdk/src/client/builder/mod.rs +++ b/crates/matrix-sdk/src/client/builder/mod.rs @@ -105,13 +105,17 @@ pub struct ClientBuilder { } impl ClientBuilder { + const DEFAULT_CROSS_PROCESS_STORE_LOCKS_HOLDER_NAME: &str = "main"; + pub(crate) fn new() -> Self { Self { homeserver_cfg: None, #[cfg(feature = "experimental-sliding-sync")] sliding_sync_version_builder: SlidingSyncVersionBuilder::Native, http_cfg: None, - store_config: BuilderStoreConfig::Custom(StoreConfig::default()), + store_config: BuilderStoreConfig::Custom(StoreConfig::new( + Self::DEFAULT_CROSS_PROCESS_STORE_LOCKS_HOLDER_NAME.to_owned(), + )), request_config: Default::default(), respect_login_well_known: true, server_versions: None, @@ -123,7 +127,8 @@ impl ClientBuilder { room_key_recipient_strategy: Default::default(), #[cfg(feature = "e2e-encryption")] decryption_trust_requirement: TrustRequirement::Untrusted, - cross_process_store_locks_holder_name: "main".to_owned(), + cross_process_store_locks_holder_name: + Self::DEFAULT_CROSS_PROCESS_STORE_LOCKS_HOLDER_NAME.to_owned(), } } @@ -210,7 +215,6 @@ impl ClientBuilder { path: path.as_ref().to_owned(), cache_path: None, passphrase: passphrase.map(ToOwned::to_owned), - event_cache_store_lock_holder: "matrix-sdk".to_owned(), }; self } @@ -228,7 +232,6 @@ impl ClientBuilder { path: path.as_ref().to_owned(), cache_path: Some(cache_path.as_ref().to_owned()), passphrase: passphrase.map(ToOwned::to_owned), - event_cache_store_lock_holder: "matrix-sdk".to_owned(), }; self } @@ -239,7 +242,6 @@ impl ClientBuilder { self.store_config = BuilderStoreConfig::IndexedDb { name: name.to_owned(), passphrase: passphrase.map(ToOwned::to_owned), - event_cache_store_lock_holder: "matrix-sdk".to_owned(), }; self } @@ -260,7 +262,9 @@ impl ClientBuilder { /// # let custom_state_store = MemoryStore::new(); /// use matrix_sdk::{config::StoreConfig, Client}; /// - /// let store_config = StoreConfig::new().state_store(custom_state_store); + /// let store_config = + /// StoreConfig::new("cross-process-store-locks-holder-name".to_owned()) + /// .state_store(custom_state_store); /// let client_builder = Client::builder().store_config(store_config); /// ``` pub fn store_config(mut self, store_config: StoreConfig) -> Self { @@ -473,13 +477,17 @@ impl ClientBuilder { base_client } else { #[allow(unused_mut)] - let mut client = - BaseClient::with_store_config(build_store_config(self.store_config).await?); + let mut client = BaseClient::with_store_config( + build_store_config(self.store_config, &self.cross_process_store_locks_holder_name) + .await?, + ); + #[cfg(feature = "e2e-encryption")] { client.room_key_recipient_strategy = self.room_key_recipient_strategy; client.decryption_trust_requirement = self.decryption_trust_requirement; } + client }; @@ -564,20 +572,16 @@ pub fn sanitize_server_name(s: &str) -> crate::Result Result { #[allow(clippy::infallible_destructuring_match)] let store_config = match builder_config { #[cfg(feature = "sqlite")] - BuilderStoreConfig::Sqlite { - path, - cache_path, - passphrase, - event_cache_store_lock_holder, - } => { - let store_config = StoreConfig::new() + BuilderStoreConfig::Sqlite { path, cache_path, passphrase } => { + let store_config = StoreConfig::new(cross_process_store_locks_holder_name.to_owned()) .state_store( matrix_sdk_sqlite::SqliteStateStore::open(&path, passphrase.as_deref()).await?, ) @@ -587,8 +591,6 @@ async fn build_store_config( passphrase.as_deref(), ) .await?, - "default-key".to_owned(), - event_cache_store_lock_holder, ); #[cfg(feature = "e2e-encryption")] @@ -600,11 +602,11 @@ async fn build_store_config( } #[cfg(feature = "indexeddb")] - BuilderStoreConfig::IndexedDb { name, passphrase, event_cache_store_lock_holder } => { + BuilderStoreConfig::IndexedDb { name, passphrase } => { build_indexeddb_store_config( &name, passphrase.as_deref(), - event_cache_store_lock_holder, + cross_process_store_locks_holder_name, ) .await? } @@ -620,28 +622,28 @@ async fn build_store_config( async fn build_indexeddb_store_config( name: &str, passphrase: Option<&str>, - event_cache_store_lock_holder: String, + cross_process_store_locks_holder_name: &str, ) -> Result { + let cross_process_store_locks_holder_name = cross_process_store_locks_holder_name.to_owned(); + #[cfg(feature = "e2e-encryption")] let store_config = { let (state_store, crypto_store) = matrix_sdk_indexeddb::open_stores_with_name(name, passphrase).await?; - StoreConfig::new().state_store(state_store).crypto_store(crypto_store) + StoreConfig::new(cross_process_store_locks_holder_name) + .state_store(state_store) + .crypto_store(crypto_store) }; #[cfg(not(feature = "e2e-encryption"))] let store_config = { let state_store = matrix_sdk_indexeddb::open_state_store(name, passphrase).await?; - StoreConfig::new().state_store(state_store) + StoreConfig::new(cross_process_store_locks_holder_name).state_store(state_store) }; let store_config = { tracing::warn!("The IndexedDB backend does not implement an event cache store, falling back to the in-memory event cache store…"); - store_config.event_cache_store( - matrix_sdk_base::event_cache_store::MemoryStore::new(), - "default-key".to_owned(), - event_cache_store_lock_holder, - ) + store_config.event_cache_store(matrix_sdk_base::event_cache_store::MemoryStore::new()) }; Ok(store_config) @@ -651,7 +653,7 @@ async fn build_indexeddb_store_config( async fn build_indexeddb_store_config( _name: &str, _passphrase: Option<&str>, - _event_cache_store_lock_holder: String, + _event_cache_store_lock_holder_name: &str, ) -> Result { panic!("the IndexedDB is only available on the 'wasm32' arch") } @@ -696,13 +698,11 @@ enum BuilderStoreConfig { path: std::path::PathBuf, cache_path: Option, passphrase: Option, - event_cache_store_lock_holder: String, }, #[cfg(feature = "indexeddb")] IndexedDb { name: String, passphrase: Option, - event_cache_store_lock_holder: String, }, Custom(StoreConfig), } @@ -770,6 +770,7 @@ pub(crate) mod tests { use assert_matches::assert_matches; use matrix_sdk_test::{async_test, test_json}; use serde_json::{json_internal, Value as JsonValue}; + #[cfg(feature = "experimental-sliding-sync")] use url::Url; use wiremock::{ matchers::{method, path}, diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index c82eb57b2b9..07d21a304b8 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -2233,7 +2233,12 @@ impl Client { #[cfg(feature = "experimental-sliding-sync")] self.sliding_sync_version(), self.inner.http_client.clone(), - self.inner.base_client.clone_with_in_memory_state_store().await?, + self.inner + .base_client + .clone_with_in_memory_state_store( + &self.inner.cross_process_store_locks_holder_name, + ) + .await?, self.inner.server_capabilities.read().await.clone(), self.inner.respect_login_well_known, self.inner.event_cache.clone(), @@ -2763,7 +2768,10 @@ pub(crate) mod tests { let memory_store = Arc::new(MemoryStore::new()); let client = Client::builder() .insecure_server_name_no_tls(server_name) - .store_config(StoreConfig::new().state_store(memory_store.clone())) + .store_config( + StoreConfig::new("cross-process-store-locks-holder-name".to_owned()) + .state_store(memory_store.clone()), + ) .build() .await .unwrap(); @@ -2782,7 +2790,10 @@ pub(crate) mod tests { let client = Client::builder() .insecure_server_name_no_tls(server_name) - .store_config(StoreConfig::new().state_store(memory_store.clone())) + .store_config( + StoreConfig::new("cross-process-store-locks-holder-name".to_owned()) + .state_store(memory_store.clone()), + ) .build() .await .unwrap(); diff --git a/crates/matrix-sdk/tests/integration/client.rs b/crates/matrix-sdk/tests/integration/client.rs index 589087296a5..a8d7a9e1115 100644 --- a/crates/matrix-sdk/tests/integration/client.rs +++ b/crates/matrix-sdk/tests/integration/client.rs @@ -1366,7 +1366,8 @@ async fn test_restore_room() { store.save_changes(&changes).await.unwrap(); // Build a client with that store. - let store_config = StoreConfig::new().state_store(store); + let store_config = + StoreConfig::new("cross-process-store-locks-holder-name".to_owned()).state_store(store); let client = Client::builder() .homeserver_url("http://localhost:1234") .request_config(RequestConfig::new().disable_retry()) diff --git a/crates/matrix-sdk/tests/integration/send_queue.rs b/crates/matrix-sdk/tests/integration/send_queue.rs index 55455dd0ff9..4cce220323d 100644 --- a/crates/matrix-sdk/tests/integration/send_queue.rs +++ b/crates/matrix-sdk/tests/integration/send_queue.rs @@ -1715,7 +1715,10 @@ async fn test_reloading_rooms_with_unsent_events() { let client = Client::builder() .homeserver_url(server.uri()) .server_versions([MatrixVersion::V1_0]) - .store_config(StoreConfig::new().state_store(store.clone())) + .store_config( + StoreConfig::new("cross-process-store-locks-holder-name".to_owned()) + .state_store(store.clone()), + ) .request_config(RequestConfig::new().disable_retry()) .build() .await @@ -1792,7 +1795,9 @@ async fn test_reloading_rooms_with_unsent_events() { let client = Client::builder() .homeserver_url(server.uri()) .server_versions([MatrixVersion::V1_0]) - .store_config(StoreConfig::new().state_store(store)) + .store_config( + StoreConfig::new("cross-process-store-locks-holder-name".to_owned()).state_store(store), + ) .request_config(RequestConfig::new().disable_retry()) .build() .await