From 3a183b4c22c75fccc3e214544adc6d96a777cae1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 31 Jan 2023 19:34:17 +0100 Subject: [PATCH 01/13] chore(sdk): Tweak the instrumentation for some encryption related methods --- crates/matrix-sdk/src/encryption/mod.rs | 1 - crates/matrix-sdk/src/room/joined.rs | 5 +++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/encryption/mod.rs b/crates/matrix-sdk/src/encryption/mod.rs index 139fc1a66..0f0755379 100644 --- a/crates/matrix-sdk/src/encryption/mod.rs +++ b/crates/matrix-sdk/src/encryption/mod.rs @@ -270,7 +270,6 @@ impl Client { /// /// * `users` - The list of user/device pairs that we should claim keys for. #[cfg(feature = "e2e-encryption")] - #[instrument(skip_all)] pub(crate) async fn claim_one_time_keys( &self, users: impl Iterator, diff --git a/crates/matrix-sdk/src/room/joined.rs b/crates/matrix-sdk/src/room/joined.rs index 30376a716..827c3d841 100644 --- a/crates/matrix-sdk/src/room/joined.rs +++ b/crates/matrix-sdk/src/room/joined.rs @@ -328,8 +328,9 @@ impl Joined { /// /// Does nothing if no room key needs to be shared. #[cfg(feature = "e2e-encryption")] + #[instrument(skip_all, fields(room_id = ?self.room_id()))] async fn preshare_room_key(&self) -> Result<()> { - // TODO expose this publicly so people can pre-share a group session if + // TODO: expose this publicly so people can pre-share a group session if // e.g. a user starts to type a message for a room. if let Some(mutex) = self.client.inner.group_session_locks.get(self.inner.room_id()).map(|m| m.clone()) @@ -379,8 +380,8 @@ impl Joined { /// # Panics /// /// Panics if the client isn't logged in. - #[instrument] #[cfg(feature = "e2e-encryption")] + #[instrument(skip_all)] async fn share_room_key(&self) -> Result<()> { let requests = self.client.base_client().share_room_key(self.inner.room_id()).await?; From e9cef35f99190bd9dae9c44ac959a021a1a1ccdb Mon Sep 17 00:00:00 2001 From: Anderas Date: Wed, 1 Feb 2023 14:06:59 +0000 Subject: [PATCH 02/13] Add matrix-sdk-sqlite with a CryptoStore implementation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note about "Write-Ahead Log" (WAL) mode: The SQLite WAL mode has a bunch of advantages that are quite nice to have: 1. WAL is significantly faster in most scenarios. 2. WAL provides more concurrency as readers do not block writers and a writer does not block readers. Reading and writing can proceed concurrently. 3. Disk I/O operations tends to be more sequential using WAL. 4. WAL uses many fewer fsync() operations and is thus less vulnerable to problems on systems where the fsync() system call is broken. The downsides of WAL mode don't really affect us. So let's turn it on. More info: https://www.sqlite.org/wal.html Co-authored-by: Jonas Platte Co-authored-by: Damir Jelić --- Cargo.lock | 124 +- bindings/matrix-sdk-crypto-ffi/Cargo.toml | 6 +- bindings/matrix-sdk-crypto-ffi/src/lib.rs | 37 +- bindings/matrix-sdk-crypto-ffi/src/machine.rs | 24 +- crates/matrix-sdk-crypto/src/gossiping/mod.rs | 2 +- .../src/session_manager/group_sessions.rs | 2 +- .../src/store/integration_tests.rs | 4 +- .../src/store/memorystore.rs | 5 +- crates/matrix-sdk-crypto/src/store/mod.rs | 4 +- .../matrix-sdk-indexeddb/src/crypto_store.rs | 2 +- crates/matrix-sdk-sled/src/crypto_store.rs | 2 +- crates/matrix-sdk-sqlite/Cargo.toml | 55 + .../matrix-sdk-sqlite/migrations/001_init.sql | 56 + crates/matrix-sdk-sqlite/src/crypto_store.rs | 1045 +++++++++++++++++ crates/matrix-sdk-sqlite/src/lib.rs | 125 ++ crates/matrix-sdk-sqlite/src/utils.rs | 114 ++ xtask/src/ci.rs | 3 + 17 files changed, 1577 insertions(+), 33 deletions(-) create mode 100644 crates/matrix-sdk-sqlite/Cargo.toml create mode 100644 crates/matrix-sdk-sqlite/migrations/001_init.sql create mode 100644 crates/matrix-sdk-sqlite/src/crypto_store.rs create mode 100644 crates/matrix-sdk-sqlite/src/lib.rs create mode 100644 crates/matrix-sdk-sqlite/src/utils.rs diff --git a/Cargo.lock b/Cargo.lock index b91ff234b..7946958e0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1196,6 +1196,29 @@ name = "deadpool-runtime" version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eaa37046cc0f6c3cc6090fbdbf73ef0b8ef4cfcc37f6befc0020f63e8cf121e1" +dependencies = [ + "tokio", +] + +[[package]] +name = "deadpool-sqlite" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e026821eaacbce25ff0d54405e4421d71656fcae3e4a9323461280fcda6dbc7d" +dependencies = [ + "deadpool", + "deadpool-sync", + "rusqlite", +] + +[[package]] +name = "deadpool-sync" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b1bea344b64b32537fde6e0f0179b1ede34d435636719dd40fe6a0f28218a61c" +dependencies = [ + "deadpool", +] [[package]] name = "debugid" @@ -1603,6 +1626,18 @@ dependencies = [ "once_cell", ] +[[package]] +name = "fallible-iterator" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4443176a9f2c162692bd3d352d745ef9413eec5782a80d8fd6f8a1ac692a07f7" + +[[package]] +name = "fallible-streaming-iterator" +version = "0.1.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7360491ce676a36bf9bb3c56c1aa791658183a54d2744120f27285738d90465a" + [[package]] name = "fancy-regex" version = "0.7.1" @@ -1957,6 +1992,15 @@ dependencies = [ "ahash 0.7.6", ] +[[package]] +name = "hashlink" +version = "0.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "69fe1fcf8b4278d860ad0548329f892a3631fb63f82574df68275f34cdbe0ffa" +dependencies = [ + "hashbrown", +] + [[package]] name = "heck" version = "0.4.0" @@ -2471,6 +2515,17 @@ dependencies = [ "winapi", ] +[[package]] +name = "libsqlite3-sys" +version = "0.25.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "29f835d03d717946d28b1d1ed632eb6f0e24a299388ee623d0c23118d3e8a7fa" +dependencies = [ + "cc", + "pkg-config", + "vcpkg", +] + [[package]] name = "link-cplusplus" version = "1.0.8" @@ -2767,7 +2822,7 @@ dependencies = [ "js_int", "matrix-sdk-common", "matrix-sdk-crypto", - "matrix-sdk-sled", + "matrix-sdk-sqlite", "pbkdf2", "rand 0.8.5", "ruma", @@ -2938,6 +2993,37 @@ dependencies = [ "tracing", ] +[[package]] +name = "matrix-sdk-sqlite" +version = "0.1.0" +dependencies = [ + "async-stream", + "async-trait", + "ctor", + "dashmap", + "deadpool-sqlite", + "fs_extra", + "futures-core", + "futures-util", + "glob", + "matrix-sdk-base", + "matrix-sdk-common", + "matrix-sdk-crypto", + "matrix-sdk-store-encryption", + "matrix-sdk-test", + "once_cell", + "rmp-serde", + "ruma", + "rusqlite", + "serde", + "serde_json", + "tempfile", + "thiserror", + "tokio", + "tracing", + "tracing-subscriber", +] + [[package]] name = "matrix-sdk-store-encryption" version = "0.2.0" @@ -3970,6 +4056,28 @@ dependencies = [ "winapi", ] +[[package]] +name = "rmp" +version = "0.8.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44519172358fd6d58656c86ab8e7fbc9e1490c3e8f14d35ed78ca0dd07403c9f" +dependencies = [ + "byteorder", + "num-traits", + "paste", +] + +[[package]] +name = "rmp-serde" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c5b13be192e0220b8afb7222aa5813cb62cc269ebb5cac346ca6487681d2913e" +dependencies = [ + "byteorder", + "rmp", + "serde", +] + [[package]] name = "ruma" version = "0.7.4" @@ -4078,6 +4186,20 @@ dependencies = [ "toml", ] +[[package]] +name = "rusqlite" +version = "0.28.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "01e213bc3ecb39ac32e81e51ebe31fd888a940515173e3a18a35f8c6e896422a" +dependencies = [ + "bitflags", + "fallible-iterator", + "fallible-streaming-iterator", + "hashlink", + "libsqlite3-sys", + "smallvec", +] + [[package]] name = "rustc-demangle" version = "0.1.21" diff --git a/bindings/matrix-sdk-crypto-ffi/Cargo.toml b/bindings/matrix-sdk-crypto-ffi/Cargo.toml index e8b71ab27..eeccbf84e 100644 --- a/bindings/matrix-sdk-crypto-ffi/Cargo.toml +++ b/bindings/matrix-sdk-crypto-ffi/Cargo.toml @@ -46,9 +46,9 @@ path = "../../crates/matrix-sdk-crypto" version = "0.6.0" features = ["qrcode", "backups_v1"] -[dependencies.matrix-sdk-sled] -path = "../../crates/matrix-sdk-sled" -version = "0.2.0" +[dependencies.matrix-sdk-sqlite] +path = "../../crates/matrix-sdk-sqlite" +version = "0.1.0" default_features = false features = ["crypto-store"] diff --git a/bindings/matrix-sdk-crypto-ffi/src/lib.rs b/bindings/matrix-sdk-crypto-ffi/src/lib.rs index a300ff99f..4527025f3 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/lib.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/lib.rs @@ -34,6 +34,7 @@ use matrix_sdk_crypto::{ types::{EventEncryptionAlgorithm as RustEventEncryptionAlgorithm, SigningKey}, EncryptionSettings as RustEncryptionSettings, LocalTrust, }; +use matrix_sdk_sqlite::SqliteCryptoStore; pub use responses::{ BootstrapCrossSigningResult, DeviceLists, KeysImportResult, OutgoingVerificationRequest, Request, RequestType, SignatureUploadRequest, UploadSigningKeysRequest, @@ -164,6 +165,20 @@ impl From for MigrationError { /// * `progress_listener` - A callback that can be used to introspect the /// progress of the migration. pub fn migrate( + data: MigrationData, + path: &str, + passphrase: Option, + progress_listener: Box, +) -> anyhow::Result<()> { + use tokio::runtime::Runtime; + let runtime = Runtime::new()?; + runtime.block_on(async move { + migrate_data(data, path, passphrase, progress_listener).await?; + Ok(()) + }) +} + +async fn migrate_data( mut data: MigrationData, path: &str, passphrase: Option, @@ -173,8 +188,6 @@ pub fn migrate( olm::PrivateCrossSigningIdentity, store::{Changes as RustChanges, CryptoStore, RecoveryKey}, }; - use matrix_sdk_sled::SledCryptoStore; - use tokio::runtime::Runtime; use vodozemac::{ megolm::InboundGroupSession, olm::{Account, Session}, @@ -196,8 +209,7 @@ pub fn migrate( progress_listener.on_progress(progress as i32, total as i32) }; - let runtime = Runtime::new()?; - let store = runtime.block_on(SledCryptoStore::open(path, passphrase.as_deref()))?; + let store = SqliteCryptoStore::open(path, passphrase.as_deref()).await?; processed_steps += 1; listener(processed_steps, total_steps); @@ -294,11 +306,13 @@ pub fn migrate( data.backup_recovery_key.map(|k| RecoveryKey::from_base58(k.as_str())).transpose()?; let cross_signing = PrivateCrossSigningIdentity::empty((*user_id).into()); - runtime.block_on(cross_signing.import_secrets_unchecked( - data.cross_signing.master_key.as_deref(), - data.cross_signing.self_signing_key.as_deref(), - data.cross_signing.user_signing_key.as_deref(), - ))?; + cross_signing + .import_secrets_unchecked( + data.cross_signing.master_key.as_deref(), + data.cross_signing.self_signing_key.as_deref(), + data.cross_signing.user_signing_key.as_deref(), + ) + .await?; data.cross_signing.master_key.zeroize(); data.cross_signing.self_signing_key.zeroize(); @@ -314,8 +328,7 @@ pub fn migrate( .collect::>()?; let tracked_users: Vec<_> = tracked_users.iter().map(|(u, d)| (&**u, *d)).collect(); - - runtime.block_on(store.save_tracked_users(tracked_users.as_slice()))?; + store.save_tracked_users(tracked_users.as_slice()).await?; processed_steps += 1; listener(processed_steps, total_steps); @@ -329,7 +342,7 @@ pub fn migrate( backup_version: data.backup_version, ..Default::default() }; - runtime.block_on(store.save_changes(changes))?; + store.save_changes(changes).await?; processed_steps += 1; listener(processed_steps, total_steps); diff --git a/bindings/matrix-sdk-crypto-ffi/src/machine.rs b/bindings/matrix-sdk-crypto-ffi/src/machine.rs index d0d17e9bd..97c72c173 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/machine.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/machine.rs @@ -1,6 +1,7 @@ use std::{ collections::{BTreeMap, HashMap}, io::Cursor, + mem::ManuallyDrop, ops::Deref, sync::Arc, time::Duration, @@ -59,10 +60,23 @@ use crate::{ /// A high level state machine that handles E2EE for Matrix. pub struct OlmMachine { - pub(crate) inner: InnerMachine, + pub(crate) inner: ManuallyDrop, pub(crate) runtime: Runtime, } +impl Drop for OlmMachine { + fn drop(&mut self) { + // SAFETY: self.inner is never used again, which is the only requirement + // for ManuallyDrop::take to be used safely. + let inner = unsafe { ManuallyDrop::take(&mut self.inner) }; + let _guard = self.runtime.enter(); + // Dropping the inner OlmMachine must happen within a tokio context + // because deadpool drops sqlite connections in the DB pool on tokio's + // blocking threadpool to avoid blocking async worker threads. + drop(inner); + } +} + /// A pair of outgoing room key requests, both of those are sendToDevice /// requests. pub struct KeyRequestPair { @@ -160,14 +174,14 @@ impl OlmMachine { let runtime = Runtime::new().expect("Couldn't create a tokio runtime"); let store = runtime - .block_on(matrix_sdk_sled::SledCryptoStore::open(path, passphrase.as_deref())) + .block_on(matrix_sdk_sqlite::SqliteCryptoStore::open(path, passphrase.as_deref())) .map_err(|e| match e { // This is a bit of an error in the sled store, the // CryptoStore returns an `OpenStoreError` which has a // variant for the state store. Not sure what to do about // this. - matrix_sdk_sled::OpenStoreError::Crypto(r) => r.into(), - matrix_sdk_sled::OpenStoreError::Sled(s) => CryptoStoreError::CryptoStore( + matrix_sdk_sqlite::OpenStoreError::Crypto(r) => r.into(), + matrix_sdk_sqlite::OpenStoreError::Sqlite(s) => CryptoStoreError::CryptoStore( matrix_sdk_crypto::store::CryptoStoreError::backend(s), ), _ => unreachable!(), @@ -178,7 +192,7 @@ impl OlmMachine { let inner = runtime.block_on(InnerMachine::with_store(&user_id, device_id, Arc::new(store)))?; - Ok(OlmMachine { inner, runtime }) + Ok(OlmMachine { inner: ManuallyDrop::new(inner), runtime }) } /// Get the display name of our own device. diff --git a/crates/matrix-sdk-crypto/src/gossiping/mod.rs b/crates/matrix-sdk-crypto/src/gossiping/mod.rs index cc52991b6..f0487d371 100644 --- a/crates/matrix-sdk-crypto/src/gossiping/mod.rs +++ b/crates/matrix-sdk-crypto/src/gossiping/mod.rs @@ -77,7 +77,7 @@ pub struct GossipRequest { } /// An enum over the various secret request types we can have. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub enum SecretInfo { /// Info for the `m.room_key_request` variant KeyRequest(SupportedKeyInfo), diff --git a/crates/matrix-sdk-crypto/src/session_manager/group_sessions.rs b/crates/matrix-sdk-crypto/src/session_manager/group_sessions.rs index e4e47b72d..af8783a67 100644 --- a/crates/matrix-sdk-crypto/src/session_manager/group_sessions.rs +++ b/crates/matrix-sdk-crypto/src/session_manager/group_sessions.rs @@ -68,7 +68,7 @@ impl GroupSessionCache { // and put it in the cache. if let Some(s) = self.sessions.get(room_id) { Ok(Some(s.clone())) - } else if let Some(s) = self.store.get_outbound_group_sessions(room_id).await? { + } else if let Some(s) = self.store.get_outbound_group_session(room_id).await? { for request_id in s.pending_request_ids() { self.sessions_being_shared.insert(request_id, s.clone()); } diff --git a/crates/matrix-sdk-crypto/src/store/integration_tests.rs b/crates/matrix-sdk-crypto/src/store/integration_tests.rs index f940ff496..b80414ff2 100644 --- a/crates/matrix-sdk-crypto/src/store/integration_tests.rs +++ b/crates/matrix-sdk-crypto/src/store/integration_tests.rs @@ -202,7 +202,7 @@ macro_rules! cryptostore_integration_tests { let dir = "load_outbound_group_session"; let (account, store) = get_loaded_store(dir.clone()).await; let room_id = room_id!("!test:localhost"); - assert!(store.get_outbound_group_sessions(&room_id).await.unwrap().is_none()); + assert!(store.get_outbound_group_session(&room_id).await.unwrap().is_none()); let (session, _) = account.create_group_session_pair_with_defaults(&room_id).await; @@ -219,7 +219,7 @@ macro_rules! cryptostore_integration_tests { store.load_account().await.unwrap(); - assert!(store.get_outbound_group_sessions(&room_id).await.unwrap().is_some()); + assert!(store.get_outbound_group_session(&room_id).await.unwrap().is_some()); } #[async_test] diff --git a/crates/matrix-sdk-crypto/src/store/memorystore.rs b/crates/matrix-sdk-crypto/src/store/memorystore.rs index ab326a6aa..e7d026115 100644 --- a/crates/matrix-sdk-crypto/src/store/memorystore.rs +++ b/crates/matrix-sdk-crypto/src/store/memorystore.rs @@ -188,10 +188,7 @@ impl CryptoStore for MemoryStore { Ok(()) } - async fn get_outbound_group_sessions( - &self, - _: &RoomId, - ) -> Result> { + async fn get_outbound_group_session(&self, _: &RoomId) -> Result> { Ok(None) } diff --git a/crates/matrix-sdk-crypto/src/store/mod.rs b/crates/matrix-sdk-crypto/src/store/mod.rs index 1127b5dc6..7a6aae2ab 100644 --- a/crates/matrix-sdk-crypto/src/store/mod.rs +++ b/crates/matrix-sdk-crypto/src/store/mod.rs @@ -839,9 +839,9 @@ pub trait CryptoStore: AsyncTraitDeps { /// Get the backup keys we have stored. async fn load_backup_keys(&self) -> Result; - /// Get the outbound group sessions we have stored that is used for the + /// Get the outbound group session we have stored that is used for the /// given room. - async fn get_outbound_group_sessions( + async fn get_outbound_group_session( &self, room_id: &RoomId, ) -> Result>; diff --git a/crates/matrix-sdk-indexeddb/src/crypto_store.rs b/crates/matrix-sdk-indexeddb/src/crypto_store.rs index 3e8860d6f..567180414 100644 --- a/crates/matrix-sdk-indexeddb/src/crypto_store.rs +++ b/crates/matrix-sdk-indexeddb/src/crypto_store.rs @@ -963,7 +963,7 @@ impl CryptoStore for IndexeddbCryptoStore { self.get_inbound_group_sessions().await.map_err(|e| e.into()) } - async fn get_outbound_group_sessions( + async fn get_outbound_group_session( &self, room_id: &RoomId, ) -> Result, CryptoStoreError> { diff --git a/crates/matrix-sdk-sled/src/crypto_store.rs b/crates/matrix-sdk-sled/src/crypto_store.rs index fe2294d5e..b386db638 100644 --- a/crates/matrix-sdk-sled/src/crypto_store.rs +++ b/crates/matrix-sdk-sled/src/crypto_store.rs @@ -844,7 +844,7 @@ impl CryptoStore for SledCryptoStore { self.reset_backup_state().await } - async fn get_outbound_group_sessions( + async fn get_outbound_group_session( &self, room_id: &RoomId, ) -> Result> { diff --git a/crates/matrix-sdk-sqlite/Cargo.toml b/crates/matrix-sdk-sqlite/Cargo.toml new file mode 100644 index 000000000..15a7c80b9 --- /dev/null +++ b/crates/matrix-sdk-sqlite/Cargo.toml @@ -0,0 +1,55 @@ +[package] +name = "matrix-sdk-sqlite" +version = "0.1.0" +edition = "2021" +repository = "https://github.com/matrix-org/matrix-rust-sdk" +description = "Sqlite storage backend for matrix-sdk" +license = "Apache-2.0" +rust-version = { workspace = true } + +[features] +default = [] + +crypto-store = [ + "dep:matrix-sdk-base", + "dep:matrix-sdk-crypto", + "matrix-sdk-base?/e2e-encryption", +] + +[dependencies] +async-stream = { workspace = true } +async-trait = { workspace = true } +dashmap = { workspace = true } +deadpool-sqlite = "0.5.0" +fs_extra = "1.2.0" +futures-core = "0.3.21" +futures-util = { version = "0.3.21", default-features = false } +matrix-sdk-base = { version = "0.6.0", path = "../matrix-sdk-base", optional = true } +matrix-sdk-common = { version = "0.6.0", path = "../matrix-sdk-common" } +matrix-sdk-crypto = { version = "0.6.0", path = "../matrix-sdk-crypto", optional = true } +matrix-sdk-store-encryption = { version = "0.2.0", path = "../matrix-sdk-store-encryption" } +rmp-serde = "1.1.1" +ruma = { workspace = true } +rusqlite = { version = "0.28.0", features = ["bundled"] } +serde = { workspace = true } +serde_json = { workspace = true } +thiserror = { workspace = true } +tokio = { version = "1.23.1", default-features = false, features = [ + "sync", + "fs", +] } +tracing = { workspace = true } + +[dev-dependencies] +ctor = { workspace = true } +glob = "0.3.0" +matrix-sdk-base = { path = "../matrix-sdk-base", features = ["testing"] } +matrix-sdk-crypto = { path = "../matrix-sdk-crypto", features = ["testing"] } +matrix-sdk-test = { path = "../../testing/matrix-sdk-test" } +once_cell = { workspace = true } +tempfile = "3.3.0" +tokio = { version = "1.23.1", default-features = false, features = [ + "rt-multi-thread", + "macros", +] } +tracing-subscriber = { version = "0.3.16", features = ["env-filter"] } diff --git a/crates/matrix-sdk-sqlite/migrations/001_init.sql b/crates/matrix-sdk-sqlite/migrations/001_init.sql new file mode 100644 index 000000000..0c1966e9c --- /dev/null +++ b/crates/matrix-sdk-sqlite/migrations/001_init.sql @@ -0,0 +1,56 @@ +CREATE TABLE "kv" ( + "key" TEXT PRIMARY KEY NOT NULL, + "value" BLOB NOT NULL +); + +CREATE TABLE "session" ( + "session_id" BLOB PRIMARY KEY NOT NULL, + "sender_key" BLOB NOT NULL, + "data" BLOB NOT NULL +); +CREATE INDEX "session_sender_key_idx" + ON "session" ("sender_key"); + +CREATE TABLE "inbound_group_session" ( + "session_id" BLOB PRIMARY KEY NOT NULL, + "room_id" BLOB NOT NULL, + "backed_up" INTEGER NOT NULL, + "data" BLOB NOT NULL +); +CREATE INDEX "inbound_group_session_room_id_idx" + ON "inbound_group_session" ("room_id"); + +CREATE TABLE "outbound_group_session" ( + "room_id" BLOB PRIMARY KEY NOT NULL, + "data" BLOB NOT NULL +); + +CREATE TABLE "device" ( + "user_id" BLOB NOT NULL, + "device_id" BLOB NOT NULL, + "data" BLOB NOT NULL, + + PRIMARY KEY ("user_id", "device_id") +); +CREATE INDEX "device_user_id" + ON "device" ("user_id"); + +CREATE TABLE "identity" ( + "user_id" BLOB PRIMARY KEY NOT NULL, + "data" BLOB NOT NULL +); + +CREATE TABLE "tracked_user" ( + "user_id" BLOB PRIMARY KEY NOT NULL, + "data" BLOB NOT NULL +); + +CREATE TABLE "olm_hash" ( + "data" BLOB PRIMARY KEY NOT NULL +); + +CREATE TABLE "key_requests" ( + "request_id" BLOB PRIMARY KEY NOT NULL, + "sent_out" INTEGER NOT NULL, + "data" BLOB NOT NULL +); diff --git a/crates/matrix-sdk-sqlite/src/crypto_store.rs b/crates/matrix-sdk-sqlite/src/crypto_store.rs new file mode 100644 index 000000000..897286ec8 --- /dev/null +++ b/crates/matrix-sdk-sqlite/src/crypto_store.rs @@ -0,0 +1,1045 @@ +// Copyright 2022 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::{ + collections::HashMap, + path::{Path, PathBuf}, + sync::{Arc, RwLock}, +}; + +use async_trait::async_trait; +use deadpool_sqlite::{Object as SqliteConn, Pool as SqlitePool, Runtime}; +use matrix_sdk_common::locks::Mutex; +use matrix_sdk_crypto::{ + olm::{ + IdentityKeys, InboundGroupSession, OutboundGroupSession, PickledInboundGroupSession, + PrivateCrossSigningIdentity, Session, + }, + store::{ + caches::SessionStore, BackupKeys, Changes, CryptoStore, CryptoStoreError, + Result as StoreResult, RoomKeyCounts, + }, + GossipRequest, ReadOnlyAccount, ReadOnlyDevice, ReadOnlyUserIdentities, SecretInfo, + TrackedUser, +}; +use matrix_sdk_store_encryption::StoreCipher; +use ruma::{DeviceId, OwnedDeviceId, RoomId, TransactionId, UserId}; +use rusqlite::OptionalExtension; +use serde::{de::DeserializeOwned, Serialize}; +use tokio::fs; +use tracing::{debug, error, instrument, warn}; + +use crate::{ + get_or_create_store_cipher, + utils::{Key, SqliteObjectExt}, + OpenStoreError, SqliteConnectionExt as _, SqliteObjectStoreExt, +}; + +#[derive(Clone, Debug)] +pub struct AccountInfo { + user_id: Arc, + device_id: Arc, + identity_keys: Arc, +} + +#[derive(Debug)] +enum Error { + Crypto(CryptoStoreError), + Sqlite(rusqlite::Error), + Pool(deadpool_sqlite::PoolError), +} + +impl From for Error { + fn from(value: CryptoStoreError) -> Self { + Self::Crypto(value) + } +} + +impl From for Error { + fn from(value: rusqlite::Error) -> Self { + Self::Sqlite(value) + } +} + +impl From for Error { + fn from(value: deadpool_sqlite::PoolError) -> Self { + Self::Pool(value) + } +} + +impl From for CryptoStoreError { + fn from(value: Error) -> Self { + match value { + Error::Crypto(c) => c, + Error::Sqlite(b) => CryptoStoreError::backend(b), + Error::Pool(b) => CryptoStoreError::backend(b), + } + } +} + +type Result = std::result::Result; + +/// A sqlite based cryptostore. +#[derive(Clone)] +pub struct SqliteCryptoStore { + store_cipher: Option>, + path: Option, + pool: SqlitePool, + + // DB values cached in memory + account_info: Arc>>, + session_cache: SessionStore, +} + +impl std::fmt::Debug for SqliteCryptoStore { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + if let Some(path) = &self.path { + f.debug_struct("SledCryptoStore").field("path", &path).finish() + } else { + f.debug_struct("SledCryptoStore").field("path", &"memory store").finish() + } + } +} + +impl SqliteCryptoStore { + /// Open the sqlite-based crypto store at the given path using the given + /// passphrase to encrypt private data. + pub async fn open( + path: impl AsRef, + passphrase: Option<&str>, + ) -> Result { + let path = path.as_ref(); + fs::create_dir_all(path).await.map_err(CryptoStoreError::from)?; + let cfg = deadpool_sqlite::Config::new(path.join("matrix-sdk-crypto.sqlite3")); + let pool = cfg.create_pool(Runtime::Tokio1)?; + + Self::open_with_pool(pool, passphrase).await + } + + /// Create a sqlite-based crypto store using the given sqlite database pool. + /// The given passphrase will be used to encrypt private data. + pub async fn open_with_pool( + pool: SqlitePool, + passphrase: Option<&str>, + ) -> Result { + let conn = pool.get().await.map_err(CryptoStoreError::backend)?; + run_migrations(&conn).await?; + let store_cipher = match passphrase { + Some(p) => Some(Arc::new(get_or_create_store_cipher(p, &conn).await?)), + None => None, + }; + + Ok(SqliteCryptoStore { + store_cipher, + path: None, + pool, + account_info: Arc::new(RwLock::new(None)), + session_cache: SessionStore::new(), + }) + } + + fn serialize_value(&self, value: &impl Serialize) -> Result, CryptoStoreError> { + let serialized = rmp_serde::to_vec_named(value).map_err(CryptoStoreError::backend)?; + + if let Some(key) = &self.store_cipher { + let encrypted = + key.encrypt_value_data(serialized).map_err(CryptoStoreError::backend)?; + rmp_serde::to_vec_named(&encrypted).map_err(CryptoStoreError::backend) + } else { + Ok(serialized) + } + } + + fn deserialize_value(&self, value: &[u8]) -> Result { + if let Some(key) = &self.store_cipher { + let encrypted = rmp_serde::from_slice(value).map_err(CryptoStoreError::backend)?; + let decrypted = key.decrypt_value_data(encrypted).map_err(CryptoStoreError::backend)?; + + rmp_serde::from_slice(&decrypted).map_err(CryptoStoreError::backend) + } else { + rmp_serde::from_slice(value).map_err(CryptoStoreError::backend) + } + } + + fn deserialize_pickled_inbound_group_session( + &self, + value: &[u8], + backed_up: bool, + ) -> Result { + let mut pickle: PickledInboundGroupSession = self.deserialize_value(value)?; + // backed_up SQL column is source of truth, backed_up field in pickle + // needed for other stores though + pickle.backed_up = backed_up; + Ok(pickle) + } + + fn deserialize_key_request( + &self, + value: &[u8], + sent_out: bool, + ) -> Result { + let mut request: GossipRequest = self.deserialize_value(value)?; + // sent_out SQL column is source of truth, sent_out field in serialized value + // needed for other stores though + request.sent_out = sent_out; + Ok(request) + } + + fn encode_key(&self, table_name: &str, key: impl AsRef<[u8]>) -> Key { + let bytes = key.as_ref(); + if let Some(store_cipher) = &self.store_cipher { + Key::Hashed(store_cipher.hash_key(table_name, bytes)) + } else { + Key::Plain(bytes.to_owned()) + } + } + + fn get_account_info(&self) -> Option { + self.account_info.read().unwrap().clone() + } + + async fn acquire(&self) -> Result { + Ok(self.pool.get().await?) + } + + async fn load_tracked_users(&self) -> Result> { + self.acquire() + .await? + .get_tracked_users() + .await? + .iter() + .map(|value| Ok(self.deserialize_value(value)?)) + .collect() + } + + async fn save_tracked_users( + &self, + tracked_users: &[(&UserId, bool)], + ) -> Result<(), CryptoStoreError> { + let users: Vec<(Key, Vec)> = tracked_users + .iter() + .map(|(u, d)| { + let user_id = self.encode_key("tracked_users", u.as_bytes()); + let data = + self.serialize_value(&TrackedUser { user_id: (*u).into(), dirty: *d })?; + Ok((user_id, data)) + }) + .collect::>()?; + + Ok(self.acquire().await?.add_tracked_users(users).await?) + } +} + +const DATABASE_VERSION: u8 = 1; + +async fn run_migrations(conn: &SqliteConn) -> Result<(), CryptoStoreError> { + let kv_exists = conn + .query_row( + "SELECT count(*) FROM sqlite_master WHERE type = 'table' AND name = 'kv'", + (), + |row| row.get::<_, u32>(0), + ) + .await + .map_err(CryptoStoreError::backend)? + > 0; + + let version = if kv_exists { + match conn.get_kv("version").await?.as_deref() { + Some([v]) => *v, + Some(_) => { + error!("version database field has multiple bytes"); + return Ok(()); + } + None => { + error!("version database field is missing"); + return Ok(()); + } + } + } else { + 0 + }; + + if version == 0 { + debug!("Creating database"); + } else if version < DATABASE_VERSION { + debug!(version, new_version = DATABASE_VERSION, "Upgrading database"); + } + + if version < 1 { + // First turn on WAL mode, this can't be done in the transaction, it fails with + // the error message: "cannot change into wal mode from within a transaction". + conn.execute_batch("PRAGMA journal_mode = wal;") + .await + .map_err(CryptoStoreError::backend)?; + conn.with_transaction(|txn| txn.execute_batch(include_str!("../migrations/001_init.sql"))) + .await + .map_err(CryptoStoreError::backend)?; + } + + conn.set_kv("version", vec![DATABASE_VERSION]).await.map_err(CryptoStoreError::backend)?; + + Ok(()) +} + +trait SqliteConnectionExt { + fn set_session( + &self, + session_id: &[u8], + sender_key: &[u8], + data: &[u8], + ) -> rusqlite::Result<()>; + + fn set_inbound_group_session( + &self, + room_id: &[u8], + session_id: &[u8], + data: &[u8], + backed_up: bool, + ) -> rusqlite::Result<()>; + + fn set_outbound_group_session(&self, room_id: &[u8], data: &[u8]) -> rusqlite::Result<()>; + + fn set_device(&self, user_id: &[u8], device_id: &[u8], data: &[u8]) -> rusqlite::Result<()>; + fn delete_device(&self, user_id: &[u8], device_id: &[u8]) -> rusqlite::Result<()>; + + fn set_identity(&self, user_id: &[u8], data: &[u8]) -> rusqlite::Result<()>; + + fn add_olm_hash(&self, data: &[u8]) -> rusqlite::Result<()>; + + fn set_key_request( + &self, + request_id: &[u8], + sent_out: bool, + data: &[u8], + ) -> rusqlite::Result<()>; +} + +impl SqliteConnectionExt for rusqlite::Connection { + fn set_session( + &self, + session_id: &[u8], + sender_key: &[u8], + data: &[u8], + ) -> rusqlite::Result<()> { + self.execute( + "INSERT INTO session (session_id, sender_key, data) + VALUES (?1, ?2, ?3) + ON CONFLICT (session_id) DO UPDATE SET data = ?3", + (session_id, sender_key, data), + )?; + Ok(()) + } + + fn set_inbound_group_session( + &self, + room_id: &[u8], + session_id: &[u8], + data: &[u8], + backed_up: bool, + ) -> rusqlite::Result<()> { + self.execute( + "INSERT INTO inbound_group_session (session_id, room_id, data, backed_up) \ + VALUES (?1, ?2, ?3, ?4) + ON CONFLICT (session_id) DO UPDATE SET data = ?3, backed_up = ?4", + (session_id, room_id, data, backed_up), + )?; + Ok(()) + } + + fn set_outbound_group_session(&self, room_id: &[u8], data: &[u8]) -> rusqlite::Result<()> { + self.execute( + "INSERT INTO outbound_group_session (room_id, data) \ + VALUES (?1, ?2) + ON CONFLICT (room_id) DO UPDATE SET data = ?2", + (room_id, data), + )?; + Ok(()) + } + + fn set_device(&self, user_id: &[u8], device_id: &[u8], data: &[u8]) -> rusqlite::Result<()> { + self.execute( + "INSERT INTO device (user_id, device_id, data) \ + VALUES (?1, ?2, ?3) + ON CONFLICT (user_id, device_id) DO UPDATE SET data = ?3", + (user_id, device_id, data), + )?; + Ok(()) + } + + fn delete_device(&self, user_id: &[u8], device_id: &[u8]) -> rusqlite::Result<()> { + self.execute( + "DELETE FROM device WHERE user_id = ? AND device_id = ?", + (user_id, device_id), + )?; + Ok(()) + } + + fn set_identity(&self, user_id: &[u8], data: &[u8]) -> rusqlite::Result<()> { + self.execute( + "INSERT INTO identity (user_id, data) \ + VALUES (?1, ?2) + ON CONFLICT (user_id) DO UPDATE SET data = ?2", + (user_id, data), + )?; + Ok(()) + } + + fn add_olm_hash(&self, data: &[u8]) -> rusqlite::Result<()> { + self.execute("INSERT INTO olm_hash (data) VALUES (?) ON CONFLICT DO NOTHING", (data,))?; + Ok(()) + } + + fn set_key_request( + &self, + request_id: &[u8], + sent_out: bool, + data: &[u8], + ) -> rusqlite::Result<()> { + self.execute( + "INSERT INTO key_requests (request_id, sent_out, data) + VALUES (?1, ?2, ?3) + ON CONFLICT (request_id) DO UPDATE SET sent_out = ?2, data = ?3", + (request_id, sent_out, data), + )?; + Ok(()) + } +} + +#[async_trait] +trait SqliteObjectCryptoStoreExt: SqliteObjectExt { + async fn get_sessions_for_sender_key(&self, sender_key: Key) -> Result>> { + Ok(self + .prepare("SELECT data FROM session WHERE sender_key = ?", |mut stmt| { + stmt.query((sender_key,))?.mapped(|row| row.get(0)).collect() + }) + .await?) + } + + async fn get_inbound_group_session( + &self, + session_id: Key, + ) -> Result, Vec)>> { + Ok(self + .query_row( + "SELECT room_id, data FROM inbound_group_session WHERE session_id = ?", + (session_id,), + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .await + .optional()?) + } + + async fn get_inbound_group_sessions(&self) -> Result, bool)>> { + Ok(self + .prepare("SELECT data, backed_up FROM inbound_group_session", |mut stmt| { + stmt.query(())?.mapped(|row| Ok((row.get(0)?, row.get(1)?))).collect() + }) + .await?) + } + + async fn get_inbound_group_session_counts(&self) -> Result { + let total = self + .query_row("SELECT count(*) FROM inbound_group_session", (), |row| row.get(0)) + .await?; + let backed_up = self + .query_row( + "SELECT count(*) FROM inbound_group_session WHERE backed_up = TRUE", + (), + |row| row.get(0), + ) + .await?; + Ok(RoomKeyCounts { total, backed_up }) + } + + async fn get_inbound_group_sessions_for_backup(&self, limit: usize) -> Result>> { + Ok(self + .prepare( + "SELECT data FROM inbound_group_session WHERE backed_up = FALSE LIMIT ?", + move |mut stmt| stmt.query((limit,))?.mapped(|row| row.get(0)).collect(), + ) + .await?) + } + + async fn reset_inbound_group_session_backup_state(&self) -> Result<()> { + self.execute("UPDATE inbound_group_session SET backed_up = FALSE", ()).await?; + Ok(()) + } + + async fn get_outbound_group_session(&self, room_id: Key) -> Result>> { + Ok(self + .query_row( + "SELECT data FROM outbound_group_session WHERE room_id = ?", + (room_id,), + |row| row.get(0), + ) + .await + .optional()?) + } + + async fn get_device(&self, user_id: Key, device_id: Key) -> Result>> { + Ok(self + .query_row( + "SELECT data FROM device WHERE user_id = ? AND device_id = ?", + (user_id, device_id), + |row| row.get(0), + ) + .await + .optional()?) + } + + async fn get_user_devices(&self, user_id: Key) -> Result>> { + Ok(self + .prepare("SELECT data FROM device WHERE user_id = ?", |mut stmt| { + stmt.query((user_id,))?.mapped(|row| row.get(0)).collect() + }) + .await?) + } + + async fn get_user_identity(&self, user_id: Key) -> Result>> { + Ok(self + .query_row("SELECT data FROM identity WHERE user_id = ?", (user_id,), |row| row.get(0)) + .await + .optional()?) + } + + async fn has_olm_hash(&self, data: Vec) -> Result { + Ok(self + .query_row("SELECT count(*) FROM olm_hash WHERE data = ?", (data,), |row| { + row.get::<_, i32>(0) + }) + .await? + > 0) + } + + async fn get_tracked_users(&self) -> Result>> { + Ok(self + .prepare("SELECT data FROM tracked_user", |mut stmt| { + stmt.query(())?.mapped(|row| row.get(0)).collect() + }) + .await?) + } + + async fn add_tracked_users(&self, users: Vec<(Key, Vec)>) -> Result<()> { + Ok(self + .prepare( + "INSERT INTO tracked_user (user_id, data) \ + VALUES (?1, ?2) \ + ON CONFLICT (user_id) DO UPDATE SET data = ?2", + |mut stmt| { + for (user_id, data) in users { + stmt.execute((user_id, data))?; + } + + Ok(()) + }, + ) + .await?) + } + + async fn get_outgoing_secret_request( + &self, + request_id: Key, + ) -> Result, bool)>> { + Ok(self + .query_row( + "SELECT data, sent_out FROM key_requests WHERE request_id = ?", + (request_id,), + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .await + .optional()?) + } + + async fn get_outgoing_secret_requests(&self) -> Result, bool)>> { + Ok(self + .prepare("SELECT data, sent_out FROM key_requests", |mut stmt| { + stmt.query(())?.mapped(|row| Ok((row.get(0)?, row.get(1)?))).collect() + }) + .await?) + } + + async fn get_unsent_secret_requests(&self) -> Result>> { + Ok(self + .prepare("SELECT data FROM key_requests WHERE sent_out = FALSE", |mut stmt| { + stmt.query(())?.mapped(|row| row.get(0)).collect() + }) + .await?) + } + + async fn delete_key_request(&self, request_id: Key) -> Result<()> { + self.execute("DELETE FROM key_requests WHERE request_id = ?", (request_id,)).await?; + Ok(()) + } +} + +#[async_trait] +impl SqliteObjectCryptoStoreExt for deadpool_sqlite::Object {} + +#[async_trait] +impl CryptoStore for SqliteCryptoStore { + async fn load_account(&self) -> StoreResult> { + let conn = self.acquire().await?; + if let Some(pickle) = conn.get_kv("account").await? { + let pickle = self.deserialize_value(&pickle)?; + + let account = ReadOnlyAccount::from_pickle(pickle)?; + + let account_info = AccountInfo { + user_id: account.user_id.clone(), + device_id: account.device_id.clone(), + identity_keys: account.identity_keys.clone(), + }; + + *self.account_info.write().unwrap() = Some(account_info); + + Ok(Some(account)) + } else { + Ok(None) + } + } + + async fn save_account(&self, account: ReadOnlyAccount) -> StoreResult<()> { + let account_info = AccountInfo { + user_id: account.user_id.clone(), + device_id: account.device_id.clone(), + identity_keys: account.identity_keys.clone(), + }; + *self.account_info.write().unwrap() = Some(account_info); + + let pickled_account = account.pickle().await; + let serialized_account = self.serialize_value(&pickled_account)?; + self.acquire().await?.set_kv("account", serialized_account).await?; + Ok(()) + } + + async fn load_identity(&self) -> StoreResult> { + let conn = self.acquire().await?; + if let Some(i) = conn.get_kv("identity").await? { + let pickle = self.deserialize_value(&i)?; + Ok(Some( + PrivateCrossSigningIdentity::from_pickle(pickle) + .await + .map_err(|_| CryptoStoreError::UnpicklingError)?, + )) + } else { + Ok(None) + } + } + + async fn save_changes(&self, changes: Changes) -> StoreResult<()> { + let pickled_account = if let Some(account) = changes.account { + let account_info = AccountInfo { + user_id: account.user_id.clone(), + device_id: account.device_id.clone(), + identity_keys: account.identity_keys.clone(), + }; + + *self.account_info.write().unwrap() = Some(account_info); + Some(account.pickle().await) + } else { + None + }; + + let pickled_private_identity = + if let Some(i) = changes.private_identity { Some(i.pickle().await?) } else { None }; + + let mut session_changes = Vec::new(); + for session in changes.sessions { + let session_id = self.encode_key("session", session.session_id()); + let sender_key = self.encode_key("session", session.sender_key().to_base64()); + let pickle = session.pickle().await; + session_changes.push((session_id, sender_key, pickle)); + + self.session_cache.add(session).await; + } + + let mut inbound_session_changes = Vec::new(); + for session in changes.inbound_group_sessions { + let room_id = self.encode_key("inbound_group_session", session.room_id().as_bytes()); + let session_id = self.encode_key("inbound_group_session", session.session_id()); + let pickle = session.pickle().await; + inbound_session_changes.push((room_id, session_id, pickle)); + } + + let mut outbound_session_changes = Vec::new(); + for session in changes.outbound_group_sessions { + let room_id = self.encode_key("outbound_group_session", session.room_id().as_bytes()); + let pickle = session.pickle().await; + outbound_session_changes.push((room_id, pickle)); + } + + let this = self.clone(); + self.acquire() + .await? + .with_transaction(move |txn| { + if let Some(pickled_account) = pickled_account { + let serialized_account = this.serialize_value(&pickled_account)?; + txn.set_kv("account", &serialized_account)?; + } + + if let Some(pickled_private_identity) = &pickled_private_identity { + let serialized_private_identity = + this.serialize_value(pickled_private_identity)?; + txn.set_kv("identity", &serialized_private_identity)?; + } + + if let Some(recovery_key) = &changes.recovery_key { + let serialized_recovery_key = this.serialize_value(recovery_key)?; + txn.set_kv("recovery_key_v1", &serialized_recovery_key)?; + } + + if let Some(backup_version) = &changes.backup_version { + let serialized_backup_version = this.serialize_value(backup_version)?; + txn.set_kv("backup_version_v1", &serialized_backup_version)?; + } + + for device in changes.devices.new.iter().chain(&changes.devices.changed) { + let user_id = this.encode_key("device", device.user_id().as_bytes()); + let device_id = this.encode_key("device", device.device_id().as_bytes()); + let data = this.serialize_value(&device)?; + txn.set_device(&user_id, &device_id, &data)?; + } + + for device in &changes.devices.deleted { + let user_id = this.encode_key("device", device.user_id().as_bytes()); + let device_id = this.encode_key("device", device.device_id().as_bytes()); + txn.delete_device(&user_id, &device_id)?; + } + + for identity in changes.identities.changed.iter().chain(&changes.identities.new) { + let user_id = this.encode_key("identity", identity.user_id().as_bytes()); + let data = this.serialize_value(&identity)?; + txn.set_identity(&user_id, &data)?; + } + + for (session_id, sender_key, pickle) in &session_changes { + let serialized_session = this.serialize_value(&pickle)?; + txn.set_session(session_id, sender_key, &serialized_session)?; + } + + for (room_id, session_id, pickle) in &inbound_session_changes { + let serialized_session = this.serialize_value(&pickle)?; + txn.set_inbound_group_session( + room_id, + session_id, + &serialized_session, + pickle.backed_up, + )?; + } + + for (room_id, pickle) in &outbound_session_changes { + let serialized_session = this.serialize_value(&pickle)?; + txn.set_outbound_group_session(room_id, &serialized_session)?; + } + + for hash in &changes.message_hashes { + let hash = serde_json::to_vec(hash).map_err(CryptoStoreError::from)?; + txn.add_olm_hash(&hash)?; + } + + for request in changes.key_requests { + let request_id = this.encode_key("key_requests", request.request_id.as_bytes()); + let serialized_request = this.serialize_value(&request)?; + txn.set_key_request(&request_id, request.sent_out, &serialized_request)?; + } + + Ok::<_, Error>(()) + }) + .await?; + + Ok(()) + } + + async fn get_sessions( + &self, + sender_key: &str, + ) -> StoreResult>>>> { + let account_info = self.get_account_info().ok_or(CryptoStoreError::AccountUnset)?; + + if self.session_cache.get(sender_key).is_none() { + let sessions = self + .acquire() + .await? + .get_sessions_for_sender_key(self.encode_key("session", sender_key.as_bytes())) + .await? + .into_iter() + .map(|bytes| { + let pickle = self.deserialize_value(&bytes)?; + Ok(Session::from_pickle( + account_info.user_id.clone(), + account_info.device_id.clone(), + account_info.identity_keys.clone(), + pickle, + )) + }) + .collect::>()?; + + self.session_cache.set_for_sender(sender_key, sessions); + } + + Ok(self.session_cache.get(sender_key)) + } + + #[instrument(skip(self))] + async fn get_inbound_group_session( + &self, + room_id: &RoomId, + session_id: &str, + ) -> StoreResult> { + let session_id = self.encode_key("inbound_group_session", session_id); + let Some((room_id_from_db, value)) = + self.acquire().await?.get_inbound_group_session(session_id).await? + else { + return Ok(None); + }; + + let room_id = self.encode_key("inbound_group_session", room_id.as_bytes()); + if *room_id != room_id_from_db { + warn!("expected room_id for session_id doesn't match what's in the DB"); + return Ok(None); + } + + let pickle = self.deserialize_value(&value)?; + + Ok(Some(InboundGroupSession::from_pickle(pickle)?)) + } + + async fn get_inbound_group_sessions(&self) -> StoreResult> { + self.acquire() + .await? + .get_inbound_group_sessions() + .await? + .into_iter() + .map(|(value, backed_up)| { + let pickle = self.deserialize_pickled_inbound_group_session(&value, backed_up)?; + Ok(InboundGroupSession::from_pickle(pickle)?) + }) + .collect() + } + + async fn inbound_group_session_counts(&self) -> StoreResult { + Ok(self.acquire().await?.get_inbound_group_session_counts().await?) + } + + async fn inbound_group_sessions_for_backup( + &self, + limit: usize, + ) -> StoreResult> { + self.acquire() + .await? + .get_inbound_group_sessions_for_backup(limit) + .await? + .into_iter() + .map(|value| { + let pickle = self.deserialize_pickled_inbound_group_session(&value, false)?; + Ok(InboundGroupSession::from_pickle(pickle)?) + }) + .collect() + } + + async fn reset_backup_state(&self) -> StoreResult<()> { + Ok(self.acquire().await?.reset_inbound_group_session_backup_state().await?) + } + + async fn load_backup_keys(&self) -> StoreResult { + let conn = self.acquire().await?; + + let backup_version = conn + .get_kv("backup_version_v1") + .await? + .map(|value| self.deserialize_value(&value)) + .transpose()?; + + let recovery_key = conn + .get_kv("recovery_key_v1") + .await? + .map(|value| self.deserialize_value(&value)) + .transpose()?; + + Ok(BackupKeys { backup_version, recovery_key }) + } + + async fn get_outbound_group_session( + &self, + room_id: &RoomId, + ) -> StoreResult> { + let room_id = self.encode_key("outbound_group_session", room_id.as_bytes()); + let Some(value) = self.acquire().await?.get_outbound_group_session(room_id).await? else { + return Ok(None); + }; + + let account_info = self.get_account_info().ok_or(CryptoStoreError::AccountUnset)?; + + let pickle = self.deserialize_value(&value)?; + let session = OutboundGroupSession::from_pickle( + account_info.device_id, + account_info.identity_keys, + pickle, + )?; + + return Ok(Some(session)); + } + + async fn load_tracked_users(&self) -> StoreResult> { + Ok(self.load_tracked_users().await?) + } + + async fn save_tracked_users(&self, users: &[(&UserId, bool)]) -> StoreResult<()> { + self.save_tracked_users(users).await + } + + async fn get_device( + &self, + user_id: &UserId, + device_id: &DeviceId, + ) -> StoreResult> { + let user_id = self.encode_key("device", user_id.as_bytes()); + let device_id = self.encode_key("device", device_id.as_bytes()); + Ok(self + .acquire() + .await? + .get_device(user_id, device_id) + .await? + .map(|value| self.deserialize_value(&value)) + .transpose()?) + } + + async fn get_user_devices( + &self, + user_id: &UserId, + ) -> StoreResult> { + let user_id = self.encode_key("device", user_id.as_bytes()); + self.acquire() + .await? + .get_user_devices(user_id) + .await? + .into_iter() + .map(|value| { + let device: ReadOnlyDevice = self.deserialize_value(&value)?; + Ok((device.device_id().to_owned(), device)) + }) + .collect() + } + + async fn get_user_identity( + &self, + user_id: &UserId, + ) -> StoreResult> { + let user_id = self.encode_key("identity", user_id.as_bytes()); + Ok(self + .acquire() + .await? + .get_user_identity(user_id) + .await? + .map(|value| self.deserialize_value(&value)) + .transpose()?) + } + + async fn is_message_known( + &self, + message_hash: &matrix_sdk_crypto::olm::OlmMessageHash, + ) -> StoreResult { + let value = serde_json::to_vec(message_hash)?; + Ok(self.acquire().await?.has_olm_hash(value).await?) + } + + async fn get_outgoing_secret_requests( + &self, + request_id: &TransactionId, + ) -> StoreResult> { + let request_id = self.encode_key("key_requests", request_id.as_bytes()); + Ok(self + .acquire() + .await? + .get_outgoing_secret_request(request_id) + .await? + .map(|(value, sent_out)| self.deserialize_key_request(&value, sent_out)) + .transpose()?) + } + + async fn get_secret_request_by_info( + &self, + key_info: &SecretInfo, + ) -> StoreResult> { + let requests = self.acquire().await?.get_outgoing_secret_requests().await?; + for (request, sent_out) in requests { + let request = self.deserialize_key_request(&request, sent_out)?; + if request.info == *key_info { + return Ok(Some(request)); + } + } + Ok(None) + } + + async fn get_unsent_secret_requests(&self) -> StoreResult> { + self.acquire() + .await? + .get_unsent_secret_requests() + .await? + .iter() + .map(|value| { + let request = self.deserialize_key_request(value, false)?; + Ok(request) + }) + .collect() + } + + async fn delete_outgoing_secret_requests(&self, request_id: &TransactionId) -> StoreResult<()> { + let request_id = self.encode_key("key_requests", request_id.as_bytes()); + Ok(self.acquire().await?.delete_key_request(request_id).await?) + } +} + +#[cfg(test)] +mod tests { + use matrix_sdk_crypto::cryptostore_integration_tests; + use once_cell::sync::Lazy; + use tempfile::{tempdir, TempDir}; + + use super::SqliteCryptoStore; + + static TMP_DIR: Lazy = Lazy::new(|| tempdir().unwrap()); + + async fn get_store(name: &str, passphrase: Option<&str>) -> SqliteCryptoStore { + let tmpdir_path = TMP_DIR.path().join(name); + + SqliteCryptoStore::open(tmpdir_path.to_str().unwrap(), passphrase) + .await + .expect("Can't create a passphrase protected store") + } + + cryptostore_integration_tests!(); +} + +#[cfg(test)] +mod encrypted_tests { + use matrix_sdk_crypto::cryptostore_integration_tests; + use once_cell::sync::Lazy; + use tempfile::{tempdir, TempDir}; + + use super::SqliteCryptoStore; + + static TMP_DIR: Lazy = Lazy::new(|| tempdir().unwrap()); + + async fn get_store(name: &str, passphrase: Option<&str>) -> SqliteCryptoStore { + let tmpdir_path = TMP_DIR.path().join(name); + let pass = passphrase.unwrap_or("default_test_password"); + + SqliteCryptoStore::open(tmpdir_path.to_str().unwrap(), Some(pass)) + .await + .expect("Can't create a passphrase protected store") + } + + cryptostore_integration_tests!(); +} diff --git a/crates/matrix-sdk-sqlite/src/lib.rs b/crates/matrix-sdk-sqlite/src/lib.rs new file mode 100644 index 000000000..97858e232 --- /dev/null +++ b/crates/matrix-sdk-sqlite/src/lib.rs @@ -0,0 +1,125 @@ +// Copyright 2022 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#[cfg(feature = "crypto-store")] +use async_trait::async_trait; +use deadpool_sqlite::CreatePoolError; +#[cfg(feature = "crypto-store")] +use deadpool_sqlite::Object as SqliteConn; +#[cfg(feature = "crypto-store")] +use matrix_sdk_crypto::{store::Result, CryptoStoreError}; +#[cfg(feature = "crypto-store")] +use matrix_sdk_store_encryption::StoreCipher; +#[cfg(feature = "crypto-store")] +use rusqlite::OptionalExtension; +use thiserror::Error; +use tracing::error; + +#[cfg(feature = "crypto-store")] +mod crypto_store; +#[cfg(feature = "crypto-store")] +mod utils; + +#[cfg(feature = "crypto-store")] +pub use self::crypto_store::SqliteCryptoStore; +#[cfg(feature = "crypto-store")] +use self::utils::SqliteObjectExt; + +/// All the errors that can occur when opening a sled store. +#[derive(Error, Debug)] +#[non_exhaustive] +pub enum OpenStoreError { + /// An error occurred with the crypto store implementation. + #[cfg(feature = "crypto-store")] + #[error(transparent)] + Crypto(#[from] CryptoStoreError), + + /// An error occurred with sqlite. + #[error(transparent)] + Sqlite(#[from] CreatePoolError), +} + +#[cfg(feature = "crypto-store")] +async fn get_or_create_store_cipher(passphrase: &str, conn: &SqliteConn) -> Result { + let encrypted_cipher = conn.get_kv("cipher").await?; + + let cipher = if let Some(encrypted) = encrypted_cipher { + StoreCipher::import(passphrase, &encrypted) + .map_err(|_| CryptoStoreError::UnpicklingError)? + } else { + let cipher = StoreCipher::new().map_err(CryptoStoreError::backend)?; + #[cfg(not(test))] + let export = cipher.export(passphrase); + #[cfg(test)] + let export = cipher._insecure_export_fast_for_testing(passphrase); + conn.set_kv("cipher", export.map_err(CryptoStoreError::backend)?).await?; + cipher + }; + + Ok(cipher) +} + +#[cfg(feature = "crypto-store")] +trait SqliteConnectionExt { + fn set_kv(&self, key: &str, value: &[u8]) -> rusqlite::Result<()>; +} + +#[cfg(feature = "crypto-store")] +impl SqliteConnectionExt for rusqlite::Connection { + fn set_kv(&self, key: &str, value: &[u8]) -> rusqlite::Result<()> { + self.execute( + "INSERT INTO kv VALUES (?1, ?2) ON CONFLICT (key) DO UPDATE SET value = ?2", + (key, value), + )?; + Ok(()) + } +} + +#[cfg(feature = "crypto-store")] +#[async_trait] +trait SqliteObjectStoreExt: SqliteObjectExt { + async fn get_kv(&self, key: &str) -> Result>> { + let key = key.to_owned(); + self.query_row("SELECT value FROM kv WHERE key = ?", (key,), |row| row.get(0)) + .await + .optional() + .map_err(CryptoStoreError::backend) + } + + async fn set_kv(&self, key: &str, value: Vec) -> Result<()>; +} + +#[cfg(feature = "crypto-store")] +#[async_trait] +impl SqliteObjectStoreExt for deadpool_sqlite::Object { + async fn set_kv(&self, key: &str, value: Vec) -> Result<()> { + let key = key.to_owned(); + self.interact(move |conn| conn.set_kv(&key, &value)) + .await + .unwrap() + .map_err(CryptoStoreError::backend)?; + + Ok(()) + } +} + +#[cfg(test)] +#[ctor::ctor] +fn init_logging() { + use tracing_subscriber::{layer::SubscriberExt, util::SubscriberInitExt}; + tracing_subscriber::registry() + .with(tracing_subscriber::EnvFilter::from_default_env()) + .with(tracing_subscriber::fmt::layer().with_test_writer()) + .init(); +} diff --git a/crates/matrix-sdk-sqlite/src/utils.rs b/crates/matrix-sdk-sqlite/src/utils.rs new file mode 100644 index 000000000..79ea904e3 --- /dev/null +++ b/crates/matrix-sdk-sqlite/src/utils.rs @@ -0,0 +1,114 @@ +// Copyright 2022 The Matrix.org Foundation C.I.C. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +use std::ops::Deref; + +use async_trait::async_trait; +use rusqlite::{Params, Row, Statement, Transaction}; + +#[derive(Debug)] +pub(crate) enum Key { + Plain(Vec), + Hashed([u8; 32]), +} + +impl Deref for Key { + type Target = [u8]; + + fn deref(&self) -> &Self::Target { + match self { + Key::Plain(slice) => slice, + Key::Hashed(bytes) => bytes, + } + } +} + +impl rusqlite::ToSql for Key { + fn to_sql(&self) -> rusqlite::Result> { + self.deref().to_sql() + } +} + +#[async_trait] +pub(crate) trait SqliteObjectExt { + async fn execute

(&self, sql: &'static str, params: P) -> rusqlite::Result + where + P: Params + Send + 'static; + + async fn execute_batch(&self, sql: &'static str) -> rusqlite::Result<()>; + + async fn prepare(&self, sql: &'static str, f: F) -> rusqlite::Result + where + T: Send + 'static, + F: FnOnce(Statement<'_>) -> rusqlite::Result + Send + 'static; + + async fn query_row(&self, sql: &'static str, params: P, f: F) -> rusqlite::Result + where + T: Send + 'static, + P: Params + Send + 'static, + F: FnOnce(&Row<'_>) -> rusqlite::Result + Send + 'static; + + async fn with_transaction(&self, f: F) -> Result + where + T: Send + 'static, + E: From + Send + 'static, + F: FnOnce(&Transaction<'_>) -> Result + Send + 'static; +} + +#[async_trait] +impl SqliteObjectExt for deadpool_sqlite::Object { + async fn execute

(&self, sql: &'static str, params: P) -> rusqlite::Result + where + P: Params + Send + 'static, + { + self.interact(move |conn| conn.execute(sql, params)).await.unwrap() + } + + async fn execute_batch(&self, sql: &'static str) -> rusqlite::Result<()> { + self.interact(move |conn| conn.execute_batch(sql)).await.unwrap() + } + + async fn prepare(&self, sql: &'static str, f: F) -> rusqlite::Result + where + T: Send + 'static, + F: FnOnce(Statement<'_>) -> rusqlite::Result + Send + 'static, + { + self.interact(move |conn| f(conn.prepare(sql)?)).await.unwrap() + } + + async fn query_row(&self, sql: &'static str, params: P, f: F) -> rusqlite::Result + where + T: Send + 'static, + P: Params + Send + 'static, + F: FnOnce(&Row<'_>) -> rusqlite::Result + Send + 'static, + { + self.interact(move |conn| conn.query_row(sql, params, f)).await.unwrap() + } + + async fn with_transaction(&self, f: F) -> Result + where + T: Send + 'static, + E: From + Send + 'static, + F: FnOnce(&Transaction<'_>) -> Result + Send + 'static, + { + self.interact(move |conn| { + let txn = conn.transaction()?; + let result = f(&txn)?; + txn.commit()?; + Ok(result) + }) + .await + .unwrap() + } +} diff --git a/xtask/src/ci.rs b/xtask/src/ci.rs index 098514e67..3a801ffe3 100644 --- a/xtask/src/ci.rs +++ b/xtask/src/ci.rs @@ -249,6 +249,9 @@ fn run_crypto_tests() -> Result<()> { cmd!("rustup run stable cargo nextest run -p matrix-sdk-crypto-ffi").run()?; + cmd!("rustup run stable cargo nextest run -p matrix-sdk-sqlite --features crypto-store") + .run()?; + Ok(()) } From 7b044ef5ddaaf223be4c1d4af8704ff861a3cac1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 1 Feb 2023 18:25:48 +0100 Subject: [PATCH 03/13] fix(crypto): Ignore the usage and signatures when comparing cross signing keys Co-authored-by: Denis Kasak --- .../src/identities/manager.rs | 34 +++-- .../matrix-sdk-crypto/src/identities/user.rs | 140 ++++++++++++++++-- .../matrix-sdk-crypto/src/olm/signing/mod.rs | 2 +- .../src/types/cross_signing_key.rs | 2 +- 4 files changed, 149 insertions(+), 29 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/identities/manager.rs b/crates/matrix-sdk-crypto/src/identities/manager.rs index 061ab0f3f..bd9b7722b 100644 --- a/crates/matrix-sdk-crypto/src/identities/manager.rs +++ b/crates/matrix-sdk-crypto/src/identities/manager.rs @@ -756,10 +756,10 @@ pub(crate) mod testing { .expect("Can't parse the keys upload response") } - pub fn own_key_query() -> KeyQueryResponse { + pub fn own_key_query_with_user_id(user_id: &UserId) -> KeyQueryResponse { let data = response_from_file(&json!({ "device_keys": { - "@example:localhost": { + user_id: { "WSKKLTJZCL": { "algorithms": [ "m.olm.v1.curve25519-aes-sha2", @@ -771,12 +771,12 @@ pub(crate) mod testing { "ed25519:WSKKLTJZCL": "lQ+eshkhgKoo+qp9Qgnj3OX5PBoWMU5M9zbuEevwYqE" }, "signatures": { - "@example:localhost": { + user_id: { "ed25519:WSKKLTJZCL": "SKpIUnq7QK0xleav0PrIQyKjVm+TgZr7Yi8cKjLeZDtkgyToE2d4/e3Aj79dqOlLB92jFVE4d1cM/Ry04wFwCA", "ed25519:0C8lCBxrvrv/O7BQfsKnkYogHZX3zAgw3RfJuyiq210": "9UGu1iC5YhFCdELGfB29YaV+QE0t/X5UDSsPf4QcdZyXIwyp9zBbHX2lh9vWudNQ+akZpaq7ZRaaM+4TCnw/Ag" } }, - "user_id": "@example:localhost", + "user_id": user_id, "unsigned": { "device_display_name": "Cross signing capable" } @@ -792,11 +792,11 @@ pub(crate) mod testing { "ed25519:LVWOVGOXME": "k+NC3L7CBD6fBClcHBrKLOkqCyGNSKhWXiH5Q2STRnA" }, "signatures": { - "@example:localhost": { + user_id: { "ed25519:LVWOVGOXME": "39Ir5Bttpc5+bQwzLj7rkjm5E5/cp/JTbMJ/t0enj6J5w9MXVBFOUqqM2hpaRaRwILMMpwYbJ8IOGjl0Y/MGAw" } }, - "user_id": "@example:localhost", + "user_id": user_id, "unsigned": { "device_display_name": "Non-cross signing" } @@ -805,8 +805,8 @@ pub(crate) mod testing { }, "failures": {}, "master_keys": { - "@example:localhost": { - "user_id": "@example:localhost", + user_id: { + "user_id": user_id, "usage": [ "master" ], @@ -814,15 +814,15 @@ pub(crate) mod testing { "ed25519:rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0": "rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0" }, "signatures": { - "@example:localhost": { + user_id: { "ed25519:WSKKLTJZCL": "ZzJp1wtmRdykXAUEItEjNiFlBrxx8L6/Vaen9am8AuGwlxxJtOkuY4m+4MPLvDPOgavKHLsrRuNLAfCeakMlCQ" } } } }, "self_signing_keys": { - "@example:localhost": { - "user_id": "@example:localhost", + user_id: { + "user_id": user_id, "usage": [ "self_signing" ], @@ -830,15 +830,15 @@ pub(crate) mod testing { "ed25519:0C8lCBxrvrv/O7BQfsKnkYogHZX3zAgw3RfJuyiq210": "0C8lCBxrvrv/O7BQfsKnkYogHZX3zAgw3RfJuyiq210" }, "signatures": { - "@example:localhost": { + user_id: { "ed25519:rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0": "AC7oDUW4rUhtInwb4lAoBJ0wAuu4a5k+8e34B5+NKsDB8HXRwgVwUWN/MRWc/sJgtSbVlhzqS9THEmQQ1C51Bw" } } } }, "user_signing_keys": { - "@example:localhost": { - "user_id": "@example:localhost", + user_id: { + "user_id": user_id, "usage": [ "user_signing" ], @@ -846,7 +846,7 @@ pub(crate) mod testing { "ed25519:DU9z4gBFKFKCk7a13sW9wjT0Iyg7Hqv5f0BPM7DEhPo": "DU9z4gBFKFKCk7a13sW9wjT0Iyg7Hqv5f0BPM7DEhPo" }, "signatures": { - "@example:localhost": { + user_id: { "ed25519:rJ2TAGkEOP6dX41Ksll6cl8K3J48l8s/59zaXyvl2p0": "C4L2sx9frGqj8w41KyynHGqwUbbwBYRZpYCB+6QWnvQFA5Oi/1PJj8w5anwzEsoO0TWmLYmf7FXuAGewanOWDg" } } @@ -857,6 +857,10 @@ pub(crate) mod testing { .expect("Can't parse the keys upload response") } + pub fn own_key_query() -> KeyQueryResponse { + own_key_query_with_user_id(user_id()) + } + pub fn key_query( identity: UploadSigningKeysRequest, device_keys: DeviceKeys, diff --git a/crates/matrix-sdk-crypto/src/identities/user.rs b/crates/matrix-sdk-crypto/src/identities/user.rs index e573af6f5..e64c219d8 100644 --- a/crates/matrix-sdk-crypto/src/identities/user.rs +++ b/crates/matrix-sdk-crypto/src/identities/user.rs @@ -277,21 +277,49 @@ impl UserIdentity { /// /// Master keys are used to sign other cross signing keys, the self signing and /// user signing keys of an user will be signed by their master key. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] #[serde(try_from = "CrossSigningKey")] pub struct MasterPubkey(Arc); +macro_rules! impl_partial_eq { + ($key_type: ty) => { + impl PartialEq for $key_type { + /// The `PartialEq` implementation compares the user ID, the usage and the + /// key material, ignoring signatures. + /// + /// The usage could be safely ignored since the type guarantees it has the + /// correct usage by construction -- it is impossible to construct a + /// value of a particular key type with an incorrect usage. However, we + /// check it anyway, to codify the notion that the same key material + /// with a different usage results in a logically different key. + /// + /// The signatures are provided by other devices and don't alter the + /// identity of the key itself. + fn eq(&self, other: &Self) -> bool { + self.user_id() == other.user_id() + && self.keys() == other.keys() + && self.usage() == other.usage() + } + } + impl Eq for $key_type {} + }; +} + +impl_partial_eq!(MasterPubkey); +impl_partial_eq!(SelfSigningPubkey); +impl_partial_eq!(UserSigningPubkey); + /// Wrapper for a cross signing key marking it as a self signing key. /// /// Self signing keys are used to sign the user's own devices. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] #[serde(try_from = "CrossSigningKey")] pub struct SelfSigningPubkey(Arc); /// Wrapper for a cross signing key marking it as a user signing key. /// /// User signing keys are used to sign the master keys of other users. -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize)] #[serde(try_from = "CrossSigningKey")] pub struct UserSigningPubkey(Arc); @@ -299,7 +327,7 @@ impl TryFrom for MasterPubkey { type Error = serde_json::Error; fn try_from(key: CrossSigningKey) -> Result { - if key.usage.contains(&KeyUsage::Master) { + if key.usage.contains(&KeyUsage::Master) && key.usage.len() == 1 { Ok(Self(key.into())) } else { Err(serde::de::Error::custom(format!( @@ -314,7 +342,7 @@ impl TryFrom for SelfSigningPubkey { type Error = serde_json::Error; fn try_from(key: CrossSigningKey) -> Result { - if key.usage.contains(&KeyUsage::SelfSigning) { + if key.usage.contains(&KeyUsage::SelfSigning) && key.usage.len() == 1 { Ok(Self(key.into())) } else { Err(serde::de::Error::custom(format!( @@ -329,7 +357,7 @@ impl TryFrom for UserSigningPubkey { type Error = serde_json::Error; fn try_from(key: CrossSigningKey) -> Result { - if key.usage.contains(&KeyUsage::UserSigning) { + if key.usage.contains(&KeyUsage::UserSigning) && key.usage.len() == 1 { Ok(Self(key.into())) } else { Err(serde::de::Error::custom(format!( @@ -945,6 +973,8 @@ pub(crate) mod testing { use ruma::{api::client::keys::get_keys::v3::Response as KeyQueryResponse, user_id}; use super::{ReadOnlyOwnUserIdentity, ReadOnlyUserIdentity}; + #[cfg(test)] + use crate::{identities::manager::testing::other_user_id, olm::PrivateCrossSigningIdentity}; use crate::{ identities::{ manager::testing::{other_key_query, own_key_query}, @@ -987,6 +1017,13 @@ pub(crate) mod testing { own_identity(&own_key_query()) } + /// Generate default other "own" identity for tests + #[cfg(test)] + pub async fn get_other_own_identity() -> ReadOnlyOwnUserIdentity { + let private_identity = PrivateCrossSigningIdentity::new(other_user_id().into()).await; + ReadOnlyOwnUserIdentity::from_private(&private_identity).await + } + /// Generate default other identify for tests pub fn get_other_identity() -> ReadOnlyUserIdentity { let user_id = user_id!("@example2:localhost"); @@ -1009,8 +1046,9 @@ pub(crate) mod tests { use assert_matches::assert_matches; use matrix_sdk_common::locks::Mutex; use matrix_sdk_test::async_test; - use ruma::user_id; + use ruma::{encryption::KeyUsage, user_id, DeviceKeyId}; use serde_json::{json, Value}; + use vodozemac::Ed25519Signature; use super::{ testing::{device, get_other_identity, get_own_identity}, @@ -1018,8 +1056,9 @@ pub(crate) mod tests { }; use crate::{ identities::{ - manager::testing::own_key_query, Device, MasterPubkey, SelfSigningPubkey, - UserSigningPubkey, + manager::testing::{own_key_query, own_key_query_with_user_id}, + user::testing::get_other_own_identity, + Device, MasterPubkey, SelfSigningPubkey, UserSigningPubkey, }, olm::{PrivateCrossSigningIdentity, ReadOnlyAccount}, store::MemoryStore, @@ -1153,8 +1192,85 @@ pub(crate) mod tests { // It should now be impossible to deserialize the keys into their corresponding // high-level cross-signing key structs. - assert_matches!(serde_json::from_value::(master_key_json), Err(_)); - assert_matches!(serde_json::from_value::(self_signing_key_json), Err(_)); - assert_matches!(serde_json::from_value::(user_signing_key_json), Err(_)); + assert_matches!(serde_json::from_value::(master_key_json.clone()), Err(_)); + assert_matches!( + serde_json::from_value::(self_signing_key_json.clone()), + Err(_) + ); + assert_matches!( + serde_json::from_value::(user_signing_key_json.clone()), + Err(_) + ); + + // Add additional usages. + let usage = master_key_json.get_mut("usage").unwrap(); + *usage = json!(["master", "user_signing"]); + let usage = self_signing_key_json.get_mut("usage").unwrap(); + *usage = json!(["self_signing", "user_signing"]); + let usage = user_signing_key_json.get_mut("usage").unwrap(); + *usage = json!(["user_signing", "self_signing"]); + + // It should still be impossible to deserialize the keys into their + // corresponding high-level cross-signing key structs. + assert_matches!(serde_json::from_value::(master_key_json.clone()), Err(_)); + assert_matches!( + serde_json::from_value::(self_signing_key_json.clone()), + Err(_) + ); + assert_matches!( + serde_json::from_value::(user_signing_key_json.clone()), + Err(_) + ); + } + + #[async_test] + async fn partial_eq_cross_signing_keys() { + macro_rules! test_partial_eq { + ($key_type:ident, $key_field:ident, $field:ident, $usage:expr) => { + let user_id = user_id!("@example:localhost"); + let response = own_key_query(); + let raw = response.$field.get(user_id).unwrap(); + let key: $key_type = raw.deserialize_as().unwrap(); + + // A different key is naturally not the same as our key. + let other_identity = get_other_own_identity().await; + let other_key = other_identity.$key_field; + assert_ne!(key, other_key); + + // However, not even our own key material with another user ID is the same. + let other_user_id = user_id!("@example2:localhost"); + let other_response = own_key_query_with_user_id(&other_user_id); + let other_raw = other_response.$field.get(other_user_id).unwrap(); + let other_key: $key_type = other_raw.deserialize_as().unwrap(); + assert_ne!(key, other_key); + + // Now let's add another signature to our key. + let signature = Ed25519Signature::from_base64( + "mia28GKixFzOWKJ0h7Bdrdy2fjxiHCsst1qpe467FbW85H61UlshtKBoAXfTLlVfi0FX+/noJ8B3noQPnY+9Cg" + ).expect("The signature can always be decoded"); + let mut other_key: CrossSigningKey = raw.deserialize_as().unwrap(); + other_key.signatures.add_signature( + user_id.to_owned(), + DeviceKeyId::from_parts(ruma::DeviceKeyAlgorithm::Ed25519, "DEVICEID".into()), + signature, + ); + let other_key = other_key.try_into().unwrap(); + + // Additional signatures are fine, adding more does not change the key's identity. + assert_eq!(key, other_key); + + // However changing the usage results in a different key. + let mut other_key: CrossSigningKey = raw.deserialize_as().unwrap(); + other_key.usage.push($usage); + let other_key = $key_type { 0: other_key.into() }; + assert_ne!(key, other_key); + }; + } + + // The last argument is deliberately some usage which is *not* correct for the + // type. + test_partial_eq!(MasterPubkey, master_key, master_keys, KeyUsage::SelfSigning); + test_partial_eq!(SelfSigningPubkey, self_signing_key, self_signing_keys, KeyUsage::Master); + test_partial_eq!(UserSigningPubkey, user_signing_key, user_signing_keys, KeyUsage::Master); } } diff --git a/crates/matrix-sdk-crypto/src/olm/signing/mod.rs b/crates/matrix-sdk-crypto/src/olm/signing/mod.rs index f9cfa9690..50f3c8a7c 100644 --- a/crates/matrix-sdk-crypto/src/olm/signing/mod.rs +++ b/crates/matrix-sdk-crypto/src/olm/signing/mod.rs @@ -581,7 +581,7 @@ impl PrivateCrossSigningIdentity { #[cfg(any(test, feature = "testing"))] #[allow(dead_code)] /// Testing helper to reset this CrossSigning with a fresh one using the - /// local ideniy + /// local identity pub async fn reset(&mut self) { let new = Self::new(self.user_id().to_owned()).await; *self = new diff --git a/crates/matrix-sdk-crypto/src/types/cross_signing_key.rs b/crates/matrix-sdk-crypto/src/types/cross_signing_key.rs index 7efa28d45..e1014881a 100644 --- a/crates/matrix-sdk-crypto/src/types/cross_signing_key.rs +++ b/crates/matrix-sdk-crypto/src/types/cross_signing_key.rs @@ -32,7 +32,7 @@ use vodozemac::{Ed25519PublicKey, KeyError}; use super::{Signatures, SigningKeys}; /// A cross signing key. -#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize)] pub struct CrossSigningKey { /// The ID of the user the key belongs to. pub user_id: OwnedUserId, From ab0e27622e2f9b6bc88a04416527a6d5a7c07b8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Wed, 1 Feb 2023 22:55:47 +0100 Subject: [PATCH 04/13] feat(bindings): Add support to setup a OpenTracing based logger --- .github/workflows/bindings_ci.yml | 20 ++ .github/workflows/ci.yml | 15 ++ Cargo.lock | 275 ++++++++++++++++++++++++ bindings/matrix-sdk-ffi/Cargo.toml | 4 + bindings/matrix-sdk-ffi/src/platform.rs | 112 ++++++++++ 5 files changed, 426 insertions(+) diff --git a/.github/workflows/bindings_ci.yml b/.github/workflows/bindings_ci.yml index e85c00a21..1d47bb6ed 100644 --- a/.github/workflows/bindings_ci.yml +++ b/.github/workflows/bindings_ci.yml @@ -26,6 +26,11 @@ jobs: - name: Checkout repo uses: actions/checkout@v2 + - name: Install Protoc + uses: arduino/setup-protoc@v1 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + - name: Check xtask cache uses: actions/cache@v3 id: xtask-cache @@ -58,6 +63,11 @@ jobs: - name: Checkout uses: actions/checkout@v1 + - name: Install Protoc + uses: arduino/setup-protoc@v1 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + - name: Install Rust uses: actions-rs/toolchain@v1 with: @@ -204,6 +214,11 @@ jobs: - name: Checkout repo uses: actions/checkout@v2 + - name: Install Protoc + uses: arduino/setup-protoc@v1 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + - name: Check xtask cache uses: actions/cache@v3 id: xtask-cache @@ -236,6 +251,11 @@ jobs: - name: Checkout uses: actions/checkout@v1 + - name: Install Protoc + uses: arduino/setup-protoc@v1 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + - name: Install Rust uses: actions-rs/toolchain@v1 with: diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0c48cb3b5..7795a8715 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,6 +22,11 @@ jobs: - name: Checkout repo uses: actions/checkout@v2 + - name: Install Protoc + uses: arduino/setup-protoc@v1 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + - name: Check xtask cache uses: actions/cache@v3 id: xtask-cache @@ -187,6 +192,11 @@ jobs: - name: Checkout uses: actions/checkout@v1 + - name: Install Protoc + uses: arduino/setup-protoc@v1 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + - name: Install Rust uses: actions-rs/toolchain@v1 with: @@ -382,6 +392,11 @@ jobs: - name: Checkout the repo uses: actions/checkout@v3 + - name: Install Protoc + uses: arduino/setup-protoc@v1 + with: + repo-token: ${{ secrets.GITHUB_TOKEN }} + - name: Install Rust uses: actions-rs/toolchain@v1 with: diff --git a/Cargo.lock b/Cargo.lock index 7946958e0..22a5c59c7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -408,6 +408,12 @@ version = "0.20.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0ea22880d78093b0cbe17c89f64a7d457941e65759157ec6cb31a31d652b05e5" +[[package]] +name = "base64" +version = "0.21.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a4a4ddaa51a5bc52a6948f74c06d20aaaddb71924eab79b8c97a8c556e942d6a" + [[package]] name = "base64ct" version = "1.5.3" @@ -1680,6 +1686,12 @@ dependencies = [ "winapi", ] +[[package]] +name = "fixedbitset" +version = "0.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ce7134b9999ecaf8bcd65542e436736ef32ddca1b3e06094cb6ec5755203b80" + [[package]] name = "flate2" version = "1.0.25" @@ -2141,6 +2153,18 @@ dependencies = [ "tokio-rustls", ] +[[package]] +name = "hyper-timeout" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbb958482e8c7be4bc3cf272a766a2b0bf1a6755e7a6ae777f017a31d11b13b1" +dependencies = [ + "hyper", + "pin-project-lite", + "tokio", + "tokio-io-timeout", +] + [[package]] name = "hyper-tls" version = "0.5.0" @@ -2887,6 +2911,7 @@ version = "0.2.0" dependencies = [ "android_logger", "anyhow", + "base64 0.21.0", "extension-trait", "futures-core", "futures-signals", @@ -2895,12 +2920,15 @@ dependencies = [ "matrix-sdk", "mime", "once_cell", + "opentelemetry", + "opentelemetry-otlp", "sanitize-filename-reader-friendly", "serde_json", "thiserror", "tokio", "tokio-stream", "tracing", + "tracing-opentelemetry", "tracing-subscriber", "uniffi", "uniffi_build", @@ -3152,6 +3180,12 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "multimap" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5ce46fe64a9d73be07dcbe690a38ce1b293be448fd8ce1e6c1b8062c9f72c6a" + [[package]] name = "napi" version = "2.10.5" @@ -3439,6 +3473,101 @@ dependencies = [ "vcpkg", ] +[[package]] +name = "opentelemetry" +version = "0.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "69d6c3d7288a106c0a363e4b0e8d308058d56902adefb16f4936f417ffef086e" +dependencies = [ + "opentelemetry_api", + "opentelemetry_sdk", +] + +[[package]] +name = "opentelemetry-http" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1edc79add46364183ece1a4542592ca593e6421c60807232f5b8f7a31703825d" +dependencies = [ + "async-trait", + "bytes", + "http", + "opentelemetry_api", + "reqwest", +] + +[[package]] +name = "opentelemetry-otlp" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d1c928609d087790fc936a1067bdc310ae702bdf3b090c3f281b713622c8bbde" +dependencies = [ + "async-trait", + "futures", + "futures-util", + "http", + "opentelemetry", + "opentelemetry-http", + "opentelemetry-proto", + "prost", + "reqwest", + "thiserror", + "tokio", + "tonic", +] + +[[package]] +name = "opentelemetry-proto" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d61a2f56df5574508dd86aaca016c917489e589ece4141df1b5e349af8d66c28" +dependencies = [ + "futures", + "futures-util", + "opentelemetry", + "prost", + "tonic", + "tonic-build", +] + +[[package]] +name = "opentelemetry_api" +version = "0.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c24f96e21e7acc813c7a8394ee94978929db2bcc46cf6b5014fc612bf7760c22" +dependencies = [ + "fnv", + "futures-channel", + "futures-util", + "indexmap", + "js-sys", + "once_cell", + "pin-project-lite", + "thiserror", +] + +[[package]] +name = "opentelemetry_sdk" +version = "0.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1ca41c4933371b61c2a2f214bf16931499af4ec90543604ec828f7a625c09113" +dependencies = [ + "async-trait", + "crossbeam-channel", + "dashmap", + "fnv", + "futures-channel", + "futures-executor", + "futures-util", + "once_cell", + "opentelemetry_api", + "percent-encoding", + "rand 0.8.5", + "thiserror", + "tokio", + "tokio-stream", +] + [[package]] name = "os_str_bytes" version = "6.4.1" @@ -3540,6 +3669,16 @@ version = "2.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "478c572c3d73181ff3c2539045f6eb99e5491218eae919370993b890cdbdd98e" +[[package]] +name = "petgraph" +version = "0.6.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6d5014253a1331579ce62aa67443b4a658c5e7dd03d4bc6d302b94474888143" +dependencies = [ + "fixedbitset", + "indexmap", +] + [[package]] name = "pin-project" version = "1.0.12" @@ -3685,6 +3824,16 @@ version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" +[[package]] +name = "prettyplease" +version = "0.1.23" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e97e3215779627f01ee256d2fad52f3d95e8e1c11e9fc6fd08f7cd455d5d5c78" +dependencies = [ + "proc-macro2", + "syn", +] + [[package]] name = "proc-macro-crate" version = "1.2.1" @@ -3756,6 +3905,28 @@ dependencies = [ "prost-derive", ] +[[package]] +name = "prost-build" +version = "0.11.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cb5320c680de74ba083512704acb90fe00f28f79207286a848e730c45dd73ed6" +dependencies = [ + "bytes", + "heck", + "itertools", + "lazy_static", + "log", + "multimap", + "petgraph", + "prettyplease", + "prost", + "prost-types", + "regex", + "syn", + "tempfile", + "which", +] + [[package]] name = "prost-derive" version = "0.11.5" @@ -3769,6 +3940,16 @@ dependencies = [ "syn", ] +[[package]] +name = "prost-types" +version = "0.11.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "017f79637768cde62820bc2d4fe0e45daaa027755c323ad077767c6c5f173091" +dependencies = [ + "bytes", + "prost", +] + [[package]] name = "pulldown-cmark" version = "0.9.2" @@ -4957,6 +5138,16 @@ dependencies = [ "windows-sys", ] +[[package]] +name = "tokio-io-timeout" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "30b74022ada614a1b4834de765f9bb43877f910cc8ce4be40e89042c9223a8bf" +dependencies = [ + "pin-project-lite", + "tokio", +] + [[package]] name = "tokio-macros" version = "1.8.2" @@ -5035,6 +5226,51 @@ dependencies = [ "serde", ] +[[package]] +name = "tonic" +version = "0.8.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f219fad3b929bef19b1f86fbc0358d35daed8f2cac972037ac0dc10bbb8d5fb" +dependencies = [ + "async-stream", + "async-trait", + "axum", + "base64 0.13.1", + "bytes", + "futures-core", + "futures-util", + "h2", + "http", + "http-body", + "hyper", + "hyper-timeout", + "percent-encoding", + "pin-project", + "prost", + "prost-derive", + "tokio", + "tokio-stream", + "tokio-util", + "tower", + "tower-layer", + "tower-service", + "tracing", + "tracing-futures", +] + +[[package]] +name = "tonic-build" +version = "0.8.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5bf5e9b9c0f7e0a7c027dcfaba7b2c60816c7049171f679d99ee2ff65d0de8c4" +dependencies = [ + "prettyplease", + "proc-macro2", + "prost-build", + "quote", + "syn", +] + [[package]] name = "tower" version = "0.4.13" @@ -5043,9 +5279,13 @@ checksum = "b8fa9be0de6cf49e536ce1851f987bd21a43b771b09473c3549a6c853db37c1c" dependencies = [ "futures-core", "futures-util", + "indexmap", "pin-project", "pin-project-lite", + "rand 0.8.5", + "slab", "tokio", + "tokio-util", "tower-layer", "tower-service", "tracing", @@ -5127,6 +5367,16 @@ dependencies = [ "tracing-subscriber", ] +[[package]] +name = "tracing-futures" +version = "0.2.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "97d095ae15e245a057c8e8451bab9b3ee1e1f68e9ba2b4fbc18d0ac5237835f2" +dependencies = [ + "pin-project", + "tracing", +] + [[package]] name = "tracing-log" version = "0.1.3" @@ -5138,6 +5388,20 @@ dependencies = [ "tracing-core", ] +[[package]] +name = "tracing-opentelemetry" +version = "0.18.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "21ebb87a95ea13271332df069020513ab70bdb5637ca42d6e492dc3bbbad48de" +dependencies = [ + "once_cell", + "opentelemetry", + "tracing", + "tracing-core", + "tracing-log", + "tracing-subscriber", +] + [[package]] name = "tracing-subscriber" version = "0.3.16" @@ -5679,6 +5943,17 @@ version = "0.1.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9193164d4de03a926d909d3bc7c30543cecb35400c02114792c2cae20d5e2dbb" +[[package]] +name = "which" +version = "4.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2441c784c52b289a054b7201fc93253e288f094e2f4be9058343127c4226a269" +dependencies = [ + "either", + "libc", + "once_cell", +] + [[package]] name = "wildmatch" version = "2.1.1" diff --git a/bindings/matrix-sdk-ffi/Cargo.toml b/bindings/matrix-sdk-ffi/Cargo.toml index 82d2e4135..b8a8af700 100644 --- a/bindings/matrix-sdk-ffi/Cargo.toml +++ b/bindings/matrix-sdk-ffi/Cargo.toml @@ -17,6 +17,7 @@ uniffi_build = { workspace = true, features = ["builtin-bindgen"] } [dependencies] anyhow = { workspace = true } +base64 = "0.21" extension-trait = "1.0.1" futures-core = "0.3.17" futures-signals = { version = "0.3.30", default-features = false } @@ -25,9 +26,12 @@ mime = "0.3.16" # FIXME: we currently can't feature flag anything in the api.udl, therefore we must enforce experimental-sliding-sync being exposed here.. # see https://github.com/matrix-org/matrix-rust-sdk/issues/1014 once_cell = { workspace = true } +opentelemetry = { version = "0.18.0", features = ["rt-tokio"] } +opentelemetry-otlp = { version = "0.11.0", features = ["tokio", "reqwest-client", "http-proto"] } sanitize-filename-reader-friendly = "2.2.1" serde_json = { workspace = true } thiserror = { workspace = true } +tracing-opentelemetry = { version = "0.18.0" } tokio = { version = "1", features = ["rt-multi-thread", "macros"] } tokio-stream = "0.1.8" uniffi = { workspace = true } diff --git a/bindings/matrix-sdk-ffi/src/platform.rs b/bindings/matrix-sdk-ffi/src/platform.rs index b1ed9702b..b506c4744 100644 --- a/bindings/matrix-sdk-ffi/src/platform.rs +++ b/bindings/matrix-sdk-ffi/src/platform.rs @@ -1,9 +1,25 @@ +use std::collections::HashMap; + #[cfg(target_os = "android")] use android as platform_impl; +use base64::{engine::general_purpose::STANDARD, Engine}; +use futures_core::future::BoxFuture; #[cfg(target_os = "ios")] use ios as platform_impl; +use opentelemetry::{ + sdk::{ + trace::{BatchMessage, TraceRuntime, Tracer}, + util::tokio_interval_stream, + Resource, + }, + KeyValue, +}; +use opentelemetry_otlp::{Protocol, WithExportConfig}; #[cfg(not(any(target_os = "ios", target_os = "android")))] use other as platform_impl; +use tokio::runtime::Handle; + +use crate::RUNTIME; #[cfg(target_os = "android")] mod android { @@ -24,6 +40,75 @@ mod android { } } +#[derive(Clone, Debug)] +struct TracingRuntime { + runtime: Handle, +} + +impl opentelemetry::runtime::Runtime for TracingRuntime { + type Interval = tokio_stream::wrappers::IntervalStream; + type Delay = ::std::pin::Pin>; + + fn interval(&self, duration: std::time::Duration) -> Self::Interval { + let _guard = self.runtime.enter(); + tokio_interval_stream(duration) + } + + fn spawn(&self, future: BoxFuture<'static, ()>) { + #[allow(clippy::let_underscore_future)] + let _ = self.runtime.spawn(future); + } + + fn delay(&self, duration: std::time::Duration) -> Self::Delay { + let _guard = self.runtime.enter(); + Box::pin(tokio::time::sleep(duration)) + } +} + +impl TraceRuntime for TracingRuntime { + type Receiver = tokio_stream::wrappers::ReceiverStream; + type Sender = tokio::sync::mpsc::Sender; + + fn batch_message_channel(&self, capacity: usize) -> (Self::Sender, Self::Receiver) { + let (sender, receiver) = tokio::sync::mpsc::channel(capacity); + (sender, tokio_stream::wrappers::ReceiverStream::new(receiver)) + } +} + +pub fn create_otlp_tracer( + user: String, + password: String, + otlp_endpoint: String, + client_name: String, +) -> anyhow::Result { + let runtime = RUNTIME.handle().to_owned(); + + let auth = STANDARD.encode(format!("{user}:{password}")); + let headers = HashMap::from([("Authorization".to_owned(), format!("Basic {auth}"))]); + let http_client = matrix_sdk::reqwest::ClientBuilder::new().build()?; + + let exporter = opentelemetry_otlp::new_exporter() + .http() + .with_http_client(http_client) + .with_protocol(Protocol::HttpBinary) + .with_endpoint(otlp_endpoint) + .with_headers(headers); + + let tracer_runtime = TracingRuntime { runtime: runtime.to_owned() }; + + let _guard = runtime.enter(); + let tracer = opentelemetry_otlp::new_pipeline() + .tracing() + .with_exporter(exporter) + .with_trace_config( + opentelemetry::sdk::trace::config() + .with_resource(Resource::new(vec![KeyValue::new("service.name", client_name)])), + ) + .install_batch(tracer_runtime)?; + + Ok(tracer) +} + #[cfg(target_os = "ios")] mod ios { use std::io; @@ -35,6 +120,26 @@ mod ios { .with(fmt::layer().with_ansi(false).with_writer(io::stderr)) .init(); } + + pub fn setup_otlp_tracing( + configuration: String, + user: String, + password: String, + otlp_endpoint: String, + ) -> anyhow::Result<()> { + let otlp_tracer = + super::create_otlp_tracer(user, password, otlp_endpoint, "element-x-ios".to_owned())?; + + let otlp_layer = tracing_opentelemetry::layer().with_tracer(otlp_tracer); + + tracing_subscriber::registry() + .with(EnvFilter::new(configuration)) + .with(fmt::layer().with_ansi(false).with_writer(io::stderr)) + .with(otlp_layer) + .init(); + + Ok(()) + } } #[cfg(not(any(target_os = "ios", target_os = "android")))] @@ -55,3 +160,10 @@ mod other { pub fn setup_tracing(filter: String) { platform_impl::setup_tracing(filter) } + +#[cfg(target_os = "ios")] +#[uniffi::export] +pub fn setup_otlp_tracing(filter: String, user: String, password: String, otlp_endpoint: String) { + platform_impl::setup_otlp_tracing(filter, user, password, otlp_endpoint) + .expect("Couldn't configure the OpenTelemetry tracer") +} From a48fd77c4a4ead58f6f4c195f809a1c14dfc3a0f Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Mon, 30 Jan 2023 18:21:14 +0100 Subject: [PATCH 05/13] refactor(sdk): Rename LocalEventTimelineItemSendState => EventSendState --- bindings/matrix-sdk-ffi/src/lib.rs | 13 ++++++------- bindings/matrix-sdk-ffi/src/timeline.rs | 12 +++++------- .../matrix-sdk/src/room/timeline/event_handler.rs | 8 ++++---- crates/matrix-sdk/src/room/timeline/event_item.rs | 10 +++++----- crates/matrix-sdk/src/room/timeline/mod.rs | 7 +++---- crates/matrix-sdk/src/room/timeline/tests.rs | 8 ++++---- 6 files changed, 27 insertions(+), 31 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/lib.rs b/bindings/matrix-sdk-ffi/src/lib.rs index 4cb893531..90a43ad37 100644 --- a/bindings/matrix-sdk-ffi/src/lib.rs +++ b/bindings/matrix-sdk-ffi/src/lib.rs @@ -90,13 +90,12 @@ mod uniffi_types { SlidingSyncViewBuilder, StoppableSpawn, UnreadNotificationsCount, }, timeline::{ - EmoteMessageContent, EncryptedMessage, EventTimelineItem, FileInfo, FileMessageContent, - FormattedBody, ImageInfo, ImageMessageContent, InsertAtData, - LocalEventTimelineItemSendState, MembershipChange, Message, MessageFormat, MessageType, - NoticeMessageContent, OtherState, Profile, Reaction, TextMessageContent, ThumbnailInfo, - TimelineChange, TimelineDiff, TimelineItem, TimelineItemContent, - TimelineItemContentKind, UpdateAtData, VideoInfo, VideoMessageContent, - VirtualTimelineItem, + EmoteMessageContent, EncryptedMessage, EventSendState, EventTimelineItem, FileInfo, + FileMessageContent, FormattedBody, ImageInfo, ImageMessageContent, InsertAtData, + MembershipChange, Message, MessageFormat, MessageType, NoticeMessageContent, + OtherState, Profile, Reaction, TextMessageContent, ThumbnailInfo, TimelineChange, + TimelineDiff, TimelineItem, TimelineItemContent, TimelineItemContentKind, UpdateAtData, + VideoInfo, VideoMessageContent, VirtualTimelineItem, }, }; } diff --git a/bindings/matrix-sdk-ffi/src/timeline.rs b/bindings/matrix-sdk-ffi/src/timeline.rs index abcbc1a7e..89d933d0a 100644 --- a/bindings/matrix-sdk-ffi/src/timeline.rs +++ b/bindings/matrix-sdk-ffi/src/timeline.rs @@ -172,7 +172,7 @@ impl TimelineItem { /// This type represents the “send state” of a local event timeline item. #[derive(Clone, uniffi::Enum)] -pub enum LocalEventTimelineItemSendState { +pub enum EventSendState { /// The local event has not been sent yet. NotSendYet, /// The local event has been sent to the server, but unsuccessfully: The @@ -182,11 +182,9 @@ pub enum LocalEventTimelineItemSendState { Sent, } -impl From - for LocalEventTimelineItemSendState -{ - fn from(value: matrix_sdk::room::timeline::LocalEventTimelineItemSendState) -> Self { - use matrix_sdk::room::timeline::LocalEventTimelineItemSendState::*; +impl From for EventSendState { + fn from(value: matrix_sdk::room::timeline::EventSendState) -> Self { + use matrix_sdk::room::timeline::EventSendState::*; match value { NotSentYet => Self::NotSendYet, @@ -267,7 +265,7 @@ impl EventTimelineItem { self.0.raw().map(|r| r.json().get().to_owned()) } - pub fn local_send_state(&self) -> Option { + pub fn local_send_state(&self) -> Option { use matrix_sdk::room::timeline::EventTimelineItem::*; match &self.0 { diff --git a/crates/matrix-sdk/src/room/timeline/event_handler.rs b/crates/matrix-sdk/src/room/timeline/event_handler.rs index ed5bb7f70..229e712fa 100644 --- a/crates/matrix-sdk/src/room/timeline/event_handler.rs +++ b/crates/matrix-sdk/src/room/timeline/event_handler.rs @@ -42,9 +42,9 @@ use tracing::{debug, error, field::debug, info, instrument, trace, warn}; use super::{ event_item::{ - AnyOtherFullStateEventContent, BundledReactions, LocalEventTimelineItem, - LocalEventTimelineItemSendState, MemberProfileChange, OtherState, Profile, - RemoteEventTimelineItem, RoomMembershipChange, Sticker, + AnyOtherFullStateEventContent, BundledReactions, EventSendState, LocalEventTimelineItem, + MemberProfileChange, OtherState, Profile, RemoteEventTimelineItem, RoomMembershipChange, + Sticker, }, find_read_marker, rfind_event_by_id, rfind_event_item, EventTimelineItem, Message, ReactionGroup, TimelineInnerMetadata, TimelineItem, TimelineItemContent, VirtualTimelineItem, @@ -533,7 +533,7 @@ impl<'a, 'i> TimelineEventHandler<'a, 'i> { match &self.flow { Flow::Local { txn_id, timestamp } => { EventTimelineItem::Local(LocalEventTimelineItem { - send_state: LocalEventTimelineItemSendState::NotSentYet, + send_state: EventSendState::NotSentYet, transaction_id: txn_id.to_owned(), event_id: None, sender, diff --git a/crates/matrix-sdk/src/room/timeline/event_item.rs b/crates/matrix-sdk/src/room/timeline/event_item.rs index 504f95bbd..e4fc6e0fb 100644 --- a/crates/matrix-sdk/src/room/timeline/event_item.rs +++ b/crates/matrix-sdk/src/room/timeline/event_item.rs @@ -208,9 +208,9 @@ impl EventTimelineItem { } } -/// This type represents the “send state” of a local event timeline item. +/// This type represents the "send state" of a local event timeline item. #[derive(Debug, Copy, Clone, PartialEq)] -pub enum LocalEventTimelineItemSendState { +pub enum EventSendState { /// The local event has not been sent yet. NotSentYet, /// The local event has been sent to the server, but unsuccessfully: The @@ -223,7 +223,7 @@ pub enum LocalEventTimelineItemSendState { #[derive(Debug, Clone)] pub struct LocalEventTimelineItem { /// The send state of this local event. - pub send_state: LocalEventTimelineItemSendState, + pub send_state: EventSendState, /// The transaction ID. pub transaction_id: OwnedTransactionId, /// The event ID received from the server in the event-sending response. @@ -251,8 +251,8 @@ impl LocalEventTimelineItem { pub(super) fn with_event_id(&self, event_id: Option) -> Self { Self { send_state: match &event_id { - Some(_) => LocalEventTimelineItemSendState::Sent, - None => LocalEventTimelineItemSendState::SendingFailed, + Some(_) => EventSendState::Sent, + None => EventSendState::SendingFailed, }, event_id, ..self.clone() diff --git a/crates/matrix-sdk/src/room/timeline/mod.rs b/crates/matrix-sdk/src/room/timeline/mod.rs index 5bb2b5049..18055d981 100644 --- a/crates/matrix-sdk/src/room/timeline/mod.rs +++ b/crates/matrix-sdk/src/room/timeline/mod.rs @@ -49,10 +49,9 @@ mod virtual_item; pub use self::{ event_item::{ - AnyOtherFullStateEventContent, BundledReactions, EncryptedMessage, EventTimelineItem, - LocalEventTimelineItemSendState, MemberProfileChange, MembershipChange, Message, - OtherState, Profile, ReactionGroup, RoomMembershipChange, Sticker, TimelineDetails, - TimelineItemContent, + AnyOtherFullStateEventContent, BundledReactions, EncryptedMessage, EventSendState, + EventTimelineItem, MemberProfileChange, MembershipChange, Message, OtherState, Profile, + ReactionGroup, RoomMembershipChange, Sticker, TimelineDetails, TimelineItemContent, }, pagination::{PaginationOptions, PaginationOutcome}, virtual_item::VirtualTimelineItem, diff --git a/crates/matrix-sdk/src/room/timeline/tests.rs b/crates/matrix-sdk/src/room/timeline/tests.rs index b7be5e4c8..2e6df850a 100644 --- a/crates/matrix-sdk/src/room/timeline/tests.rs +++ b/crates/matrix-sdk/src/room/timeline/tests.rs @@ -58,7 +58,7 @@ use super::{ EventTimelineItem, MembershipChange, Profile, TimelineInner, TimelineItem, TimelineItemContent, VirtualTimelineItem, }; -use crate::room::timeline::event_item::LocalEventTimelineItemSendState; +use crate::room::timeline::event_item::EventSendState; static ALICE: Lazy<&UserId> = Lazy::new(|| user_id!("@alice:server.name")); static BOB: Lazy<&UserId> = Lazy::new(|| user_id!("@bob:other.server")); @@ -388,7 +388,7 @@ async fn remote_echo_full_trip() { { let item = assert_matches!(stream.next().await, Some(VecDiff::Push { value }) => value); let event = item.as_event().unwrap().as_local().unwrap(); - assert_eq!(event.send_state, LocalEventTimelineItemSendState::NotSentYet); + assert_eq!(event.send_state, EventSendState::NotSentYet); } // Scenario 2: The local event has not been sent to the server successfully, it @@ -400,7 +400,7 @@ async fn remote_echo_full_trip() { let item = assert_matches!(stream.next().await, Some(VecDiff::UpdateAt { value, index: 1 }) => value); let event = item.as_event().unwrap().as_local().unwrap(); - assert_eq!(event.send_state, LocalEventTimelineItemSendState::SendingFailed); + assert_eq!(event.send_state, EventSendState::SendingFailed); } // Scenario 3: The local event has been sent successfully to the server and an @@ -412,7 +412,7 @@ async fn remote_echo_full_trip() { let item = assert_matches!(stream.next().await, Some(VecDiff::UpdateAt { value, index: 1 }) => value); let event = item.as_event().unwrap().as_local().unwrap(); - assert_eq!(event.send_state, LocalEventTimelineItemSendState::Sent); + assert_eq!(event.send_state, EventSendState::Sent); event_id }; From c8021cf2baffccae62d42c2f8ffd7d8aea297847 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Mon, 30 Jan 2023 18:48:18 +0100 Subject: [PATCH 06/13] refactor(sdk): Move LocalEventTimelineItem#event_id into send_state --- bindings/matrix-sdk-ffi/src/timeline.rs | 10 ++-- .../src/room/timeline/event_handler.rs | 1 - .../src/room/timeline/event_item.rs | 46 +++++++++---------- crates/matrix-sdk/src/room/timeline/inner.rs | 45 +++++++++--------- crates/matrix-sdk/src/room/timeline/tests.rs | 21 ++++++--- .../tests/integration/room/timeline.rs | 7 +-- 6 files changed, 68 insertions(+), 62 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/timeline.rs b/bindings/matrix-sdk-ffi/src/timeline.rs index 89d933d0a..aae2a3536 100644 --- a/bindings/matrix-sdk-ffi/src/timeline.rs +++ b/bindings/matrix-sdk-ffi/src/timeline.rs @@ -179,17 +179,17 @@ pub enum EventSendState { /// sending has failed. SendingFailed, /// The local event has been sent successfully to the server. - Sent, + Sent { event_id: String }, } -impl From for EventSendState { - fn from(value: matrix_sdk::room::timeline::EventSendState) -> Self { +impl From<&matrix_sdk::room::timeline::EventSendState> for EventSendState { + fn from(value: &matrix_sdk::room::timeline::EventSendState) -> Self { use matrix_sdk::room::timeline::EventSendState::*; match value { NotSentYet => Self::NotSendYet, SendingFailed => Self::SendingFailed, - Sent => Self::Sent, + Sent { event_id } => Self::Sent { event_id: event_id.to_string() }, } } } @@ -269,7 +269,7 @@ impl EventTimelineItem { use matrix_sdk::room::timeline::EventTimelineItem::*; match &self.0 { - Local(local_event) => Some(local_event.send_state.into()), + Local(local_event) => Some((&local_event.send_state).into()), Remote(_) => None, } } diff --git a/crates/matrix-sdk/src/room/timeline/event_handler.rs b/crates/matrix-sdk/src/room/timeline/event_handler.rs index 229e712fa..3ad83729d 100644 --- a/crates/matrix-sdk/src/room/timeline/event_handler.rs +++ b/crates/matrix-sdk/src/room/timeline/event_handler.rs @@ -535,7 +535,6 @@ impl<'a, 'i> TimelineEventHandler<'a, 'i> { EventTimelineItem::Local(LocalEventTimelineItem { send_state: EventSendState::NotSentYet, transaction_id: txn_id.to_owned(), - event_id: None, sender, sender_profile, timestamp: *timestamp, diff --git a/crates/matrix-sdk/src/room/timeline/event_item.rs b/crates/matrix-sdk/src/room/timeline/event_item.rs index e4fc6e0fb..f73c981e2 100644 --- a/crates/matrix-sdk/src/room/timeline/event_item.rs +++ b/crates/matrix-sdk/src/room/timeline/event_item.rs @@ -88,10 +88,10 @@ impl EventTimelineItem { /// case of a remote event. pub fn unique_identifier(&self) -> String { match self { - Self::Local(LocalEventTimelineItem { transaction_id, event_id, .. }) => { - match event_id { - Some(event_id) => event_id.to_string(), - None => transaction_id.to_string(), + Self::Local(LocalEventTimelineItem { transaction_id, send_state, .. }) => { + match send_state { + EventSendState::Sent { event_id } => event_id.to_string(), + _ => transaction_id.to_string(), } } @@ -123,7 +123,7 @@ impl EventTimelineItem { /// of the send request that created the event. pub fn event_id(&self) -> Option<&EventId> { match self { - Self::Local(local_event) => local_event.event_id.as_deref(), + Self::Local(local_event) => local_event.event_id(), Self::Remote(remote_event) => Some(&remote_event.event_id), } } @@ -209,7 +209,7 @@ impl EventTimelineItem { } /// This type represents the "send state" of a local event timeline item. -#[derive(Debug, Copy, Clone, PartialEq)] +#[derive(Clone, Debug, PartialEq, Eq)] pub enum EventSendState { /// The local event has not been sent yet. NotSentYet, @@ -217,7 +217,10 @@ pub enum EventSendState { /// sending has failed. SendingFailed, /// The local event has been sent successfully to the server. - Sent, + Sent { + /// The event ID assigned by the server. + event_id: OwnedEventId, + }, } #[derive(Debug, Clone)] @@ -226,8 +229,6 @@ pub struct LocalEventTimelineItem { pub send_state: EventSendState, /// The transaction ID. pub transaction_id: OwnedTransactionId, - /// The event ID received from the server in the event-sending response. - pub event_id: Option, /// The sender of the event. pub sender: OwnedUserId, /// The sender's profile of the event. @@ -239,25 +240,20 @@ pub struct LocalEventTimelineItem { } impl LocalEventTimelineItem { - /// Clone the current event item, and update its `event_id`. + /// Get the event ID of this item. /// - /// `event_id` is optional: - /// * `Some(_)` means the local event has been sent successfully to the - /// server, its send state will be moved to - /// [`LocalEventTimelineItemSendState::Sent`]. - /// * `None` means the local event has been failed to be sent to the - /// server, its send state will be moved to - /// [`LocalEventTimelineItemSendState::SendingFailed`]. - pub(super) fn with_event_id(&self, event_id: Option) -> Self { - Self { - send_state: match &event_id { - Some(_) => EventSendState::Sent, - None => EventSendState::SendingFailed, - }, - event_id, - ..self.clone() + /// Will be `Some` if and only if `send_state` is `EventSendState::Sent`. + pub fn event_id(&self) -> Option<&EventId> { + match &self.send_state { + EventSendState::Sent { event_id } => Some(event_id), + _ => None, } } + + /// Clone the current event item, and update its `send_state`. + pub(super) fn with_send_state(&self, send_state: EventSendState) -> Self { + Self { send_state, ..self.clone() } + } } impl From for EventTimelineItem { diff --git a/crates/matrix-sdk/src/room/timeline/inner.rs b/crates/matrix-sdk/src/room/timeline/inner.rs index d5a1f2fd8..b3ad25ebf 100644 --- a/crates/matrix-sdk/src/room/timeline/inner.rs +++ b/crates/matrix-sdk/src/room/timeline/inner.rs @@ -20,10 +20,10 @@ use ruma::{ AnySyncTimelineEvent, }, serde::Raw, - MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedTransactionId, OwnedUserId, RoomId, + EventId, MilliSecondsSinceUnixEpoch, OwnedEventId, OwnedTransactionId, OwnedUserId, RoomId, TransactionId, UserId, }; -use tracing::{debug, error, info, warn}; +use tracing::{debug, error, field::debug, info, warn}; #[cfg(feature = "e2e-encryption")] use tracing::{instrument, trace}; @@ -32,7 +32,7 @@ use super::{ update_read_marker, Flow, HandleEventResult, TimelineEventHandler, TimelineEventKind, TimelineEventMetadata, TimelineItemPosition, }, - rfind_event_item, EventTimelineItem, Profile, TimelineItem, + rfind_event_item, EventSendState, EventTimelineItem, Profile, TimelineItem, }; use crate::{ events::SyncTimelineEventWithoutContent, @@ -160,36 +160,40 @@ impl TimelineInner

{ ) -> crate::error::Result<()> { match response { Ok(response) => { - self.update_event_id_of_local_event(txn_id, Some(response.event_id)); + self.update_event_send_state( + txn_id, + EventSendState::Sent { event_id: response.event_id }, + ); Ok(()) } Err(error) => { - self.update_event_id_of_local_event(txn_id, None); + self.update_event_send_state(txn_id, EventSendState::SendingFailed); Err(error) } } } - /// Update the event ID of a local event represented by a transaction ID. - /// - /// If the event ID is `None`, it means there is no event ID returned by the - /// server, so the sending has failed. If the event ID is `Some(_)`, it - /// means the sending has been successful. + /// Update the send state of a local event represented by a transaction ID. /// /// If no local event is found, a warning is raised. - pub(super) fn update_event_id_of_local_event( + pub(super) fn update_event_send_state( &self, txn_id: &TransactionId, - event_id: Option, + send_state: EventSendState, ) { let mut lock = self.items.lock_mut(); + let new_event_id: Option<&EventId> = match &send_state { + EventSendState::Sent { event_id } => Some(event_id), + _ => None, + }; + // Look for the local event by the transaction ID or event ID. let result = rfind_event_item(&lock, |it| { it.transaction_id() == Some(txn_id) - || event_id.is_some() && it.event_id() == event_id.as_deref() + || new_event_id.is_some() && it.event_id() == new_event_id }); let Some((idx, item)) = result else { @@ -204,16 +208,15 @@ impl TimelineInner

{ return; }; - // An event ID already exists, that's a broken state, let's emit an - // error but also override to the given event ID. - if let Some(existing_event_id) = &item.event_id { - error!( - ?existing_event_id, new_event_id = ?event_id, ?txn_id, - "Local echo already has an event ID" - ); + // The event was already marked as sent, that's a broken state, let's + // emit an error but also override to the given sent state. + if let EventSendState::Sent { event_id: existing_event_id } = &item.send_state { + let new_event_id = new_event_id.map(debug); + error!(?existing_event_id, ?new_event_id, ?txn_id, "Local echo already marked as sent"); } - lock.set_cloned(idx, Arc::new(TimelineItem::Event(item.with_event_id(event_id).into()))); + let new_item = TimelineItem::Event(item.with_send_state(send_state).into()); + lock.set_cloned(idx, Arc::new(new_item)); } /// Handle a back-paginated event. diff --git a/crates/matrix-sdk/src/room/timeline/tests.rs b/crates/matrix-sdk/src/room/timeline/tests.rs index 2e6df850a..e92ea523d 100644 --- a/crates/matrix-sdk/src/room/timeline/tests.rs +++ b/crates/matrix-sdk/src/room/timeline/tests.rs @@ -394,11 +394,12 @@ async fn remote_echo_full_trip() { // Scenario 2: The local event has not been sent to the server successfully, it // has failed. In this case, there is no event ID. { - let event_id = None; + timeline.inner.update_event_send_state(&txn_id, EventSendState::SendingFailed); - timeline.inner.update_event_id_of_local_event(&txn_id, event_id); - - let item = assert_matches!(stream.next().await, Some(VecDiff::UpdateAt { value, index: 1 }) => value); + let item = assert_matches!( + stream.next().await, + Some(VecDiff::UpdateAt { value, index: 1 }) => value + ); let event = item.as_event().unwrap().as_local().unwrap(); assert_eq!(event.send_state, EventSendState::SendingFailed); } @@ -408,11 +409,17 @@ async fn remote_echo_full_trip() { let event_id = { let event_id = event_id!("$W6mZSLWMmfuQQ9jhZWeTxFIM"); - timeline.inner.update_event_id_of_local_event(&txn_id, Some(event_id.to_owned())); + timeline.inner.update_event_send_state( + &txn_id, + EventSendState::Sent { event_id: event_id.to_owned() }, + ); - let item = assert_matches!(stream.next().await, Some(VecDiff::UpdateAt { value, index: 1 }) => value); + let item = assert_matches!( + stream.next().await, + Some(VecDiff::UpdateAt { value, index: 1 }) => value + ); let event = item.as_event().unwrap().as_local().unwrap(); - assert_eq!(event.send_state, EventSendState::Sent); + assert_matches!(event.send_state, EventSendState::Sent { .. }); event_id }; diff --git a/crates/matrix-sdk/tests/integration/room/timeline.rs b/crates/matrix-sdk/tests/integration/room/timeline.rs index d28047012..88c4631eb 100644 --- a/crates/matrix-sdk/tests/integration/room/timeline.rs +++ b/crates/matrix-sdk/tests/integration/room/timeline.rs @@ -8,7 +8,8 @@ use futures_util::StreamExt; use matrix_sdk::{ config::SyncSettings, room::timeline::{ - AnyOtherFullStateEventContent, PaginationOptions, TimelineItemContent, VirtualTimelineItem, + AnyOtherFullStateEventContent, EventSendState, PaginationOptions, TimelineItemContent, + VirtualTimelineItem, }, ruma::MilliSecondsSinceUnixEpoch, }; @@ -186,7 +187,7 @@ async fn echo() { let local_echo = assert_matches!(timeline_stream.next().await, Some(VecDiff::Push { value }) => value); let item = local_echo.as_event().unwrap().as_local().unwrap(); - assert!(item.event_id.is_none()); + assert_matches!(&item.send_state, EventSendState::NotSentYet); let msg = assert_matches!(&item.content, TimelineItemContent::Message(msg) => msg); let text = assert_matches!(msg.msgtype(), MessageType::Text(text) => text); @@ -200,7 +201,7 @@ async fn echo() { Some(VecDiff::UpdateAt { index: 1, value }) => value ); let item = sent_confirmation.as_event().unwrap().as_local().unwrap(); - assert!(item.event_id.is_some()); + assert_matches!(&item.send_state, EventSendState::Sent { .. }); ev_builder.add_joined_room(JoinedRoomBuilder::new(room_id).add_timeline_event( TimelineTestEvent::Custom(json!({ From 3a1eb62c38653cb1a62088c3fa1f9cd5e91c63cc Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Mon, 30 Jan 2023 19:27:38 +0100 Subject: [PATCH 07/13] refactor(sdk): Expose event sending errors through timeline item MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit … instead of through the return value of Timeline::send. --- bindings/matrix-sdk-ffi/src/api.udl | 5 +-- bindings/matrix-sdk-ffi/src/room.rs | 45 +++++++++++-------- bindings/matrix-sdk-ffi/src/timeline.rs | 4 +- .../src/room/timeline/event_item.rs | 9 +++- crates/matrix-sdk/src/room/timeline/inner.rs | 25 ----------- crates/matrix-sdk/src/room/timeline/mod.rs | 13 +++--- crates/matrix-sdk/src/room/timeline/tests.rs | 21 ++++++--- .../tests/integration/room/timeline.rs | 2 +- labs/jack-in/src/app/model.rs | 8 +--- 9 files changed, 61 insertions(+), 71 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/api.udl b/bindings/matrix-sdk-ffi/src/api.udl index 27a92d5c1..f241b0055 100644 --- a/bindings/matrix-sdk-ffi/src/api.udl +++ b/bindings/matrix-sdk-ffi/src/api.udl @@ -259,14 +259,13 @@ interface Room { // Raises an exception if there are no timeline listeners. [Throws=ClientError] void paginate_backwards(PaginationOptions opts); - + [Throws=ClientError] void send_read_receipt(string event_id); - + [Throws=ClientError] void send_read_marker(string fully_read_event_id, string? read_receipt_event_id); - [Throws=ClientError] void send(RoomMessageEventContent msg, string? txn_id); [Throws=ClientError] diff --git a/bindings/matrix-sdk-ffi/src/room.rs b/bindings/matrix-sdk-ffi/src/room.rs index d6696aa00..bf8c6f8dd 100644 --- a/bindings/matrix-sdk-ffi/src/room.rs +++ b/bindings/matrix-sdk-ffi/src/room.rs @@ -295,16 +295,18 @@ impl Room { }) } - pub fn send(&self, msg: Arc, txn_id: Option) -> Result<()> { + pub fn send(&self, msg: Arc, txn_id: Option) { let timeline = match &*self.timeline.read().unwrap() { Some(t) => Arc::clone(t), - None => bail!("Timeline not set up, can't send message"), + None => { + error!("Timeline not set up, can't send message"); + return; + } }; - RUNTIME.block_on(async move { - timeline.send((*msg).to_owned().into(), txn_id.as_deref().map(Into::into)).await?; - Ok(()) - }) + RUNTIME.spawn(async move { + timeline.send((*msg).to_owned().into(), txn_id.as_deref().map(Into::into)).await; + }); } pub fn send_reply( @@ -326,24 +328,27 @@ impl Room { let event_id: &EventId = in_reply_to_event_id.as_str().try_into().context("Failed to create EventId.")?; - RUNTIME.block_on(async move { + let reply_content = RUNTIME.block_on(async move { let timeline_event = room.event(event_id).await.context("Couldn't find event.")?; let event_content = timeline_event .event .deserialize_as::() - .context("Couldn't deserialise event")?; + .context("Couldn't deserialize event")?; let original_message = event_content.as_original().context("Couldn't retrieve original message.")?; - let reply_content = RoomMessageEventContent::text_markdown(msg) - .make_reply_to(original_message, ForwardThread::Yes); + anyhow::Ok( + RoomMessageEventContent::text_markdown(msg) + .make_reply_to(original_message, ForwardThread::Yes), + ) + })?; - timeline.send(reply_content.into(), txn_id.as_deref().map(Into::into)).await?; - - Ok(()) - }) + RUNTIME.spawn(async move { + timeline.send(reply_content.into(), txn_id.as_deref().map(Into::into)).await; + }); + Ok(()) } pub fn edit( @@ -365,7 +370,7 @@ impl Room { let event_id: &EventId = original_event_id.as_str().try_into().context("Failed to create EventId.")?; - RUNTIME.block_on(async move { + let edited_content = RUNTIME.block_on(async move { let timeline_event = room.event(event_id).await.context("Couldn't find event.")?; let event_content = timeline_event @@ -384,11 +389,13 @@ impl Room { let mut edited_content = RoomMessageEventContent::text_markdown(new_msg); edited_content.relates_to = Some(Relation::Replacement(replacement)); + Ok(edited_content) + })?; - timeline.send(edited_content.into(), txn_id.as_deref().map(Into::into)).await?; - - Ok(()) - }) + RUNTIME.spawn(async move { + timeline.send(edited_content.into(), txn_id.as_deref().map(Into::into)).await; + }); + Ok(()) } /// Redacts an event from the room. diff --git a/bindings/matrix-sdk-ffi/src/timeline.rs b/bindings/matrix-sdk-ffi/src/timeline.rs index aae2a3536..a079b44d4 100644 --- a/bindings/matrix-sdk-ffi/src/timeline.rs +++ b/bindings/matrix-sdk-ffi/src/timeline.rs @@ -177,7 +177,7 @@ pub enum EventSendState { NotSendYet, /// The local event has been sent to the server, but unsuccessfully: The /// sending has failed. - SendingFailed, + SendingFailed { error: String }, /// The local event has been sent successfully to the server. Sent { event_id: String }, } @@ -188,7 +188,7 @@ impl From<&matrix_sdk::room::timeline::EventSendState> for EventSendState { match value { NotSentYet => Self::NotSendYet, - SendingFailed => Self::SendingFailed, + SendingFailed { error } => Self::SendingFailed { error: error.to_string() }, Sent { event_id } => Self::Sent { event_id: event_id.to_string() }, } } diff --git a/crates/matrix-sdk/src/room/timeline/event_item.rs b/crates/matrix-sdk/src/room/timeline/event_item.rs index f73c981e2..5a9e64665 100644 --- a/crates/matrix-sdk/src/room/timeline/event_item.rs +++ b/crates/matrix-sdk/src/room/timeline/event_item.rs @@ -52,6 +52,8 @@ use ruma::{ OwnedTransactionId, OwnedUserId, TransactionId, UserId, }; +use crate::Error; + /// An item in the timeline that represents at least one event. /// /// There is always one main event that gives the `EventTimelineItem` its @@ -209,13 +211,16 @@ impl EventTimelineItem { } /// This type represents the "send state" of a local event timeline item. -#[derive(Clone, Debug, PartialEq, Eq)] +#[derive(Clone, Debug)] pub enum EventSendState { /// The local event has not been sent yet. NotSentYet, /// The local event has been sent to the server, but unsuccessfully: The /// sending has failed. - SendingFailed, + SendingFailed { + /// Details about how sending the event failed. + error: Arc, + }, /// The local event has been sent successfully to the server. Sent { /// The event ID assigned by the server. diff --git a/crates/matrix-sdk/src/room/timeline/inner.rs b/crates/matrix-sdk/src/room/timeline/inner.rs index b3ad25ebf..139408c4e 100644 --- a/crates/matrix-sdk/src/room/timeline/inner.rs +++ b/crates/matrix-sdk/src/room/timeline/inner.rs @@ -14,7 +14,6 @@ use matrix_sdk_base::{ locks::Mutex, }; use ruma::{ - api::client::message::send_message_event::v3::Response as SendMessageEventResponse, events::{ fully_read::FullyReadEvent, relation::Annotation, AnyMessageLikeEventContent, AnySyncTimelineEvent, @@ -151,30 +150,6 @@ impl TimelineInner

{ .handle_event(kind); } - /// Handle the response returned by the server when a local event has been - /// sent. - pub(super) fn handle_local_event_send_response( - &self, - txn_id: &TransactionId, - response: crate::error::Result, - ) -> crate::error::Result<()> { - match response { - Ok(response) => { - self.update_event_send_state( - txn_id, - EventSendState::Sent { event_id: response.event_id }, - ); - - Ok(()) - } - Err(error) => { - self.update_event_send_state(txn_id, EventSendState::SendingFailed); - - Err(error) - } - } - } - /// Update the send state of a local event represented by a transaction ID. /// /// If no local event is found, a warning is raised. diff --git a/crates/matrix-sdk/src/room/timeline/mod.rs b/crates/matrix-sdk/src/room/timeline/mod.rs index 18055d981..19fb2aa5b 100644 --- a/crates/matrix-sdk/src/room/timeline/mod.rs +++ b/crates/matrix-sdk/src/room/timeline/mod.rs @@ -370,11 +370,7 @@ impl Timeline { /// [`MessageLikeUnsigned`]: ruma::events::MessageLikeUnsigned /// [`SyncMessageLikeEvent`]: ruma::events::SyncMessageLikeEvent #[instrument(skip(self, content), fields(room_id = ?self.room().room_id()))] - pub async fn send( - &self, - content: AnyMessageLikeEventContent, - txn_id: Option<&TransactionId>, - ) -> Result<()> { + pub async fn send(&self, content: AnyMessageLikeEventContent, txn_id: Option<&TransactionId>) { let txn_id = txn_id.map_or_else(TransactionId::new, ToOwned::to_owned); self.inner.handle_local_event(txn_id.clone(), content.clone()).await; @@ -383,7 +379,12 @@ impl Timeline { let room = Joined { inner: self.room().clone() }; let response = room.send(content, Some(&txn_id)).await; - self.inner.handle_local_event_send_response(&txn_id, response) + + let send_state = match response { + Ok(response) => EventSendState::Sent { event_id: response.event_id }, + Err(error) => EventSendState::SendingFailed { error: Arc::new(error) }, + }; + self.inner.update_event_send_state(&txn_id, send_state); } } diff --git a/crates/matrix-sdk/src/room/timeline/tests.rs b/crates/matrix-sdk/src/room/timeline/tests.rs index e92ea523d..11e651c9a 100644 --- a/crates/matrix-sdk/src/room/timeline/tests.rs +++ b/crates/matrix-sdk/src/room/timeline/tests.rs @@ -14,9 +14,12 @@ //! Unit tests (based on private methods) for the timeline API. -use std::sync::{ - atomic::{AtomicU32, Ordering::SeqCst}, - Arc, +use std::{ + io, + sync::{ + atomic::{AtomicU32, Ordering::SeqCst}, + Arc, + }, }; use assert_matches::assert_matches; @@ -58,7 +61,7 @@ use super::{ EventTimelineItem, MembershipChange, Profile, TimelineInner, TimelineItem, TimelineItemContent, VirtualTimelineItem, }; -use crate::room::timeline::event_item::EventSendState; +use crate::{room::timeline::event_item::EventSendState, Error}; static ALICE: Lazy<&UserId> = Lazy::new(|| user_id!("@alice:server.name")); static BOB: Lazy<&UserId> = Lazy::new(|| user_id!("@bob:other.server")); @@ -388,20 +391,24 @@ async fn remote_echo_full_trip() { { let item = assert_matches!(stream.next().await, Some(VecDiff::Push { value }) => value); let event = item.as_event().unwrap().as_local().unwrap(); - assert_eq!(event.send_state, EventSendState::NotSentYet); + assert_matches!(event.send_state, EventSendState::NotSentYet); } // Scenario 2: The local event has not been sent to the server successfully, it // has failed. In this case, there is no event ID. { - timeline.inner.update_event_send_state(&txn_id, EventSendState::SendingFailed); + let some_io_error = Error::Io(io::Error::new(io::ErrorKind::Other, "this is a test")); + timeline.inner.update_event_send_state( + &txn_id, + EventSendState::SendingFailed { error: Arc::new(some_io_error) }, + ); let item = assert_matches!( stream.next().await, Some(VecDiff::UpdateAt { value, index: 1 }) => value ); let event = item.as_event().unwrap().as_local().unwrap(); - assert_eq!(event.send_state, EventSendState::SendingFailed); + assert_matches!(event.send_state, EventSendState::SendingFailed { .. }); } // Scenario 3: The local event has been sent successfully to the server and an diff --git a/crates/matrix-sdk/tests/integration/room/timeline.rs b/crates/matrix-sdk/tests/integration/room/timeline.rs index 88c4631eb..0bb890a24 100644 --- a/crates/matrix-sdk/tests/integration/room/timeline.rs +++ b/crates/matrix-sdk/tests/integration/room/timeline.rs @@ -194,7 +194,7 @@ async fn echo() { assert_eq!(text.body, "Hello, World!"); // Wait for the sending to finish and assert everything was successful - send_hdl.await.unwrap().unwrap(); + send_hdl.await.unwrap(); let sent_confirmation = assert_matches!( timeline_stream.next().await, diff --git a/labs/jack-in/src/app/model.rs b/labs/jack-in/src/app/model.rs index 435a007c8..5a9b93200 100644 --- a/labs/jack-in/src/app/model.rs +++ b/labs/jack-in/src/app/model.rs @@ -7,7 +7,7 @@ use std::{ops::Deref, time::Duration}; use futures::executor::block_on; use matrix_sdk::{ruma::events::room::message::RoomMessageEventContent, Client}; use tokio::sync::mpsc; -use tracing::{error, info, warn}; +use tracing::warn; use tuirealm::{ props::{Alignment, Borders, Color}, terminal::TerminalBridge, @@ -215,11 +215,7 @@ impl Update for Model { if let Some(tl) = self.sliding_sync.room_timeline.lock_ref().deref() { block_on(async move { // fire and forget - match tl.send(RoomMessageEventContent::text_plain(m).into(), None).await - { - Ok(_r) => info!("Message send"), - Err(e) => error!("Sending message failed: {e}"), - } + tl.send(RoomMessageEventContent::text_plain(m).into(), None).await; }); } else { warn!("asked to send message, but no room is selected"); From e068b4987ed77dee6109357d100cdbdc07402e18 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Wed, 1 Feb 2023 11:51:17 +0100 Subject: [PATCH 08/13] doc(sdk): Document what happens when timeline event sending fails --- crates/matrix-sdk/src/room/timeline/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/matrix-sdk/src/room/timeline/mod.rs b/crates/matrix-sdk/src/room/timeline/mod.rs index 19fb2aa5b..234dd0be4 100644 --- a/crates/matrix-sdk/src/room/timeline/mod.rs +++ b/crates/matrix-sdk/src/room/timeline/mod.rs @@ -351,6 +351,9 @@ impl Timeline { /// If the encryption feature is enabled, this method will transparently /// encrypt the room message if the room is encrypted. /// + /// If sending the message fails, the local echo item will change its + /// `send_state` to [`EventSendState::SendingFailed`]. + /// /// # Arguments /// /// * `content` - The content of the message event. From 8a1b1eccd61fdd57534927d3304310741c9be5eb Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Wed, 1 Feb 2023 11:12:59 +0100 Subject: [PATCH 09/13] doc(sdk): Fix docs that refer to TimelineKey --- crates/matrix-sdk/src/room/timeline/event_item.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/crates/matrix-sdk/src/room/timeline/event_item.rs b/crates/matrix-sdk/src/room/timeline/event_item.rs index 5a9e64665..a78024b1c 100644 --- a/crates/matrix-sdk/src/room/timeline/event_item.rs +++ b/crates/matrix-sdk/src/room/timeline/event_item.rs @@ -57,8 +57,8 @@ use crate::Error; /// An item in the timeline that represents at least one event. /// /// There is always one main event that gives the `EventTimelineItem` its -/// identity (see [key](Self::key)) but in many cases, additional events like -/// reactions and edits are also part of the item. +/// identity but in many cases, additional events like reactions and edits are +/// also part of the item. #[derive(Debug, Clone)] pub enum EventTimelineItem { /// An event item that has been sent, but not yet acknowledged by the @@ -119,10 +119,10 @@ impl EventTimelineItem { /// If this returns `Some(_)`, the event was successfully created by the /// server. /// - /// Even if the [`key()`](Self::key) of this timeline item holds a - /// transaction ID, this can be `Some(_)` as the event ID can be known not - /// just from the remote echo via `sync_events`, but also from the response - /// of the send request that created the event. + /// Even if this is a [`Local`](Self::Local) event,, this can be `Some(_)` + /// as the event ID can be known not just from the remote echo via + /// `sync_events`, but also from the response of the send request that + /// created the event. pub fn event_id(&self) -> Option<&EventId> { match self { Self::Local(local_event) => local_event.event_id(), From ce973b35e9d84a92da188e622b5ef959f51a1268 Mon Sep 17 00:00:00 2001 From: Jonas Platte Date: Wed, 1 Feb 2023 11:22:28 +0100 Subject: [PATCH 10/13] chore: Upgrade uniffi to 0.23.0 --- .cargo/config.toml | 1 + .github/workflows/bindings_ci.yml | 8 -- Cargo.lock | 89 ++++++++++++++----- Cargo.toml | 7 +- bindings/apple/build_crypto_xcframework.sh | 2 +- bindings/kotlin/README.md | 1 - bindings/kotlin/scripts/build_crypto.sh | 5 +- bindings/kotlin/scripts/build_sdk.sh | 5 +- bindings/matrix-sdk-crypto-ffi/Cargo.toml | 3 +- bindings/matrix-sdk-crypto-ffi/build.rs | 2 +- .../matrix-sdk-crypto-ffi/src/uniffi_api.rs | 2 +- bindings/matrix-sdk-ffi/Cargo.toml | 3 +- bindings/matrix-sdk-ffi/build.rs | 2 +- bindings/matrix-sdk-ffi/src/uniffi_api.rs | 2 +- uniffi-bindgen/Cargo.toml | 8 ++ uniffi-bindgen/src/main.rs | 3 + xtask/Cargo.toml | 1 + xtask/src/ci.rs | 4 +- 18 files changed, 95 insertions(+), 53 deletions(-) create mode 100644 uniffi-bindgen/Cargo.toml create mode 100644 uniffi-bindgen/src/main.rs diff --git a/.cargo/config.toml b/.cargo/config.toml index 53c737cc8..c5345f6cd 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -10,6 +10,7 @@ target-applies-to-host = false [alias] xtask = "run --package xtask --" +uniffi-bindgen = "run --package uniffi-bindgen --" [doc.extern-map.registries] crates-io = "https://docs.rs/" diff --git a/.github/workflows/bindings_ci.yml b/.github/workflows/bindings_ci.yml index 1d47bb6ed..b53a81a8c 100644 --- a/.github/workflows/bindings_ci.yml +++ b/.github/workflows/bindings_ci.yml @@ -16,8 +16,6 @@ env: CARGO_TERM_COLOR: always MATRIX_SDK_CRYPTO_NODEJS_PATH: bindings/matrix-sdk-crypto-nodejs MATRIX_SDK_CRYPTO_JS_PATH: bindings/matrix-sdk-crypto-js - # keep in sync with uniffi dependency in root Cargo.toml - UNIFFI_REV: 249a78b6f3f35661f1530e53811134e1bf012608 jobs: xtask-linux: @@ -84,12 +82,6 @@ jobs: path: target/debug/xtask key: xtask-linux-${{ hashFiles('Cargo.toml', 'xtask/**') }} - - name: Install Uniffi - uses: actions-rs/cargo@v1 - with: - command: install - args: uniffi_bindgen --git https://github.com/mozilla/uniffi-rs --rev ${{ env.UNIFFI_REV }} - - name: Build library & generate bindings run: target/debug/xtask ci bindings diff --git a/Cargo.lock b/Cargo.lock index 22a5c59c7..096af4311 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2859,8 +2859,6 @@ dependencies = [ "tracing", "tracing-subscriber", "uniffi", - "uniffi_build", - "uniffi_macros", "vodozemac", "zeroize", ] @@ -2931,8 +2929,6 @@ dependencies = [ "tracing-opentelemetry", "tracing-subscriber", "uniffi", - "uniffi_build", - "uniffi_macros", "zeroize", ] @@ -4748,6 +4744,12 @@ version = "1.6.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "74233d3b3b2f6d4b006dc19dee745e73e2a6bfb6f93607cd3b02bd5b00797d7c" +[[package]] +name = "siphasher" +version = "0.3.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7bd3e3206899af3f8b12af284fafc038cc1dc2b41d1b89dd17297221c5d225de" + [[package]] name = "slab" version = "0.4.7" @@ -5557,29 +5559,36 @@ checksum = "f962df74c8c05a667b5ee8bcf162993134c104e96440b663c8daa176dc772d8c" [[package]] name = "uniffi" -version = "0.21.0" -source = "git+https://github.com/mozilla/uniffi-rs?rev=249a78b6f3f35661f1530e53811134e1bf012608#249a78b6f3f35661f1530e53811134e1bf012608" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f71cc01459bc34cfe43fabf32b39f1228709bc6db1b3a664a92940af3d062376" dependencies = [ "anyhow", - "bytes", "camino", - "log", - "once_cell", - "paste", - "static_assertions", + "clap 3.2.23", + "uniffi_bindgen", + "uniffi_build", + "uniffi_core", "uniffi_macros", ] +[[package]] +name = "uniffi-bindgen" +version = "0.1.0" +dependencies = [ + "uniffi", +] + [[package]] name = "uniffi_bindgen" -version = "0.21.0" -source = "git+https://github.com/mozilla/uniffi-rs?rev=249a78b6f3f35661f1530e53811134e1bf012608#249a78b6f3f35661f1530e53811134e1bf012608" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dbbba5103051c18f10b22f80a74439ddf7100273f217a547005d2735b2498994" dependencies = [ "anyhow", "askama", "bincode", "camino", - "clap 3.2.23", "fs-err", "glob", "goblin", @@ -5596,18 +5605,46 @@ dependencies = [ [[package]] name = "uniffi_build" -version = "0.21.0" -source = "git+https://github.com/mozilla/uniffi-rs?rev=249a78b6f3f35661f1530e53811134e1bf012608#249a78b6f3f35661f1530e53811134e1bf012608" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ee1a28368ff3d83717e3d3e2e15a66269c43488c3f036914131bb68892f29fb" dependencies = [ "anyhow", "camino", "uniffi_bindgen", ] +[[package]] +name = "uniffi_checksum_derive" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "03de61393a42b4ad4984a3763c0600594ac3e57e5aaa1d05cede933958987c03" +dependencies = [ + "quote", + "syn", +] + +[[package]] +name = "uniffi_core" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a2b4852d638d74ca2d70e450475efb6d91fe6d54a7cd8d6bd80ad2ee6cd7daa" +dependencies = [ + "anyhow", + "bytes", + "camino", + "cargo_metadata", + "log", + "once_cell", + "paste", + "static_assertions", +] + [[package]] name = "uniffi_macros" -version = "0.21.0" -source = "git+https://github.com/mozilla/uniffi-rs?rev=249a78b6f3f35661f1530e53811134e1bf012608#249a78b6f3f35661f1530e53811134e1bf012608" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fa03394de21e759e0022f1ea8d992d2e39290d735b9ed52b1f74b20a684f794e" dependencies = [ "bincode", "camino", @@ -5624,16 +5661,20 @@ dependencies = [ [[package]] name = "uniffi_meta" -version = "0.21.0" -source = "git+https://github.com/mozilla/uniffi-rs?rev=249a78b6f3f35661f1530e53811134e1bf012608#249a78b6f3f35661f1530e53811134e1bf012608" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "66fdab2c436aed7a6391bec64204ec33948bfed9b11b303235740771f85c4ea6" dependencies = [ "serde", + "siphasher", + "uniffi_checksum_derive", ] [[package]] name = "uniffi_testing" -version = "0.21.0" -source = "git+https://github.com/mozilla/uniffi-rs?rev=249a78b6f3f35661f1530e53811134e1bf012608#249a78b6f3f35661f1530e53811134e1bf012608" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "92b0570953ec41d97ce23e3b92161ac18231670a1f97523258a6d2ab76d7f76c" dependencies = [ "anyhow", "camino", @@ -5932,7 +5973,8 @@ dependencies = [ [[package]] name = "weedle2" version = "4.0.0" -source = "git+https://github.com/mozilla/uniffi-rs?rev=249a78b6f3f35661f1530e53811134e1bf012608#249a78b6f3f35661f1530e53811134e1bf012608" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2e79c5206e1f43a2306fd64bdb95025ee4228960f2e6c5a8b173f3caaf807741" dependencies = [ "nom", ] @@ -6124,6 +6166,7 @@ dependencies = [ "fs_extra", "serde", "serde_json", + "uniffi", "uniffi_bindgen", "xshell", ] diff --git a/Cargo.toml b/Cargo.toml index 91f4581e7..8e5fad15c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,6 +9,7 @@ members = [ "testing/*", "examples/*", "labs/*", + "uniffi-bindgen", "xtask", ] # xtask, labs, testing and the bindings should only be built when invoked explicitly. @@ -35,10 +36,8 @@ serde_html_form = "0.2.0" serde_json = "1.0.91" thiserror = "1.0.38" tracing = { version = "0.1.36", default-features = false, features = ["std"] } -uniffi = { git = "https://github.com/mozilla/uniffi-rs", rev = "249a78b6f3f35661f1530e53811134e1bf012608" } -uniffi_macros = { git = "https://github.com/mozilla/uniffi-rs", rev = "249a78b6f3f35661f1530e53811134e1bf012608" } -uniffi_bindgen = { git = "https://github.com/mozilla/uniffi-rs", rev = "249a78b6f3f35661f1530e53811134e1bf012608" } -uniffi_build = { git = "https://github.com/mozilla/uniffi-rs", rev = "249a78b6f3f35661f1530e53811134e1bf012608", features = ["builtin-bindgen"] } +uniffi = "0.23.0" +uniffi_bindgen = "0.23.0" vodozemac = { git = "https://github.com/matrix-org/vodozemac", rev = "12b24e909107c1fac23245376f294eaf48ba186a" } zeroize = "1.3.0" diff --git a/bindings/apple/build_crypto_xcframework.sh b/bindings/apple/build_crypto_xcframework.sh index 11e29fd40..db1a9e4e3 100755 --- a/bindings/apple/build_crypto_xcframework.sh +++ b/bindings/apple/build_crypto_xcframework.sh @@ -51,7 +51,7 @@ lipo -create \ -output "${GENERATED_DIR}/simulator/libmatrix_sdk_crypto_ffi.a" # Generate uniffi files -uniffi-bindgen generate \ +cargo uniffi-bindgen generate \ --language swift \ --lib-file "${TARGET_DIR}/aarch64-apple-ios-sim/${REL_TYPE_DIR}/libmatrix_sdk_crypto_ffi.a" \ --config "${SRC_ROOT}/bindings/${TARGET_CRATE}/uniffi.toml" \ diff --git a/bindings/kotlin/README.md b/bindings/kotlin/README.md index 5dbac22bf..ebb206b7e 100644 --- a/bindings/kotlin/README.md +++ b/bindings/kotlin/README.md @@ -5,7 +5,6 @@ This project and build scripts demonstrate how to create an aar and how to impor ## Prerequisites * the Rust toolchain -* UniFFI - `cargo install uniffi_bindgen` * cargo-ndk < 2.12.0 `cargo install cargo-ndk --version 2.11.0` * android targets (e.g. `rustup target add \ aarch64-linux-android \ diff --git a/bindings/kotlin/scripts/build_crypto.sh b/bindings/kotlin/scripts/build_crypto.sh index 86756e848..d0ee97102 100755 --- a/bindings/kotlin/scripts/build_crypto.sh +++ b/bindings/kotlin/scripts/build_crypto.sh @@ -45,12 +45,12 @@ cargo ndk --target i686-linux-android -o ${SDK_TARGET_DIR}/ build "${RELEASE_FLA # Generate uniffi files echo -e "Generate uniffi kotlin file" -uniffi-bindgen generate "${SRC_ROOT}/bindings/${TARGET_CRATE}/src/olm.udl" \ +cargo uniffi-bindgen generate "${SRC_ROOT}/bindings/${TARGET_CRATE}/src/olm.udl" \ --language kotlin \ --config "${SRC_ROOT}/bindings/${TARGET_CRATE}/uniffi.toml" \ --out-dir ${GENERATED_DIR} \ --lib-file "${BASE_TARGET_DIR}/x86_64-linux-android/${RELEASE_TYPE_DIR}/libmatrix_sdk_crypto_ffi.a" - + # Create android library cd "${KOTLIN_ROOT}" ./gradlew :crypto:crypto-android:assemble @@ -64,4 +64,3 @@ echo -e "Cleaning up temporary files" rm -r "${BUILD_DIR}" rm -r "${SDK_TARGET_DIR}" - diff --git a/bindings/kotlin/scripts/build_sdk.sh b/bindings/kotlin/scripts/build_sdk.sh index 30c8d92b6..e7f3249f7 100755 --- a/bindings/kotlin/scripts/build_sdk.sh +++ b/bindings/kotlin/scripts/build_sdk.sh @@ -43,11 +43,11 @@ cargo ndk --target i686-linux-android -o ${SDK_TARGET_DIR}/ build "${RELEASE_FLA # Generate uniffi files echo -e "Generate uniffi kotlin file" -uniffi-bindgen generate "${SRC_ROOT}/bindings/matrix-sdk-ffi/src/api.udl" \ +cargo uniffi-bindgen generate "${SRC_ROOT}/bindings/matrix-sdk-ffi/src/api.udl" \ --language kotlin \ --out-dir ${GENERATED_DIR} \ --lib-file "${BASE_TARGET_DIR}/x86_64-linux-android/${RELEASE_TYPE_DIR}/libmatrix_sdk_ffi.a" - + # Create android library cd "${KOTLIN_ROOT}" ./gradlew :sdk:sdk-android:assemble @@ -61,4 +61,3 @@ echo -e "Cleaning up temporary files" rm -r "${BUILD_DIR}" rm -r "${SDK_TARGET_DIR}" - diff --git a/bindings/matrix-sdk-crypto-ffi/Cargo.toml b/bindings/matrix-sdk-crypto-ffi/Cargo.toml index eeccbf84e..0c8c876b7 100644 --- a/bindings/matrix-sdk-crypto-ffi/Cargo.toml +++ b/bindings/matrix-sdk-crypto-ffi/Cargo.toml @@ -29,7 +29,6 @@ tracing = { workspace = true } tracing-subscriber = { version = "0.3.11", features = ["env-filter"] } # keep in sync with uniffi dependency in matrix-sdk-ffi, and uniffi_bindgen in ffi CI job uniffi = { workspace = true } -uniffi_macros = { workspace = true } vodozemac = { workspace = true } zeroize = { workspace = true, features = ["zeroize_derive"] } @@ -58,7 +57,7 @@ default_features = false features = ["rt-multi-thread"] [build-dependencies] -uniffi_build = { workspace = true, features = ["builtin-bindgen"] } +uniffi = { workspace = true, features = ["build"] } [dev-dependencies] tempfile = "3.3.0" diff --git a/bindings/matrix-sdk-crypto-ffi/build.rs b/bindings/matrix-sdk-crypto-ffi/build.rs index bfce95467..97f5d7ab7 100644 --- a/bindings/matrix-sdk-crypto-ffi/build.rs +++ b/bindings/matrix-sdk-crypto-ffi/build.rs @@ -1,3 +1,3 @@ fn main() { - uniffi_build::generate_scaffolding("./src/olm.udl").unwrap(); + uniffi::generate_scaffolding("./src/olm.udl").unwrap(); } diff --git a/bindings/matrix-sdk-crypto-ffi/src/uniffi_api.rs b/bindings/matrix-sdk-crypto-ffi/src/uniffi_api.rs index e858c71b1..df681245f 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/uniffi_api.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/uniffi_api.rs @@ -2,4 +2,4 @@ use crate::*; -uniffi_macros::include_scaffolding!("olm"); +uniffi::include_scaffolding!("olm"); diff --git a/bindings/matrix-sdk-ffi/Cargo.toml b/bindings/matrix-sdk-ffi/Cargo.toml index b8a8af700..06e5e2fef 100644 --- a/bindings/matrix-sdk-ffi/Cargo.toml +++ b/bindings/matrix-sdk-ffi/Cargo.toml @@ -13,7 +13,7 @@ repository = "https://github.com/matrix-org/matrix-rust-sdk" crate-type = ["cdylib", "staticlib"] [build-dependencies] -uniffi_build = { workspace = true, features = ["builtin-bindgen"] } +uniffi = { workspace = true, features = ["build"] } [dependencies] anyhow = { workspace = true } @@ -35,7 +35,6 @@ tracing-opentelemetry = { version = "0.18.0" } tokio = { version = "1", features = ["rt-multi-thread", "macros"] } tokio-stream = "0.1.8" uniffi = { workspace = true } -uniffi_macros = { workspace = true } zeroize = { workspace = true } diff --git a/bindings/matrix-sdk-ffi/build.rs b/bindings/matrix-sdk-ffi/build.rs index 40ec554ca..9ee0a05d0 100644 --- a/bindings/matrix-sdk-ffi/build.rs +++ b/bindings/matrix-sdk-ffi/build.rs @@ -1,3 +1,3 @@ fn main() { - uniffi_build::generate_scaffolding("./src/api.udl").expect("Building the UDL file failed"); + uniffi::generate_scaffolding("./src/api.udl").expect("Building the UDL file failed"); } diff --git a/bindings/matrix-sdk-ffi/src/uniffi_api.rs b/bindings/matrix-sdk-ffi/src/uniffi_api.rs index 119d1d4e5..3d6676f8f 100644 --- a/bindings/matrix-sdk-ffi/src/uniffi_api.rs +++ b/bindings/matrix-sdk-ffi/src/uniffi_api.rs @@ -2,4 +2,4 @@ use crate::*; -uniffi_macros::include_scaffolding!("api"); +uniffi::include_scaffolding!("api"); diff --git a/uniffi-bindgen/Cargo.toml b/uniffi-bindgen/Cargo.toml new file mode 100644 index 000000000..7b318d646 --- /dev/null +++ b/uniffi-bindgen/Cargo.toml @@ -0,0 +1,8 @@ +[package] +name = "uniffi-bindgen" +version = "0.1.0" +edition = "2021" +publish = false + +[dependencies] +uniffi = { workspace = true, features = ["cli"] } diff --git a/uniffi-bindgen/src/main.rs b/uniffi-bindgen/src/main.rs new file mode 100644 index 000000000..f6cff6cf1 --- /dev/null +++ b/uniffi-bindgen/src/main.rs @@ -0,0 +1,3 @@ +fn main() { + uniffi::uniffi_bindgen_main() +} diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index 47d71d2ac..04f96324c 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -14,5 +14,6 @@ clap = { version = "4.0.18", features = ["derive"] } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } fs_extra = "1" +uniffi = { workspace = true } uniffi_bindgen = { workspace = true } xshell = "0.1.17" diff --git a/xtask/src/ci.rs b/xtask/src/ci.rs index 3a801ffe3..d2c763651 100644 --- a/xtask/src/ci.rs +++ b/xtask/src/ci.rs @@ -127,7 +127,7 @@ fn check_bindings() -> Result<()> { cmd!("rustup run stable cargo build -p matrix-sdk-crypto-ffi -p matrix-sdk-ffi").run()?; cmd!( " - uniffi-bindgen generate + rustup run stable cargo run -p uniffi-bindgen -- generate --language kotlin --language swift --lib-file target/debug/libmatrix_sdk_ffi.a @@ -138,7 +138,7 @@ fn check_bindings() -> Result<()> { .run()?; cmd!( " - uniffi-bindgen generate + rustup run stable cargo run -p uniffi-bindgen -- generate --language kotlin --language swift --lib-file target/debug/libmatrix_sdk_crypto_ffi.a From 00638ba74c2e63cfb09921a6d2db0035f39dd234 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= <76261501+zecakeh@users.noreply.github.com> Date: Thu, 2 Feb 2023 13:06:11 +0100 Subject: [PATCH 11/13] feat(sdk): Allow to fetch replied to messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Kévin Commaille --- bindings/matrix-sdk-ffi/src/timeline.rs | 2 +- crates/matrix-sdk/src/error.rs | 5 + .../src/room/timeline/event_handler.rs | 10 +- .../src/room/timeline/event_item.rs | 113 ++++++++++-- crates/matrix-sdk/src/room/timeline/inner.rs | 85 ++++++++- crates/matrix-sdk/src/room/timeline/mod.rs | 49 +++++- .../tests/integration/room/timeline.rs | 165 +++++++++++++++++- 7 files changed, 405 insertions(+), 24 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/timeline.rs b/bindings/matrix-sdk-ffi/src/timeline.rs index a079b44d4..132c79b06 100644 --- a/bindings/matrix-sdk-ffi/src/timeline.rs +++ b/bindings/matrix-sdk-ffi/src/timeline.rs @@ -463,7 +463,7 @@ impl Message { // This event ID string will be replaced by something more useful later. pub fn in_reply_to(&self) -> Option { - self.0.in_reply_to().map(ToString::to_string) + self.0.in_reply_to().map(|r| r.event_id.to_string()) } pub fn is_edited(&self) -> bool { diff --git a/crates/matrix-sdk/src/error.rs b/crates/matrix-sdk/src/error.rs index de68e557b..687c14867 100644 --- a/crates/matrix-sdk/src/error.rs +++ b/crates/matrix-sdk/src/error.rs @@ -241,6 +241,11 @@ pub enum Error { #[error(transparent)] SlidingSync(#[from] crate::sliding_sync::Error), + /// An error occurred in the timeline. + #[cfg(feature = "experimental-timeline")] + #[error(transparent)] + Timeline(#[from] crate::room::timeline::Error), + /// The client is in inconsistent state. This happens when we set a room to /// a specific type, but then cannot get it in this type. #[error("The internal client state is inconsistent.")] diff --git a/crates/matrix-sdk/src/room/timeline/event_handler.rs b/crates/matrix-sdk/src/room/timeline/event_handler.rs index 3ad83729d..d0fbc3442 100644 --- a/crates/matrix-sdk/src/room/timeline/event_handler.rs +++ b/crates/matrix-sdk/src/room/timeline/event_handler.rs @@ -46,8 +46,9 @@ use super::{ MemberProfileChange, OtherState, Profile, RemoteEventTimelineItem, RoomMembershipChange, Sticker, }, - find_read_marker, rfind_event_by_id, rfind_event_item, EventTimelineItem, Message, - ReactionGroup, TimelineInnerMetadata, TimelineItem, TimelineItemContent, VirtualTimelineItem, + find_read_marker, rfind_event_by_id, rfind_event_item, EventTimelineItem, InReplyToDetails, + Message, ReactionGroup, TimelineInnerMetadata, TimelineItem, TimelineItemContent, + VirtualTimelineItem, }; use crate::{events::SyncTimelineEventWithoutContent, room::timeline::MembershipChange}; @@ -821,10 +822,7 @@ impl NewEventTimelineItem { let edited = relations.replace.is_some(); let content = TimelineItemContent::Message(Message { msgtype: c.msgtype, - in_reply_to: c.relates_to.and_then(|rel| match rel { - message::Relation::Reply { in_reply_to } => Some(in_reply_to.event_id), - _ => None, - }), + in_reply_to: c.relates_to.and_then(InReplyToDetails::from_relation), edited, }); diff --git a/crates/matrix-sdk/src/room/timeline/event_item.rs b/crates/matrix-sdk/src/room/timeline/event_item.rs index a78024b1c..cb0ed7c72 100644 --- a/crates/matrix-sdk/src/room/timeline/event_item.rs +++ b/crates/matrix-sdk/src/room/timeline/event_item.rs @@ -15,7 +15,7 @@ use std::{fmt, ops::Deref, sync::Arc}; use indexmap::IndexMap; -use matrix_sdk_base::deserialized_responses::EncryptionInfo; +use matrix_sdk_base::deserialized_responses::{EncryptionInfo, TimelineEvent}; use ruma::{ events::{ policy::rule::{ @@ -33,7 +33,7 @@ use ruma::{ history_visibility::RoomHistoryVisibilityEventContent, join_rules::RoomJoinRulesEventContent, member::{Change, RoomMemberEventContent}, - message::MessageType, + message::{self, MessageType, Relation}, name::RoomNameEventContent, pinned_events::RoomPinnedEventsEventContent, power_levels::RoomPowerLevelsEventContent, @@ -44,15 +44,16 @@ use ruma::{ }, space::{child::SpaceChildEventContent, parent::SpaceParentEventContent}, sticker::StickerEventContent, - AnyFullStateEventContent, AnySyncTimelineEvent, FullStateEventContent, - MessageLikeEventType, StateEventType, + AnyFullStateEventContent, AnyMessageLikeEventContent, AnySyncTimelineEvent, + AnyTimelineEvent, FullStateEventContent, MessageLikeEventType, StateEventType, }, serde::Raw, EventId, MilliSecondsSinceUnixEpoch, OwnedDeviceId, OwnedEventId, OwnedMxcUri, OwnedTransactionId, OwnedUserId, TransactionId, UserId, }; -use crate::Error; +use super::inner::ProfileProvider; +use crate::{Error, Result}; /// An item in the timeline that represents at least one event. /// @@ -295,6 +296,11 @@ impl RemoteEventTimelineItem { Self { reactions, ..self.clone() } } + /// Clone the current event item, and update its `content`. + pub(super) fn with_content(&self, content: TimelineItemContent) -> Self { + Self { content, ..self.clone() } + } + /// Clone the current event item, change its `content` to /// [`TimelineItemContent::RedactedMessage`], and reset its `reactions`. pub(super) fn to_redacted(&self) -> Self { @@ -364,6 +370,9 @@ pub enum TimelineDetails { /// The details are available. Ready(T), + + /// An error occurred when fetching the details. + Error(Arc), } /// The content of an [`EventTimelineItem`]. @@ -436,10 +445,7 @@ impl TimelineItemContent { #[derive(Clone)] pub struct Message { pub(super) msgtype: MessageType, - // TODO: Add everything required to display the replied-to event, plus a - // 'loading' state that is entered at first, until the user requests the - // reply to be loaded. - pub(super) in_reply_to: Option, + pub(super) in_reply_to: Option, pub(super) edited: bool, } @@ -456,15 +462,19 @@ impl Message { self.msgtype.body() } - /// Get the event ID of the event this message is replying to, if any. - pub fn in_reply_to(&self) -> Option<&EventId> { - self.in_reply_to.as_deref() + /// Get the event this message is replying to, if any. + pub fn in_reply_to(&self) -> Option<&InReplyToDetails> { + self.in_reply_to.as_ref() } /// Get the edit state of this message (has been edited: `true` / `false`). pub fn is_edited(&self) -> bool { self.edited } + + pub(super) fn with_in_reply_to(&self, in_reply_to: InReplyToDetails) -> Self { + Self { in_reply_to: Some(in_reply_to), ..self.clone() } + } } impl fmt::Debug for Message { @@ -475,6 +485,85 @@ impl fmt::Debug for Message { } } +/// Details about an event being replied to. +#[derive(Clone, Debug)] +pub struct InReplyToDetails { + /// The ID of the event. + pub event_id: OwnedEventId, + + /// The details of the event. + /// + /// Use [`Timeline::fetch_item_details`] to fetch the data if it is + /// unavailable. The `replies_nesting_level` field in + /// [`TimelineDetailsSettings`] decides if this should be fetched. + /// + /// [`Timeline::fetch_item_details`]: super::Timeline::fetch_item_details + /// [`TimelineDetailsSettings`]: super::TimelineDetailsSettings + pub details: TimelineDetails>, +} + +impl InReplyToDetails { + pub(super) fn from_relation(relation: Relation) -> Option { + match relation { + message::Relation::Reply { in_reply_to } => { + Some(Self { event_id: in_reply_to.event_id, details: TimelineDetails::Unavailable }) + } + _ => None, + } + } +} + +/// An event that is replied to. +#[derive(Clone, Debug)] +pub struct RepliedToEvent { + pub(super) message: Message, + pub(super) sender: OwnedUserId, + pub(super) sender_profile: Profile, +} + +impl RepliedToEvent { + /// Get the message of this event. + pub fn message(&self) -> &Message { + &self.message + } + + /// Get the sender of this event. + pub fn sender(&self) -> &UserId { + &self.sender + } + + /// Get the profile of the sender. + pub fn sender_profile(&self) -> &Profile { + &self.sender_profile + } + + pub(super) async fn try_from_timeline_event( + timeline_event: TimelineEvent, + profile_provider: &P, + ) -> Result { + let event = match timeline_event.event.deserialize() { + Ok(AnyTimelineEvent::MessageLike(event)) => event, + _ => { + return Err(super::Error::UnsupportedEvent.into()); + } + }; + + let Some(AnyMessageLikeEventContent::RoomMessage(c)) = event.original_content() else { + return Err(super::Error::UnsupportedEvent.into()); + }; + + let message = Message { + msgtype: c.msgtype, + in_reply_to: c.relates_to.and_then(InReplyToDetails::from_relation), + edited: event.relations().replace.is_some(), + }; + let sender = event.sender().to_owned(); + let sender_profile = profile_provider.profile(&sender).await; + + Ok(Self { message, sender, sender_profile }) + } +} + /// Metadata about an `m.room.encrypted` event that could not be decrypted. #[derive(Clone, Debug)] pub enum EncryptedMessage { diff --git a/crates/matrix-sdk/src/room/timeline/inner.rs b/crates/matrix-sdk/src/room/timeline/inner.rs index 139408c4e..7d6b726ed 100644 --- a/crates/matrix-sdk/src/room/timeline/inner.rs +++ b/crates/matrix-sdk/src/room/timeline/inner.rs @@ -31,11 +31,13 @@ use super::{ update_read_marker, Flow, HandleEventResult, TimelineEventHandler, TimelineEventKind, TimelineEventMetadata, TimelineItemPosition, }, - rfind_event_item, EventSendState, EventTimelineItem, Profile, TimelineItem, + rfind_event_by_id, rfind_event_item, EventSendState, EventTimelineItem, InReplyToDetails, + Message, Profile, RepliedToEvent, TimelineDetails, TimelineItem, TimelineItemContent, }; use crate::{ events::SyncTimelineEventWithoutContent, room::{self, timeline::event_item::RemoteEventTimelineItem}, + Result, }; #[derive(Debug)] @@ -368,12 +370,93 @@ impl TimelineInner

{ .await; } } + + fn update_event_item(&self, index: usize, event_item: EventTimelineItem) { + self.items.lock_mut().set_cloned(index, Arc::new(TimelineItem::Event(event_item))) + } } impl TimelineInner { pub(super) fn room(&self) -> &room::Common { &self.profile_provider } + + pub(super) async fn fetch_in_reply_to_details( + &self, + index: usize, + mut item: RemoteEventTimelineItem, + ) -> Result { + let TimelineItemContent::Message(message) = item.content.clone() else { + return Ok(item); + }; + let Some(in_reply_to) = message.in_reply_to() else { + return Ok(item); + }; + + let details = + self.fetch_replied_to_event(index, &item, &message, &in_reply_to.event_id).await; + + // We need to be sure to have the latest position of the event as it might have + // changed while waiting for the request. + let (index, _) = rfind_event_by_id(&self.items(), &item.event_id) + .ok_or(super::Error::RemoteEventNotInTimeline)?; + + item = item.with_content(TimelineItemContent::Message(message.with_in_reply_to( + InReplyToDetails { event_id: in_reply_to.event_id.clone(), details }, + ))); + self.update_event_item(index, item.clone().into()); + + Ok(item) + } + + async fn fetch_replied_to_event( + &self, + index: usize, + item: &RemoteEventTimelineItem, + message: &Message, + in_reply_to: &EventId, + ) -> TimelineDetails> { + if let Some((_, item)) = rfind_event_by_id(&self.items(), in_reply_to) { + let details = match item.content() { + TimelineItemContent::Message(message) => { + TimelineDetails::Ready(Box::new(RepliedToEvent { + message: message.clone(), + sender: item.sender().to_owned(), + sender_profile: item.sender_profile().clone(), + })) + } + _ => TimelineDetails::Error(Arc::new(super::Error::UnsupportedEvent.into())), + }; + + return details; + }; + + self.update_event_item( + index, + item.with_content(TimelineItemContent::Message(message.with_in_reply_to( + InReplyToDetails { + event_id: in_reply_to.to_owned(), + details: TimelineDetails::Pending, + }, + ))) + .into(), + ); + + match self.room().event(in_reply_to).await { + Ok(timeline_event) => { + match RepliedToEvent::try_from_timeline_event( + timeline_event, + &self.profile_provider, + ) + .await + { + Ok(event) => TimelineDetails::Ready(Box::new(event)), + Err(e) => TimelineDetails::Error(Arc::new(e)), + } + } + Err(e) => TimelineDetails::Error(Arc::new(e)), + } + } } #[async_trait] diff --git a/crates/matrix-sdk/src/room/timeline/mod.rs b/crates/matrix-sdk/src/room/timeline/mod.rs index 234dd0be4..458c54d5a 100644 --- a/crates/matrix-sdk/src/room/timeline/mod.rs +++ b/crates/matrix-sdk/src/room/timeline/mod.rs @@ -28,6 +28,7 @@ use ruma::{ events::{fully_read::FullyReadEventContent, AnyMessageLikeEventContent}, EventId, MilliSecondsSinceUnixEpoch, TransactionId, }; +use thiserror::Error; use tracing::{error, instrument, warn}; use super::Joined; @@ -50,8 +51,9 @@ mod virtual_item; pub use self::{ event_item::{ AnyOtherFullStateEventContent, BundledReactions, EncryptedMessage, EventSendState, - EventTimelineItem, MemberProfileChange, MembershipChange, Message, OtherState, Profile, - ReactionGroup, RoomMembershipChange, Sticker, TimelineDetails, TimelineItemContent, + EventTimelineItem, InReplyToDetails, MemberProfileChange, MembershipChange, Message, + OtherState, Profile, ReactionGroup, RepliedToEvent, RoomMembershipChange, Sticker, + TimelineDetails, TimelineItemContent, }, pagination::{PaginationOptions, PaginationOutcome}, virtual_item::VirtualTimelineItem, @@ -389,6 +391,36 @@ impl Timeline { }; self.inner.update_event_send_state(&txn_id, send_state); } + + /// Fetch unavailable details about the event with the given ID. + /// + /// This method only works for IDs of [`RemoteEventTimelineItem`]s, to + /// prevent losing details when a local echo is replaced by its remote + /// echo. + /// + /// This method tries to make all the requests it can. If an error is + /// encountered for a given request, it is forwarded with the + /// [`TimelineDetails::Error`] variant. + /// + /// # Arguments + /// + /// * `event_id` - The event ID of the event to fetch details for. + /// + /// # Errors + /// + /// Returns an error if the identifier doesn't match any event with a remote + /// echo in the timeline, or if the event is removed from the timeline + /// before all requests are handled. + #[instrument(skip(self), fields(room_id = ?self.room().room_id()))] + pub async fn fetch_event_details(&self, event_id: &EventId) -> Result<()> { + let (index, item) = rfind_event_by_id(&self.inner.items(), event_id) + .and_then(|(pos, item)| item.as_remote().map(|item| (pos, item.clone()))) + .ok_or(Error::RemoteEventNotInTimeline)?; + + self.inner.fetch_in_reply_to_details(index, item).await?; + + Ok(()) + } } /// A single entry in timeline. @@ -473,3 +505,16 @@ fn rfind_event_by_id<'a>( fn find_read_marker(items: &[Arc]) -> Option { items.iter().rposition(|item| item.is_read_marker()) } + +/// Errors specific to the timeline. +#[derive(Error, Debug)] +#[non_exhaustive] +pub enum Error { + /// The requested event with a remote echo is not in the timeline. + #[error("Event with remote echo not found in timeline")] + RemoteEventNotInTimeline, + + /// The event is currently unsupported for this use case. + #[error("Unsupported event")] + UnsupportedEvent, +} diff --git a/crates/matrix-sdk/tests/integration/room/timeline.rs b/crates/matrix-sdk/tests/integration/room/timeline.rs index 0bb890a24..8aab02552 100644 --- a/crates/matrix-sdk/tests/integration/room/timeline.rs +++ b/crates/matrix-sdk/tests/integration/room/timeline.rs @@ -8,10 +8,11 @@ use futures_util::StreamExt; use matrix_sdk::{ config::SyncSettings, room::timeline::{ - AnyOtherFullStateEventContent, EventSendState, PaginationOptions, TimelineItemContent, - VirtualTimelineItem, + AnyOtherFullStateEventContent, Error as TimelineError, EventSendState, PaginationOptions, + TimelineDetails, TimelineItemContent, VirtualTimelineItem, }, ruma::MilliSecondsSinceUnixEpoch, + Error, }; use matrix_sdk_common::executor::spawn; use matrix_sdk_test::{ @@ -553,3 +554,163 @@ async fn read_marker() { assert_matches!(timeline_stream.next().await, Some(VecDiff::Push { value }) => value); assert_matches!(marker.as_virtual().unwrap(), VirtualTimelineItem::ReadMarker); } + +#[async_test] +async fn in_reply_to_details() { + let room_id = room_id!("!a98sd12bjh:example.org"); + let (client, server) = logged_in_client().await; + let sync_settings = SyncSettings::new().timeout(Duration::from_millis(3000)); + + let mut ev_builder = EventBuilder::new(); + ev_builder.add_joined_room(JoinedRoomBuilder::new(room_id)); + + mock_sync(&server, ev_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + let room = client.get_room(room_id).unwrap(); + let timeline = room.timeline().await; + let mut timeline_stream = timeline.signal().to_stream(); + + // The event doesn't exist. + assert_matches!( + timeline.fetch_event_details(event_id!("$fakeevent")).await, + Err(Error::Timeline(TimelineError::RemoteEventNotInTimeline)) + ); + + ev_builder.add_joined_room( + JoinedRoomBuilder::new(room_id) + .add_timeline_event(TimelineTestEvent::Custom(json!({ + "content": { + "body": "hello", + "msgtype": "m.text", + }, + "event_id": "$event1", + "origin_server_ts": 152037280, + "sender": "@alice:example.org", + "type": "m.room.message", + }))) + .add_timeline_event(TimelineTestEvent::Custom(json!({ + "content": { + "body": "hello to you too", + "msgtype": "m.text", + "m.relates_to": { + "m.in_reply_to": { + "event_id": "$event1", + }, + }, + }, + "event_id": "$event2", + "origin_server_ts": 152045456, + "sender": "@bob:example.org", + "type": "m.room.message", + }))), + ); + + mock_sync(&server, ev_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + let _day_divider = + assert_matches!(timeline_stream.next().await, Some(VecDiff::Push { value }) => value); + let first = + assert_matches!(timeline_stream.next().await, Some(VecDiff::Push { value }) => value); + assert_matches!(first.as_event().unwrap().content(), TimelineItemContent::Message(_)); + let second = + assert_matches!(timeline_stream.next().await, Some(VecDiff::Push { value }) => value); + let second_event = second.as_event().unwrap().as_remote().unwrap(); + let message = + assert_matches!(&second_event.content, TimelineItemContent::Message(message) => message); + let in_reply_to = message.in_reply_to().unwrap(); + assert_eq!(in_reply_to.event_id, event_id!("$event1")); + assert_matches!(in_reply_to.details, TimelineDetails::Unavailable); + + // Fetch details locally first. + timeline.fetch_event_details(&second_event.event_id).await.unwrap(); + + let second = assert_matches!(timeline_stream.next().await, Some(VecDiff::UpdateAt { index: 2, value }) => value); + let message = assert_matches!(second.as_event().unwrap().content(), TimelineItemContent::Message(message) => message); + assert_matches!(message.in_reply_to().unwrap().details, TimelineDetails::Ready(_)); + + ev_builder.add_joined_room(JoinedRoomBuilder::new(room_id).add_timeline_event( + TimelineTestEvent::Custom(json!({ + "content": { + "body": "you were right", + "msgtype": "m.text", + "m.relates_to": { + "m.in_reply_to": { + "event_id": "$remoteevent", + }, + }, + }, + "event_id": "$event3", + "origin_server_ts": 152046694, + "sender": "@bob:example.org", + "type": "m.room.message", + })), + )); + + mock_sync(&server, ev_builder.build_json_sync_response(), None).await; + let _response = client.sync_once(sync_settings.clone()).await.unwrap(); + server.reset().await; + + let third = + assert_matches!(timeline_stream.next().await, Some(VecDiff::Push { value }) => value); + let third_event = third.as_event().unwrap().as_remote().unwrap(); + let message = + assert_matches!(&third_event.content, TimelineItemContent::Message(message) => message); + let in_reply_to = message.in_reply_to().unwrap(); + assert_eq!(in_reply_to.event_id, event_id!("$remoteevent")); + assert_matches!(in_reply_to.details, TimelineDetails::Unavailable); + + Mock::given(method("GET")) + .and(path_regex(r"^/_matrix/client/r0/rooms/.*/event/\$remoteevent")) + .and(header("authorization", "Bearer 1234")) + .respond_with(ResponseTemplate::new(404).set_body_json(json!({ + "errcode": "M_NOT_FOUND", + "error": "Event not found.", + }))) + .expect(1) + .mount(&server) + .await; + + // Fetch details remotely if we can't find them locally. + timeline.fetch_event_details(&third_event.event_id).await.unwrap(); + server.reset().await; + + let third = assert_matches!(timeline_stream.next().await, Some(VecDiff::UpdateAt { index: 3, value }) => value); + let message = assert_matches!(third.as_event().unwrap().content(), TimelineItemContent::Message(message) => message); + assert_matches!(message.in_reply_to().unwrap().details, TimelineDetails::Pending); + + let third = assert_matches!(timeline_stream.next().await, Some(VecDiff::UpdateAt { index: 3, value }) => value); + let message = assert_matches!(third.as_event().unwrap().content(), TimelineItemContent::Message(message) => message); + assert_matches!(message.in_reply_to().unwrap().details, TimelineDetails::Error(_)); + + Mock::given(method("GET")) + .and(path_regex(r"^/_matrix/client/r0/rooms/.*/event/\$remoteevent")) + .and(header("authorization", "Bearer 1234")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "content": { + "body": "Alice is gonna arrive soon", + "msgtype": "m.text", + }, + "room_id": room_id, + "event_id": "$event0", + "origin_server_ts": 152024004, + "sender": "@admin:example.org", + "type": "m.room.message", + }))) + .expect(1) + .mount(&server) + .await; + + timeline.fetch_event_details(&third_event.event_id).await.unwrap(); + + let third = assert_matches!(timeline_stream.next().await, Some(VecDiff::UpdateAt { index: 3, value }) => value); + let message = assert_matches!(third.as_event().unwrap().content(), TimelineItemContent::Message(message) => message); + assert_matches!(message.in_reply_to().unwrap().details, TimelineDetails::Pending); + + let third = assert_matches!(timeline_stream.next().await, Some(VecDiff::UpdateAt { index: 3, value }) => value); + let message = assert_matches!(third.as_event().unwrap().content(), TimelineItemContent::Message(message) => message); + assert_matches!(message.in_reply_to().unwrap().details, TimelineDetails::Ready(_)); +} From a9ba2dd54608e548c8027cdee51c666e5dbe73a4 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 2 Feb 2023 13:25:36 +0100 Subject: [PATCH 12/13] feat(sdk): `SlidingSync::get_room` now takes a ref to `OwnedRoomId`. Before this patch, `SlidingSync::get_room` was taking ownership of an `OwnedRoomId`, to only use it as a reference. Thus, calling this method was creating useless clones. This patch updates `SlidingSync::get_room` to receive a reference to `OwnedRoomId`. --- bindings/matrix-sdk-ffi/src/sliding_sync.rs | 3 ++- crates/matrix-sdk/src/sliding_sync.rs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/sliding_sync.rs b/bindings/matrix-sdk-ffi/src/sliding_sync.rs index 1c3d76bd4..1e3c749ff 100644 --- a/bindings/matrix-sdk-ffi/src/sliding_sync.rs +++ b/bindings/matrix-sdk-ffi/src/sliding_sync.rs @@ -642,7 +642,8 @@ impl SlidingSync { pub fn get_room(&self, room_id: String) -> anyhow::Result>> { let runner = self.inner.clone(); - Ok(self.inner.get_room(OwnedRoomId::try_from(room_id)?).map(|inner| { + + Ok(self.inner.get_room(&OwnedRoomId::try_from(room_id)?).map(|inner| { Arc::new(SlidingSyncRoom { inner, runner, diff --git a/crates/matrix-sdk/src/sliding_sync.rs b/crates/matrix-sdk/src/sliding_sync.rs index bf91446d1..f6aa77bab 100644 --- a/crates/matrix-sdk/src/sliding_sync.rs +++ b/crates/matrix-sdk/src/sliding_sync.rs @@ -814,8 +814,8 @@ impl SlidingSync { } /// Lookup a specific room - pub fn get_room(&self, room_id: OwnedRoomId) -> Option { - self.rooms.lock_ref().get(&room_id).cloned() + pub fn get_room(&self, room_id: &OwnedRoomId) -> Option { + self.rooms.lock_ref().get(room_id).cloned() } fn update_to_device_since(&self, since: String) { From ae8a8c6cc3d7c9eb00afe547dc436299a88be122 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 2 Feb 2023 13:36:32 +0100 Subject: [PATCH 13/13] feat(sdk): `SldingSync::get_room` takes a `&RoomId`. --- bindings/matrix-sdk-ffi/src/sliding_sync.rs | 4 ++-- crates/matrix-sdk/src/sliding_sync.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/sliding_sync.rs b/bindings/matrix-sdk-ffi/src/sliding_sync.rs index 1e3c749ff..408e93745 100644 --- a/bindings/matrix-sdk-ffi/src/sliding_sync.rs +++ b/bindings/matrix-sdk-ffi/src/sliding_sync.rs @@ -13,7 +13,7 @@ use matrix_sdk::ruma::{ v4::RoomSubscription as RumaRoomSubscription, UnreadNotificationsCount as RumaUnreadNotificationsCount, }, - assign, IdParseError, OwnedRoomId, UInt, + assign, IdParseError, OwnedRoomId, RoomId, UInt, }; pub use matrix_sdk::{ room::timeline::Timeline, ruma::api::client::sync::sync_events::v4::SyncRequestListFilters, @@ -643,7 +643,7 @@ impl SlidingSync { pub fn get_room(&self, room_id: String) -> anyhow::Result>> { let runner = self.inner.clone(); - Ok(self.inner.get_room(&OwnedRoomId::try_from(room_id)?).map(|inner| { + Ok(self.inner.get_room(<&RoomId>::try_from(room_id.as_str())?).map(|inner| { Arc::new(SlidingSyncRoom { inner, runner, diff --git a/crates/matrix-sdk/src/sliding_sync.rs b/crates/matrix-sdk/src/sliding_sync.rs index f6aa77bab..56d693a71 100644 --- a/crates/matrix-sdk/src/sliding_sync.rs +++ b/crates/matrix-sdk/src/sliding_sync.rs @@ -814,7 +814,7 @@ impl SlidingSync { } /// Lookup a specific room - pub fn get_room(&self, room_id: &OwnedRoomId) -> Option { + pub fn get_room(&self, room_id: &RoomId) -> Option { self.rooms.lock_ref().get(room_id).cloned() }