Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UTD hook: stop re-reporting of UTDs on app relaunch #3519

Merged
merged 13 commits into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ futures-executor = "0.3.21"
futures-util = { version = "0.3.26", default-features = false, features = [
"alloc",
] }
growable-bloom-filter = "2.1.0"
http = "1.1.0"
imbl = "2.0.0"
itertools = "0.12.0"
Expand Down
35 changes: 27 additions & 8 deletions bindings/matrix-sdk-ffi/src/sync_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use matrix_sdk_ui::{
UnableToDecryptHook, UnableToDecryptInfo as SdkUnableToDecryptInfo, UtdHookManager,
},
};
use tracing::error;

use crate::{
error::ClientError, helpers::unwrap_or_clone_arc, room_list::RoomListService, TaskHandle,
Expand Down Expand Up @@ -93,14 +94,19 @@ impl SyncService {

#[derive(Clone, uniffi::Object)]
pub struct SyncServiceBuilder {
client: Client,
builder: MatrixSyncServiceBuilder,

utd_hook: Option<Arc<UtdHookManager>>,
}

impl SyncServiceBuilder {
pub(crate) fn new(client: Client) -> Arc<Self> {
Arc::new(Self { builder: MatrixSyncService::builder(client), utd_hook: None })
Arc::new(Self {
client: client.clone(),
builder: MatrixSyncService::builder(client),
utd_hook: None,
})
}
}

Expand All @@ -109,20 +115,33 @@ impl SyncServiceBuilder {
pub fn with_cross_process_lock(self: Arc<Self>, app_identifier: Option<String>) -> Arc<Self> {
let this = unwrap_or_clone_arc(self);
let builder = this.builder.with_cross_process_lock(app_identifier);
Arc::new(Self { builder, utd_hook: this.utd_hook })
Arc::new(Self { client: this.client, builder, utd_hook: this.utd_hook })
}

pub fn with_utd_hook(self: Arc<Self>, delegate: Box<dyn UnableToDecryptDelegate>) -> Arc<Self> {
pub async fn with_utd_hook(
self: Arc<Self>,
delegate: Box<dyn UnableToDecryptDelegate>,
) -> Arc<Self> {
// UTDs detected before this duration may be reclassified as "late decryption"
// events (or discarded, if they get decrypted fast enough).
const UTD_HOOK_GRACE_PERIOD: Duration = Duration::from_secs(60);

let this = unwrap_or_clone_arc(self);
let utd_hook = Some(Arc::new(
UtdHookManager::new(Arc::new(UtdHook { delegate }))
.with_max_delay(UTD_HOOK_GRACE_PERIOD),
));
Arc::new(Self { builder: this.builder, utd_hook })

let mut utd_hook = UtdHookManager::new(Arc::new(UtdHook { delegate }), this.client.clone())
.with_max_delay(UTD_HOOK_GRACE_PERIOD);

if let Err(e) = utd_hook.reload_from_store().await {
error!("Unable to reload UTD hook data from data store: {}", e);
// Carry on with the setup anyway; we shouldn't fail setup just
// because the UTD hook failed to load its data.
}

Arc::new(Self {
client: this.client,
builder: this.builder,
utd_hook: Some(Arc::new(utd_hook)),
})
}

pub async fn finish(self: Arc<Self>) -> Result<Arc<SyncService>, ClientError> {
Expand Down
1 change: 1 addition & 0 deletions crates/matrix-sdk-base/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ bitflags = { version = "2.4.0", features = ["serde"] }
eyeball = { workspace = true }
eyeball-im = { workspace = true }
futures-util = { workspace = true }
growable-bloom-filter = { workspace = true }
http = { workspace = true, optional = true }
matrix-sdk-common = { workspace = true }
matrix-sdk-crypto = { workspace = true, optional = true }
Expand Down
40 changes: 40 additions & 0 deletions crates/matrix-sdk-base/src/store/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::collections::{BTreeMap, BTreeSet};
use assert_matches::assert_matches;
use assert_matches2::assert_let;
use async_trait::async_trait;
use growable_bloom_filter::GrowableBloomBuilder;
use matrix_sdk_test::test_json;
use ruma::{
api::client::media::get_content_thumbnail::v3::Method,
Expand Down Expand Up @@ -62,6 +63,8 @@ pub trait StateStoreIntegrationTests {
async fn test_user_avatar_url_saving(&self);
/// Test sync token saving.
async fn test_sync_token_saving(&self);
/// Test UtdHookManagerData saving.
async fn test_utd_hook_manager_data_saving(&self);
/// Test stripped room member saving.
async fn test_stripped_member_saving(&self);
/// Test room power levels saving.
Expand Down Expand Up @@ -594,6 +597,37 @@ impl StateStoreIntegrationTests for DynStateStore {
assert_matches!(self.get_kv_data(StateStoreDataKey::SyncToken).await, Ok(None));
}

async fn test_utd_hook_manager_data_saving(&self) {
// Before any data is written, the getter should return None.
assert!(
self.get_kv_data(StateStoreDataKey::UtdHookManagerData)
.await
.expect("Could not read data")
.is_none(),
"Store was not empty at start"
);

// Put some data in the store...
let data = GrowableBloomBuilder::new().build();
self.set_kv_data(
StateStoreDataKey::UtdHookManagerData,
StateStoreDataValue::UtdHookManagerData(data.clone()),
)
.await
.expect("Could not save data");

// ... and check it comes back.
let read_data = self
.get_kv_data(StateStoreDataKey::UtdHookManagerData)
.await
.expect("Could not read data")
.expect("no data found")
.into_utd_hook_manager_data()
.expect("not UtdHookManagerData");

assert_eq!(read_data, data);
}

async fn test_stripped_member_saving(&self) {
let room_id = room_id!("!test_stripped_member_saving:localhost");
let user_id = user_id();
Expand Down Expand Up @@ -1351,6 +1385,12 @@ macro_rules! statestore_integration_tests {
store.test_sync_token_saving().await
}

#[async_test]
async fn test_utd_hook_manager_data_saving() {
let store = get_store().await.expect("creating store failed").into_state_store();
store.test_utd_hook_manager_data_saving().await;
}

#[async_test]
async fn test_stripped_member_saving() {
let store = get_store().await.unwrap().into_state_store();
Expand Down
19 changes: 19 additions & 0 deletions crates/matrix-sdk-base/src/store/memory_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use std::{
};

use async_trait::async_trait;
use growable_bloom_filter::GrowableBloom;
use matrix_sdk_common::{instant::Instant, ring_buffer::RingBuffer};
use ruma::{
canonical_json::{redact, RedactedBecause},
Expand Down Expand Up @@ -52,6 +53,7 @@ pub struct MemoryStore {
user_avatar_url: StdRwLock<HashMap<String, String>>,
sync_token: StdRwLock<Option<String>>,
filters: StdRwLock<HashMap<String, String>>,
utd_hook_manager_data: StdRwLock<Option<GrowableBloom>>,
account_data: StdRwLock<HashMap<GlobalAccountDataEventType, Raw<AnyGlobalAccountDataEvent>>>,
profiles: StdRwLock<HashMap<OwnedRoomId, HashMap<OwnedUserId, MinimalRoomMemberEvent>>>,
display_names: StdRwLock<HashMap<OwnedRoomId, HashMap<String, BTreeSet<OwnedUserId>>>>,
Expand Down Expand Up @@ -94,6 +96,7 @@ impl Default for MemoryStore {
user_avatar_url: Default::default(),
sync_token: Default::default(),
filters: Default::default(),
utd_hook_manager_data: Default::default(),
account_data: Default::default(),
profiles: Default::default(),
display_names: Default::default(),
Expand Down Expand Up @@ -186,6 +189,12 @@ impl StateStore for MemoryStore {
.get(user_id.as_str())
.cloned()
.map(StateStoreDataValue::RecentlyVisitedRooms),
StateStoreDataKey::UtdHookManagerData => self
.utd_hook_manager_data
.read()
.unwrap()
.clone()
.map(StateStoreDataValue::UtdHookManagerData),
})
}

Expand Down Expand Up @@ -219,6 +228,13 @@ impl StateStore for MemoryStore {
.expect("Session data not a list of recently visited rooms"),
);
}
StateStoreDataKey::UtdHookManagerData => {
*self.utd_hook_manager_data.write().unwrap() = Some(
value
.into_utd_hook_manager_data()
.expect("Session data not the hook manager data"),
);
}
}

Ok(())
Expand All @@ -236,6 +252,9 @@ impl StateStore for MemoryStore {
StateStoreDataKey::RecentlyVisitedRooms(user_id) => {
self.recently_visited_rooms.write().unwrap().remove(user_id.as_str());
}
StateStoreDataKey::UtdHookManagerData => {
*self.utd_hook_manager_data.write().unwrap() = None
}
}
Ok(())
}
Expand Down
18 changes: 18 additions & 0 deletions crates/matrix-sdk-base/src/store/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use std::{

use as_variant::as_variant;
use async_trait::async_trait;
use growable_bloom_filter::GrowableBloom;
use matrix_sdk_common::AsyncTraitDeps;
use ruma::{
events::{
Expand Down Expand Up @@ -808,6 +809,10 @@ pub enum StateStoreDataValue {

/// A list of recently visited room identifiers for the current user
RecentlyVisitedRooms(Vec<String>),

/// Persistent data for
/// `matrix_sdk_ui::unable_to_decrypt_hook::UtdHookManager`.
UtdHookManagerData(GrowableBloom),
}

impl StateStoreDataValue {
Expand All @@ -830,6 +835,11 @@ impl StateStoreDataValue {
pub fn into_recently_visited_rooms(self) -> Option<Vec<String>> {
as_variant!(self, Self::RecentlyVisitedRooms)
}

/// Get this value if it is the data for the `UtdHookManager`.
pub fn into_utd_hook_manager_data(self) -> Option<GrowableBloom> {
as_variant!(self, Self::UtdHookManagerData)
}
}

/// A key for key-value data.
Expand All @@ -846,6 +856,10 @@ pub enum StateStoreDataKey<'a> {

/// Recently visited room identifiers
RecentlyVisitedRooms(&'a UserId),

/// Persistent data for
/// `matrix_sdk_ui::unable_to_decrypt_hook::UtdHookManager`.
UtdHookManagerData,
}

impl StateStoreDataKey<'_> {
Expand All @@ -860,4 +874,8 @@ impl StateStoreDataKey<'_> {
/// Key prefix to use for the
/// [`RecentlyVisitedRooms`][Self::RecentlyVisitedRooms] variant.
pub const RECENTLY_VISITED_ROOMS: &'static str = "recently_visited_rooms";

/// Key to use for the [`UtdHookManagerData`][Self::UtdHookManagerData]
/// variant.
pub const UTD_HOOK_MANAGER_DATA: &'static str = "utd_hook_manager_data";
}
3 changes: 2 additions & 1 deletion crates/matrix-sdk-indexeddb/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ rustdoc-args = ["--cfg", "docsrs"]

[features]
default = ["e2e-encryption", "state-store"]
state-store = ["dep:matrix-sdk-base"]
state-store = ["dep:matrix-sdk-base", "growable-bloom-filter"]
e2e-encryption = ["dep:matrix-sdk-crypto"]
testing = ["matrix-sdk-crypto?/testing"]

Expand All @@ -24,6 +24,7 @@ anyhow = { workspace = true }
async-trait = { workspace = true }
base64 = { workspace = true }
gloo-utils = { version = "0.2.0", features = ["serde"] }
growable-bloom-filter = { workspace = true, optional = true }
indexed_db_futures = "0.4.1"
js-sys = { version = "0.3.58" }
matrix-sdk-base = { workspace = true, features = ["js"], optional = true }
Expand Down
11 changes: 11 additions & 0 deletions crates/matrix-sdk-indexeddb/src/state_store/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use std::{
use anyhow::anyhow;
use async_trait::async_trait;
use gloo_utils::format::JsValueSerdeExt;
use growable_bloom_filter::GrowableBloom;
use indexed_db_futures::prelude::*;
use matrix_sdk_base::{
deserialized_responses::RawAnySyncOrStrippedState,
Expand Down Expand Up @@ -388,6 +389,9 @@ impl IndexeddbStateStore {
StateStoreDataKey::RecentlyVisitedRooms(user_id) => {
self.encode_key(keys::KV, (StateStoreDataKey::RECENTLY_VISITED_ROOMS, user_id))
}
StateStoreDataKey::UtdHookManagerData => {
self.encode_key(keys::KV, StateStoreDataKey::UTD_HOOK_MANAGER_DATA)
}
}
}
}
Expand Down Expand Up @@ -449,6 +453,10 @@ impl_state_store!({
.map(|f| self.deserialize_event::<Vec<String>>(&f))
.transpose()?
.map(StateStoreDataValue::RecentlyVisitedRooms),
StateStoreDataKey::UtdHookManagerData => value
.map(|f| self.deserialize_event::<GrowableBloom>(&f))
.transpose()?
.map(StateStoreDataValue::UtdHookManagerData),
};

Ok(value)
Expand All @@ -475,6 +483,9 @@ impl_state_store!({
.into_recently_visited_rooms()
.expect("Session data not a recently visited room list"),
),
StateStoreDataKey::UtdHookManagerData => self.serialize_event(
&value.into_utd_hook_manager_data().expect("Session data not UtdHookManagerData"),
),
};

let tx =
Expand Down
Loading
Loading