feat(ui): EncryptionSyncService and Notification are using Client::cross_process_store_locks_holder_name.

This patch removes the `process_id` argument from
`EncryptionSyncService::new()` and replaces it by
`Client::cross_process_store_locks_holder_name`. The “process ID” is
set when the `Client` is converted into another `Client` tailore for
notification in `NotificationClient` with `Client::notification_client`
which now has a new `cross_process_store_locks_holder_name` argument.
This commit is contained in:
Ivan Enderlin
2024-11-06 16:55:04 +01:00
parent 90b8ba3c2e
commit 563c3aae31
9 changed files with 55 additions and 43 deletions
+2 -2
View File
@@ -112,9 +112,9 @@ impl SyncServiceBuilder {
#[matrix_sdk_ffi_macros::export]
impl SyncServiceBuilder {
pub fn with_cross_process_lock(self: Arc<Self>, app_identifier: Option<String>) -> Arc<Self> {
pub fn with_cross_process_lock(self: Arc<Self>) -> Arc<Self> {
let this = unwrap_or_clone_arc(self);
let builder = this.builder.with_cross_process_lock(app_identifier);
let builder = this.builder.with_cross_process_lock();
Arc::new(Self { client: this.client, builder, utd_hook: this.utd_hook })
}
@@ -88,11 +88,7 @@ impl EncryptionSyncService {
/// Creates a new instance of a `EncryptionSyncService`.
///
/// This will create and manage an instance of [`matrix_sdk::SlidingSync`].
/// The `process_id` is used as the identifier of that instance, as such
/// make sure to not reuse a name used by another process, at the risk
/// of causing problems.
pub async fn new(
process_id: String,
client: Client,
poll_and_network_timeouts: Option<(Duration, Duration)>,
with_locking: WithLocking,
@@ -119,7 +115,13 @@ impl EncryptionSyncService {
if with_locking {
// Gently try to enable the cross-process lock on behalf of the user.
match client.encryption().enable_cross_process_store_lock(process_id).await {
match client
.encryption()
.enable_cross_process_store_lock(
client.cross_process_store_locks_holder_name().to_owned(),
)
.await
{
Ok(()) | Err(matrix_sdk::Error::BadCryptoStoreState) => {
// Ignore; we've already set the crypto store lock to
// something, and that's sufficient as
@@ -111,7 +111,8 @@ impl NotificationClient {
parent_client: Client,
process_setup: NotificationProcessSetup,
) -> Result<Self, Error> {
let client = parent_client.notification_client().await?;
let client = parent_client.notification_client(Self::LOCK_ID.to_owned()).await?;
Ok(NotificationClient {
client,
parent_client,
@@ -242,7 +243,6 @@ impl NotificationClient {
};
let encryption_sync = EncryptionSyncService::new(
Self::LOCK_ID.to_owned(),
self.client.clone(),
Some((Duration::from_secs(3), Duration::from_secs(4))),
with_locking,
+4 -13
View File
@@ -435,15 +435,11 @@ pub struct SyncServiceBuilder {
/// Is the cross-process lock for the crypto store enabled?
with_cross_process_lock: bool,
/// Application identifier, used as the cross-process lock value, if
/// applicable.
identifier: String,
}
impl SyncServiceBuilder {
fn new(client: Client) -> Self {
Self { client, with_cross_process_lock: false, identifier: "app".to_owned() }
Self { client, with_cross_process_lock: false }
}
/// Enables the cross-process lock, if the sync service is being built in a
@@ -454,14 +450,10 @@ impl SyncServiceBuilder {
/// external process attempting to decrypt notifications. In general,
/// `with_cross_process_lock` should not be called.
///
/// An app identifier can be provided too, to identify the current process;
/// if it's not provided, a default value of "app" is used as the
/// application identifier.
pub fn with_cross_process_lock(mut self, app_identifier: Option<String>) -> Self {
/// Be sure to have configured
/// [`Client::cross_process_store_locks_holder_name`] accordingly.
pub fn with_cross_process_lock(mut self) -> Self {
self.with_cross_process_lock = true;
if let Some(app_identifier) = app_identifier {
self.identifier = app_identifier;
}
self
}
@@ -477,7 +469,6 @@ impl SyncServiceBuilder {
let encryption_sync = Arc::new(
EncryptionSyncService::new(
self.identifier,
self.client,
None,
WithLocking::from(self.with_cross_process_lock),
@@ -40,8 +40,7 @@ async fn test_smoke_encryption_sync_works() -> anyhow::Result<()> {
let sync_permit = Arc::new(AsyncMutex::new(EncryptionSyncPermit::new_for_testing()));
let sync_permit_guard = sync_permit.clone().lock_owned().await;
let encryption_sync =
EncryptionSyncService::new("tests".to_owned(), client, None, WithLocking::Yes).await?;
let encryption_sync = EncryptionSyncService::new(client, None, WithLocking::Yes).await?;
let stream = encryption_sync.sync(sync_permit_guard);
pin_mut!(stream);
@@ -186,8 +185,7 @@ async fn test_encryption_sync_one_fixed_iteration() -> anyhow::Result<()> {
let sync_permit = Arc::new(AsyncMutex::new(EncryptionSyncPermit::new_for_testing()));
let sync_permit_guard = sync_permit.lock_owned().await;
let encryption_sync =
EncryptionSyncService::new("tests".to_owned(), client, None, WithLocking::Yes).await?;
let encryption_sync = EncryptionSyncService::new(client, None, WithLocking::Yes).await?;
// Run all the iterations.
encryption_sync.run_fixed_iterations(1, sync_permit_guard).await?;
@@ -218,8 +216,7 @@ async fn test_encryption_sync_two_fixed_iterations() -> anyhow::Result<()> {
let sync_permit = Arc::new(AsyncMutex::new(EncryptionSyncPermit::new_for_testing()));
let sync_permit_guard = sync_permit.lock_owned().await;
let encryption_sync =
EncryptionSyncService::new("tests".to_owned(), client, None, WithLocking::Yes).await?;
let encryption_sync = EncryptionSyncService::new(client, None, WithLocking::Yes).await?;
encryption_sync.run_fixed_iterations(2, sync_permit_guard).await?;
@@ -254,8 +251,7 @@ async fn test_encryption_sync_always_reloads_todevice_token() -> anyhow::Result<
let sync_permit = Arc::new(AsyncMutex::new(EncryptionSyncPermit::new_for_testing()));
let sync_permit_guard = sync_permit.lock_owned().await;
let encryption_sync =
EncryptionSyncService::new("tests".to_owned(), client.clone(), None, WithLocking::Yes)
.await?;
EncryptionSyncService::new(client.clone(), None, WithLocking::Yes).await?;
let stream = encryption_sync.sync(sync_permit_guard);
pin_mut!(stream);
@@ -363,15 +359,14 @@ async fn test_notification_client_does_not_upload_duplicate_one_time_keys() -> a
info!("Creating the notification client");
let notification_client = client
.notification_client()
.notification_client("tests".to_owned())
.await
.expect("We should be able to build a notification client");
let sync_permit = Arc::new(AsyncMutex::new(EncryptionSyncPermit::new_for_testing()));
let sync_permit_guard = sync_permit.lock_owned().await;
let encryption_sync =
EncryptionSyncService::new("tests".to_owned(), client.clone(), None, WithLocking::Yes)
.await?;
EncryptionSyncService::new(client.clone(), None, WithLocking::Yes).await?;
let stream = encryption_sync.sync(sync_permit_guard);
pin_mut!(stream);
+11 -5
View File
@@ -2253,7 +2253,15 @@ impl Client {
}
/// Create a new specialized `Client` that can process notifications.
pub async fn notification_client(&self) -> Result<Client> {
///
/// See [`CrossProcessStoreLock::new`] to learn more about
/// `cross_process_store_locks_holder_name`.
///
/// [`CrossProcessStoreLock::new`]: matrix_sdk_common::store_locks::CrossProcessStoreLock::new
pub async fn notification_client(
&self,
cross_process_store_locks_holder_name: String,
) -> Result<Client> {
let client = Client {
inner: ClientInner::new(
self.inner.auth_ctx.clone(),
@@ -2264,9 +2272,7 @@ impl Client {
self.inner.http_client.clone(),
self.inner
.base_client
.clone_with_in_memory_state_store(
&self.inner.cross_process_store_locks_holder_name,
)
.clone_with_in_memory_state_store(&cross_process_store_locks_holder_name)
.await?,
self.inner.server_capabilities.read().await.clone(),
self.inner.respect_login_well_known,
@@ -2274,7 +2280,7 @@ impl Client {
self.inner.send_queue_data.clone(),
#[cfg(feature = "e2e-encryption")]
self.inner.e2ee.encryption_settings,
self.inner.cross_process_store_locks_holder_name.clone(),
cross_process_store_locks_holder_name,
)
.await,
};
+2
View File
@@ -1458,6 +1458,8 @@ impl Encryption {
/// caches.
///
/// The provided `lock_value` must be a unique identifier for this process.
/// Check [`Client::cross_process_store_locks_holder_name`] to
/// get the global value.
pub async fn enable_cross_process_store_lock(&self, lock_value: String) -> Result<(), Error> {
// If the lock has already been created, don't recreate it from scratch.
if let Some(prev_lock) = self.client.locks().cross_process_crypto_store_lock.get() {
@@ -37,6 +37,7 @@ pub struct TestClientBuilder {
use_sqlite_dir: Option<SqlitePath>,
encryption_settings: EncryptionSettings,
http_proxy: Option<String>,
cross_process_store_locks_holder_name: Option<String>,
}
impl TestClientBuilder {
@@ -52,6 +53,7 @@ impl TestClientBuilder {
use_sqlite_dir: None,
encryption_settings: Default::default(),
http_proxy: None,
cross_process_store_locks_holder_name: None,
}
}
@@ -79,6 +81,11 @@ impl TestClientBuilder {
self
}
pub fn cross_process_store_locks_holder_name(mut self, holder_name: String) -> Self {
self.cross_process_store_locks_holder_name = Some(holder_name);
self
}
fn common_client_builder(&self) -> ClientBuilder {
let homeserver_url =
option_env!("HOMESERVER_URL").unwrap_or("http://localhost:8228").to_owned();
@@ -90,6 +97,11 @@ impl TestClientBuilder {
.with_encryption_settings(self.encryption_settings)
.request_config(RequestConfig::short_retry());
if let Some(holder_name) = &self.cross_process_store_locks_holder_name {
client_builder =
client_builder.cross_process_store_locks_holder_name(holder_name.clone());
}
if let Some(proxy) = &self.http_proxy {
client_builder = client_builder.proxy(proxy);
}
@@ -125,14 +125,18 @@ impl ClientWrapper {
/// Otherwise, a random path is used.
///
/// The contained SyncService always has a cross-process lock. If
/// app_identifier is supplied, it is used to identify this client's
/// process. If not, the default name is used.
/// `cross_process_store_locks_holder_name` is supplied, it is used to
/// identify this client's process. If not, the default name is used.
async fn new(
username: &str,
sqlite_dir: Option<&Path>,
app_identifier: Option<String>,
cross_process_store_locks_holder_name: Option<String>,
) -> Self {
let builder = TestClientBuilder::new(username);
let mut builder = TestClientBuilder::new(username);
if let Some(holder_name) = cross_process_store_locks_holder_name {
builder = builder.cross_process_store_locks_holder_name(holder_name);
}
let builder = if let Some(sqlite_dir) = sqlite_dir {
builder.use_sqlite_dir(sqlite_dir)
@@ -153,7 +157,7 @@ impl ClientWrapper {
let client = SyncTokenAwareClient::new(inner_client.clone());
let sync_service = SyncService::builder(inner_client)
.with_cross_process_lock(app_identifier)
.with_cross_process_lock()
.build()
.await
.expect("Failed to create sync service");