From 18c155beb5ea10acbb3622c14912785e141d0357 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Thu, 22 Feb 2024 17:50:07 +0100 Subject: [PATCH 01/74] feat: room_directory_sync basic sync loop --- crates/matrix-sdk/src/lib.rs | 1 + crates/matrix-sdk/src/room_directory_sync.rs | 53 ++++++++++++++++++++ 2 files changed, 54 insertions(+) create mode 100644 crates/matrix-sdk/src/room_directory_sync.rs diff --git a/crates/matrix-sdk/src/lib.rs b/crates/matrix-sdk/src/lib.rs index 63211aa91be..77cb7f60840 100644 --- a/crates/matrix-sdk/src/lib.rs +++ b/crates/matrix-sdk/src/lib.rs @@ -48,6 +48,7 @@ pub mod notification_settings; #[cfg(feature = "experimental-oidc")] pub mod oidc; pub mod room; +pub mod room_directory_sync; pub mod utils; pub mod futures { //! Named futures returned from methods on types in [the crate root][crate]. diff --git a/crates/matrix-sdk/src/room_directory_sync.rs b/crates/matrix-sdk/src/room_directory_sync.rs new file mode 100644 index 00000000000..abecfabce72 --- /dev/null +++ b/crates/matrix-sdk/src/room_directory_sync.rs @@ -0,0 +1,53 @@ +// Copyright 2024 Mauro Romito +// Copyright 2024 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 eyeball_im::ObservableVector; +use futures_core::Stream; +use ruma::OwnedRoomId; + +use crate::Client; + +#[derive(Clone)] +struct RoomDescription { + room_id: OwnedRoomId, +} +struct RoomDirectorySync { + next_token: Option, + client: Client, + results: ObservableVector, +} + +impl RoomDirectorySync { + async fn sync(&mut self) { + // TODO: we may want to have the limit be a configurable parameter of the sync? + // TODO: same for the server? + loop { + let response = self.client.public_rooms(None, self.next_token.as_deref(), None).await; + if let Ok(response) = response { + self.next_token = response.next_batch.clone(); + self.results.append( + response + .chunk + .into_iter() + .map(|room| RoomDescription { room_id: room.room_id }) + .collect(), + ); + if self.next_token.is_none() { + break; + } + } + } + } +} From 8ac6845607665aca6afb48b283c69827c1b45d9c Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Mon, 26 Feb 2024 17:32:52 +0100 Subject: [PATCH 02/74] feat: paginated public room search --- crates/matrix-sdk/src/lib.rs | 2 +- .../matrix-sdk/src/room_directory_search.rs | 97 +++++++++++++++++++ crates/matrix-sdk/src/room_directory_sync.rs | 53 ---------- .../src/tests.rs | 1 + .../src/tests/room_directory_search.rs | 71 ++++++++++++++ 5 files changed, 170 insertions(+), 54 deletions(-) create mode 100644 crates/matrix-sdk/src/room_directory_search.rs delete mode 100644 crates/matrix-sdk/src/room_directory_sync.rs create mode 100644 testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs diff --git a/crates/matrix-sdk/src/lib.rs b/crates/matrix-sdk/src/lib.rs index 77cb7f60840..4134fa3c9a3 100644 --- a/crates/matrix-sdk/src/lib.rs +++ b/crates/matrix-sdk/src/lib.rs @@ -48,7 +48,7 @@ pub mod notification_settings; #[cfg(feature = "experimental-oidc")] pub mod oidc; pub mod room; -pub mod room_directory_sync; +pub mod room_directory_search; pub mod utils; pub mod futures { //! Named futures returned from methods on types in [the crate root][crate]. diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs new file mode 100644 index 00000000000..13a94032aed --- /dev/null +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -0,0 +1,97 @@ +// Copyright 2024 Mauro Romito +// Copyright 2024 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 eyeball_im::{ObservableVector, VectorDiff}; +use futures_core::Stream; +use ruma::{ + api::client::directory::get_public_rooms_filtered::v3::Request as PublicRoomsFilterRequest, + directory::{Filter, PublicRoomJoinRule}, + OwnedMxcUri, OwnedRoomAliasId, OwnedRoomId, +}; + +use crate::Client; + +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct RoomDescription { + pub room_id: OwnedRoomId, + pub name: Option, + pub topic: Option, + pub alias: Option, + pub avatar_url: Option, + pub join_rule: PublicRoomJoinRule, + pub is_world_readable: bool, + pub joined_members: u64, +} + +pub struct RoomDirectorySearch { + batch_size: u32, + filter: Option, + next_token: Option, + client: Client, + results: ObservableVector, +} + +impl RoomDirectorySearch { + pub fn new(client: Client) -> Self { + Self { + batch_size: 0, + filter: None, + next_token: None, + client, + results: ObservableVector::new(), + } + } + + pub async fn search(&mut self, filter: Option, batch_size: u32) { + self.filter = filter; + self.batch_size = batch_size; + self.next_token = None; + self.results.clear(); + self.next_page().await; + } + + pub async fn next_page(&mut self) { + let mut filter = Filter::new(); + filter.generic_search_term = self.filter.clone(); + + let mut request = PublicRoomsFilterRequest::new(); + request.filter = filter; + request.limit = Some(self.batch_size.into()); + request.since = self.next_token.clone(); + if let Ok(response) = self.client.public_rooms_filtered(request).await { + self.next_token = response.next_batch; + self.results.append( + response + .chunk + .into_iter() + .map(|room| RoomDescription { + room_id: room.room_id, + name: room.name, + topic: room.topic, + alias: room.canonical_alias, + avatar_url: room.avatar_url, + join_rule: room.join_rule, + is_world_readable: room.world_readable, + joined_members: room.num_joined_members.into(), + }) + .collect(), + ); + } + } + + pub fn results(&self) -> impl Stream> { + self.results.subscribe().into_stream() + } +} diff --git a/crates/matrix-sdk/src/room_directory_sync.rs b/crates/matrix-sdk/src/room_directory_sync.rs deleted file mode 100644 index abecfabce72..00000000000 --- a/crates/matrix-sdk/src/room_directory_sync.rs +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright 2024 Mauro Romito -// Copyright 2024 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 eyeball_im::ObservableVector; -use futures_core::Stream; -use ruma::OwnedRoomId; - -use crate::Client; - -#[derive(Clone)] -struct RoomDescription { - room_id: OwnedRoomId, -} -struct RoomDirectorySync { - next_token: Option, - client: Client, - results: ObservableVector, -} - -impl RoomDirectorySync { - async fn sync(&mut self) { - // TODO: we may want to have the limit be a configurable parameter of the sync? - // TODO: same for the server? - loop { - let response = self.client.public_rooms(None, self.next_token.as_deref(), None).await; - if let Ok(response) = response { - self.next_token = response.next_batch.clone(); - self.results.append( - response - .chunk - .into_iter() - .map(|room| RoomDescription { room_id: room.room_id }) - .collect(), - ); - if self.next_token.is_none() { - break; - } - } - } - } -} diff --git a/testing/matrix-sdk-integration-testing/src/tests.rs b/testing/matrix-sdk-integration-testing/src/tests.rs index ee80945ad2f..507d8e4b6ab 100644 --- a/testing/matrix-sdk-integration-testing/src/tests.rs +++ b/testing/matrix-sdk-integration-testing/src/tests.rs @@ -4,4 +4,5 @@ mod invitations; mod reactions; mod redaction; mod repeated_join; +mod room_directory_search; mod sliding_sync; diff --git a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs new file mode 100644 index 00000000000..9e3d93a42aa --- /dev/null +++ b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs @@ -0,0 +1,71 @@ +// Copyright 2024 Mauro Romito +// Copyright 2024 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 anyhow::Result; +use eyeball_im::VectorDiff; +use futures::{FutureExt, StreamExt}; +use matrix_sdk::{ + room_directory_search::RoomDirectorySearch, + ruma::api::client::room::{create_room::v3::Request as CreateRoomRequest, Visibility}, +}; +use stream_assert::{assert_next_eq, assert_pending}; + +use crate::helpers::TestClientBuilder; + +#[tokio::test(flavor = "multi_thread")] +async fn test_room_directory_search_no_filter() -> Result<()> { + let alice = TestClientBuilder::new("alice".to_owned()).use_sqlite().build().await?; + for _ in 0..10 { + let mut request: CreateRoomRequest = CreateRoomRequest::new(); + request.visibility = Visibility::Public; + alice.create_room(request).await?; + } + let mut room_directory_search = RoomDirectorySearch::new(alice); + let mut stream = room_directory_search.results(); + room_directory_search.search(None, 10).await; + assert_next_eq!(stream, VectorDiff::Clear); + if let VectorDiff::Append { values } = stream.next().now_or_never().unwrap().unwrap() { + assert_eq!(values.len(), 10); + } else { + panic!("Expected a Vector::Append"); + } + assert_pending!(stream); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_room_directory_search_filter() -> Result<()> { + let alice = TestClientBuilder::new("alice".to_owned()).use_sqlite().build().await?; + let mut request: CreateRoomRequest = CreateRoomRequest::new(); + request.visibility = Visibility::Public; + alice.create_room(request).await?; + + let mut request: CreateRoomRequest = CreateRoomRequest::new(); + request.visibility = Visibility::Public; + request.name = Some("test".to_owned()); + alice.create_room(request).await?; + + let mut room_directory_search = RoomDirectorySearch::new(alice); + let mut stream = room_directory_search.results(); + room_directory_search.search(Some("test".to_owned()), 10).await; + assert_next_eq!(stream, VectorDiff::Clear); + if let VectorDiff::Append { values } = stream.next().now_or_never().unwrap().unwrap() { + assert_eq!(values.len(), 1); + } else { + panic!("Expected a Vector::Append"); + } + assert_pending!(stream); + Ok(()) +} From a79c5286d7c7764e5b0f1ae8b939cf51722ec525 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 27 Feb 2024 10:58:13 +0100 Subject: [PATCH 03/74] improved the tests --- .../matrix-sdk/src/room_directory_search.rs | 16 ++++++++++++++ .../src/tests/room_directory_search.rs | 22 ++++++++++++++++--- 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index 13a94032aed..844f12df934 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -41,6 +41,7 @@ pub struct RoomDirectorySearch { next_token: Option, client: Client, results: ObservableVector, + is_at_last_page: bool, } impl RoomDirectorySearch { @@ -51,6 +52,7 @@ impl RoomDirectorySearch { next_token: None, client, results: ObservableVector::new(), + is_at_last_page: false, } } @@ -59,10 +61,14 @@ impl RoomDirectorySearch { self.batch_size = batch_size; self.next_token = None; self.results.clear(); + self.is_at_last_page = false; self.next_page().await; } pub async fn next_page(&mut self) { + if self.is_at_last_page { + return; + } let mut filter = Filter::new(); filter.generic_search_term = self.filter.clone(); @@ -72,6 +78,9 @@ impl RoomDirectorySearch { request.since = self.next_token.clone(); if let Ok(response) = self.client.public_rooms_filtered(request).await { self.next_token = response.next_batch; + if self.next_token.is_none() { + self.is_at_last_page = true; + } self.results.append( response .chunk @@ -94,4 +103,11 @@ impl RoomDirectorySearch { pub fn results(&self) -> impl Stream> { self.results.subscribe().into_stream() } + + pub fn loaded_pages(&self) -> usize { + if self.batch_size == 0 { + return 0; + } + self.results.len() / self.batch_size as usize + } } diff --git a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs index 9e3d93a42aa..0bfc61caa27 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs @@ -21,27 +21,43 @@ use matrix_sdk::{ ruma::api::client::room::{create_room::v3::Request as CreateRoomRequest, Visibility}, }; use stream_assert::{assert_next_eq, assert_pending}; +use tracing::warn; use crate::helpers::TestClientBuilder; #[tokio::test(flavor = "multi_thread")] async fn test_room_directory_search_no_filter() -> Result<()> { let alice = TestClientBuilder::new("alice".to_owned()).use_sqlite().build().await?; - for _ in 0..10 { + for index in 0..8 { let mut request: CreateRoomRequest = CreateRoomRequest::new(); request.visibility = Visibility::Public; + let name = format!("test_room_{}", index); + request.name = Some(name); alice.create_room(request).await?; } let mut room_directory_search = RoomDirectorySearch::new(alice); let mut stream = room_directory_search.results(); - room_directory_search.search(None, 10).await; + room_directory_search.search(None, 5).await; assert_next_eq!(stream, VectorDiff::Clear); if let VectorDiff::Append { values } = stream.next().now_or_never().unwrap().unwrap() { - assert_eq!(values.len(), 10); + warn!("Values: {:?}", values); + assert_eq!(values.len(), 5); } else { panic!("Expected a Vector::Append"); } assert_pending!(stream); + + room_directory_search.next_page().await; + if let VectorDiff::Append { values } = stream.next().now_or_never().unwrap().unwrap() { + warn!("Values: {:?}", values); + assert_eq!(values.len(), 3); + } else { + panic!("Expected a Vector::Append"); + } + assert_pending!(stream); + + room_directory_search.next_page().await; + assert_pending!(stream); Ok(()) } From 26b0b32e5505901a7167dbf03377cfaed60a8b0b Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 27 Feb 2024 14:37:22 +0100 Subject: [PATCH 04/74] feat(bindings): ffi layer started implementation also improved the integration test for the filtered case --- bindings/matrix-sdk-ffi/src/client.rs | 9 +++ bindings/matrix-sdk-ffi/src/lib.rs | 1 + .../src/room_directory_search.rs | 73 +++++++++++++++++++ .../matrix-sdk/src/room_directory_search.rs | 56 +++++++------- .../src/tests/room_directory_search.rs | 62 ++++++---------- 5 files changed, 137 insertions(+), 64 deletions(-) create mode 100644 bindings/matrix-sdk-ffi/src/room_directory_search.rs diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index 54406134ef0..cde3c488a2c 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -58,6 +58,7 @@ use crate::{ encryption::Encryption, notification::NotificationClientBuilder, notification_settings::NotificationSettings, + room_directory_search::RoomDirectorySearch, sync_service::{SyncService, SyncServiceBuilder}, task_handle::TaskHandle, ClientError, @@ -740,6 +741,14 @@ impl Client { } }))) } + + pub fn room_directory_search(&self) -> Arc { + Arc::new(RoomDirectorySearch { + inner: matrix_sdk::room_directory_search::RoomDirectorySearch::new( + (*self.inner).clone(), + ), + }) + } } #[uniffi::export(callback_interface)] diff --git a/bindings/matrix-sdk-ffi/src/lib.rs b/bindings/matrix-sdk-ffi/src/lib.rs index bcc60db3a56..d90deea3e10 100644 --- a/bindings/matrix-sdk-ffi/src/lib.rs +++ b/bindings/matrix-sdk-ffi/src/lib.rs @@ -32,6 +32,7 @@ mod notification; mod notification_settings; mod platform; mod room; +mod room_directory_search; mod room_info; mod room_list; mod room_member; diff --git a/bindings/matrix-sdk-ffi/src/room_directory_search.rs b/bindings/matrix-sdk-ffi/src/room_directory_search.rs new file mode 100644 index 00000000000..11dff161984 --- /dev/null +++ b/bindings/matrix-sdk-ffi/src/room_directory_search.rs @@ -0,0 +1,73 @@ +// Copyright 2024 Mauro Romito +// Copyright 2024 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 matrix_sdk::{room_directory_search::RoomDirectorySearch as SdkRoomDirectorySearch, Client}; + +use crate::error::ClientError; + +#[derive(uniffi::Enum)] +pub enum PublicRoomJoinRule { + Public, + Knock, +} + +impl PublicRoomJoinRule { + fn convert(value: ruma::directory::PublicRoomJoinRule) -> Option { + match value { + ruma::directory::PublicRoomJoinRule::Public => Some(Self::Public), + ruma::directory::PublicRoomJoinRule::Knock => Some(Self::Knock), + _ => None, + } + } +} + +#[derive(uniffi::Record)] +pub struct RoomDescription { + pub room_id: String, + pub name: Option, + pub topic: Option, + pub alias: Option, + pub avatar_url: Option, + pub join_rule: Option, + pub is_world_readable: bool, + pub joined_members: u64, +} + +impl From for RoomDescription { + fn from(value: matrix_sdk::room_directory_search::RoomDescription) -> Self { + Self { + room_id: value.room_id.to_string(), + name: value.name, + topic: value.topic, + alias: value.alias.map(|alias| alias.to_string()), + avatar_url: value.avatar_url.map(|url| url.to_string()), + join_rule: PublicRoomJoinRule::convert(value.join_rule), + is_world_readable: value.is_world_readable, + joined_members: value.joined_members, + } + } +} + +#[derive(uniffi::Object)] +pub struct RoomDirectorySearch { + pub(crate) inner: SdkRoomDirectorySearch, +} + +// #[uniffi::export(async_runtime = "tokio")] +// impl RoomDirectorySearch { +// pub async fn next_page(self) -> Result<(), ClientError> { +// self.inner.next_page().await.map_err(Into::into) +// } +// } diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index 844f12df934..be85138c25c 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -21,7 +21,7 @@ use ruma::{ OwnedMxcUri, OwnedRoomAliasId, OwnedRoomId, }; -use crate::Client; +use crate::{Client, Result}; #[derive(Clone, Debug, Eq, PartialEq)] pub struct RoomDescription { @@ -56,18 +56,18 @@ impl RoomDirectorySearch { } } - pub async fn search(&mut self, filter: Option, batch_size: u32) { + pub async fn search(&mut self, filter: Option, batch_size: u32) -> Result<()> { self.filter = filter; self.batch_size = batch_size; self.next_token = None; self.results.clear(); self.is_at_last_page = false; - self.next_page().await; + self.next_page().await } - pub async fn next_page(&mut self) { + pub async fn next_page(&mut self) -> Result<()> { if self.is_at_last_page { - return; + return Ok(()); } let mut filter = Filter::new(); filter.generic_search_term = self.filter.clone(); @@ -76,28 +76,28 @@ impl RoomDirectorySearch { request.filter = filter; request.limit = Some(self.batch_size.into()); request.since = self.next_token.clone(); - if let Ok(response) = self.client.public_rooms_filtered(request).await { - self.next_token = response.next_batch; - if self.next_token.is_none() { - self.is_at_last_page = true; - } - self.results.append( - response - .chunk - .into_iter() - .map(|room| RoomDescription { - room_id: room.room_id, - name: room.name, - topic: room.topic, - alias: room.canonical_alias, - avatar_url: room.avatar_url, - join_rule: room.join_rule, - is_world_readable: room.world_readable, - joined_members: room.num_joined_members.into(), - }) - .collect(), - ); + let response = self.client.public_rooms_filtered(request).await?; + self.next_token = response.next_batch; + if self.next_token.is_none() { + self.is_at_last_page = true; } + self.results.append( + response + .chunk + .into_iter() + .map(|room| RoomDescription { + room_id: room.room_id, + name: room.name, + topic: room.topic, + alias: room.canonical_alias, + avatar_url: room.avatar_url, + join_rule: room.join_rule, + is_world_readable: room.world_readable, + joined_members: room.num_joined_members.into(), + }) + .collect(), + ); + Ok(()) } pub fn results(&self) -> impl Stream> { @@ -110,4 +110,8 @@ impl RoomDirectorySearch { } self.results.len() / self.batch_size as usize } + + pub fn is_at_last_page(&self) -> bool { + self.is_at_last_page + } } diff --git a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs index 0bfc61caa27..5eaf5e262c5 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs @@ -20,68 +20,54 @@ use matrix_sdk::{ room_directory_search::RoomDirectorySearch, ruma::api::client::room::{create_room::v3::Request as CreateRoomRequest, Visibility}, }; +use rand::{thread_rng, Rng}; use stream_assert::{assert_next_eq, assert_pending}; use tracing::warn; use crate::helpers::TestClientBuilder; #[tokio::test(flavor = "multi_thread")] -async fn test_room_directory_search_no_filter() -> Result<()> { +async fn test_room_directory_search_filter() -> Result<()> { let alice = TestClientBuilder::new("alice".to_owned()).use_sqlite().build().await?; - for index in 0..8 { + let search_string = random_string(32); + for index in 0..25 { let mut request: CreateRoomRequest = CreateRoomRequest::new(); request.visibility = Visibility::Public; - let name = format!("test_room_{}", index); - request.name = Some(name); + request.name = Some(format!("{}{}", search_string, index)); alice.create_room(request).await?; } let mut room_directory_search = RoomDirectorySearch::new(alice); let mut stream = room_directory_search.results(); - room_directory_search.search(None, 5).await; + room_directory_search.search(Some(search_string), 10).await?; assert_next_eq!(stream, VectorDiff::Clear); - if let VectorDiff::Append { values } = stream.next().now_or_never().unwrap().unwrap() { - warn!("Values: {:?}", values); - assert_eq!(values.len(), 5); - } else { - panic!("Expected a Vector::Append"); + + for _ in 0..2 { + if let VectorDiff::Append { values } = stream.next().now_or_never().unwrap().unwrap() { + warn!("Values: {:?}", values); + assert_eq!(values.len(), 10); + } else { + panic!("Expected a Vector::Append"); + } + assert_pending!(stream); + room_directory_search.next_page().await?; } - assert_pending!(stream); - room_directory_search.next_page().await; if let VectorDiff::Append { values } = stream.next().now_or_never().unwrap().unwrap() { warn!("Values: {:?}", values); - assert_eq!(values.len(), 3); + assert_eq!(values.len(), 5); } else { panic!("Expected a Vector::Append"); } assert_pending!(stream); - - room_directory_search.next_page().await; + room_directory_search.next_page().await?; assert_pending!(stream); Ok(()) } -#[tokio::test(flavor = "multi_thread")] -async fn test_room_directory_search_filter() -> Result<()> { - let alice = TestClientBuilder::new("alice".to_owned()).use_sqlite().build().await?; - let mut request: CreateRoomRequest = CreateRoomRequest::new(); - request.visibility = Visibility::Public; - alice.create_room(request).await?; - - let mut request: CreateRoomRequest = CreateRoomRequest::new(); - request.visibility = Visibility::Public; - request.name = Some("test".to_owned()); - alice.create_room(request).await?; - - let mut room_directory_search = RoomDirectorySearch::new(alice); - let mut stream = room_directory_search.results(); - room_directory_search.search(Some("test".to_owned()), 10).await; - assert_next_eq!(stream, VectorDiff::Clear); - if let VectorDiff::Append { values } = stream.next().now_or_never().unwrap().unwrap() { - assert_eq!(values.len(), 1); - } else { - panic!("Expected a Vector::Append"); - } - assert_pending!(stream); - Ok(()) +fn random_string(length: usize) -> String { + thread_rng() + .sample_iter(&rand::distributions::Alphanumeric) + .take(length) + .map(char::from) + .collect() } From d9231be1bad94722b8a39c415710c154a4999ae8 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 27 Feb 2024 17:08:55 +0100 Subject: [PATCH 05/74] feat(bindings): listener code --- bindings/matrix-sdk-ffi/src/client.rs | 8 +- .../src/room_directory_search.rs | 88 +++++++++++++++++-- .../matrix-sdk/src/room_directory_search.rs | 2 +- 3 files changed, 84 insertions(+), 14 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index cde3c488a2c..7e9a4fa8a07 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -743,11 +743,9 @@ impl Client { } pub fn room_directory_search(&self) -> Arc { - Arc::new(RoomDirectorySearch { - inner: matrix_sdk::room_directory_search::RoomDirectorySearch::new( - (*self.inner).clone(), - ), - }) + Arc::new(RoomDirectorySearch::new( + matrix_sdk::room_directory_search::RoomDirectorySearch::new((*self.inner).clone()), + )) } } diff --git a/bindings/matrix-sdk-ffi/src/room_directory_search.rs b/bindings/matrix-sdk-ffi/src/room_directory_search.rs index 11dff161984..80e7d30ad02 100644 --- a/bindings/matrix-sdk-ffi/src/room_directory_search.rs +++ b/bindings/matrix-sdk-ffi/src/room_directory_search.rs @@ -13,9 +13,15 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::{fmt::Debug, sync::Arc}; + +use eyeball_im::VectorDiff; +use futures_util::{pin_mut, StreamExt}; use matrix_sdk::{room_directory_search::RoomDirectorySearch as SdkRoomDirectorySearch, Client}; +use tokio::sync::RwLock; -use crate::error::ClientError; +use super::RUNTIME; +use crate::{error::ClientError, task_handle::TaskHandle}; #[derive(uniffi::Enum)] pub enum PublicRoomJoinRule { @@ -62,12 +68,78 @@ impl From for RoomDescriptio #[derive(uniffi::Object)] pub struct RoomDirectorySearch { - pub(crate) inner: SdkRoomDirectorySearch, + pub(crate) inner: RwLock, +} + +impl RoomDirectorySearch { + pub fn new(inner: SdkRoomDirectorySearch) -> Self { + Self { inner: RwLock::new(inner) } + } +} + +#[uniffi::export(async_runtime = "tokio")] +impl RoomDirectorySearch { + pub async fn next_page(&self) -> Result<(), ClientError> { + let mut inner = self.inner.write().await; + inner.next_page().await?; + Ok(()) + } + + pub async fn search(&self, filter: Option, batch_size: u32) -> Result<(), ClientError> { + let mut inner = self.inner.write().await; + inner.search(filter, batch_size).await?; + Ok(()) + } + + pub async fn loaded_pages(&self) -> Result { + let inner = self.inner.read().await; + Ok(inner.loaded_pages() as u32) + } + + pub async fn is_at_last_page(&self) -> Result { + let inner = self.inner.read().await; + Ok(inner.is_at_last_page()) + } + + pub async fn entries( + &self, + listener: Box, + ) -> RoomDirectorySearchEntriesResult { + let entries_stream = self.inner.read().await.results(); + RoomDirectorySearchEntriesResult { + entries_stream: Arc::new(TaskHandle::new(RUNTIME.spawn(async move { + pin_mut!(entries_stream); + + while let Some(diff) = entries_stream.next().await { + match diff { + VectorDiff::Clear => { + listener.on_update(RoomDirectorySearchEntryUpdate::Clear); + } + VectorDiff::Append { values } => { + listener.on_update(RoomDirectorySearchEntryUpdate::Append { + values: values.into_iter().map(|value| value.into()).collect(), + }); + } + _ => {} + } + } + }))), + } + } +} + +#[derive(uniffi::Record)] +pub struct RoomDirectorySearchEntriesResult { + pub entries_stream: Arc, +} + +#[derive(uniffi::Enum)] +pub enum RoomDirectorySearchEntryUpdate { + Clear, + Append { values: Vec }, } -// #[uniffi::export(async_runtime = "tokio")] -// impl RoomDirectorySearch { -// pub async fn next_page(self) -> Result<(), ClientError> { -// self.inner.next_page().await.map_err(Into::into) -// } -// } +#[uniffi::export(callback_interface)] +pub trait RoomDirectorySearchEntriesListener: Send + Sync + Debug { + fn on_update(&self, room_entries_update: RoomDirectorySearchEntryUpdate); +} diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index be85138c25c..1ab090a73e5 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -13,7 +13,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use eyeball_im::{ObservableVector, VectorDiff}; +use eyeball_im::{ObservableVector, VectorDiff, VectorSubscriber}; use futures_core::Stream; use ruma::{ api::client::directory::get_public_rooms_filtered::v3::Request as PublicRoomsFilterRequest, From caa9a7d8be135ba0506a906ffd19e174337eb6e4 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 27 Feb 2024 17:31:26 +0100 Subject: [PATCH 06/74] tests: code improvement for the filter integration test --- .../src/tests/room_directory_search.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs index 5eaf5e262c5..a2e8fdb4a6d 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs @@ -33,7 +33,9 @@ async fn test_room_directory_search_filter() -> Result<()> { for index in 0..25 { let mut request: CreateRoomRequest = CreateRoomRequest::new(); request.visibility = Visibility::Public; - request.name = Some(format!("{}{}", search_string, index)); + let name = format!("{}_{}", search_string, index); + warn!("room name: {}", name); + request.name = Some(name); alice.create_room(request).await?; } let mut room_directory_search = RoomDirectorySearch::new(alice); @@ -61,6 +63,17 @@ async fn test_room_directory_search_filter() -> Result<()> { assert_pending!(stream); room_directory_search.next_page().await?; assert_pending!(stream); + + // This should reset the state completely + room_directory_search.search(None, 25).await?; + assert_next_eq!(stream, VectorDiff::Clear); + if let VectorDiff::Append { values } = stream.next().now_or_never().unwrap().unwrap() { + warn!("Values: {:?}", values); + assert_eq!(values.len(), 25); + } else { + panic!("Expected a Vector::Append"); + } + assert_pending!(stream); Ok(()) } From 37d95571e992f969e6cc9f6d46fd50dc0220867a Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Tue, 27 Feb 2024 17:40:03 +0100 Subject: [PATCH 07/74] test: fixed a test by a adding a small delay --- .../src/tests/room_directory_search.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs index a2e8fdb4a6d..eafab919bf5 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs @@ -13,6 +13,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::time::Duration; + use anyhow::Result; use eyeball_im::VectorDiff; use futures::{FutureExt, StreamExt}; @@ -22,6 +24,7 @@ use matrix_sdk::{ }; use rand::{thread_rng, Rng}; use stream_assert::{assert_next_eq, assert_pending}; +use tokio::time::sleep; use tracing::warn; use crate::helpers::TestClientBuilder; @@ -38,6 +41,7 @@ async fn test_room_directory_search_filter() -> Result<()> { request.name = Some(name); alice.create_room(request).await?; } + sleep(Duration::from_secs(1)).await; let mut room_directory_search = RoomDirectorySearch::new(alice); let mut stream = room_directory_search.results(); room_directory_search.search(Some(search_string), 10).await?; From 9c33540af8b80dd28532c73e9ede2796745423ce Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Wed, 28 Feb 2024 16:18:35 +0100 Subject: [PATCH 08/74] tests: improved tests and added a unit test --- .../matrix-sdk/src/room_directory_search.rs | 100 ++++++++++++++---- .../src/tests/room_directory_search.rs | 43 +++----- 2 files changed, 97 insertions(+), 46 deletions(-) diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index 1ab090a73e5..a4d5bc8f332 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -13,8 +13,9 @@ // See the License for the specific language governing permissions and // limitations under the License. -use eyeball_im::{ObservableVector, VectorDiff, VectorSubscriber}; +use eyeball_im::{ObservableVector, VectorDiff}; use futures_core::Stream; +use imbl::Vector; use ruma::{ api::client::directory::get_public_rooms_filtered::v3::Request as PublicRoomsFilterRequest, directory::{Filter, PublicRoomJoinRule}, @@ -35,6 +36,21 @@ pub struct RoomDescription { pub joined_members: u64, } +impl From for RoomDescription { + fn from(value: ruma::directory::PublicRoomsChunk) -> Self { + Self { + room_id: value.room_id, + name: value.name, + topic: value.topic, + alias: value.canonical_alias, + avatar_url: value.avatar_url, + join_rule: value.join_rule, + is_world_readable: value.world_readable, + joined_members: value.num_joined_members.into(), + } + } +} + pub struct RoomDirectorySearch { batch_size: u32, filter: Option, @@ -81,27 +97,14 @@ impl RoomDirectorySearch { if self.next_token.is_none() { self.is_at_last_page = true; } - self.results.append( - response - .chunk - .into_iter() - .map(|room| RoomDescription { - room_id: room.room_id, - name: room.name, - topic: room.topic, - alias: room.canonical_alias, - avatar_url: room.avatar_url, - join_rule: room.join_rule, - is_world_readable: room.world_readable, - joined_members: room.num_joined_members.into(), - }) - .collect(), - ); + self.results.append(response.chunk.into_iter().map(Into::into).collect()); Ok(()) } - pub fn results(&self) -> impl Stream> { - self.results.subscribe().into_stream() + pub fn results( + &self, + ) -> (Vector, impl Stream>>) { + self.results.subscribe().into_values_and_batched_stream() } pub fn loaded_pages(&self) -> usize { @@ -115,3 +118,62 @@ impl RoomDirectorySearch { self.is_at_last_page } } + +#[cfg(test)] +mod tests { + use matrix_sdk_test::{async_test, test_json}; + use ruma::{OwnedRoomId, RoomAliasId, RoomId}; + use wiremock::{http::Method, Match, Mock, MockServer, Request, ResponseTemplate}; + + use crate::{ + room_directory_search::{RoomDescription, RoomDirectorySearch}, + test_utils::logged_in_client, + Client, + }; + + struct RoomDirectorySearchMatcher; + + impl Match for RoomDirectorySearchMatcher { + fn matches(&self, request: &Request) -> bool { + let match_url = request.url.path() == "/_matrix/client/v3/publicRooms"; + let match_method = request.method == Method::Post; + match_url && match_method + } + } + + #[async_test] + async fn search_success() { + let (server, client) = new_client().await; + + let mut room_directory_search = RoomDirectorySearch::new(client); + Mock::given(RoomDirectorySearchMatcher) + .respond_with(ResponseTemplate::new(200).set_body_json(&*test_json::PUBLIC_ROOMS)) + .mount(&server) + .await; + + room_directory_search.search(None, 1).await.unwrap(); + let (results, _) = room_directory_search.results(); + assert_eq!(results.len(), 1); + assert_eq!( + results[0], + RoomDescription { + room_id: RoomId::parse("!ol19s:bleecker.street").unwrap(), + name: Some("CHEESE".into()), + topic: Some("Tasty tasty cheese".into()), + alias: RoomAliasId::parse("#room:example.com").unwrap().into(), + avatar_url: Some("mxc://bleeker.street/CHEDDARandBRIE".into()), + join_rule: ruma::directory::PublicRoomJoinRule::Public, + is_world_readable: true, + joined_members: 37, + } + ); + assert_eq!(room_directory_search.is_at_last_page, false); + assert_eq!(room_directory_search.loaded_pages(), 1); + } + + async fn new_client() -> (MockServer, Client) { + let server = MockServer::start().await; + let client = logged_in_client(Some(server.uri())).await; + (server, client) + } +} diff --git a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs index eafab919bf5..e21bcb4bbc9 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs @@ -16,8 +16,9 @@ use std::time::Duration; use anyhow::Result; +use assert_matches::assert_matches; use eyeball_im::VectorDiff; -use futures::{FutureExt, StreamExt}; +use futures::StreamExt; use matrix_sdk::{ room_directory_search::RoomDirectorySearch, ruma::api::client::room::{create_room::v3::Request as CreateRoomRequest, Visibility}, @@ -43,40 +44,28 @@ async fn test_room_directory_search_filter() -> Result<()> { } sleep(Duration::from_secs(1)).await; let mut room_directory_search = RoomDirectorySearch::new(alice); - let mut stream = room_directory_search.results(); + let (values, mut stream) = room_directory_search.results(); + assert!(values.is_empty()); room_directory_search.search(Some(search_string), 10).await?; - assert_next_eq!(stream, VectorDiff::Clear); + let results_batch = stream.next().await.unwrap(); + assert_matches!(&results_batch[0], VectorDiff::Clear); + assert_matches!(&results_batch[1], VectorDiff::Append { values } => { assert_eq!(values.len(), 10); }); - for _ in 0..2 { - if let VectorDiff::Append { values } = stream.next().now_or_never().unwrap().unwrap() { - warn!("Values: {:?}", values); - assert_eq!(values.len(), 10); - } else { - panic!("Expected a Vector::Append"); - } - assert_pending!(stream); - room_directory_search.next_page().await?; - } - - if let VectorDiff::Append { values } = stream.next().now_or_never().unwrap().unwrap() { - warn!("Values: {:?}", values); - assert_eq!(values.len(), 5); - } else { - panic!("Expected a Vector::Append"); - } + room_directory_search.next_page().await?; + room_directory_search.next_page().await?; + let results_batch = stream.next().await.unwrap(); + assert_eq!(results_batch.len(), 2); + assert_matches!(&results_batch[0], VectorDiff::Append { values } => { assert_eq!(values.len(), 10); }); + assert_matches!(&results_batch[1], VectorDiff::Append { values } => { assert_eq!(values.len(), 5); }); assert_pending!(stream); room_directory_search.next_page().await?; assert_pending!(stream); // This should reset the state completely room_directory_search.search(None, 25).await?; - assert_next_eq!(stream, VectorDiff::Clear); - if let VectorDiff::Append { values } = stream.next().now_or_never().unwrap().unwrap() { - warn!("Values: {:?}", values); - assert_eq!(values.len(), 25); - } else { - panic!("Expected a Vector::Append"); - } + let results_batch = stream.next().await.unwrap(); + assert_matches!(&results_batch[0], VectorDiff::Clear); + assert_matches!(&results_batch[1], VectorDiff::Append { values } => { assert_eq!(values.len(), 25); }); assert_pending!(stream); Ok(()) } From 690ed4611da9b5a83e39894b9a051bea99ac1241 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Wed, 28 Feb 2024 19:01:42 +0100 Subject: [PATCH 09/74] tests: unit tests have been completed --- .../matrix-sdk/src/room_directory_search.rs | 203 +++++++++++++++--- .../src/test_json/api_responses.rs | 23 ++ testing/matrix-sdk-test/src/test_json/mod.rs | 4 +- 3 files changed, 204 insertions(+), 26 deletions(-) diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index a4d5bc8f332..9014d531f8c 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -122,8 +122,12 @@ impl RoomDirectorySearch { #[cfg(test)] mod tests { use matrix_sdk_test::{async_test, test_json}; - use ruma::{OwnedRoomId, RoomAliasId, RoomId}; - use wiremock::{http::Method, Match, Mock, MockServer, Request, ResponseTemplate}; + use ruma::{directory::Filter, serde::Raw, RoomAliasId, RoomId}; + use serde_json::Value as JsonValue; + use wiremock::{ + matchers::{method, path_regex}, + Match, Mock, MockServer, Request, ResponseTemplate, + }; use crate::{ room_directory_search::{RoomDescription, RoomDirectorySearch}, @@ -131,22 +135,77 @@ mod tests { Client, }; - struct RoomDirectorySearchMatcher; + struct RoomDirectorySearchMatcher { + next_token: Option, + filter_term: Option, + } impl Match for RoomDirectorySearchMatcher { fn matches(&self, request: &Request) -> bool { - let match_url = request.url.path() == "/_matrix/client/v3/publicRooms"; - let match_method = request.method == Method::Post; - match_url && match_method + let Ok(body) = request.body_json::>() else { + return false; + }; + + // The body's `since` field is set equal to the matcher's next_token. + if !body.get_field::("since").is_ok_and(|s| s == self.next_token) { + return false; + } + + // The body's `filter` field has `generic_search_term` equal to the matcher's + // next_token. + if !body.get_field::("filter").is_ok_and(|s| { + if self.filter_term.is_none() { + s.is_none() || s.is_some_and(|s| s.generic_search_term.is_none()) + } else { + s.is_some_and(|s| s.generic_search_term == self.filter_term) + } + }) { + return false; + } + + method("POST").matches(request) + && path_regex("/_matrix/client/../publicRooms").matches(request) + } + } + + fn get_first_page_description() -> RoomDescription { + RoomDescription { + room_id: RoomId::parse("!ol19s:bleecker.street").unwrap(), + name: Some("CHEESE".into()), + topic: Some("Tasty tasty cheese".into()), + alias: None, + avatar_url: Some("mxc://bleeker.street/CHEDDARandBRIE".into()), + join_rule: ruma::directory::PublicRoomJoinRule::Public, + is_world_readable: true, + joined_members: 37, } } + fn get_second_page_description() -> RoomDescription { + RoomDescription { + room_id: RoomId::parse("!ca18r:bleecker.street").unwrap(), + name: Some("PEAR".into()), + topic: Some("Tasty tasty pear".into()), + alias: RoomAliasId::parse("#murrays:pear.bar").ok(), + avatar_url: Some("mxc://bleeker.street/pear".into()), + join_rule: ruma::directory::PublicRoomJoinRule::Knock, + is_world_readable: false, + joined_members: 20, + } + } + + async fn new_server_and_client() -> (MockServer, Client) { + let server = MockServer::start().await; + let client = logged_in_client(Some(server.uri())).await; + (server, client) + } + #[async_test] async fn search_success() { - let (server, client) = new_client().await; + let (server, client) = new_server_and_client().await; let mut room_directory_search = RoomDirectorySearch::new(client); - Mock::given(RoomDirectorySearchMatcher) + Mock::given(RoomDirectorySearchMatcher { next_token: None, filter_term: None }) .respond_with(ResponseTemplate::new(200).set_body_json(&*test_json::PUBLIC_ROOMS)) .mount(&server) .await; @@ -154,26 +213,122 @@ mod tests { room_directory_search.search(None, 1).await.unwrap(); let (results, _) = room_directory_search.results(); assert_eq!(results.len(), 1); + assert_eq!(results[0], get_first_page_description()); + assert!(!room_directory_search.is_at_last_page); + assert_eq!(room_directory_search.loaded_pages(), 1); + } + + #[async_test] + async fn search_success_paginated() { + let (server, client) = new_server_and_client().await; + + let mut room_directory_search = RoomDirectorySearch::new(client); + Mock::given(RoomDirectorySearchMatcher { next_token: None, filter_term: None }) + .respond_with(ResponseTemplate::new(200).set_body_json(&*test_json::PUBLIC_ROOMS)) + .mount(&server) + .await; + + room_directory_search.search(None, 1).await.unwrap(); + + Mock::given(RoomDirectorySearchMatcher { + next_token: Some("p190q".into()), + filter_term: None, + }) + .respond_with( + ResponseTemplate::new(200).set_body_json(&*test_json::PUBLIC_ROOMS_FINAL_PAGE), + ) + .mount(&server) + .await; + + room_directory_search.next_page().await.unwrap(); + + let (results, _) = room_directory_search.results(); assert_eq!( - results[0], - RoomDescription { - room_id: RoomId::parse("!ol19s:bleecker.street").unwrap(), - name: Some("CHEESE".into()), - topic: Some("Tasty tasty cheese".into()), - alias: RoomAliasId::parse("#room:example.com").unwrap().into(), - avatar_url: Some("mxc://bleeker.street/CHEDDARandBRIE".into()), - join_rule: ruma::directory::PublicRoomJoinRule::Public, - is_world_readable: true, - joined_members: 37, - } + results, + vec![get_first_page_description(), get_second_page_description()].into() ); - assert_eq!(room_directory_search.is_at_last_page, false); + assert!(room_directory_search.is_at_last_page); + assert_eq!(room_directory_search.loaded_pages(), 2); + } + + #[async_test] + async fn search_fails() { + let (server, client) = new_server_and_client().await; + + let mut room_directory_search = RoomDirectorySearch::new(client); + Mock::given(RoomDirectorySearchMatcher { next_token: None, filter_term: None }) + .respond_with(ResponseTemplate::new(404)) + .mount(&server) + .await; + + room_directory_search.search(None, 1).await.unwrap_err(); + let (results, _) = room_directory_search.results(); + assert_eq!(results.len(), 0); + assert!(!room_directory_search.is_at_last_page); + assert_eq!(room_directory_search.loaded_pages(), 0); + } + + #[async_test] + async fn search_fails_when_paginating() { + let (server, client) = new_server_and_client().await; + + let mut room_directory_search = RoomDirectorySearch::new(client); + Mock::given(RoomDirectorySearchMatcher { next_token: None, filter_term: None }) + .respond_with(ResponseTemplate::new(200).set_body_json(&*test_json::PUBLIC_ROOMS)) + .mount(&server) + .await; + + room_directory_search.search(None, 1).await.unwrap(); + + Mock::given(RoomDirectorySearchMatcher { + next_token: Some("p190q".into()), + filter_term: None, + }) + .respond_with(ResponseTemplate::new(404)) + .mount(&server) + .await; + + room_directory_search.next_page().await.unwrap_err(); + + let (results, _) = room_directory_search.results(); + assert_eq!(results, vec![get_first_page_description()].into()); + assert!(!room_directory_search.is_at_last_page); assert_eq!(room_directory_search.loaded_pages(), 1); } - async fn new_client() -> (MockServer, Client) { - let server = MockServer::start().await; - let client = logged_in_client(Some(server.uri())).await; - (server, client) + #[async_test] + async fn search_success_paginated_with_filter() { + let (server, client) = new_server_and_client().await; + + let mut room_directory_search = RoomDirectorySearch::new(client); + Mock::given(RoomDirectorySearchMatcher { + next_token: None, + filter_term: Some("bleecker.street".into()), + }) + .respond_with(ResponseTemplate::new(200).set_body_json(&*test_json::PUBLIC_ROOMS)) + .mount(&server) + .await; + + room_directory_search.search(Some("bleecker.street".into()), 1).await.unwrap(); + + Mock::given(RoomDirectorySearchMatcher { + next_token: Some("p190q".into()), + filter_term: Some("bleecker.street".into()), + }) + .respond_with( + ResponseTemplate::new(200).set_body_json(&*test_json::PUBLIC_ROOMS_FINAL_PAGE), + ) + .mount(&server) + .await; + + room_directory_search.next_page().await.unwrap(); + + let (results, _) = room_directory_search.results(); + assert_eq!( + results, + vec![get_first_page_description(), get_second_page_description()].into() + ); + assert!(room_directory_search.is_at_last_page); + assert_eq!(room_directory_search.loaded_pages(), 2); } } diff --git a/testing/matrix-sdk-test/src/test_json/api_responses.rs b/testing/matrix-sdk-test/src/test_json/api_responses.rs index d534697717a..230022ba462 100644 --- a/testing/matrix-sdk-test/src/test_json/api_responses.rs +++ b/testing/matrix-sdk-test/src/test_json/api_responses.rs @@ -239,6 +239,7 @@ pub static NOT_FOUND: Lazy = Lazy::new(|| { }); /// `GET /_matrix/client/v3/publicRooms` +/// `POST /_matrix/client/v3/publicRooms` pub static PUBLIC_ROOMS: Lazy = Lazy::new(|| { json!({ "chunk": [ @@ -261,6 +262,28 @@ pub static PUBLIC_ROOMS: Lazy = Lazy::new(|| { }) }); +/// `GET /_matrix/client/v3/publicRooms` +/// `POST /_matrix/client/v3/publicRooms`` +pub static PUBLIC_ROOMS_FINAL_PAGE: Lazy = Lazy::new(|| { + json!({ + "chunk": [ + { + "canonical_alias": "#murrays:pear.bar", + "avatar_url": "mxc://bleeker.street/pear", + "guest_can_join": false, + "name": "PEAR", + "num_joined_members": 20, + "room_id": "!ca18r:bleecker.street", + "topic": "Tasty tasty pear", + "world_readable": false, + "join_rule": "knock" + } + ], + "prev_batch": "p190q", + "total_room_count_estimate": 115 + }) +}); + /// `POST /_matrix/client/v3/refresh` without new refresh token. pub static REFRESH_TOKEN: Lazy = Lazy::new(|| { json!({ diff --git a/testing/matrix-sdk-test/src/test_json/mod.rs b/testing/matrix-sdk-test/src/test_json/mod.rs index e5a6a5255e1..9c29bba81a1 100644 --- a/testing/matrix-sdk-test/src/test_json/mod.rs +++ b/testing/matrix-sdk-test/src/test_json/mod.rs @@ -18,8 +18,8 @@ pub mod sync_events; pub use api_responses::{ DEVICES, GET_ALIAS, KEYS_QUERY, KEYS_QUERY_TWO_DEVICES_ONE_SIGNED, KEYS_UPLOAD, LOGIN, LOGIN_RESPONSE_ERR, LOGIN_TYPES, LOGIN_WITH_DISCOVERY, LOGIN_WITH_REFRESH_TOKEN, NOT_FOUND, - PUBLIC_ROOMS, REFRESH_TOKEN, REFRESH_TOKEN_WITH_REFRESH_TOKEN, REGISTRATION_RESPONSE_ERR, - UNKNOWN_TOKEN_SOFT_LOGOUT, VERSIONS, WELL_KNOWN, WHOAMI, + PUBLIC_ROOMS, PUBLIC_ROOMS_FINAL_PAGE, REFRESH_TOKEN, REFRESH_TOKEN_WITH_REFRESH_TOKEN, + REGISTRATION_RESPONSE_ERR, UNKNOWN_TOKEN_SOFT_LOGOUT, VERSIONS, WELL_KNOWN, WHOAMI, }; pub use members::MEMBERS; pub use sync::{ From 3e35f163b7971db79e9b38f573c3902a6a2e9ed2 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Wed, 28 Feb 2024 19:17:58 +0100 Subject: [PATCH 10/74] docs: updated documentation and removed code from ffi that will be changed soon --- .../src/room_directory_search.rs | 37 ++++++++++--------- .../matrix-sdk/src/room_directory_search.rs | 15 ++++++++ 2 files changed, 35 insertions(+), 17 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room_directory_search.rs b/bindings/matrix-sdk-ffi/src/room_directory_search.rs index 80e7d30ad02..3035165dd6f 100644 --- a/bindings/matrix-sdk-ffi/src/room_directory_search.rs +++ b/bindings/matrix-sdk-ffi/src/room_directory_search.rs @@ -16,8 +16,8 @@ use std::{fmt::Debug, sync::Arc}; use eyeball_im::VectorDiff; -use futures_util::{pin_mut, StreamExt}; -use matrix_sdk::{room_directory_search::RoomDirectorySearch as SdkRoomDirectorySearch, Client}; +use futures_util::pin_mut; +use matrix_sdk::room_directory_search::RoomDirectorySearch as SdkRoomDirectorySearch; use tokio::sync::RwLock; use super::RUNTIME; @@ -108,21 +108,24 @@ impl RoomDirectorySearch { let entries_stream = self.inner.read().await.results(); RoomDirectorySearchEntriesResult { entries_stream: Arc::new(TaskHandle::new(RUNTIME.spawn(async move { - pin_mut!(entries_stream); - - while let Some(diff) = entries_stream.next().await { - match diff { - VectorDiff::Clear => { - listener.on_update(RoomDirectorySearchEntryUpdate::Clear); - } - VectorDiff::Append { values } => { - listener.on_update(RoomDirectorySearchEntryUpdate::Append { - values: values.into_iter().map(|value| value.into()).collect(), - }); - } - _ => {} - } - } + // TODO: This needs to get improved with sensei Ivan + // pin_mut!(entries_stream); + + // while let Some(diff) = entries_stream.next().await { + // match diff { + // VectorDiff::Clear => { + // + // listener.on_update(RoomDirectorySearchEntryUpdate::Clear); + // } + // VectorDiff::Append { values } => { + // + // listener.on_update(RoomDirectorySearchEntryUpdate::Append { + // values: values.into_iter().map(|value| + // value.into()).collect(), }); + // } + // _ => {} + // } + // } }))), } } diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index 9014d531f8c..3c7781fe0fb 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -72,6 +72,14 @@ impl RoomDirectorySearch { } } + /// Starts a filtered search for the server + /// If the `filter` is not provided it will search for all the rooms + /// You can specify a `batch_size`` to control the number of rooms to fetch + /// per request + /// + /// This method will clear the current search results and start a new one + /// Should never be used concurrently with another `next_page` or a + /// `search`. pub async fn search(&mut self, filter: Option, batch_size: u32) -> Result<()> { self.filter = filter; self.batch_size = batch_size; @@ -81,6 +89,9 @@ impl RoomDirectorySearch { self.next_page().await } + /// Asks the server for the next page of the current search + /// Should never be used concurrently with another `next_page` or a + /// `search`. pub async fn next_page(&mut self) -> Result<()> { if self.is_at_last_page { return Ok(()); @@ -101,12 +112,15 @@ impl RoomDirectorySearch { Ok(()) } + /// Get the initial value of the current stored room descriptions in the + /// search, and a stream of updates for them. pub fn results( &self, ) -> (Vector, impl Stream>>) { self.results.subscribe().into_values_and_batched_stream() } + /// Get the number of pages that have been loaded so far pub fn loaded_pages(&self) -> usize { if self.batch_size == 0 { return 0; @@ -114,6 +128,7 @@ impl RoomDirectorySearch { self.results.len() / self.batch_size as usize } + /// Get whether the search is at the last page pub fn is_at_last_page(&self) -> bool { self.is_at_last_page } From 70466aafb4e1de630e2ef0811e3ba4fd88673525 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Wed, 28 Feb 2024 19:25:52 +0100 Subject: [PATCH 11/74] docs: more documentation for types and the mod --- crates/matrix-sdk/src/room_directory_search.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index 3c7781fe0fb..49921e921c6 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -13,6 +13,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +//! Types for searching the public room directory. + use eyeball_im::{ObservableVector, VectorDiff}; use futures_core::Stream; use imbl::Vector; @@ -24,15 +26,24 @@ use ruma::{ use crate::{Client, Result}; +/// This struct represents a single result of a room directory search. #[derive(Clone, Debug, Eq, PartialEq)] pub struct RoomDescription { + /// The room's ID. pub room_id: OwnedRoomId, + /// The name of the room, if any. pub name: Option, + /// The topic of the room, if any. pub topic: Option, + /// The canonical alias of the room, if any. pub alias: Option, + /// The room's avatar URL, if any. pub avatar_url: Option, + /// The room's join rule. pub join_rule: PublicRoomJoinRule, + /// Whether can be previewed pub is_world_readable: bool, + /// The number of members that have joined the room. pub joined_members: u64, } @@ -51,6 +62,7 @@ impl From for RoomDescription { } } +#[derive(Debug)] pub struct RoomDirectorySearch { batch_size: u32, filter: Option, From 4dd7c3093c43d1f5c00b46fb4f5409e1d55574b3 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Wed, 28 Feb 2024 19:41:31 +0100 Subject: [PATCH 12/74] tests: improved the tests by adding the limit into the check for the request --- crates/matrix-sdk/src/room_directory_search.rs | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index 49921e921c6..7ffff43cfd4 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -165,6 +165,7 @@ mod tests { struct RoomDirectorySearchMatcher { next_token: Option, filter_term: Option, + limit: u32, } impl Match for RoomDirectorySearchMatcher { @@ -178,6 +179,10 @@ mod tests { return false; } + if !body.get_field::("limit").is_ok_and(|s| s == Some(self.limit)) { + return false; + } + // The body's `filter` field has `generic_search_term` equal to the matcher's // next_token. if !body.get_field::("filter").is_ok_and(|s| { @@ -232,7 +237,7 @@ mod tests { let (server, client) = new_server_and_client().await; let mut room_directory_search = RoomDirectorySearch::new(client); - Mock::given(RoomDirectorySearchMatcher { next_token: None, filter_term: None }) + Mock::given(RoomDirectorySearchMatcher { next_token: None, filter_term: None, limit: 1 }) .respond_with(ResponseTemplate::new(200).set_body_json(&*test_json::PUBLIC_ROOMS)) .mount(&server) .await; @@ -250,7 +255,7 @@ mod tests { let (server, client) = new_server_and_client().await; let mut room_directory_search = RoomDirectorySearch::new(client); - Mock::given(RoomDirectorySearchMatcher { next_token: None, filter_term: None }) + Mock::given(RoomDirectorySearchMatcher { next_token: None, filter_term: None, limit: 1 }) .respond_with(ResponseTemplate::new(200).set_body_json(&*test_json::PUBLIC_ROOMS)) .mount(&server) .await; @@ -260,6 +265,7 @@ mod tests { Mock::given(RoomDirectorySearchMatcher { next_token: Some("p190q".into()), filter_term: None, + limit: 1, }) .respond_with( ResponseTemplate::new(200).set_body_json(&*test_json::PUBLIC_ROOMS_FINAL_PAGE), @@ -283,7 +289,7 @@ mod tests { let (server, client) = new_server_and_client().await; let mut room_directory_search = RoomDirectorySearch::new(client); - Mock::given(RoomDirectorySearchMatcher { next_token: None, filter_term: None }) + Mock::given(RoomDirectorySearchMatcher { next_token: None, filter_term: None, limit: 1 }) .respond_with(ResponseTemplate::new(404)) .mount(&server) .await; @@ -300,7 +306,7 @@ mod tests { let (server, client) = new_server_and_client().await; let mut room_directory_search = RoomDirectorySearch::new(client); - Mock::given(RoomDirectorySearchMatcher { next_token: None, filter_term: None }) + Mock::given(RoomDirectorySearchMatcher { next_token: None, filter_term: None, limit: 1 }) .respond_with(ResponseTemplate::new(200).set_body_json(&*test_json::PUBLIC_ROOMS)) .mount(&server) .await; @@ -310,6 +316,7 @@ mod tests { Mock::given(RoomDirectorySearchMatcher { next_token: Some("p190q".into()), filter_term: None, + limit: 1, }) .respond_with(ResponseTemplate::new(404)) .mount(&server) @@ -331,6 +338,7 @@ mod tests { Mock::given(RoomDirectorySearchMatcher { next_token: None, filter_term: Some("bleecker.street".into()), + limit: 1, }) .respond_with(ResponseTemplate::new(200).set_body_json(&*test_json::PUBLIC_ROOMS)) .mount(&server) @@ -341,6 +349,7 @@ mod tests { Mock::given(RoomDirectorySearchMatcher { next_token: Some("p190q".into()), filter_term: Some("bleecker.street".into()), + limit: 1, }) .respond_with( ResponseTemplate::new(200).set_body_json(&*test_json::PUBLIC_ROOMS_FINAL_PAGE), From 4b1eefca80d96bf6577c4851a726fb623ac5c72b Mon Sep 17 00:00:00 2001 From: Mauro <34335419+Velin92@users.noreply.github.com> Date: Thu, 29 Feb 2024 11:52:31 +0100 Subject: [PATCH 13/74] Apply suggestions from code review Co-authored-by: Ivan Enderlin Signed-off-by: Mauro <34335419+Velin92@users.noreply.github.com> --- .../src/room_directory_search.rs | 36 ++++++------------- .../matrix-sdk/src/room_directory_search.rs | 35 +++++++++++------- 2 files changed, 33 insertions(+), 38 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room_directory_search.rs b/bindings/matrix-sdk-ffi/src/room_directory_search.rs index 3035165dd6f..06822de56ad 100644 --- a/bindings/matrix-sdk-ffi/src/room_directory_search.rs +++ b/bindings/matrix-sdk-ffi/src/room_directory_search.rs @@ -101,33 +101,19 @@ impl RoomDirectorySearch { Ok(inner.is_at_last_page()) } - pub async fn entries( + pub async fn results( &self, listener: Box, - ) -> RoomDirectorySearchEntriesResult { - let entries_stream = self.inner.read().await.results(); - RoomDirectorySearchEntriesResult { - entries_stream: Arc::new(TaskHandle::new(RUNTIME.spawn(async move { - // TODO: This needs to get improved with sensei Ivan - // pin_mut!(entries_stream); - - // while let Some(diff) = entries_stream.next().await { - // match diff { - // VectorDiff::Clear => { - // - // listener.on_update(RoomDirectorySearchEntryUpdate::Clear); - // } - // VectorDiff::Append { values } => { - // - // listener.on_update(RoomDirectorySearchEntryUpdate::Append { - // values: values.into_iter().map(|value| - // value.into()).collect(), }); - // } - // _ => {} - // } - // } - }))), - } + ) -> TaskHandle { + let (initial_values, stream) = self.inner.read().await.results(); + + TaskHandle::new(RUNTIME.spawn(async move { + listener.on_update(vec![VectorDiff::Reset { values: initial_values }.map(Into::into)]); + + while let Some(diffs) = stream.next().await { + listener.on_update(diffs.into_iter().map(|diff| diff.map(Into::into)).collect()); + } + })) } } diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index 7ffff43cfd4..a0fdf50b590 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -27,6 +27,8 @@ use ruma::{ use crate::{Client, Result}; /// This struct represents a single result of a room directory search. +/// +/// It's produced by [`RoomDirectorySearch::results`]. #[derive(Clone, Debug, Eq, PartialEq)] pub struct RoomDescription { /// The room's ID. @@ -84,12 +86,13 @@ impl RoomDirectorySearch { } } - /// Starts a filtered search for the server - /// If the `filter` is not provided it will search for all the rooms + /// Starts a filtered search for the server. + /// + /// If the `filter` is not provided it will search for all the rooms. /// You can specify a `batch_size`` to control the number of rooms to fetch - /// per request + /// per request. /// - /// This method will clear the current search results and start a new one + /// This method will clear the current search results and start a new one. /// Should never be used concurrently with another `next_page` or a /// `search`. pub async fn search(&mut self, filter: Option, batch_size: u32) -> Result<()> { @@ -101,7 +104,8 @@ impl RoomDirectorySearch { self.next_page().await } - /// Asks the server for the next page of the current search + /// Asks the server for the next page of the current search. + /// /// Should never be used concurrently with another `next_page` or a /// `search`. pub async fn next_page(&mut self) -> Result<()> { @@ -124,7 +128,7 @@ impl RoomDirectorySearch { Ok(()) } - /// Get the initial value of the current stored room descriptions in the + /// Get the initial values of the current stored room descriptions in the /// search, and a stream of updates for them. pub fn results( &self, @@ -132,12 +136,13 @@ impl RoomDirectorySearch { self.results.subscribe().into_values_and_batched_stream() } - /// Get the number of pages that have been loaded so far + /// Get the number of pages that have been loaded so far. pub fn loaded_pages(&self) -> usize { if self.batch_size == 0 { return 0; } - self.results.len() / self.batch_size as usize + + (self.results.len() / self.batch_size).ceil() as usize } /// Get whether the search is at the last page @@ -243,10 +248,11 @@ mod tests { .await; room_directory_search.search(None, 1).await.unwrap(); - let (results, _) = room_directory_search.results(); + let (results, stream) = room_directory_search.results(); + assert_pending!(stream); assert_eq!(results.len(), 1); assert_eq!(results[0], get_first_page_description()); - assert!(!room_directory_search.is_at_last_page); + assert!(!room_directory_search.is_at_last_page()); assert_eq!(room_directory_search.loaded_pages(), 1); } @@ -294,11 +300,14 @@ mod tests { .mount(&server) .await; - room_directory_search.search(None, 1).await.unwrap_err(); - let (results, _) = room_directory_search.results(); + let search = room_directory_search.search(None, 1).await; + assert!(search.is_err()); + + let (results, stream) = room_directory_search.results(); assert_eq!(results.len(), 0); - assert!(!room_directory_search.is_at_last_page); + assert!(!room_directory_search.is_at_last_page()); assert_eq!(room_directory_search.loaded_pages(), 0); + assert_pending!(stream); } #[async_test] From 2163ab03ec5cf265508572077c055004bf588a5d Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Thu, 29 Feb 2024 12:48:49 +0100 Subject: [PATCH 14/74] tests: test improvements and added a new test --- .../src/room_directory_search.rs | 38 ++++++++- .../matrix-sdk/src/room_directory_search.rs | 81 ++++++++++++++----- .../src/tests/room_directory_search.rs | 2 +- 3 files changed, 96 insertions(+), 25 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room_directory_search.rs b/bindings/matrix-sdk-ffi/src/room_directory_search.rs index 06822de56ad..ea549b53090 100644 --- a/bindings/matrix-sdk-ffi/src/room_directory_search.rs +++ b/bindings/matrix-sdk-ffi/src/room_directory_search.rs @@ -16,6 +16,7 @@ use std::{fmt::Debug, sync::Arc}; use eyeball_im::VectorDiff; +use futures::StreamExt; use futures_util::pin_mut; use matrix_sdk::room_directory_search::RoomDirectorySearch as SdkRoomDirectorySearch; use tokio::sync::RwLock; @@ -106,10 +107,12 @@ impl RoomDirectorySearch { listener: Box, ) -> TaskHandle { let (initial_values, stream) = self.inner.read().await.results(); - + TaskHandle::new(RUNTIME.spawn(async move { - listener.on_update(vec![VectorDiff::Reset { values: initial_values }.map(Into::into)]); - + listener.on_update(RoomDirectorySearchEntryUpdate::Reset { + values: initial_values.into_iter().map(Into::into).collect(), + }); + while let Some(diffs) = stream.next().await { listener.on_update(diffs.into_iter().map(|diff| diff.map(Into::into)).collect()); } @@ -124,8 +127,35 @@ pub struct RoomDirectorySearchEntriesResult { #[derive(uniffi::Enum)] pub enum RoomDirectorySearchEntryUpdate { - Clear, Append { values: Vec }, + Clear, + PushFront { value: RoomDescription }, + PushBack { value: RoomDescription }, + PopFront, + PopBack, + Insert { index: u32, value: RoomDescription }, + Set { index: u32, value: RoomDescription }, + Remove { index: u32 }, + Truncate { length: u32 }, + Reset { values: Vec }, +} + +impl From> for RoomDirectorySearchEntryUpdate { + fn from(diff: VectorDiff) -> Self { + match diff { + VectorDiff::Append { values } => Self::Append { values }, + VectorDiff::Clear => Self::Clear, + VectorDiff::PushFront { value } => Self::PushFront { value }, + VectorDiff::PushBack { value } => Self::PushBack { value }, + VectorDiff::PopFront => Self::PopFront, + VectorDiff::PopBack => Self::PopBack, + VectorDiff::Insert { index, value } => Self::Insert { index, value }, + VectorDiff::Set { index, value } => Self::Set { index, value }, + VectorDiff::Remove { index } => Self::Remove { index }, + VectorDiff::Truncate { length } => Self::Truncate { length }, + VectorDiff::Reset { values } => Self::Reset { values }, + } + } } #[uniffi::export(callback_interface)] diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index a0fdf50b590..b438ff6f29b 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -141,8 +141,7 @@ impl RoomDirectorySearch { if self.batch_size == 0 { return 0; } - - (self.results.len() / self.batch_size).ceil() as usize + (self.results.len() as f64 / self.batch_size as f64).ceil() as usize } /// Get whether the search is at the last page @@ -153,9 +152,13 @@ impl RoomDirectorySearch { #[cfg(test)] mod tests { + use assert_matches::assert_matches; + use eyeball_im::VectorDiff; + use futures_util::StreamExt; use matrix_sdk_test::{async_test, test_json}; use ruma::{directory::Filter, serde::Raw, RoomAliasId, RoomId}; use serde_json::Value as JsonValue; + use stream_assert::assert_pending; use wiremock::{ matchers::{method, path_regex}, Match, Mock, MockServer, Request, ResponseTemplate, @@ -248,7 +251,7 @@ mod tests { .await; room_directory_search.search(None, 1).await.unwrap(); - let (results, stream) = room_directory_search.results(); + let (results, mut stream) = room_directory_search.results(); assert_pending!(stream); assert_eq!(results.len(), 1); assert_eq!(results[0], get_first_page_description()); @@ -267,6 +270,10 @@ mod tests { .await; room_directory_search.search(None, 1).await.unwrap(); + let (initial_results, mut stream) = room_directory_search.results(); + assert_eq!(initial_results, vec![get_first_page_description()].into()); + assert!(!room_directory_search.is_at_last_page()); + assert_eq!(room_directory_search.loaded_pages(), 1); Mock::given(RoomDirectorySearchMatcher { next_token: Some("p190q".into()), @@ -281,13 +288,11 @@ mod tests { room_directory_search.next_page().await.unwrap(); - let (results, _) = room_directory_search.results(); - assert_eq!( - results, - vec![get_first_page_description(), get_second_page_description()].into() - ); - assert!(room_directory_search.is_at_last_page); + let results_batch: Vec> = stream.next().await.unwrap(); + assert_matches!(&results_batch[0], VectorDiff::Append { values } => { assert_eq!(values, &vec![get_second_page_description()].into()); }); + assert!(room_directory_search.is_at_last_page()); assert_eq!(room_directory_search.loaded_pages(), 2); + assert_pending!(stream); } #[async_test] @@ -300,10 +305,9 @@ mod tests { .mount(&server) .await; - let search = room_directory_search.search(None, 1).await; - assert!(search.is_err()); + assert!(room_directory_search.next_page().await.is_err()); - let (results, stream) = room_directory_search.results(); + let (results, mut stream) = room_directory_search.results(); assert_eq!(results.len(), 0); assert!(!room_directory_search.is_at_last_page()); assert_eq!(room_directory_search.loaded_pages(), 0); @@ -331,11 +335,11 @@ mod tests { .mount(&server) .await; - room_directory_search.next_page().await.unwrap_err(); + assert!(room_directory_search.next_page().await.is_err()); let (results, _) = room_directory_search.results(); assert_eq!(results, vec![get_first_page_description()].into()); - assert!(!room_directory_search.is_at_last_page); + assert!(!room_directory_search.is_at_last_page()); assert_eq!(room_directory_search.loaded_pages(), 1); } @@ -354,6 +358,10 @@ mod tests { .await; room_directory_search.search(Some("bleecker.street".into()), 1).await.unwrap(); + let (initial_results, mut stream) = room_directory_search.results(); + assert_eq!(initial_results, vec![get_first_page_description()].into()); + assert!(!room_directory_search.is_at_last_page()); + assert_eq!(room_directory_search.loaded_pages(), 1); Mock::given(RoomDirectorySearchMatcher { next_token: Some("p190q".into()), @@ -368,12 +376,45 @@ mod tests { room_directory_search.next_page().await.unwrap(); - let (results, _) = room_directory_search.results(); - assert_eq!( - results, - vec![get_first_page_description(), get_second_page_description()].into() - ); - assert!(room_directory_search.is_at_last_page); + let results_batch: Vec> = stream.next().await.unwrap(); + assert_matches!(&results_batch[0], VectorDiff::Append { values } => { assert_eq!(values, &vec![get_second_page_description()].into()); }); + assert!(room_directory_search.is_at_last_page()); assert_eq!(room_directory_search.loaded_pages(), 2); + assert_pending!(stream); + } + + #[async_test] + async fn search_followed_by_another_search_with_filter() { + let (server, client) = new_server_and_client().await; + + let mut room_directory_search = RoomDirectorySearch::new(client); + Mock::given(RoomDirectorySearchMatcher { next_token: None, filter_term: None, limit: 1 }) + .respond_with(ResponseTemplate::new(200).set_body_json(&*test_json::PUBLIC_ROOMS)) + .mount(&server) + .await; + + room_directory_search.search(None, 1).await.unwrap(); + let (initial_results, mut stream) = room_directory_search.results(); + assert_eq!(initial_results, vec![get_first_page_description()].into()); + assert!(!room_directory_search.is_at_last_page()); + assert_eq!(room_directory_search.loaded_pages(), 1); + + Mock::given(RoomDirectorySearchMatcher { + next_token: None, + filter_term: Some("bleecker.street".into()), + limit: 1, + }) + .respond_with(ResponseTemplate::new(200).set_body_json(&*test_json::PUBLIC_ROOMS)) + .mount(&server) + .await; + + room_directory_search.search(Some("bleecker.street".into()), 1).await.unwrap(); + + let results_batch: Vec> = stream.next().await.unwrap(); + assert_matches!(&results_batch[0], VectorDiff::Clear); + assert_matches!(&results_batch[1], VectorDiff::Append { values } => { assert_eq!(values, &vec![get_first_page_description()].into()); }); + assert!(!room_directory_search.is_at_last_page()); + assert_eq!(room_directory_search.loaded_pages(), 1); + assert_pending!(stream); } } diff --git a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs index e21bcb4bbc9..3348bd14303 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs @@ -47,7 +47,7 @@ async fn test_room_directory_search_filter() -> Result<()> { let (values, mut stream) = room_directory_search.results(); assert!(values.is_empty()); room_directory_search.search(Some(search_string), 10).await?; - let results_batch = stream.next().await.unwrap(); + let results_batch: Vec> = stream.next().await.unwrap(); assert_matches!(&results_batch[0], VectorDiff::Clear); assert_matches!(&results_batch[1], VectorDiff::Append { values } => { assert_eq!(values.len(), 10); }); From 2e3ced1fb2f2586ff4176467d927e60366788805 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Thu, 29 Feb 2024 12:58:34 +0100 Subject: [PATCH 15/74] docs: more documentation --- .../matrix-sdk/src/room_directory_search.rs | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index b438ff6f29b..e81a2a9bfbe 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -63,7 +63,24 @@ impl From for RoomDescription { } } } - +/// RoomDirectorySearch allows searching the public room directory, with the +/// capability of using a filter and a batch_size. This struct is also +/// responsible for keeping the current state of the search, and exposing an +/// update of stream of the results, reset the search, or ask for the next page. +/// +/// # Example +/// +/// ```rust +/// use matrix_sdk::room_directory_search::RoomDirectorySearch; +/// +/// # fn main() -> Result<()> { +/// let room_directory_search = RoomDirectorySearch(client); +/// room_directory_search.search(None, 10).await?; +/// let (results, mut stream) = room_directory_search.results(); +/// room_directory_search.next_page().await?; +/// ... +/// # } +/// ``` #[derive(Debug)] pub struct RoomDirectorySearch { batch_size: u32, @@ -75,6 +92,7 @@ pub struct RoomDirectorySearch { } impl RoomDirectorySearch { + /// Constructor for the `RoomDirectorySearch`, requires a `Client`. pub fn new(client: Client) -> Self { Self { batch_size: 0, @@ -326,6 +344,12 @@ mod tests { room_directory_search.search(None, 1).await.unwrap(); + let (results, mut stream) = room_directory_search.results(); + assert_eq!(results, vec![get_first_page_description()].into()); + assert!(!room_directory_search.is_at_last_page()); + assert_eq!(room_directory_search.loaded_pages(), 1); + assert_pending!(stream); + Mock::given(RoomDirectorySearchMatcher { next_token: Some("p190q".into()), filter_term: None, @@ -336,11 +360,10 @@ mod tests { .await; assert!(room_directory_search.next_page().await.is_err()); - - let (results, _) = room_directory_search.results(); assert_eq!(results, vec![get_first_page_description()].into()); assert!(!room_directory_search.is_at_last_page()); assert_eq!(room_directory_search.loaded_pages(), 1); + assert_pending!(stream); } #[async_test] From 8890bf3cee182bf6961499caebf601bb4055fb50 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Thu, 29 Feb 2024 14:10:45 +0100 Subject: [PATCH 16/74] feat(bindings): improved and fixed ffi code --- .../src/room_directory_search.rs | 69 ++++++++++--------- 1 file changed, 38 insertions(+), 31 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room_directory_search.rs b/bindings/matrix-sdk-ffi/src/room_directory_search.rs index ea549b53090..c566a9001ad 100644 --- a/bindings/matrix-sdk-ffi/src/room_directory_search.rs +++ b/bindings/matrix-sdk-ffi/src/room_directory_search.rs @@ -16,8 +16,7 @@ use std::{fmt::Debug, sync::Arc}; use eyeball_im::VectorDiff; -use futures::StreamExt; -use futures_util::pin_mut; +use futures_util::StreamExt; use matrix_sdk::room_directory_search::RoomDirectorySearch as SdkRoomDirectorySearch; use tokio::sync::RwLock; @@ -92,29 +91,20 @@ impl RoomDirectorySearch { Ok(()) } - pub async fn loaded_pages(&self) -> Result { - let inner = self.inner.read().await; - Ok(inner.loaded_pages() as u32) - } - - pub async fn is_at_last_page(&self) -> Result { - let inner = self.inner.read().await; - Ok(inner.is_at_last_page()) - } - - pub async fn results( - &self, - listener: Box, - ) -> TaskHandle { - let (initial_values, stream) = self.inner.read().await.results(); + pub async fn state(&self, listener: Box) -> TaskHandle { + let (initial_values, mut stream) = self.inner.read().await.results(); TaskHandle::new(RUNTIME.spawn(async move { - listener.on_update(RoomDirectorySearchEntryUpdate::Reset { - values: initial_values.into_iter().map(Into::into).collect(), + listener.on_update(RoomDirectorySearchState { + updates: vec![RoomDirectorySearchEntryUpdate::Reset { + values: initial_values.into_iter().map(Into::into).collect(), + }], + is_at_last_page: self.inner.read().await.is_at_last_page(), + loaded_pages: self.inner.read().await.loaded_pages() as u32, }); while let Some(diffs) = stream.next().await { - listener.on_update(diffs.into_iter().map(|diff| diff.map(Into::into)).collect()); + listener.on_update(diffs.into_iter().map(|diff| diff.into()).collect()); } })) } @@ -140,25 +130,42 @@ pub enum RoomDirectorySearchEntryUpdate { Reset { values: Vec }, } -impl From> for RoomDirectorySearchEntryUpdate { - fn from(diff: VectorDiff) -> Self { +impl From> + for RoomDirectorySearchEntryUpdate +{ + fn from(diff: VectorDiff) -> Self { match diff { - VectorDiff::Append { values } => Self::Append { values }, + VectorDiff::Append { values } => { + Self::Append { values: values.into_iter().map(|v| v.into()).collect() } + } VectorDiff::Clear => Self::Clear, - VectorDiff::PushFront { value } => Self::PushFront { value }, - VectorDiff::PushBack { value } => Self::PushBack { value }, + VectorDiff::PushFront { value } => Self::PushFront { value: value.into() }, + VectorDiff::PushBack { value } => Self::PushBack { value: value.into() }, VectorDiff::PopFront => Self::PopFront, VectorDiff::PopBack => Self::PopBack, - VectorDiff::Insert { index, value } => Self::Insert { index, value }, - VectorDiff::Set { index, value } => Self::Set { index, value }, - VectorDiff::Remove { index } => Self::Remove { index }, - VectorDiff::Truncate { length } => Self::Truncate { length }, - VectorDiff::Reset { values } => Self::Reset { values }, + VectorDiff::Insert { index, value } => { + Self::Insert { index: index as u32, value: value.into() } + } + VectorDiff::Set { index, value } => { + Self::Set { index: index as u32, value: value.into() } + } + VectorDiff::Remove { index } => Self::Remove { index: index as u32 }, + VectorDiff::Truncate { length } => Self::Truncate { length: length as u32 }, + VectorDiff::Reset { values } => { + Self::Reset { values: values.into_iter().map(|v| v.into()).collect() } + } } } } +#[derive(uniffi::Record)] +struct RoomDirectorySearchState { + pub updates: Vec, + pub is_at_last_page: bool, + pub loaded_pages: u32, +} + #[uniffi::export(callback_interface)] pub trait RoomDirectorySearchEntriesListener: Send + Sync + Debug { - fn on_update(&self, room_entries_update: RoomDirectorySearchEntryUpdate); + fn on_update(&self, room_entries_update: RoomDirectorySearchState); } From b2b9b5fa120bc24277f7d4530d635216d4c58aee Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Thu, 29 Feb 2024 14:29:03 +0100 Subject: [PATCH 17/74] feat(bindings): reverted some code from ffi --- .../src/room_directory_search.rs | 34 ++++++++++--------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room_directory_search.rs b/bindings/matrix-sdk-ffi/src/room_directory_search.rs index c566a9001ad..35a5629ecb6 100644 --- a/bindings/matrix-sdk-ffi/src/room_directory_search.rs +++ b/bindings/matrix-sdk-ffi/src/room_directory_search.rs @@ -91,17 +91,26 @@ impl RoomDirectorySearch { Ok(()) } - pub async fn state(&self, listener: Box) -> TaskHandle { + pub async fn loaded_pages(&self) -> Result { + let inner = self.inner.read().await; + Ok(inner.loaded_pages() as u32) + } + + pub async fn is_at_last_page(&self) -> Result { + let inner = self.inner.read().await; + Ok(inner.is_at_last_page()) + } + + pub async fn results( + &self, + listener: Box, + ) -> TaskHandle { let (initial_values, mut stream) = self.inner.read().await.results(); TaskHandle::new(RUNTIME.spawn(async move { - listener.on_update(RoomDirectorySearchState { - updates: vec![RoomDirectorySearchEntryUpdate::Reset { - values: initial_values.into_iter().map(Into::into).collect(), - }], - is_at_last_page: self.inner.read().await.is_at_last_page(), - loaded_pages: self.inner.read().await.loaded_pages() as u32, - }); + listener.on_update(vec![RoomDirectorySearchEntryUpdate::Reset { + values: initial_values.into_iter().map(Into::into).collect(), + }]); while let Some(diffs) = stream.next().await { listener.on_update(diffs.into_iter().map(|diff| diff.into()).collect()); @@ -158,14 +167,7 @@ impl From> } } -#[derive(uniffi::Record)] -struct RoomDirectorySearchState { - pub updates: Vec, - pub is_at_last_page: bool, - pub loaded_pages: u32, -} - #[uniffi::export(callback_interface)] pub trait RoomDirectorySearchEntriesListener: Send + Sync + Debug { - fn on_update(&self, room_entries_update: RoomDirectorySearchState); + fn on_update(&self, room_entries_update: Vec); } From 2abe3aba4adf937a9267d94901543b327235c6b5 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Fri, 1 Mar 2024 15:21:57 +0100 Subject: [PATCH 18/74] improved docs --- crates/matrix-sdk/src/room_directory_search.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index e81a2a9bfbe..0dca713616f 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -70,15 +70,17 @@ impl From for RoomDescription { /// /// # Example /// -/// ```rust -/// use matrix_sdk::room_directory_search::RoomDirectorySearch; +/// ```no_run +/// use matrix_sdk::{room_directory_search::RoomDirectorySearch, Client}; +/// use url::Url; /// /// # fn main() -> Result<()> { +/// let homeserver = Url::parse("http://localhost:8080")?; +/// let client = Client::new(homeserver).await?; /// let room_directory_search = RoomDirectorySearch(client); /// room_directory_search.search(None, 10).await?; /// let (results, mut stream) = room_directory_search.results(); /// room_directory_search.next_page().await?; -/// ... /// # } /// ``` #[derive(Debug)] From ad1623da58e156a432b055985cb1747f30912c03 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Fri, 1 Mar 2024 15:44:24 +0100 Subject: [PATCH 19/74] docs: fixed docs --- crates/matrix-sdk/src/room_directory_search.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index 0dca713616f..9637b7bdd48 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -74,14 +74,15 @@ impl From for RoomDescription { /// use matrix_sdk::{room_directory_search::RoomDirectorySearch, Client}; /// use url::Url; /// -/// # fn main() -> Result<()> { -/// let homeserver = Url::parse("http://localhost:8080")?; -/// let client = Client::new(homeserver).await?; -/// let room_directory_search = RoomDirectorySearch(client); -/// room_directory_search.search(None, 10).await?; -/// let (results, mut stream) = room_directory_search.results(); -/// room_directory_search.next_page().await?; -/// # } +/// async { +/// let homeserver = Url::parse("http://localhost:8080")?; +/// let client = Client::new(homeserver).await?; +/// let mut room_directory_search = RoomDirectorySearch::new(client); +/// room_directory_search.search(None, 10).await?; +/// let (results, mut stream) = room_directory_search.results(); +/// room_directory_search.next_page().await?; +/// anyhow::Ok(()) +/// }; /// ``` #[derive(Debug)] pub struct RoomDirectorySearch { From 2f7b2f0451b31e664168312247e15f03e0e7b673 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Fri, 1 Mar 2024 17:07:52 +0100 Subject: [PATCH 20/74] fix: fmt --- .../src/tests/room_directory_search.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs index 3348bd14303..fb58da2eedd 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs @@ -47,7 +47,8 @@ async fn test_room_directory_search_filter() -> Result<()> { let (values, mut stream) = room_directory_search.results(); assert!(values.is_empty()); room_directory_search.search(Some(search_string), 10).await?; - let results_batch: Vec> = stream.next().await.unwrap(); + let results_batch: Vec> = + stream.next().await.unwrap(); assert_matches!(&results_batch[0], VectorDiff::Clear); assert_matches!(&results_batch[1], VectorDiff::Append { values } => { assert_eq!(values.len(), 10); }); From 9fbc2ab07c76ee499c90054315c7c5155141b87a Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Fri, 1 Mar 2024 17:17:24 +0100 Subject: [PATCH 21/74] mocking library not supported on wasm --- crates/matrix-sdk/src/room_directory_search.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index 9637b7bdd48..3eec4bc44ef 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -171,7 +171,7 @@ impl RoomDirectorySearch { } } -#[cfg(test)] +#[cfg(all(test, not(target_arch = "wasm32")))] mod tests { use assert_matches::assert_matches; use eyeball_im::VectorDiff; From e922a58cc3f40fb2fa87b67a9370e754974ac736 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Fri, 1 Mar 2024 17:25:11 +0100 Subject: [PATCH 22/74] tests: fix fmt --- .../src/tests/room_directory_search.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs index fb58da2eedd..ff3c3d35ba6 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/room_directory_search.rs @@ -24,7 +24,7 @@ use matrix_sdk::{ ruma::api::client::room::{create_room::v3::Request as CreateRoomRequest, Visibility}, }; use rand::{thread_rng, Rng}; -use stream_assert::{assert_next_eq, assert_pending}; +use stream_assert::assert_pending; use tokio::time::sleep; use tracing::warn; From a52a2329a1a47d8cf631410d28fb438f31841fdc Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Wed, 13 Mar 2024 12:43:26 +0100 Subject: [PATCH 23/74] pr suggestions improved the conversion by using a try from and changed the nex_token to a search state to indicate the current state of the search --- .../src/room_directory_search.rs | 14 +++-- .../matrix-sdk/src/room_directory_search.rs | 59 +++++++++++++------ 2 files changed, 50 insertions(+), 23 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room_directory_search.rs b/bindings/matrix-sdk-ffi/src/room_directory_search.rs index 35a5629ecb6..3218b3e2fcc 100644 --- a/bindings/matrix-sdk-ffi/src/room_directory_search.rs +++ b/bindings/matrix-sdk-ffi/src/room_directory_search.rs @@ -29,12 +29,14 @@ pub enum PublicRoomJoinRule { Knock, } -impl PublicRoomJoinRule { - fn convert(value: ruma::directory::PublicRoomJoinRule) -> Option { +impl TryFrom for PublicRoomJoinRule { + type Error = (); + + fn try_from(value: ruma::directory::PublicRoomJoinRule) -> Result { match value { - ruma::directory::PublicRoomJoinRule::Public => Some(Self::Public), - ruma::directory::PublicRoomJoinRule::Knock => Some(Self::Knock), - _ => None, + ruma::directory::PublicRoomJoinRule::Public => Ok(Self::Public), + ruma::directory::PublicRoomJoinRule::Knock => Ok(Self::Knock), + _ => Err(()), } } } @@ -59,7 +61,7 @@ impl From for RoomDescriptio topic: value.topic, alias: value.alias.map(|alias| alias.to_string()), avatar_url: value.avatar_url.map(|url| url.to_string()), - join_rule: PublicRoomJoinRule::convert(value.join_rule), + join_rule: value.join_rule.try_into().ok(), is_world_readable: value.is_world_readable, joined_members: value.joined_members, } diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index 3eec4bc44ef..96dd7274f62 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -63,6 +63,34 @@ impl From for RoomDescription { } } } + +#[derive(Default, Debug)] +enum SearchState { + /// The search has more pages and contains the next token to be used in the + /// next page request. + Next(String), + /// The search has reached the end. + End, + /// The search is in a starting state, and has yet to fetch the first page. + #[default] + Start, +} + +// if you want extra methods: +impl SearchState { + fn next_token(&self) -> Option<&str> { + if let Self::Next(next_token) = &self { + Some(next_token) + } else { + None + } + } + + fn is_at_end(&self) -> bool { + matches!(self, Self::End) + } +} + /// RoomDirectorySearch allows searching the public room directory, with the /// capability of using a filter and a batch_size. This struct is also /// responsible for keeping the current state of the search, and exposing an @@ -88,10 +116,9 @@ impl From for RoomDescription { pub struct RoomDirectorySearch { batch_size: u32, filter: Option, - next_token: Option, + search_state: SearchState, client: Client, results: ObservableVector, - is_at_last_page: bool, } impl RoomDirectorySearch { @@ -100,10 +127,9 @@ impl RoomDirectorySearch { Self { batch_size: 0, filter: None, - next_token: None, + search_state: Default::default(), client, results: ObservableVector::new(), - is_at_last_page: false, } } @@ -114,23 +140,21 @@ impl RoomDirectorySearch { /// per request. /// /// This method will clear the current search results and start a new one. - /// Should never be used concurrently with another `next_page` or a - /// `search`. + // Should never be used concurrently with another `next_page` or a + // `search`. pub async fn search(&mut self, filter: Option, batch_size: u32) -> Result<()> { self.filter = filter; self.batch_size = batch_size; - self.next_token = None; + self.search_state = Default::default(); self.results.clear(); - self.is_at_last_page = false; self.next_page().await } /// Asks the server for the next page of the current search. - /// - /// Should never be used concurrently with another `next_page` or a - /// `search`. + // Should never be used concurrently with another `next_page` or a + // `search`. pub async fn next_page(&mut self) -> Result<()> { - if self.is_at_last_page { + if self.search_state.is_at_end() { return Ok(()); } let mut filter = Filter::new(); @@ -139,11 +163,12 @@ impl RoomDirectorySearch { let mut request = PublicRoomsFilterRequest::new(); request.filter = filter; request.limit = Some(self.batch_size.into()); - request.since = self.next_token.clone(); + request.since = self.search_state.next_token().map(ToOwned::to_owned); let response = self.client.public_rooms_filtered(request).await?; - self.next_token = response.next_batch; - if self.next_token.is_none() { - self.is_at_last_page = true; + if let Some(next_token) = response.next_batch { + self.search_state = SearchState::Next(next_token); + } else { + self.search_state = SearchState::End; } self.results.append(response.chunk.into_iter().map(Into::into).collect()); Ok(()) @@ -167,7 +192,7 @@ impl RoomDirectorySearch { /// Get whether the search is at the last page pub fn is_at_last_page(&self) -> bool { - self.is_at_last_page + self.search_state.is_at_end() } } From 2f9b9942c30a452a3a59db2d4c9ec1982ab71699 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Wed, 13 Mar 2024 12:45:50 +0100 Subject: [PATCH 24/74] docs: removed useless comment --- crates/matrix-sdk/src/room_directory_search.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index 96dd7274f62..3f5febe969c 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -76,7 +76,6 @@ enum SearchState { Start, } -// if you want extra methods: impl SearchState { fn next_token(&self) -> Option<&str> { if let Self::Next(next_token) = &self { From 73b01743a58d526190de2c8cb7add3bd0f2fcc5a Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Wed, 13 Mar 2024 12:48:32 +0100 Subject: [PATCH 25/74] improved code spacing --- crates/matrix-sdk/src/room_directory_search.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index 3f5febe969c..24586d3512e 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -156,6 +156,7 @@ impl RoomDirectorySearch { if self.search_state.is_at_end() { return Ok(()); } + let mut filter = Filter::new(); filter.generic_search_term = self.filter.clone(); @@ -163,12 +164,15 @@ impl RoomDirectorySearch { request.filter = filter; request.limit = Some(self.batch_size.into()); request.since = self.search_state.next_token().map(ToOwned::to_owned); + let response = self.client.public_rooms_filtered(request).await?; + if let Some(next_token) = response.next_batch { self.search_state = SearchState::Next(next_token); } else { self.search_state = SearchState::End; } + self.results.append(response.chunk.into_iter().map(Into::into).collect()); Ok(()) } From 5e692931dd151c532eacb54b975a3b0a52f31cb5 Mon Sep 17 00:00:00 2001 From: Mauro <34335419+Velin92@users.noreply.github.com> Date: Thu, 14 Mar 2024 10:42:27 +0100 Subject: [PATCH 26/74] Apply suggestions from code review Co-authored-by: Ivan Enderlin Signed-off-by: Mauro <34335419+Velin92@users.noreply.github.com> --- bindings/matrix-sdk-ffi/src/room_directory_search.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room_directory_search.rs b/bindings/matrix-sdk-ffi/src/room_directory_search.rs index 3218b3e2fcc..7036d7f5bf0 100644 --- a/bindings/matrix-sdk-ffi/src/room_directory_search.rs +++ b/bindings/matrix-sdk-ffi/src/room_directory_search.rs @@ -30,13 +30,13 @@ pub enum PublicRoomJoinRule { } impl TryFrom for PublicRoomJoinRule { - type Error = (); + type Error = String; fn try_from(value: ruma::directory::PublicRoomJoinRule) -> Result { match value { ruma::directory::PublicRoomJoinRule::Public => Ok(Self::Public), ruma::directory::PublicRoomJoinRule::Knock => Ok(Self::Knock), - _ => Err(()), + rule => Err(format!("unsupported join rule: {rule:?}")), } } } From 4b85a81347e2e3dec6b9393d2b6087ba7914cf08 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Thu, 14 Mar 2024 16:53:18 +0100 Subject: [PATCH 27/74] docs: warning about NSFW content --- crates/matrix-sdk/src/room_directory_search.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index 24586d3512e..55e38d5d188 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -94,6 +94,8 @@ impl SearchState { /// capability of using a filter and a batch_size. This struct is also /// responsible for keeping the current state of the search, and exposing an /// update of stream of the results, reset the search, or ask for the next page. +/// NOTE: Users must take great care when using the public room search since the +/// results might contains NSFW content. /// /// # Example /// From 229105536bc815c7064834333b468b406fc040fc Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Thu, 14 Mar 2024 17:55:54 +0100 Subject: [PATCH 28/74] docs: updated docs --- crates/matrix-sdk/src/room_directory_search.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index 55e38d5d188..d8a367c0e72 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -94,7 +94,7 @@ impl SearchState { /// capability of using a filter and a batch_size. This struct is also /// responsible for keeping the current state of the search, and exposing an /// update of stream of the results, reset the search, or ask for the next page. -/// NOTE: Users must take great care when using the public room search since the +/// Users must take great care when using the public room search since the /// results might contains NSFW content. /// /// # Example From 44029009e4026d0fda23acacaa995f32ca98f550 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 18 Mar 2024 12:18:41 +0100 Subject: [PATCH 29/74] feat(sdk): Implement `ChunkIdentifier::to_last_item_position`. This patch is about an internal thing, but it makes the code easier to understand. --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index c13a2a7d293..5173f45bca5 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -402,7 +402,7 @@ impl LinkedChunk { /// /// It iterates from the last to the first item. pub fn ritems(&self) -> impl Iterator { - self.ritems_from(ItemPosition(self.latest_chunk().identifier(), 0)) + self.ritems_from(self.latest_chunk().identifier().to_last_item_position()) .expect("`iter_items_from` cannot fail because at least one empty chunk must exist") } @@ -553,6 +553,14 @@ impl ChunkIdentifierGenerator { #[repr(transparent)] pub struct ChunkIdentifier(u64); +impl ChunkIdentifier { + /// Transform the `ChunkIdentifier` into an `ItemPosition` representing the + /// last item position. + fn to_last_item_position(self) -> ItemPosition { + ItemPosition(self, 0) + } +} + /// The position of an item in a [`LinkedChunk`]. /// /// It's a pair of a chunk position and an item index. `(…, 0)` represents From 2bb07d6a4e1292b61b8ff550c19fcf9dafbc1a7a Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 18 Mar 2024 12:19:24 +0100 Subject: [PATCH 30/74] feat(sdk): Implement `LinkedChunk::items`. This patch implements the new `LinkedChunk::items` method that returns a forward iterator over items. --- .../src/event_cache/linked_chunk.rs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 5173f45bca5..029d7c8464f 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -406,6 +406,19 @@ impl LinkedChunk { .expect("`iter_items_from` cannot fail because at least one empty chunk must exist") } + /// Iterate over the items, forward. + /// + /// It iterates from the first to the last item. + pub fn items(&self) -> impl Iterator { + let first_chunk = self.first_chunk(); + let ChunkContent::Items(items) = first_chunk.content() else { + unreachable!("The first chunk is necessarily an `Items`"); + }; + + self.items_from(ItemPosition(ChunkIdentifierGenerator::FIRST_IDENTIFIER, items.len())) + .expect("`items` cannot fail because at least one empty chunk must exist") + } + /// Iterate over the items, starting from `position`, backward. /// /// It iterates from the item at `position` to the first item. @@ -447,6 +460,11 @@ impl LinkedChunk { .flatten()) } + /// Get the first chunk, as an immutable reference. + fn first_chunk(&self) -> &Chunk { + unsafe { self.first.as_ref() } + } + /// Get the latest chunk, as an immutable reference. fn latest_chunk(&self) -> &Chunk { unsafe { self.last.unwrap_or(self.first).as_ref() } @@ -1165,6 +1183,23 @@ mod tests { assert_matches!(iterator.next(), None); } + #[test] + fn test_items() { + let mut linked_chunk = LinkedChunk::::new(); + linked_chunk.push_items_back(['a', 'b']); + linked_chunk.push_gap_back(()); + linked_chunk.push_items_back(['c', 'd', 'e']); + + let mut iterator = linked_chunk.items(); + + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'a'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'b'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'c'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'd'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(3), 0), 'e'))); + assert_matches!(iterator.next(), None); + } + #[test] fn test_ritems_from() -> Result<(), LinkedChunkError> { let mut linked_chunk = LinkedChunk::::new(); From 9c4318d191cdcdcec22dbae0b1c2f0d1dc148d6a Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 18 Mar 2024 12:21:21 +0100 Subject: [PATCH 31/74] feat(sdk): Convert `ChunkIdentifier` to `ItemPosition`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch implements `From` for `ItemPosition`. It's useful when we get a `ChunkIdentifier` and we need to `insert_… _at(item_position)`. --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 029d7c8464f..e11f38f34e2 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -598,6 +598,12 @@ impl ItemPosition { } } +impl From for ItemPosition { + fn from(chunk_identifier: ChunkIdentifier) -> Self { + Self(chunk_identifier, 0) + } +} + /// An iterator over a [`LinkedChunk`] that traverses the chunk in backward /// direction (i.e. it calls `previous` on each chunk to make progress). pub struct LinkedChunkIterBackward<'a, Item, Gap, const CAP: usize> { From 4774cc8e65bef225787f0197786a4f1ef38cdeea Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 18 Mar 2024 12:22:16 +0100 Subject: [PATCH 32/74] feat(sdk): Implement `Chunk::content`. This patch implements `Chunk::content` to get an immutable reference to the content of a chunk. --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index e11f38f34e2..7490e0dad69 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -736,6 +736,11 @@ impl Chunk { self.identifier } + /// Get the content of the chunk. + pub fn content(&self) -> &ChunkContent { + &self.content + } + /// The length of the chunk, i.e. how many items are in it. /// /// It will always return 0 if it's a gap chunk. From 9dcab4ed30527707fb53b887d5ffcb87cb3d3e79 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 18 Mar 2024 12:57:18 +0100 Subject: [PATCH 33/74] feat(sdk): `ItemPosition` has the copy semantics. This patch implements `Copy` and `Clone` for `ItemPosition`. --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 7490e0dad69..5810b907110 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -583,7 +583,7 @@ impl ChunkIdentifier { /// /// It's a pair of a chunk position and an item index. `(…, 0)` represents /// the last item in the chunk. -#[derive(Debug, PartialEq)] +#[derive(Copy, Clone, Debug, PartialEq)] pub struct ItemPosition(ChunkIdentifier, usize); impl ItemPosition { From a8e522c16439d09970953cdd12a20652ef7c77c5 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 18 Mar 2024 14:11:55 +0100 Subject: [PATCH 34/74] feat(sdk): Implement `LinkedChunk::chunks`. This patch implements the `LinkedChunk::chunks` method that returns a forward iterator over the chunks. --- .../src/event_cache/linked_chunk.rs | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 5810b907110..ed39073d680 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -367,7 +367,15 @@ impl LinkedChunk { /// It iterates from the last to the first chunk. pub fn rchunks(&self) -> LinkedChunkIterBackward<'_, Item, Gap, CAP> { self.rchunks_from(self.latest_chunk().identifier()) - .expect("`iter_chunks_from` cannot fail because at least one empty chunk must exist") + .expect("`rchunks_from` cannot fail because at least one empty chunk must exist") + } + + /// Iterate over the chunks, forward. + /// + /// It iterates from the first to the last chunk. + pub fn chunks(&self) -> LinkedChunkIter<'_, Item, Gap, CAP> { + self.chunks_from(ChunkIdentifierGenerator::FIRST_IDENTIFIER) + .expect("`chunks_from` cannot fail because at least one empty chunk must exist") } /// Iterate over the chunks, starting from `identifier`, backward. @@ -403,7 +411,7 @@ impl LinkedChunk { /// It iterates from the last to the first item. pub fn ritems(&self) -> impl Iterator { self.ritems_from(self.latest_chunk().identifier().to_last_item_position()) - .expect("`iter_items_from` cannot fail because at least one empty chunk must exist") + .expect("`ritems_from` cannot fail because at least one empty chunk must exist") } /// Iterate over the items, forward. @@ -1117,6 +1125,40 @@ mod tests { assert_matches!(iterator.next(), None); } + #[test] + fn test_chunks() { + let mut linked_chunk = LinkedChunk::::new(); + linked_chunk.push_items_back(['a', 'b']); + linked_chunk.push_gap_back(()); + linked_chunk.push_items_back(['c', 'd', 'e']); + + let mut iterator = linked_chunk.chunks(); + + assert_matches!( + iterator.next(), + Some(Chunk { identifier: ChunkIdentifier(0), content: ChunkContent::Items(items), .. }) => { + assert_eq!(items, &['a', 'b']); + } + ); + assert_matches!( + iterator.next(), + Some(Chunk { identifier: ChunkIdentifier(1), content: ChunkContent::Gap(..), .. }) + ); + assert_matches!( + iterator.next(), + Some(Chunk { identifier: ChunkIdentifier(2), content: ChunkContent::Items(items), .. }) => { + assert_eq!(items, &['c', 'd']); + } + ); + assert_matches!( + iterator.next(), + Some(Chunk { identifier: ChunkIdentifier(3), content: ChunkContent::Items(items), .. }) => { + assert_eq!(items, &['e']); + } + ); + assert_matches!(iterator.next(), None); + } + #[test] fn test_rchunks_from() -> Result<(), LinkedChunkError> { let mut linked_chunk = LinkedChunk::::new(); From 88f75a85bb0031f10403b85029cf261935678eb6 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Mon, 18 Mar 2024 14:30:30 +0100 Subject: [PATCH 35/74] feat(sdk): `Chunk::is_gap` and `::is_items` are now public. --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index ed39073d680..0b8a6d9d9c2 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -720,12 +720,12 @@ impl Chunk { } /// Check whether this current chunk is a gap chunk. - fn is_gap(&self) -> bool { + pub fn is_gap(&self) -> bool { matches!(self.content, ChunkContent::Gap(..)) } /// Check whether this current chunk is an items chunk. - fn is_items(&self) -> bool { + pub fn is_items(&self) -> bool { !self.is_gap() } From 6b754acd811556c05dbacd17811510850547d0e9 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 19 Mar 2024 09:28:57 +0100 Subject: [PATCH 36/74] feat(sdk): Add `Chunk::as_ptr`. This patch adds the new `Chunk::as_ptr` method. It simplifies the code a little bit. --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 0b8a6d9d9c2..38f9dcdcb7b 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -98,7 +98,7 @@ impl LinkedChunk { if last_chunk.is_first_chunk().not() { // Maybe `last_chunk` is the same as the previous `self.last` chunk, but it's // OK. - self.last = Some(NonNull::from(last_chunk)); + self.last = Some(last_chunk.as_ptr()); } self.length += number_of_items; @@ -176,7 +176,7 @@ impl LinkedChunk { if chunk.is_first_chunk().not() && chunk.is_last_chunk() { // Maybe `chunk` is the same as the previous `self.last` chunk, but it's // OK. - self.last = Some(NonNull::from(chunk)); + self.last = Some(chunk.as_ptr()); } self.length += number_of_items; @@ -241,7 +241,7 @@ impl LinkedChunk { if chunk.is_first_chunk().not() && chunk.is_last_chunk() { // Maybe `chunk` is the same as the previous `self.last` chunk, but it's // OK. - self.last = Some(NonNull::from(chunk)); + self.last = Some(chunk.as_ptr()); } Ok(()) @@ -719,6 +719,11 @@ impl Chunk { NonNull::from(Box::leak(chunk_box)) } + /// Get the pointer to `Self`. + pub fn as_ptr(&self) -> NonNull { + NonNull::from(self) + } + /// Check whether this current chunk is a gap chunk. pub fn is_gap(&self) -> bool { matches!(self.content, ChunkContent::Gap(..)) @@ -848,7 +853,7 @@ impl Chunk { // Link to the new chunk. self.next = Some(new_chunk_ptr); // Link the new chunk to this one. - new_chunk.previous = Some(NonNull::from(self)); + new_chunk.previous = Some(self.as_ptr()); new_chunk } From 213dac2d306fd0668f0cac46235fcbe25255ed29 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 19 Mar 2024 09:30:33 +0100 Subject: [PATCH 37/74] feat(sdk): Rewrite `LinkedChunk::replace_gap_at`. This patch rewrites `LinkedChunk::replace_gap_at`. Instead of inserting new items on the _previous_ chunk of the gap and doing all the stuff here, a new items chunk is created _after_ the gap (where items are pushed), to finally unlink and remove the gap. First off, it removes an `unwrap`. Second, if the previous chunk was an items chunk, and had free space, the items would have been added in there, which is not the intended behaviour. The tests have been updated accordingly. --- .../src/event_cache/linked_chunk.rs | 50 +++++++++---------- 1 file changed, 24 insertions(+), 26 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 38f9dcdcb7b..4969cac9654 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -270,40 +270,38 @@ impl LinkedChunk { debug_assert!(chunk.is_first_chunk().not(), "A gap cannot be the first chunk"); - let (previous, number_of_items) = match &mut chunk.content { + let (maybe_last_chunk_ptr, number_of_items) = match &mut chunk.content { ChunkContent::Gap(..) => { let items = items.into_iter(); let number_of_items = items.len(); - // Find the previous chunk… - // - // SAFETY: `unwrap` is safe because we are ensured `chunk` is not the first - // chunk, so a previous chunk always exists. - let previous = chunk.previous_mut().unwrap(); + let last_inserted_chunk = chunk + // Insert a new items chunk… + .insert_next(Chunk::new_items_leaked( + chunk_identifier_generator.generate_next().unwrap(), + )) + // … and insert the items. + .push_items(items, &chunk_identifier_generator); - // … and insert the items on it. - (previous.push_items(items, &chunk_identifier_generator), number_of_items) + ( + last_inserted_chunk.is_last_chunk().then(|| last_inserted_chunk.as_ptr()), + number_of_items, + ) } ChunkContent::Items(..) => { return Err(LinkedChunkError::ChunkIsItems { identifier: chunk_identifier }) } }; - // Get the pointer to `chunk` via `previous`. - // - // SAFETY: `unwrap` is safe because we are ensured the next of the previous - // chunk is `chunk` itself. - chunk_ptr = previous.next.unwrap(); - - // Get the pointer to the `previous` via `chunk`. - let previous_ptr = chunk.previous; - // Now that new items have been pushed, we can unlink the gap chunk. chunk.unlink(); + // Get the pointer to `chunk`. + chunk_ptr = chunk.as_ptr(); + // Update `self.last` if the gap chunk was the last chunk. - if chunk.is_last_chunk() { - self.last = previous_ptr; + if let Some(last_chunk_ptr) = maybe_last_chunk_ptr { + self.last = Some(last_chunk_ptr); } self.length += number_of_items; @@ -1432,10 +1430,10 @@ mod tests { #[test] fn test_replace_gap_at() -> Result<(), LinkedChunkError> { let mut linked_chunk = LinkedChunk::::new(); - linked_chunk.push_items_back(['a', 'b', 'c']); + linked_chunk.push_items_back(['a', 'b']); linked_chunk.push_gap_back(()); - linked_chunk.push_items_back(['l', 'm', 'n']); - assert_items_eq!(linked_chunk, ['a', 'b', 'c'] [-] ['l', 'm', 'n']); + linked_chunk.push_items_back(['l', 'm']); + assert_items_eq!(linked_chunk, ['a', 'b'] [-] ['l', 'm']); // Replace a gap in the middle of the linked chunk. { @@ -1445,7 +1443,7 @@ mod tests { linked_chunk.replace_gap_at(['d', 'e', 'f', 'g', 'h'], gap_identifier)?; assert_items_eq!( linked_chunk, - ['a', 'b', 'c'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm', 'n'] + ['a', 'b'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm'] ); } @@ -1454,7 +1452,7 @@ mod tests { linked_chunk.push_gap_back(()); assert_items_eq!( linked_chunk, - ['a', 'b', 'c'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm', 'n'] [-] + ['a', 'b'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm'] [-] ); let gap_identifier = linked_chunk.chunk_identifier(Chunk::is_gap).unwrap(); @@ -1463,11 +1461,11 @@ mod tests { linked_chunk.replace_gap_at(['w', 'x', 'y', 'z'], gap_identifier)?; assert_items_eq!( linked_chunk, - ['a', 'b', 'c'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm', 'n'] ['w', 'x', 'y'] ['z'] + ['a', 'b'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm'] ['w', 'x', 'y'] ['z'] ); } - assert_eq!(linked_chunk.len(), 15); + assert_eq!(linked_chunk.len(), 13); Ok(()) } From 06e212c11dc56690a1279ba177d0dfec817f8fdf Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 19 Mar 2024 09:48:21 +0100 Subject: [PATCH 38/74] !fix rebase issue --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 4969cac9654..f27811bb29b 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -415,7 +415,7 @@ impl LinkedChunk { /// Iterate over the items, forward. /// /// It iterates from the first to the last item. - pub fn items(&self) -> impl Iterator { + pub fn items(&self) -> impl Iterator { let first_chunk = self.first_chunk(); let ChunkContent::Items(items) = first_chunk.content() else { unreachable!("The first chunk is necessarily an `Items`"); @@ -467,7 +467,7 @@ impl LinkedChunk { } /// Get the first chunk, as an immutable reference. - fn first_chunk(&self) -> &Chunk { + fn first_chunk(&self) -> &Chunk { unsafe { self.first.as_ref() } } @@ -748,7 +748,7 @@ impl Chunk { } /// Get the content of the chunk. - pub fn content(&self) -> &ChunkContent { + pub fn content(&self) -> &ChunkContent { &self.content } From 454d49aa64168f2809d941a4ab616bc16b0d9979 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 19 Mar 2024 10:05:12 +0100 Subject: [PATCH 39/74] feat(sdk): Add `Chunk::first_item_position` and `::last_item_position`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch implements `Chunk::first_item_position` and `Chunk::last_item_position` as a way to _position_ the “cursor” (`ItemPosition`) in the correct way when we want to do some particular insertion (at the beginning or at the end of a chunk). --- .../src/event_cache/linked_chunk.rs | 67 ++++++++++++++++--- 1 file changed, 58 insertions(+), 9 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index f27811bb29b..91424af88be 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -408,7 +408,7 @@ impl LinkedChunk { /// /// It iterates from the last to the first item. pub fn ritems(&self) -> impl Iterator { - self.ritems_from(self.latest_chunk().identifier().to_last_item_position()) + self.ritems_from(self.latest_chunk().last_item_position()) .expect("`ritems_from` cannot fail because at least one empty chunk must exist") } @@ -577,14 +577,6 @@ impl ChunkIdentifierGenerator { #[repr(transparent)] pub struct ChunkIdentifier(u64); -impl ChunkIdentifier { - /// Transform the `ChunkIdentifier` into an `ItemPosition` representing the - /// last item position. - fn to_last_item_position(self) -> ItemPosition { - ItemPosition(self, 0) - } -} - /// The position of an item in a [`LinkedChunk`]. /// /// It's a pair of a chunk position and an item index. `(…, 0)` represents @@ -752,6 +744,23 @@ impl Chunk { &self.content } + /// Get the [`ItemPosition`] of the first item. + /// + /// If it's a `Gap` chunk, the last part of the tuple will always be `0`. + pub fn first_item_position(&self) -> ItemPosition { + let identifier = self.identifier(); + + match &self.content { + ChunkContent::Gap(..) => ItemPosition(identifier, 0), + ChunkContent::Items(items) => ItemPosition(identifier, items.len() - 1), + } + } + + /// Get the [`ItemPosition`] of the last item. + pub fn last_item_position(&self) -> ItemPosition { + ItemPosition(self.identifier(), 0) + } + /// The length of the chunk, i.e. how many items are in it. /// /// It will always return 0 if it's a gap chunk. @@ -1469,4 +1478,44 @@ mod tests { Ok(()) } + + #[test] + fn test_chunk_item_positions() { + let mut linked_chunk = LinkedChunk::::new(); + linked_chunk.push_items_back(['a', 'b', 'c', 'd', 'e']); + linked_chunk.push_gap_back(()); + linked_chunk.push_items_back(['f']); + + assert_items_eq!(linked_chunk, ['a', 'b', 'c'] ['d', 'e'] [-] ['f']); + + let mut iterator = linked_chunk.chunks(); + + // First chunk. + { + let chunk = iterator.next().unwrap(); + assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(0), 2)); + assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(0), 0)); + } + + // Second chunk. + { + let chunk = iterator.next().unwrap(); + assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(1), 1)); + assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(1), 0)); + } + + // Gap. + { + let chunk = iterator.next().unwrap(); + assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(2), 0)); + assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(2), 0)); + } + + // Last chunk. + { + let chunk = iterator.next().unwrap(); + assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(3), 0)); + assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(3), 0)); + } + } } From 628374b8d86653e733649a506f5ae70385cd4de1 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 19 Mar 2024 10:11:58 +0100 Subject: [PATCH 40/74] feat(sdk): Optimise `LinkedChunk` iterators. This patch optimises `LinkedChunk::rchunks` and `chunks` by _not_ using `rchunks_from` and `chunks_from`. Indeed, it's faster to not call the inner `chunk` method + `unwrap`ping the result and so on. Just a tiny optimisation. This patch also uses the new `Chunk::last_item_position` method for `LinkedChunk::items`. Abstraction for the win! --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 91424af88be..8d6326c4f0a 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -349,7 +349,7 @@ impl LinkedChunk { where P: FnMut(&'a Chunk) -> bool, { - self.rchunks().find_map(|chunk| predicate(chunk).then_some(chunk.identifier())) + self.rchunks().find_map(|chunk| predicate(chunk).then(|| chunk.identifier())) } /// Search for an item, and return its position. @@ -364,16 +364,14 @@ impl LinkedChunk { /// /// It iterates from the last to the first chunk. pub fn rchunks(&self) -> LinkedChunkIterBackward<'_, Item, Gap, CAP> { - self.rchunks_from(self.latest_chunk().identifier()) - .expect("`rchunks_from` cannot fail because at least one empty chunk must exist") + LinkedChunkIterBackward::new(self.latest_chunk()) } /// Iterate over the chunks, forward. /// /// It iterates from the first to the last chunk. pub fn chunks(&self) -> LinkedChunkIter<'_, Item, Gap, CAP> { - self.chunks_from(ChunkIdentifierGenerator::FIRST_IDENTIFIER) - .expect("`chunks_from` cannot fail because at least one empty chunk must exist") + LinkedChunkIter::new(self.first_chunk()) } /// Iterate over the chunks, starting from `identifier`, backward. @@ -417,11 +415,8 @@ impl LinkedChunk { /// It iterates from the first to the last item. pub fn items(&self) -> impl Iterator { let first_chunk = self.first_chunk(); - let ChunkContent::Items(items) = first_chunk.content() else { - unreachable!("The first chunk is necessarily an `Items`"); - }; - self.items_from(ItemPosition(ChunkIdentifierGenerator::FIRST_IDENTIFIER, items.len())) + self.items_from(first_chunk.first_item_position()) .expect("`items` cannot fail because at least one empty chunk must exist") } From ffacbe866667e7f4a60bd2bb1a1d30b11161022b Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 19 Mar 2024 10:40:36 +0100 Subject: [PATCH 41/74] feat(sdk): Reverse the indices order of `ItemPosition`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, in a chunk with items `a`, `b` and `c`, their indices were 2, 1 and 0. It creates a problem when we push new items: all indices must be shifted to the left inside the same chunk. That's not optimised. This patch reverses the order, thus now `a` has index 0, `b` has index 1 and `c` has index 2. It also removes a possible bug where we use `item_index` without “reversing” it. This is now obvious that we don't need to re-compute the `item_index`, we can use it directly. --- .../src/event_cache/linked_chunk.rs | 88 ++++++++++--------- 1 file changed, 45 insertions(+), 43 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 8d6326c4f0a..5f87e30c37a 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -148,11 +148,6 @@ impl LinkedChunk { return Err(LinkedChunkError::InvalidItemIndex { index: item_index }); } - // The `ItemPosition` is computed from the latest items. Here, we manipulate the - // items in their original order: the last item comes last. Let's adjust - // `item_index`. - let item_index = current_items_length - 1 - item_index; - // Split the items. let detached_items = current_items.split_off(item_index); @@ -213,11 +208,6 @@ impl LinkedChunk { return Err(LinkedChunkError::InvalidItemIndex { index: item_index }); } - // The `ItemPosition` is computed from the latest items. Here, we manipulate the - // items in their original order: the last item comes last. Let's adjust - // `item_index`. - let item_index = current_items_length - 1 - item_index; - // Split the items. let detached_items = current_items.split_off(item_index); @@ -432,13 +422,21 @@ impl LinkedChunk { .filter_map(|chunk| match &chunk.content { ChunkContent::Gap(..) => None, ChunkContent::Items(items) => { - Some(items.iter().rev().enumerate().map(move |(item_index, item)| { - (ItemPosition(chunk.identifier(), item_index), item) + let identifier = chunk.identifier(); + + Some(items.iter().enumerate().rev().map(move |(item_index, item)| { + (ItemPosition(identifier, item_index), item) })) } }) .flatten() - .skip(position.item_index())) + .skip_while({ + let expected_index = position.item_index(); + + move |(ItemPosition(_chunk_identifier, item_index), _item)| { + *item_index != expected_index + } + })) } /// Iterate over the items, starting from `position`, forward. @@ -453,12 +451,15 @@ impl LinkedChunk { .filter_map(|chunk| match &chunk.content { ChunkContent::Gap(..) => None, ChunkContent::Items(items) => { - Some(items.iter().rev().enumerate().rev().map(move |(item_index, item)| { - (ItemPosition(chunk.identifier(), item_index), item) + let identifier = chunk.identifier(); + + Some(items.iter().enumerate().map(move |(item_index, item)| { + (ItemPosition(identifier, item_index), item) })) } }) - .flatten()) + .flatten() + .skip(position.item_index())) } /// Get the first chunk, as an immutable reference. @@ -740,9 +741,14 @@ impl Chunk { } /// Get the [`ItemPosition`] of the first item. + pub fn first_item_position(&self) -> ItemPosition { + ItemPosition(self.identifier(), 0) + } + + /// Get the [`ItemPosition`] of the last item. /// /// If it's a `Gap` chunk, the last part of the tuple will always be `0`. - pub fn first_item_position(&self) -> ItemPosition { + pub fn last_item_position(&self) -> ItemPosition { let identifier = self.identifier(); match &self.content { @@ -751,11 +757,6 @@ impl Chunk { } } - /// Get the [`ItemPosition`] of the last item. - pub fn last_item_position(&self) -> ItemPosition { - ItemPosition(self.identifier(), 0) - } - /// The length of the chunk, i.e. how many items are in it. /// /// It will always return 0 if it's a gap chunk. @@ -996,14 +997,15 @@ mod tests { { $( $all )* } { let mut iterator = $linked_chunk - .chunks_from(ChunkIdentifierGenerator::FIRST_IDENTIFIER) - .unwrap() + .chunks() .enumerate() .filter_map(|(chunk_index, chunk)| match &chunk.content { ChunkContent::Gap(..) => None, ChunkContent::Items(items) => { + let identifier = chunk.identifier(); + Some(items.iter().enumerate().map(move |(item_index, item)| { - (chunk_index, ItemPosition(chunk.identifier(), item_index), item) + (chunk_index, ItemPosition(identifier, item_index), item) })) } }) @@ -1236,10 +1238,10 @@ mod tests { let mut iterator = linked_chunk.ritems(); assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(3), 0), 'e'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'd'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'c'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'b'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'a'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'd'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'c'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'b'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'a'))); assert_matches!(iterator.next(), None); } @@ -1252,10 +1254,10 @@ mod tests { let mut iterator = linked_chunk.items(); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'a'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'b'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'c'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'd'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'a'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'b'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'c'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'd'))); assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(3), 0), 'e'))); assert_matches!(iterator.next(), None); } @@ -1270,9 +1272,9 @@ mod tests { let mut iterator = linked_chunk.ritems_from(linked_chunk.item_position(|item| *item == 'c').unwrap())?; - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'c'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'b'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'a'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'c'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'b'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'a'))); assert_matches!(iterator.next(), None); Ok(()) @@ -1288,8 +1290,8 @@ mod tests { let mut iterator = linked_chunk.items_from(linked_chunk.item_position(|item| *item == 'c').unwrap())?; - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'c'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'd'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'c'))); + assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'd'))); assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(3), 0), 'e'))); assert_matches!(iterator.next(), None); @@ -1367,7 +1369,7 @@ mod tests { ); assert_matches!( - linked_chunk.insert_items_at(['u', 'v'], ItemPosition(ChunkIdentifier(6), 0),), + linked_chunk.insert_items_at(['u', 'v'], ItemPosition(ChunkIdentifier(6), 0)), Err(LinkedChunkError::ChunkIsAGap { identifier: ChunkIdentifier(6) }) ); } @@ -1488,15 +1490,15 @@ mod tests { // First chunk. { let chunk = iterator.next().unwrap(); - assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(0), 2)); - assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(0), 0)); + assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(0), 0)); + assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(0), 2)); } // Second chunk. { let chunk = iterator.next().unwrap(); - assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(1), 1)); - assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(1), 0)); + assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(1), 0)); + assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(1), 1)); } // Gap. From 0c4b62f66453fb64e3db54b77946559641ae49b0 Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Tue, 19 Mar 2024 13:03:32 +0100 Subject: [PATCH 42/74] sdk: move `get_profile` from `Client` to `Account` (#3238) This also renames and streamlines the existing `Account::get_profile` function to `Account::fetch_profile` which now calls the more general function. --- bindings/matrix-sdk-ffi/src/client.rs | 2 +- crates/matrix-sdk/CHANGELOG.md | 1 + crates/matrix-sdk/src/account.rs | 19 +++++++++++++++---- crates/matrix-sdk/src/client/mod.rs | 11 ----------- crates/matrix-sdk/src/widget/settings/mod.rs | 2 +- 5 files changed, 18 insertions(+), 17 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index 54406134ef0..06456de64ec 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -662,7 +662,7 @@ impl Client { pub fn get_profile(&self, user_id: String) -> Result { RUNTIME.block_on(async move { let owned_user_id = UserId::parse(user_id.clone())?; - let response = self.inner.get_profile(&owned_user_id).await?; + let response = self.inner.account().fetch_user_profile_of(&owned_user_id).await?; let user_profile = UserProfile { user_id, diff --git a/crates/matrix-sdk/CHANGELOG.md b/crates/matrix-sdk/CHANGELOG.md index 9cfe0a78819..25fe72860e1 100644 --- a/crates/matrix-sdk/CHANGELOG.md +++ b/crates/matrix-sdk/CHANGELOG.md @@ -10,6 +10,7 @@ Breaking changes: - Replace `impl MediaEventContent` with `&impl MediaEventContent` in `Media::get_file`/`Media::remove_file`/`Media::get_thumbnail`/`Media::remove_thumbnail` - A custom sliding sync proxy set with `ClientBuilder::sliding_sync_proxy` now takes precedence over a discovered proxy. +- `Client::get_profile` was moved to `Account` and renamed to `Account::fetch_user_profile_of`. `Account::get_profile` was renamed to `Account::fetch_user_profile`. Additions: diff --git a/crates/matrix-sdk/src/account.rs b/crates/matrix-sdk/src/account.rs index e2d5ea45ff5..4df13fedd05 100644 --- a/crates/matrix-sdk/src/account.rs +++ b/crates/matrix-sdk/src/account.rs @@ -261,18 +261,29 @@ impl Account { /// # async { /// # let homeserver = Url::parse("http://localhost:8080")?; /// # let client = Client::new(homeserver).await?; - /// let profile = client.account().get_profile().await?; + /// let profile = client.account().fetch_user_profile().await?; /// println!( /// "You are '{:?}' with avatar '{:?}'", /// profile.displayname, profile.avatar_url /// ); /// # anyhow::Ok(()) }; /// ``` - pub async fn get_profile(&self) -> Result { + pub async fn fetch_user_profile(&self) -> Result { let user_id = self.client.user_id().ok_or(Error::AuthenticationRequired)?; + self.fetch_user_profile_of(user_id).await + } + + /// Get the profile for a given user id + /// + /// # Arguments + /// + /// * `user_id` the matrix id this function downloads the profile for + pub async fn fetch_user_profile_of( + &self, + user_id: &UserId, + ) -> Result { let request = get_profile::v3::Request::new(user_id.to_owned()); - let request_config = self.client.request_config().force_auth(); - Ok(self.client.send(request, Some(request_config)).await?) + Ok(self.client.send(request, Some(RequestConfig::short_retry().force_auth())).await?) } /// Change the password of the account. diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index 1d81a20e1b7..2242ced17af 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -48,7 +48,6 @@ use ruma::{ }, filter::{create_filter::v3::Request as FilterUploadRequest, FilterDefinition}, membership::{join_room_by_id, join_room_by_id_or_alias}, - profile::get_profile, push::{set_pusher, Pusher}, room::create_room, session::login::v3::DiscoveryInfo, @@ -2048,16 +2047,6 @@ impl Client { self.send(request, None).await } - /// Get the profile for a given user id - /// - /// # Arguments - /// - /// * `user_id` the matrix id this function downloads the profile for - pub async fn get_profile(&self, user_id: &UserId) -> Result { - let request = get_profile::v3::Request::new(user_id.to_owned()); - Ok(self.send(request, Some(RequestConfig::short_retry())).await?) - } - /// Get the notification settings of the current owner of the client. pub async fn notification_settings(&self) -> NotificationSettings { let ruleset = self.account().push_rules().await.unwrap_or_else(|_| Ruleset::new()); diff --git a/crates/matrix-sdk/src/widget/settings/mod.rs b/crates/matrix-sdk/src/widget/settings/mod.rs index 2545a49ce30..341de9b0fbb 100644 --- a/crates/matrix-sdk/src/widget/settings/mod.rs +++ b/crates/matrix-sdk/src/widget/settings/mod.rs @@ -91,7 +91,7 @@ impl WidgetSettings { props: ClientProperties, ) -> Result { self._generate_webview_url( - room.client().account().get_profile().await.unwrap_or_default(), + room.client().account().fetch_user_profile().await.unwrap_or_default(), room.own_user_id(), room.room_id(), room.client().device_id().unwrap_or("UNKNOWN".into()), From c59465c54c4cd988944a193b3c677fd686c86403 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 19 Mar 2024 13:16:37 +0100 Subject: [PATCH 43/74] crypto: Rotate fallback keys in a time-based manner (#3151) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fallback keys until now have been rotated on the basis that the homeserver tells us that a fallback key has been used. Now this leads to various problems if the server tells us too often that it has been used, i.e. if we receive the same sync response too often. It leaves us also open to the homeserver being dishonest and never telling us that the fallback key has been used. Let's avoid all these problems by just rotating the fallback key in a time-based manner. Signed-off-by: Damir Jelić Co-authored-by: Andy Balaam --- bindings/matrix-sdk-crypto-ffi/src/lib.rs | 3 +- crates/matrix-sdk-crypto/CHANGELOG.md | 6 + .../src/dehydrated_devices.rs | 2 +- crates/matrix-sdk-crypto/src/machine.rs | 2 +- crates/matrix-sdk-crypto/src/olm/account.rs | 117 +++++++++++++++--- 5 files changed, 113 insertions(+), 17 deletions(-) diff --git a/bindings/matrix-sdk-crypto-ffi/src/lib.rs b/bindings/matrix-sdk-crypto-ffi/src/lib.rs index 22623d23903..fe6ffb70572 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/lib.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/lib.rs @@ -245,7 +245,8 @@ async fn migrate_data( pickle, shared: data.account.shared, uploaded_signed_key_count: data.account.uploaded_signed_key_count as u64, - creation_local_time: MilliSecondsSinceUnixEpoch(UInt::default()), + creation_local_time: MilliSecondsSinceUnixEpoch::now(), + fallback_key_creation_timestamp: Some(MilliSecondsSinceUnixEpoch::now()), }; let account = matrix_sdk_crypto::olm::Account::from_pickle(pickled_account)?; diff --git a/crates/matrix-sdk-crypto/CHANGELOG.md b/crates/matrix-sdk-crypto/CHANGELOG.md index 0dedf89030a..722489f9ee6 100644 --- a/crates/matrix-sdk-crypto/CHANGELOG.md +++ b/crates/matrix-sdk-crypto/CHANGELOG.md @@ -1,5 +1,11 @@ # UNRELEASED +Changed: + +- Fallback keys are rotated in a time-based manner, instead of waiting for the + server to tell us that a fallback key got used. + ([#3151](https://github.com/matrix-org/matrix-rust-sdk/pull/3151)) + Breaking changes: - Rename the `OlmMachine::invalidate_group_session` method to diff --git a/crates/matrix-sdk-crypto/src/dehydrated_devices.rs b/crates/matrix-sdk-crypto/src/dehydrated_devices.rs index d33f1928219..4e4402415b3 100644 --- a/crates/matrix-sdk-crypto/src/dehydrated_devices.rs +++ b/crates/matrix-sdk-crypto/src/dehydrated_devices.rs @@ -318,7 +318,7 @@ impl DehydratedDevice { let mut transaction = self.store.transaction().await; let account = transaction.account().await?; - account.generate_fallback_key_helper(); + account.generate_fallback_key_if_needed(); let (device_keys, one_time_keys, fallback_keys) = account.keys_for_upload(); diff --git a/crates/matrix-sdk-crypto/src/machine.rs b/crates/matrix-sdk-crypto/src/machine.rs index f597863b09e..7ea6d4e296d 100644 --- a/crates/matrix-sdk-crypto/src/machine.rs +++ b/crates/matrix-sdk-crypto/src/machine.rs @@ -2305,7 +2305,7 @@ pub(crate) mod tests { .store() .with_transaction(|mut tr| async { let account = tr.account().await.unwrap(); - account.generate_fallback_key_helper(); + account.generate_fallback_key_if_needed(); account.update_uploaded_key_count(0); account.generate_one_time_keys_if_needed(); let request = machine diff --git a/crates/matrix-sdk-crypto/src/olm/account.rs b/crates/matrix-sdk-crypto/src/olm/account.rs index 8773365ebfe..7d8708db672 100644 --- a/crates/matrix-sdk-crypto/src/olm/account.rs +++ b/crates/matrix-sdk-crypto/src/olm/account.rs @@ -17,6 +17,7 @@ use std::{ fmt, ops::{Deref, Not as _}, sync::Arc, + time::Duration, }; use ruma::{ @@ -327,6 +328,14 @@ pub struct Account { /// needs to set this for us, depending on the count we will suggest the /// client to upload new keys. uploaded_signed_key_count: u64, + /// The timestamp of the last time we generated a fallback key. Fallback + /// keys are rotated in a time-based manner. This field records when we + /// either generated our first fallback key or rotated one. + /// + /// Will be `None` if we never created a fallback key, or if we're migrating + /// from a `AccountPickle` that didn't use time-based fallback key + /// rotation. + fallback_creation_timestamp: Option, } impl Deref for Account { @@ -358,6 +367,9 @@ pub struct PickledAccount { /// as creation time of own device #[serde(default = "default_account_creation_time")] pub creation_local_time: MilliSecondsSinceUnixEpoch, + /// The timestamp of the last time we generated a fallback key. + #[serde(default)] + pub fallback_key_creation_timestamp: Option, } fn default_account_creation_time() -> MilliSecondsSinceUnixEpoch { @@ -404,6 +416,7 @@ impl Account { inner: Box::new(account), shared: false, uploaded_signed_key_count: 0, + fallback_creation_timestamp: None, } } @@ -496,11 +509,11 @@ impl Account { self.generate_one_time_keys_if_needed(); } - if let Some(unused) = unused_fallback_keys { - if !unused.contains(&DeviceKeyAlgorithm::SignedCurve25519) { - // Generate a new fallback key if we don't have one. - self.generate_fallback_key_helper(); - } + // If the server supports fallback keys or if it did so in the past, shown by + // the existence of a fallback creation timestamp, generate a new one if + // we don't have one, or if the current fallback key expired. + if unused_fallback_keys.is_some() || self.fallback_creation_timestamp.is_some() { + self.generate_fallback_key_if_needed(); } } @@ -543,17 +556,61 @@ impl Account { Some(key_count as u64) } - pub(crate) fn generate_fallback_key_helper(&mut self) { - if self.inner.fallback_key().is_empty() { + /// Generate a new fallback key iff a unpublished one isn't already inside + /// of vodozemac and if the currently active one expired. + /// + /// The former is checked using [`Account::fallback_key().is_empty()`], + /// which is a hashmap that gets cleared by the + /// [`Account::mark_keys_as_published()`] call. + pub(crate) fn generate_fallback_key_if_needed(&mut self) { + if self.inner.fallback_key().is_empty() && self.fallback_key_expired() { let removed_fallback_key = self.inner.generate_fallback_key(); + self.fallback_creation_timestamp = Some(MilliSecondsSinceUnixEpoch::now()); debug!( ?removed_fallback_key, - "No unused fallback keys were found on the server, generated a new fallback key.", + "The fallback key either expired or we didn't have one: generated a new fallback key.", ); } } + /// Check if our most recent fallback key has expired. + /// + /// We consider the fallback key to be expired if it's older than a week. + /// This is the lower bound for the recommended signed pre-key bundle + /// rotation interval in the X3DH spec[1]. + /// + /// [1]: https://signal.org/docs/specifications/x3dh/#publishing-keys + fn fallback_key_expired(&self) -> bool { + const FALLBACK_KEY_MAX_AGE: Duration = Duration::from_secs(3600 * 24 * 7); + + if let Some(time) = self.fallback_creation_timestamp { + // `to_system_time()` returns `None` if the the UNIX_EPOCH + `time` doesn't fit + // into a i64. This will likely never happen, but let's rotate the + // key in case the values are messed up for some other reason. + let Some(system_time) = time.to_system_time() else { + return true; + }; + + // `elapsed()` errors if the `system_time` is in the future, this should mean + // that our clock has changed to the past, let's rotate just in case + // and then we'll get to a normal time. + let Ok(elapsed) = system_time.elapsed() else { + return true; + }; + + // Alright, our times are normal and we know how much time elapsed since the + // last time we created/rotated a fallback key. + // + // If the key is older than a week, then we rotate it. + elapsed > FALLBACK_KEY_MAX_AGE + } else { + // We never created a fallback key, or we're migrating to the time-based + // fallback key rotation, so let's generate a new fallback key. + true + } + } + fn fallback_key(&self) -> HashMap { self.inner.fallback_key() } @@ -595,6 +652,7 @@ impl Account { shared: self.shared(), uploaded_signed_key_count: self.uploaded_key_count(), creation_local_time: self.static_data.creation_local_time, + fallback_key_creation_timestamp: self.fallback_creation_timestamp, } } @@ -651,6 +709,7 @@ impl Account { inner: Box::new(account), shared: pickle.shared, uploaded_signed_key_count: pickle.uploaded_signed_key_count, + fallback_creation_timestamp: pickle.fallback_key_creation_timestamp, }) } @@ -1372,6 +1431,7 @@ mod tests { use std::{ collections::{BTreeMap, BTreeSet}, ops::Deref, + time::Duration, }; use anyhow::Result; @@ -1443,7 +1503,10 @@ mod tests { // We don't create fallback keys since we don't know if the server // supports them, we need to receive a sync response to decide if we're // going to create them or not. - assert!(fallback_keys.is_empty()); + assert!( + fallback_keys.is_empty(), + "We should not upload fallback keys until we know if the server supports them." + ); let one_time_keys = BTreeMap::from([(DeviceKeyAlgorithm::SignedCurve25519, 50u8.into())]); @@ -1451,7 +1514,11 @@ mod tests { // fallback key gets uploaded. account.update_key_counts(&one_time_keys, None); let (_, _, fallback_keys) = account.keys_for_upload(); - assert!(fallback_keys.is_empty()); + assert!( + fallback_keys.is_empty(), + "We should not upload a fallback key if we're certain that the server doesn't support \ + them." + ); // The empty array means that the server supports fallback keys but // there isn't a unused fallback key on the server. This time we upload @@ -1459,14 +1526,36 @@ mod tests { let unused_fallback_keys = &[]; account.update_key_counts(&one_time_keys, Some(unused_fallback_keys.as_ref())); let (_, _, fallback_keys) = account.keys_for_upload(); - assert!(!fallback_keys.is_empty()); + assert!( + !fallback_keys.is_empty(), + "We should upload the initial fallback key if the server supports them." + ); account.mark_keys_as_published(); - // There's an unused fallback key on the server, nothing to do here. - let unused_fallback_keys = &[DeviceKeyAlgorithm::SignedCurve25519]; + // There's no unused fallback key on the server, but our initial fallback key + // did not yet expire. + let unused_fallback_keys = &[]; account.update_key_counts(&one_time_keys, Some(unused_fallback_keys.as_ref())); let (_, _, fallback_keys) = account.keys_for_upload(); - assert!(fallback_keys.is_empty()); + assert!( + fallback_keys.is_empty(), + "We should not upload new fallback keys unless our current fallback key expires." + ); + + let fallback_key_timestamp = + account.fallback_creation_timestamp.unwrap().to_system_time().unwrap() + - Duration::from_secs(3600 * 24 * 30); + + account.fallback_creation_timestamp = + Some(MilliSecondsSinceUnixEpoch::from_system_time(fallback_key_timestamp).unwrap()); + + account.update_key_counts(&one_time_keys, None); + let (_, _, fallback_keys) = account.keys_for_upload(); + assert!( + !fallback_keys.is_empty(), + "Now that our fallback key has expired, we should try to upload a new one, even if the \ + server supposedly doesn't support fallback keys anymore" + ); Ok(()) } From 0ff9e066fbf1237d915ea12a892300a879730fbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Commaille?= Date: Tue, 19 Mar 2024 12:33:39 +0100 Subject: [PATCH 44/74] sdk: Don't enable default features of mas-oidc-client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't need the `hyper` feature as we use our own HTTP client, and the `keystore` will not be used in most cases. Signed-off-by: Kévin Commaille --- Cargo.lock | 286 ++++++++++++----------------------- crates/matrix-sdk/Cargo.toml | 2 +- 2 files changed, 100 insertions(+), 188 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6ada81b6c5e..c94b5475d69 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -719,9 +719,9 @@ dependencies = [ [[package]] name = "chrono" -version = "0.4.33" +version = "0.4.35" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9f13690e35a5e4ace198e7beea2895d29f3a9cc55015fcebe6336bd2010af9eb" +checksum = "8eaf5903dcbc0a39312feb77df2ff4c76387d591b9fc7b04a238dcf8bb62639a" dependencies = [ "android-tzdata", "iana-time-zone", @@ -2143,16 +2143,6 @@ dependencies = [ "hashbrown 0.14.3", ] -[[package]] -name = "hdrhistogram" -version = "7.5.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "765c9198f173dd59ce26ff9f95ef0aafd0a0fe01fb9d72841bc5066a4c06511d" -dependencies = [ - "byteorder", - "num-traits", -] - [[package]] name = "headers" version = "0.3.9" @@ -2322,7 +2312,6 @@ dependencies = [ "http", "hyper", "rustls", - "rustls-native-certs", "tokio", "tokio-rustls", ] @@ -2554,16 +2543,6 @@ version = "2.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f518f335dce6725a761382244631d86cf0ccb2863413590b31338feb467f9c3" -[[package]] -name = "iri-string" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "21859b667d66a4c1dacd9df0863b3efb65785474255face87f5bca39dd8407c0" -dependencies = [ - "memchr", - "serde", -] - [[package]] name = "is-terminal" version = "0.4.10" @@ -2858,9 +2837,8 @@ dependencies = [ [[package]] name = "mas-http" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da6948721a2bc05e73f8029515e05b0c7deabb6fcec51ee7f033ecbfe60b7818" +version = "0.8.0" +source = "git+https://github.com/matrix-org/matrix-authentication-service?rev=099eabd1371d2840a2f025a6372d6428039eb511#099eabd1371d2840a2f025a6372d6428039eb511" dependencies = [ "bytes", "futures-util", @@ -2868,9 +2846,7 @@ dependencies = [ "http", "http-body", "hyper", - "mas-tower", - "once_cell", - "opentelemetry", + "opentelemetry 0.22.0", "serde", "serde_json", "serde_urlencoded", @@ -2878,14 +2854,13 @@ dependencies = [ "tower", "tower-http", "tracing", - "tracing-opentelemetry", + "tracing-opentelemetry 0.23.0", ] [[package]] name = "mas-iana" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1c48820df73240471efb9fe90f90461b0029e4f0b7915e2df23633523635dfa3" +version = "0.8.0" +source = "git+https://github.com/matrix-org/matrix-authentication-service?rev=099eabd1371d2840a2f025a6372d6428039eb511#099eabd1371d2840a2f025a6372d6428039eb511" dependencies = [ "schemars", "serde", @@ -2893,9 +2868,8 @@ dependencies = [ [[package]] name = "mas-jose" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b39a251dfb34fb81d7259e91b368ee3551013406149333484ae30bd7da8c2c74" +version = "0.8.0" +source = "git+https://github.com/matrix-org/matrix-authentication-service?rev=099eabd1371d2840a2f025a6372d6428039eb511#099eabd1371d2840a2f025a6372d6428039eb511" dependencies = [ "base64ct", "chrono", @@ -2922,40 +2896,10 @@ dependencies = [ "url", ] -[[package]] -name = "mas-keystore" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a75c2a138f5805d21cf62c3947e23743349cb1303e8e3374aad14a5d571d7912" -dependencies = [ - "aead", - "base64ct", - "chacha20poly1305", - "const-oid", - "der", - "ecdsa", - "elliptic-curve", - "generic-array", - "k256", - "mas-iana", - "mas-jose", - "p256", - "p384", - "pem-rfc7468", - "pkcs1", - "pkcs8", - "rand 0.8.5", - "rsa", - "sec1", - "spki", - "thiserror", -] - [[package]] name = "mas-oidc-client" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3138f9b4240f515c740ec61e27b436f1fd5a24aabb66b51d93346c7d46e2e571" +version = "0.8.0" +source = "git+https://github.com/matrix-org/matrix-authentication-service?rev=099eabd1371d2840a2f025a6372d6428039eb511#099eabd1371d2840a2f025a6372d6428039eb511" dependencies = [ "base64ct", "bytes", @@ -2964,48 +2908,23 @@ dependencies = [ "futures-util", "headers", "http", - "http-body", - "hyper", - "hyper-rustls", "language-tags", "mas-http", "mas-iana", "mas-jose", - "mas-keystore", "mime", "oauth2-types", - "once_cell", "rand 0.8.5", - "rustls", "serde", "serde_json", "serde_urlencoded", "serde_with", "thiserror", - "tokio", "tower", - "tower-http", "tracing", "url", ] -[[package]] -name = "mas-tower" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f6beeba7843e755539b582e6240293db1e6bd428e22bc1a6ef4220b1fd2fc53d" -dependencies = [ - "http", - "opentelemetry", - "opentelemetry-http", - "opentelemetry-semantic-conventions", - "pin-project-lite", - "tokio", - "tower", - "tracing", - "tracing-opentelemetry", -] - [[package]] name = "matchers" version = "0.1.0" @@ -3268,9 +3187,9 @@ dependencies = [ "matrix-sdk-ui", "mime", "once_cell", - "opentelemetry", + "opentelemetry 0.21.0", "opentelemetry-otlp", - "opentelemetry_sdk", + "opentelemetry_sdk 0.21.2", "paranoid-android", "ruma", "sanitize-filename-reader-friendly", @@ -3282,7 +3201,7 @@ dependencies = [ "tracing", "tracing-appender", "tracing-core", - "tracing-opentelemetry", + "tracing-opentelemetry 0.22.0", "tracing-subscriber", "uniffi", "url", @@ -3713,9 +3632,8 @@ dependencies = [ [[package]] name = "oauth2-types" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fd0c3fa3366856696f31b0686570dc4a511b499e648a03e433ad8b940ed2f122" +version = "0.8.0" +source = "git+https://github.com/matrix-org/matrix-authentication-service?rev=099eabd1371d2840a2f025a6372d6428039eb511#099eabd1371d2840a2f025a6372d6428039eb511" dependencies = [ "chrono", "data-encoding", @@ -3848,6 +3766,21 @@ dependencies = [ "urlencoding", ] +[[package]] +name = "opentelemetry" +version = "0.22.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "900d57987be3f2aeb70d385fff9b27fb74c5723cc9a52d904d4f9c807a0667bf" +dependencies = [ + "futures-core", + "futures-sink", + "js-sys", + "once_cell", + "pin-project-lite", + "thiserror", + "urlencoding", +] + [[package]] name = "opentelemetry-http" version = "0.10.0" @@ -3857,7 +3790,7 @@ dependencies = [ "async-trait", "bytes", "http", - "opentelemetry", + "opentelemetry 0.21.0", "reqwest", ] @@ -3870,11 +3803,11 @@ dependencies = [ "async-trait", "futures-core", "http", - "opentelemetry", + "opentelemetry 0.21.0", "opentelemetry-http", "opentelemetry-proto", "opentelemetry-semantic-conventions", - "opentelemetry_sdk", + "opentelemetry_sdk 0.21.2", "prost 0.11.9", "reqwest", "thiserror", @@ -3888,8 +3821,8 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2e155ce5cc812ea3d1dffbd1539aed653de4bf4882d60e6e04dcf0901d674e1" dependencies = [ - "opentelemetry", - "opentelemetry_sdk", + "opentelemetry 0.21.0", + "opentelemetry_sdk 0.21.2", "prost 0.11.9", "tonic", ] @@ -3900,7 +3833,7 @@ version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f5774f1ef1f982ef2a447f6ee04ec383981a3ab99c8e77a1a7b30182e65bbc84" dependencies = [ - "opentelemetry", + "opentelemetry 0.21.0", ] [[package]] @@ -3916,7 +3849,7 @@ dependencies = [ "futures-util", "glob", "once_cell", - "opentelemetry", + "opentelemetry 0.21.0", "ordered-float", "percent-encoding", "rand 0.8.5", @@ -3925,6 +3858,25 @@ dependencies = [ "tokio-stream", ] +[[package]] +name = "opentelemetry_sdk" +version = "0.22.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9e90c7113be649e31e9a0f8b5ee24ed7a16923b322c3c5ab6367469c049d6b7e" +dependencies = [ + "async-trait", + "crossbeam-channel", + "futures-channel", + "futures-executor", + "futures-util", + "once_cell", + "opentelemetry 0.22.0", + "ordered-float", + "percent-encoding", + "rand 0.8.5", + "thiserror", +] + [[package]] name = "option-ext" version = "0.2.0" @@ -4021,26 +3973,25 @@ dependencies = [ [[package]] name = "parse-display" -version = "0.8.2" +version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c6509d08722b53e8dafe97f2027b22ccbe3a5db83cb352931e9716b0aa44bc5c" +checksum = "06af5f9333eb47bd9ba8462d612e37a8328a5cb80b13f0af4de4c3b89f52dee5" dependencies = [ - "once_cell", "parse-display-derive", "regex", + "regex-syntax 0.8.2", ] [[package]] name = "parse-display-derive" -version = "0.8.2" +version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "68517892c8daf78da08c0db777fcc17e07f2f63ef70041718f8a7630ad84f341" +checksum = "dc9252f259500ee570c75adcc4e317fa6f57a1e47747d622e0bf838002a7b790" dependencies = [ - "once_cell", "proc-macro2", "quote", "regex", - "regex-syntax 0.7.5", + "regex-syntax 0.8.2", "structmeta", "syn 2.0.48", ] @@ -4199,21 +4150,6 @@ dependencies = [ "spki", ] -[[package]] -name = "pkcs5" -version = "0.7.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e847e2c91a18bfa887dd028ec33f2fe6f25db77db3619024764914affe8b69a6" -dependencies = [ - "aes", - "cbc", - "der", - "pbkdf2", - "scrypt", - "sha2", - "spki", -] - [[package]] name = "pkcs7" version = "0.4.1" @@ -4232,8 +4168,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f950b2377845cebe5cf8b5165cb3cc1a5e0fa5cfa3e1f7f55707d8fd82e0a7b7" dependencies = [ "der", - "pkcs5", - "rand_core 0.6.4", "spki", ] @@ -4711,12 +4645,6 @@ version = "0.6.29" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f162c6dd7b008981e4d40210aca20b4bd0f9b60ca9271061b07f78537722f2e1" -[[package]] -name = "regex-syntax" -version = "0.7.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dbb5fb1acd8a1a18b3dd5be62d25485eb770e05afb408a9627d14d451bae12da" - [[package]] name = "regex-syntax" version = "0.8.2" @@ -5074,18 +5002,6 @@ dependencies = [ "sct", ] -[[package]] -name = "rustls-native-certs" -version = "0.6.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a9aace74cb666635c918e9c12bc0d348266037aa8eb599b5cba565709a8dff00" -dependencies = [ - "openssl-probe", - "rustls-pemfile", - "schannel", - "security-framework", -] - [[package]] name = "rustls-pemfile" version = "1.0.4" @@ -5117,15 +5033,6 @@ version = "1.0.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f98d2aa92eebf49b69786be48e4477826b256916e84a57ff2a4f21923b48eb4c" -[[package]] -name = "salsa20" -version = "0.10.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "97a22f5af31f73a954c10289c93e8a50cc23d971e80ee446f1f6f7137a088213" -dependencies = [ - "cipher", -] - [[package]] name = "same-file" version = "1.0.6" @@ -5159,10 +5066,13 @@ version = "0.8.16" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "45a28f4c49489add4ce10783f7911893516f15afe45d015608d41faca6bc4d29" dependencies = [ + "chrono", "dyn-clone", + "indexmap 1.9.3", "schemars_derive", "serde", "serde_json", + "url", ] [[package]] @@ -5209,17 +5119,6 @@ dependencies = [ "syn 2.0.48", ] -[[package]] -name = "scrypt" -version = "0.11.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0516a385866c09368f0b5bcd1caff3366aace790fcd46e2bb032697bb172fd1f" -dependencies = [ - "pbkdf2", - "salsa20", - "sha2", -] - [[package]] name = "sct" version = "0.7.1" @@ -5346,6 +5245,7 @@ version = "1.0.113" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "69801b70b1c3dac963ecb03a364ba0ceda9cf60c71cfe475e99864759c8b8a79" dependencies = [ + "indexmap 2.2.2", "itoa", "ryu", "serde", @@ -5385,9 +5285,9 @@ dependencies = [ [[package]] name = "serde_with" -version = "3.6.0" +version = "3.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1b0ed1662c5a68664f45b76d18deb0e234aff37207086803165c961eb695e981" +checksum = "ee80b0e361bbf88fd2f6e242ccd19cfda072cb0faa6ae694ecee08199938569a" dependencies = [ "base64 0.21.7", "chrono", @@ -5395,6 +5295,7 @@ dependencies = [ "indexmap 1.9.3", "indexmap 2.2.2", "serde", + "serde_derive", "serde_json", "serde_with_macros", "time", @@ -5402,9 +5303,9 @@ dependencies = [ [[package]] name = "serde_with_macros" -version = "3.6.0" +version = "3.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "568577ff0ef47b879f736cd66740e022f3672788cdf002a05a4e609ea5a6fb15" +checksum = "6561dc161a9224638a31d876ccdfefbc1df91d3f3a8342eddb35f055d48c7655" dependencies = [ "darling", "proc-macro2", @@ -5622,9 +5523,9 @@ checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" [[package]] name = "structmeta" -version = "0.2.0" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "78ad9e09554f0456d67a69c1584c9798ba733a5b50349a6c0d0948710523922d" +checksum = "2e1575d8d40908d70f6fd05537266b90ae71b15dbbe7a8b7dffa2b759306d329" dependencies = [ "proc-macro2", "quote", @@ -5634,9 +5535,9 @@ dependencies = [ [[package]] name = "structmeta-derive" -version = "0.2.0" +version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a60bcaff7397072dca0017d1db428e30d5002e00b6847703e2e42005c95fbe00" +checksum = "152a0b65a590ff6c3da95cabe2353ee04e6167c896b28e3b14478c2636c922fc" dependencies = [ "proc-macro2", "quote", @@ -5758,18 +5659,18 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.56" +version = "1.0.58" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d54378c645627613241d077a3a79db965db602882668f9136ac42af9ecb730ad" +checksum = "03468839009160513471e86a034bb2c5c0e4baae3b43f79ffc55c4a5427b3297" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.56" +version = "1.0.58" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fa0faa943b50f3db30a20aa7e265dbc66076993efed8463e8de414e5d06d3471" +checksum = "c61f3ba182994efc43764a46c018c347bc492c79f024e705f46567b418f6d4f7" dependencies = [ "proc-macro2", "quote", @@ -6041,7 +5942,6 @@ checksum = "b8fa9be0de6cf49e536ce1851f987bd21a43b771b09473c3549a6c853db37c1c" dependencies = [ "futures-core", "futures-util", - "hdrhistogram", "indexmap 1.9.3", "pin-project", "pin-project-lite", @@ -6067,10 +5967,7 @@ dependencies = [ "http", "http-body", "http-range-header", - "iri-string", "pin-project-lite", - "tokio", - "tower", "tower-layer", "tower-service", ] @@ -6161,8 +6058,8 @@ checksum = "c67ac25c5407e7b961fafc6f7e9aa5958fd297aada2d20fa2ae1737357e55596" dependencies = [ "js-sys", "once_cell", - "opentelemetry", - "opentelemetry_sdk", + "opentelemetry 0.21.0", + "opentelemetry_sdk 0.21.2", "smallvec", "tracing", "tracing-core", @@ -6171,6 +6068,22 @@ dependencies = [ "web-time 0.2.4", ] +[[package]] +name = "tracing-opentelemetry" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a9be14ba1bbe4ab79e9229f7f89fab8d120b865859f10527f31c033e599d2284" +dependencies = [ + "js-sys", + "once_cell", + "opentelemetry 0.22.0", + "opentelemetry_sdk 0.22.1", + "tracing", + "tracing-core", + "tracing-subscriber", + "web-time 1.0.0", +] + [[package]] name = "tracing-subscriber" version = "0.3.18" @@ -6381,7 +6294,6 @@ dependencies = [ "serde", "syn 2.0.48", "toml 0.5.11", - "uniffi_build", "uniffi_meta", ] diff --git a/crates/matrix-sdk/Cargo.toml b/crates/matrix-sdk/Cargo.toml index 48d0446ebb3..27d5af23930 100644 --- a/crates/matrix-sdk/Cargo.toml +++ b/crates/matrix-sdk/Cargo.toml @@ -86,7 +86,7 @@ imbl = { workspace = true, features = ["serde"] } indexmap = "2.0.2" js_int = "0.2.2" language-tags = { version = "0.3.2", optional = true } -mas-oidc-client = { version = "0.7.0", optional = true } +mas-oidc-client = { git = "https://github.com/matrix-org/matrix-authentication-service", rev = "099eabd1371d2840a2f025a6372d6428039eb511", default-features = false, optional = true } matrix-sdk-base = { workspace = true, features = ["uniffi"] } matrix-sdk-common = { workspace = true } matrix-sdk-indexeddb = { workspace = true, optional = true } From c6da40cf5538f2c289acd667610b23a49cb2cf73 Mon Sep 17 00:00:00 2001 From: Benjamin Bouvier Date: Tue, 19 Mar 2024 14:06:12 +0100 Subject: [PATCH 45/74] ffi: bump opentelemetry crates This gets rid of multiple duplicate crates in the dependency tree. --- Cargo.lock | 158 +++++++---------------------- bindings/matrix-sdk-ffi/Cargo.toml | 8 +- 2 files changed, 39 insertions(+), 127 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c94b5475d69..17f0edaf44e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2846,7 +2846,7 @@ dependencies = [ "http", "http-body", "hyper", - "opentelemetry 0.22.0", + "opentelemetry", "serde", "serde_json", "serde_urlencoded", @@ -2854,7 +2854,7 @@ dependencies = [ "tower", "tower-http", "tracing", - "tracing-opentelemetry 0.23.0", + "tracing-opentelemetry", ] [[package]] @@ -3187,9 +3187,9 @@ dependencies = [ "matrix-sdk-ui", "mime", "once_cell", - "opentelemetry 0.21.0", + "opentelemetry", "opentelemetry-otlp", - "opentelemetry_sdk 0.21.2", + "opentelemetry_sdk", "paranoid-android", "ruma", "sanitize-filename-reader-friendly", @@ -3201,7 +3201,7 @@ dependencies = [ "tracing", "tracing-appender", "tracing-core", - "tracing-opentelemetry 0.22.0", + "tracing-opentelemetry", "tracing-subscriber", "uniffi", "url", @@ -3750,22 +3750,6 @@ dependencies = [ "vcpkg", ] -[[package]] -name = "opentelemetry" -version = "0.21.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e32339a5dc40459130b3bd269e9892439f55b33e772d2a9d402a789baaf4e8a" -dependencies = [ - "futures-core", - "futures-sink", - "indexmap 2.2.2", - "js-sys", - "once_cell", - "pin-project-lite", - "thiserror", - "urlencoding", -] - [[package]] name = "opentelemetry" version = "0.22.0" @@ -3783,32 +3767,32 @@ dependencies = [ [[package]] name = "opentelemetry-http" -version = "0.10.0" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7f51189ce8be654f9b5f7e70e49967ed894e84a06fc35c6c042e64ac1fc5399e" +checksum = "7cbfa5308166ca861434f0b0913569579b8e587430a3d6bcd7fd671921ec145a" dependencies = [ "async-trait", "bytes", "http", - "opentelemetry 0.21.0", + "opentelemetry", "reqwest", ] [[package]] name = "opentelemetry-otlp" -version = "0.14.0" +version = "0.15.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f24cda83b20ed2433c68241f918d0f6fdec8b1d43b7a9590ab4420c5095ca930" +checksum = "1a016b8d9495c639af2145ac22387dcb88e44118e45320d9238fbf4e7889abcb" dependencies = [ "async-trait", "futures-core", "http", - "opentelemetry 0.21.0", + "opentelemetry", "opentelemetry-http", "opentelemetry-proto", "opentelemetry-semantic-conventions", - "opentelemetry_sdk 0.21.2", - "prost 0.11.9", + "opentelemetry_sdk", + "prost", "reqwest", "thiserror", "tokio", @@ -3817,30 +3801,27 @@ dependencies = [ [[package]] name = "opentelemetry-proto" -version = "0.4.0" +version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a2e155ce5cc812ea3d1dffbd1539aed653de4bf4882d60e6e04dcf0901d674e1" +checksum = "3a8fddc9b68f5b80dae9d6f510b88e02396f006ad48cac349411fbecc80caae4" dependencies = [ - "opentelemetry 0.21.0", - "opentelemetry_sdk 0.21.2", - "prost 0.11.9", + "opentelemetry", + "opentelemetry_sdk", + "prost", "tonic", ] [[package]] name = "opentelemetry-semantic-conventions" -version = "0.13.0" +version = "0.14.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f5774f1ef1f982ef2a447f6ee04ec383981a3ab99c8e77a1a7b30182e65bbc84" -dependencies = [ - "opentelemetry 0.21.0", -] +checksum = "f9ab5bd6c42fb9349dcf28af2ba9a0667f697f9bdcca045d39f2cec5543e2910" [[package]] name = "opentelemetry_sdk" -version = "0.21.2" +version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f16aec8a98a457a52664d69e0091bac3a0abd18ead9b641cb00202ba4e0efe4" +checksum = "9e90c7113be649e31e9a0f8b5ee24ed7a16923b322c3c5ab6367469c049d6b7e" dependencies = [ "async-trait", "crossbeam-channel", @@ -3849,7 +3830,7 @@ dependencies = [ "futures-util", "glob", "once_cell", - "opentelemetry 0.21.0", + "opentelemetry", "ordered-float", "percent-encoding", "rand 0.8.5", @@ -3858,25 +3839,6 @@ dependencies = [ "tokio-stream", ] -[[package]] -name = "opentelemetry_sdk" -version = "0.22.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9e90c7113be649e31e9a0f8b5ee24ed7a16923b322c3c5ab6367469c049d6b7e" -dependencies = [ - "async-trait", - "crossbeam-channel", - "futures-channel", - "futures-executor", - "futures-util", - "once_cell", - "opentelemetry 0.22.0", - "ordered-float", - "percent-encoding", - "rand 0.8.5", - "thiserror", -] - [[package]] name = "option-ext" version = "0.2.0" @@ -4359,16 +4321,6 @@ dependencies = [ "unarray", ] -[[package]] -name = "prost" -version = "0.11.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0b82eaa1d779e9a4bc1c3217db8ffbeabaae1dca241bf70183242128d48681cd" -dependencies = [ - "bytes", - "prost-derive 0.11.9", -] - [[package]] name = "prost" version = "0.12.3" @@ -4376,20 +4328,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "146c289cda302b98a28d40c8b3b90498d6e526dd24ac2ecea73e4e491685b94a" dependencies = [ "bytes", - "prost-derive 0.12.3", -] - -[[package]] -name = "prost-derive" -version = "0.11.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e5d2d8d10f3c6ded6da8b05b5fb3b8a5082514344d56c9f871412d29b4e075b4" -dependencies = [ - "anyhow", - "itertools 0.10.5", - "proc-macro2", - "quote", - "syn 1.0.109", + "prost-derive", ] [[package]] @@ -5908,16 +5847,15 @@ dependencies = [ [[package]] name = "tonic" -version = "0.9.2" +version = "0.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3082666a3a6433f7f511c7192923fa1fe07c69332d3c6a2e6bb040b569199d5a" +checksum = "76c4eb7a4e9ef9d4763600161f12f5070b92a578e1b634db88a6887844c91a13" dependencies = [ + "async-stream", "async-trait", "axum", "base64 0.21.7", "bytes", - "futures-core", - "futures-util", "h2", "http", "http-body", @@ -5925,7 +5863,7 @@ dependencies = [ "hyper-timeout", "percent-encoding", "pin-project", - "prost 0.11.9", + "prost", "tokio", "tokio-stream", "tower", @@ -6050,24 +5988,6 @@ dependencies = [ "tracing-core", ] -[[package]] -name = "tracing-opentelemetry" -version = "0.22.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c67ac25c5407e7b961fafc6f7e9aa5958fd297aada2d20fa2ae1737357e55596" -dependencies = [ - "js-sys", - "once_cell", - "opentelemetry 0.21.0", - "opentelemetry_sdk 0.21.2", - "smallvec", - "tracing", - "tracing-core", - "tracing-log", - "tracing-subscriber", - "web-time 0.2.4", -] - [[package]] name = "tracing-opentelemetry" version = "0.23.0" @@ -6076,12 +5996,14 @@ checksum = "a9be14ba1bbe4ab79e9229f7f89fab8d120b865859f10527f31c033e599d2284" dependencies = [ "js-sys", "once_cell", - "opentelemetry 0.22.0", - "opentelemetry_sdk 0.22.1", + "opentelemetry", + "opentelemetry_sdk", + "smallvec", "tracing", "tracing-core", + "tracing-log", "tracing-subscriber", - "web-time 1.0.0", + "web-time", ] [[package]] @@ -6137,7 +6059,7 @@ checksum = "34778c17965aa2a08913b57e1f34db9b4a63f5de31768b55bf20d2795f921259" dependencies = [ "getrandom 0.2.12", "rand 0.8.5", - "web-time 1.0.0", + "web-time", ] [[package]] @@ -6436,7 +6358,7 @@ dependencies = [ "hmac", "matrix-pickle", "pkcs7", - "prost 0.12.3", + "prost", "rand 0.8.5", "serde", "serde_bytes", @@ -6599,16 +6521,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "web-time" -version = "0.2.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aa30049b1c872b72c89866d458eae9f20380ab280ffd1b1e18df2d3e2d98cfe0" -dependencies = [ - "js-sys", - "wasm-bindgen", -] - [[package]] name = "web-time" version = "1.0.0" diff --git a/bindings/matrix-sdk-ffi/Cargo.toml b/bindings/matrix-sdk-ffi/Cargo.toml index 502cf34077e..8a110442112 100644 --- a/bindings/matrix-sdk-ffi/Cargo.toml +++ b/bindings/matrix-sdk-ffi/Cargo.toml @@ -32,9 +32,9 @@ futures-util = { workspace = true } matrix-sdk-ui = { workspace = true, features = ["e2e-encryption", "uniffi", "experimental-room-list-with-unified-invites"] } mime = "0.3.16" once_cell = { workspace = true } -opentelemetry = "0.21.0" -opentelemetry_sdk = { version = "0.21.0", features = ["rt-tokio"] } -opentelemetry-otlp = { version = "0.14.0", features = ["tokio", "reqwest-client", "http-proto"] } +opentelemetry = "0.22.0" +opentelemetry_sdk = { version = "0.22.0", features = ["rt-tokio"] } +opentelemetry-otlp = { version = "0.15.0", features = ["tokio", "reqwest-client", "http-proto"] } ruma = { workspace = true, features = ["html", "unstable-unspecified", "unstable-msc3488", "compat-unset-avatar", "unstable-msc3245-v1-compat"] } sanitize-filename-reader-friendly = "2.2.1" serde = { workspace = true } @@ -42,7 +42,7 @@ serde_json = { workspace = true } thiserror = { workspace = true } tracing = { workspace = true } tracing-core = { workspace = true } -tracing-opentelemetry = "0.22.0" +tracing-opentelemetry = "0.23.0" tracing-subscriber = { version = "0.3", features = ["env-filter"] } tracing-appender = { version = "0.2.2" } tokio = { version = "1", features = ["rt-multi-thread", "macros"] } From 1e35188aec90f2abe39999258a394ecda8a6318e Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Tue, 19 Mar 2024 10:33:13 -0400 Subject: [PATCH 46/74] Add a dehydrated flag to device_keys of dehydrated devices (#3164) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Hubert Chathi Co-authored-by: Damir Jelić --- Cargo.lock | 1 + bindings/matrix-sdk-crypto-ffi/src/lib.rs | 1 + crates/matrix-sdk-crypto/CHANGELOG.md | 3 ++ crates/matrix-sdk-crypto/Cargo.toml | 1 + .../src/dehydrated_devices.rs | 12 ++++++-- .../src/identities/device.rs | 5 ++++ crates/matrix-sdk-crypto/src/olm/account.rs | 28 +++++++++++++++++-- .../src/types/device_keys.rs | 10 +++++++ 8 files changed, 57 insertions(+), 4 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 17f0edaf44e..880825f6c3d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3115,6 +3115,7 @@ dependencies = [ "http", "indoc", "itertools 0.12.1", + "js_option", "matrix-sdk-common", "matrix-sdk-qrcode", "matrix-sdk-test", diff --git a/bindings/matrix-sdk-crypto-ffi/src/lib.rs b/bindings/matrix-sdk-crypto-ffi/src/lib.rs index fe6ffb70572..dc48c07f503 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/lib.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/lib.rs @@ -243,6 +243,7 @@ async fn migrate_data( user_id: parse_user_id(&data.account.user_id)?, device_id: device_id.clone(), pickle, + dehydrated: false, // dehydrated devices are never involved in migration shared: data.account.shared, uploaded_signed_key_count: data.account.uploaded_signed_key_count as u64, creation_local_time: MilliSecondsSinceUnixEpoch::now(), diff --git a/crates/matrix-sdk-crypto/CHANGELOG.md b/crates/matrix-sdk-crypto/CHANGELOG.md index 722489f9ee6..7e3b3abfea9 100644 --- a/crates/matrix-sdk-crypto/CHANGELOG.md +++ b/crates/matrix-sdk-crypto/CHANGELOG.md @@ -14,6 +14,9 @@ Breaking changes: - Move `OlmMachine::export_room_keys` to `matrix_sdk_crypto::store::Store`. (Call it with `olm_machine.store().export_room_keys(...)`.) +- Add new `dehydrated` property to `olm::account::PickledAccount`. + ([#3164](https://github.com/matrix-org/matrix-rust-sdk/pull/3164)) + Additions: - When Olm message decryption fails, report the error code(s) from the failure. diff --git a/crates/matrix-sdk-crypto/Cargo.toml b/crates/matrix-sdk-crypto/Cargo.toml index 16def859cc1..0e3f773234f 100644 --- a/crates/matrix-sdk-crypto/Cargo.toml +++ b/crates/matrix-sdk-crypto/Cargo.toml @@ -43,6 +43,7 @@ hkdf = "0.12.3" hmac = "0.12.1" http = { workspace = true, optional = true } # feature = testing only itertools = { workspace = true } +js_option = "0.1.1" matrix-sdk-qrcode = { workspace = true, optional = true } matrix-sdk-common = { workspace = true } pbkdf2 = { version = "0.12.2", default-features = false } diff --git a/crates/matrix-sdk-crypto/src/dehydrated_devices.rs b/crates/matrix-sdk-crypto/src/dehydrated_devices.rs index 4e4402415b3..dad7938c742 100644 --- a/crates/matrix-sdk-crypto/src/dehydrated_devices.rs +++ b/crates/matrix-sdk-crypto/src/dehydrated_devices.rs @@ -95,7 +95,7 @@ impl DehydratedDevices { let user_id = self.inner.user_id(); let user_identity = self.inner.store().private_identity(); - let account = Account::new(user_id); + let account = Account::new_dehydrated(user_id); let store = Arc::new(CryptoStoreWrapper::new(user_id, MemoryStore::new())); let verification_machine = VerificationMachine::new( @@ -378,6 +378,7 @@ fn expand_pickle_key(key: &[u8; 32], device_id: &DeviceId) -> Box<[u8; 32]> { mod tests { use std::{collections::BTreeMap, iter}; + use js_option::JsOption; use matrix_sdk_test::async_test; use ruma::{ api::client::keys::get_keys::v3::Response as KeysQueryResponse, assign, @@ -390,7 +391,7 @@ mod tests { create_session, get_prepared_machine_test_helper, to_device_requests_to_content, }, olm::OutboundGroupSession, - types::events::ToDeviceEvent, + types::{events::ToDeviceEvent, DeviceKeys as DeviceKeysType}, utilities::json_convert, EncryptionSettings, OlmMachine, }; @@ -477,6 +478,13 @@ mod tests { !request.fallback_keys.is_empty(), "The dehydrated device creation request should contain some fallback keys" ); + + let device_keys: DeviceKeysType = request.device_keys.deserialize_as().unwrap(); + assert_eq!( + device_keys.dehydrated, + JsOption::Some(true), + "The device keys of the dehydrated device should be marked as dehydrated." + ); } #[async_test] diff --git a/crates/matrix-sdk-crypto/src/identities/device.rs b/crates/matrix-sdk-crypto/src/identities/device.rs index cdaebf021b5..8fc0cc7639b 100644 --- a/crates/matrix-sdk-crypto/src/identities/device.rs +++ b/crates/matrix-sdk-crypto/src/identities/device.rs @@ -481,6 +481,11 @@ impl Device { Ok(raw_encrypted) } + + /// Whether or not the device is a dehydrated device. + pub fn is_dehydrated(&self) -> bool { + self.inner.inner.dehydrated.unwrap_or(false) + } } /// A read only view over all devices belonging to a user. diff --git a/crates/matrix-sdk-crypto/src/olm/account.rs b/crates/matrix-sdk-crypto/src/olm/account.rs index 7d8708db672..036367d2de1 100644 --- a/crates/matrix-sdk-crypto/src/olm/account.rs +++ b/crates/matrix-sdk-crypto/src/olm/account.rs @@ -20,6 +20,7 @@ use std::{ time::Duration, }; +use js_option::JsOption; use ruma::{ api::client::{ dehydrated_device::{DehydratedDeviceData, DehydratedDeviceV1}, @@ -162,6 +163,8 @@ pub struct StaticAccountData { pub device_id: OwnedDeviceId, /// The associated identity keys. pub identity_keys: Arc, + /// Whether the account is for a dehydrated device. + pub dehydrated: bool, // The creation time of the account in milliseconds since epoch. creation_local_time: MilliSecondsSinceUnixEpoch, } @@ -282,13 +285,17 @@ impl StaticAccountData { ), ]); - DeviceKeys::new( + let mut ret = DeviceKeys::new( (*self.user_id).to_owned(), (*self.device_id).to_owned(), Self::ALGORITHMS.iter().map(|a| (**a).clone()).collect(), keys, Default::default(), - ) + ); + if self.dehydrated { + ret.dehydrated = JsOption::Some(true); + } + ret } /// Get the user id of the owner of the account. @@ -361,6 +368,9 @@ pub struct PickledAccount { pub pickle: AccountPickle, /// Was the account shared. pub shared: bool, + /// Whether this is for a dehydrated device + #[serde(default)] + pub dehydrated: bool, /// The number of uploaded one-time keys we have on the server. pub uploaded_signed_key_count: u64, /// The local time creation of this account (milliseconds since epoch), used @@ -411,6 +421,7 @@ impl Account { user_id: user_id.into(), device_id: device_id.into(), identity_keys: Arc::new(identity_keys), + dehydrated: false, creation_local_time: MilliSecondsSinceUnixEpoch::now(), }, inner: Box::new(account), @@ -437,6 +448,17 @@ impl Account { Self::new_helper(account, user_id, &device_id) } + /// Create a new random Olm Account for a dehydrated device + pub fn new_dehydrated(user_id: &UserId) -> Self { + let account = InnerAccount::new(); + let device_id: OwnedDeviceId = + base64_encode(account.identity_keys().curve25519.as_bytes()).into(); + + let mut ret = Self::new_helper(account, user_id, &device_id); + ret.static_data.dehydrated = true; + ret + } + /// Get the immutable data for this account. pub fn static_data(&self) -> &StaticAccountData { &self.static_data @@ -650,6 +672,7 @@ impl Account { device_id: self.device_id().to_owned(), pickle, shared: self.shared(), + dehydrated: self.static_data.dehydrated, uploaded_signed_key_count: self.uploaded_key_count(), creation_local_time: self.static_data.creation_local_time, fallback_key_creation_timestamp: self.fallback_creation_timestamp, @@ -704,6 +727,7 @@ impl Account { user_id: (*pickle.user_id).into(), device_id: (*pickle.device_id).into(), identity_keys: Arc::new(identity_keys), + dehydrated: pickle.dehydrated, creation_local_time: pickle.creation_local_time, }, inner: Box::new(account), diff --git a/crates/matrix-sdk-crypto/src/types/device_keys.rs b/crates/matrix-sdk-crypto/src/types/device_keys.rs index c4bdd87c5da..0689ef19305 100644 --- a/crates/matrix-sdk-crypto/src/types/device_keys.rs +++ b/crates/matrix-sdk-crypto/src/types/device_keys.rs @@ -20,6 +20,7 @@ use std::collections::BTreeMap; +use js_option::JsOption; use ruma::{ serde::Raw, DeviceKeyAlgorithm, DeviceKeyId, OwnedDeviceId, OwnedDeviceKeyId, OwnedUserId, }; @@ -52,6 +53,10 @@ pub struct DeviceKeys { /// Signatures for the device key object. pub signatures: Signatures, + /// Whether the device is a dehydrated device or not + #[serde(default, skip_serializing_if = "JsOption::is_undefined")] + pub dehydrated: JsOption, + /// Additional data added to the device key information by intermediate /// servers, and not covered by the signatures. #[serde(default, skip_serializing_if = "UnsignedDeviceInfo::is_empty")] @@ -77,6 +82,7 @@ impl DeviceKeys { algorithms, keys, signatures, + dehydrated: JsOption::Undefined, unsigned: Default::default(), other: BTreeMap::new(), } @@ -182,6 +188,8 @@ struct DeviceKeyHelper { pub device_id: OwnedDeviceId, pub algorithms: Vec, pub keys: BTreeMap, + #[serde(default, skip_serializing_if = "JsOption::is_undefined")] + pub dehydrated: JsOption, pub signatures: Signatures, #[serde(default, skip_serializing_if = "UnsignedDeviceInfo::is_empty")] pub unsigned: UnsignedDeviceInfo, @@ -216,6 +224,7 @@ impl TryFrom for DeviceKeys { device_id: value.device_id, algorithms: value.algorithms, keys: keys?, + dehydrated: value.dehydrated, signatures: value.signatures, unsigned: value.unsigned, other: value.other, @@ -233,6 +242,7 @@ impl From for DeviceKeyHelper { device_id: value.device_id, algorithms: value.algorithms, keys, + dehydrated: value.dehydrated, signatures: value.signatures, unsigned: value.unsigned, other: value.other, From 099c8550496041e08ebce30fef40392af1693717 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 15 Mar 2024 13:05:26 +0000 Subject: [PATCH 47/74] crypto: Store TrackedUsers in MemoryStore --- .../src/store/memorystore.rs | 33 ++++++++++++++++--- crates/matrix-sdk-crypto/src/store/mod.rs | 2 +- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/store/memorystore.rs b/crates/matrix-sdk-crypto/src/store/memorystore.rs index 00385a140ef..395b54bbc1e 100644 --- a/crates/matrix-sdk-crypto/src/store/memorystore.rs +++ b/crates/matrix-sdk-crypto/src/store/memorystore.rs @@ -55,6 +55,7 @@ pub struct MemoryStore { sessions: SessionStore, inbound_group_sessions: GroupSessionStore, outbound_group_sessions: StdRwLock>, + tracked_users: StdRwLock>, olm_hashes: StdRwLock>>, devices: DeviceStore, identities: StdRwLock>, @@ -76,6 +77,7 @@ impl Default for MemoryStore { sessions: SessionStore::new(), inbound_group_sessions: GroupSessionStore::new(), outbound_group_sessions: Default::default(), + tracked_users: Default::default(), olm_hashes: Default::default(), devices: DeviceStore::new(), identities: Default::default(), @@ -315,10 +317,13 @@ impl CryptoStore for MemoryStore { } async fn load_tracked_users(&self) -> Result> { - Ok(Vec::new()) + Ok(self.tracked_users.read().unwrap().clone()) } - async fn save_tracked_users(&self, _: &[(&UserId, bool)]) -> Result<()> { + async fn save_tracked_users(&self, tracked_users: &[(&UserId, bool)]) -> Result<()> { + self.tracked_users.write().unwrap().extend(tracked_users.iter().map(|(user_id, dirty)| { + TrackedUser { user_id: user_id.to_owned().into(), dirty: *dirty } + })); Ok(()) } @@ -472,7 +477,7 @@ impl CryptoStore for MemoryStore { #[cfg(test)] mod tests { use matrix_sdk_test::async_test; - use ruma::room_id; + use ruma::{room_id, user_id}; use vodozemac::{Curve25519PublicKey, Ed25519PublicKey}; use crate::{ @@ -526,7 +531,7 @@ mod tests { #[async_test] async fn test_outbound_group_session_store() { - // Given an outbound sessions + // Given an outbound session let (account, _) = get_account_and_session_test_helper(); let room_id = room_id!("!test:localhost"); let (outbound, _) = account.create_group_session_pair_with_defaults(room_id).await; @@ -543,6 +548,26 @@ mod tests { ); } + #[async_test] + async fn test_tracked_users_store() { + // Given some tracked users + let tracked_users = + &[(user_id!("@dirty_user:s"), true), (user_id!("@clean_user:t"), false)]; + + // When we save them to the store + let store = MemoryStore::new(); + store.save_tracked_users(tracked_users).await.unwrap(); + + // Then we can get them out again + let loaded_tracked_users = + store.load_tracked_users().await.expect("failed to load tracked users"); + assert_eq!(loaded_tracked_users[0].user_id, user_id!("@dirty_user:s")); + assert!(loaded_tracked_users[0].dirty); + assert_eq!(loaded_tracked_users[1].user_id, user_id!("@clean_user:t")); + assert!(!loaded_tracked_users[1].dirty); + assert_eq!(loaded_tracked_users.len(), 2); + } + #[async_test] async fn test_device_store() { let device = get_device(); diff --git a/crates/matrix-sdk-crypto/src/store/mod.rs b/crates/matrix-sdk-crypto/src/store/mod.rs index 67969a0e23f..5ae09a50989 100644 --- a/crates/matrix-sdk-crypto/src/store/mod.rs +++ b/crates/matrix-sdk-crypto/src/store/mod.rs @@ -526,7 +526,7 @@ pub struct Changes { } /// A user for which we are tracking the list of devices. -#[derive(Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, Serialize, Deserialize)] pub struct TrackedUser { /// The user ID of the user. pub user_id: OwnedUserId, From 3ccd2e9f8fb2763ea8be46fbebf79d8807d2851f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 19 Mar 2024 15:18:27 +0100 Subject: [PATCH 48/74] crypto: Remove the KeysBackup variant of the OutgoingRequest enum Backing up room keys isn't part of the outgoing requests processing loop, instead the user is supposed to have a separate loop calling `BackupMachine::backup()`. --- bindings/matrix-sdk-crypto-ffi/src/responses.rs | 1 - crates/matrix-sdk-crypto/src/backups/mod.rs | 9 +-------- crates/matrix-sdk-crypto/src/requests.rs | 8 -------- .../src/verification/event_enums.rs | 1 - crates/matrix-sdk/src/encryption/mod.rs | 17 ----------------- 5 files changed, 1 insertion(+), 35 deletions(-) diff --git a/bindings/matrix-sdk-crypto-ffi/src/responses.rs b/bindings/matrix-sdk-crypto-ffi/src/responses.rs index 17d115b3aa7..7f3a4501648 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/responses.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/responses.rs @@ -164,7 +164,6 @@ impl From for Request { }, RoomMessage(r) => Request::from(r), KeysClaim(c) => (r.request_id().to_owned(), c.clone()).into(), - KeysBackup(b) => (r.request_id().to_owned(), b.clone()).into(), } } } diff --git a/crates/matrix-sdk-crypto/src/backups/mod.rs b/crates/matrix-sdk-crypto/src/backups/mod.rs index 6d73cbc3561..e89f867bf6a 100644 --- a/crates/matrix-sdk-crypto/src/backups/mod.rs +++ b/crates/matrix-sdk-crypto/src/backups/mod.rs @@ -39,8 +39,7 @@ use crate::{ olm::{BackedUpRoomKey, ExportedRoomKey, InboundGroupSession, SignedJsonObject}, store::{BackupDecryptionKey, BackupKeys, Changes, RoomKeyCounts, Store}, types::{MegolmV1AuthData, RoomKeyBackupInfo, Signatures}, - CryptoStoreError, Device, KeysBackupRequest, OutgoingRequest, RoomKeyImportResult, - SignatureError, + CryptoStoreError, Device, KeysBackupRequest, RoomKeyImportResult, SignatureError, }; mod keys; @@ -70,12 +69,6 @@ struct PendingBackup { sessions: BTreeMap>>, } -impl From for OutgoingRequest { - fn from(b: PendingBackup) -> Self { - OutgoingRequest { request_id: b.request_id, request: Arc::new(b.request.into()) } - } -} - /// The result of a signature verification of a signed JSON object. #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct SignatureVerification { diff --git a/crates/matrix-sdk-crypto/src/requests.rs b/crates/matrix-sdk-crypto/src/requests.rs index a716f1b0127..2a5751b7673 100644 --- a/crates/matrix-sdk-crypto/src/requests.rs +++ b/crates/matrix-sdk-crypto/src/requests.rs @@ -218,8 +218,6 @@ pub enum OutgoingRequests { /// A room message request, usually for sending in-room interactive /// verification events. RoomMessage(RoomMessageRequest), - /// A request that will back up a batch of room keys to the server. - KeysBackup(KeysBackupRequest), } #[cfg(test)] @@ -235,12 +233,6 @@ impl From for OutgoingRequests { } } -impl From for OutgoingRequests { - fn from(r: KeysBackupRequest) -> Self { - Self::KeysBackup(r) - } -} - impl From for OutgoingRequests { fn from(r: KeysClaimRequest) -> Self { Self::KeysClaim(r) diff --git a/crates/matrix-sdk-crypto/src/verification/event_enums.rs b/crates/matrix-sdk-crypto/src/verification/event_enums.rs index 45b45685f76..146d5a9a6ba 100644 --- a/crates/matrix-sdk-crypto/src/verification/event_enums.rs +++ b/crates/matrix-sdk-crypto/src/verification/event_enums.rs @@ -771,7 +771,6 @@ impl TryFrom for OutgoingContent { match value.request() { crate::OutgoingRequests::KeysUpload(_) | crate::OutgoingRequests::KeysQuery(_) - | crate::OutgoingRequests::KeysBackup(_) | crate::OutgoingRequests::SignatureUpload(_) | crate::OutgoingRequests::KeysClaim(_) => Err("Invalid request type".to_owned()), crate::OutgoingRequests::ToDeviceRequest(r) => Self::try_from(r.clone()), diff --git a/crates/matrix-sdk/src/encryption/mod.rs b/crates/matrix-sdk/src/encryption/mod.rs index de5774c1ac9..47b63cfb852 100644 --- a/crates/matrix-sdk/src/encryption/mod.rs +++ b/crates/matrix-sdk/src/encryption/mod.rs @@ -36,7 +36,6 @@ use matrix_sdk_base::crypto::{ use matrix_sdk_common::executor::spawn; use ruma::{ api::client::{ - backup::add_backup_keys::v3::Response as KeysBackupResponse, keys::{ get_keys, upload_keys, upload_signing_keys::v3::Request as UploadSigningKeysRequest, }, @@ -541,27 +540,11 @@ impl Client { let response = self.send(request.clone(), None).await?; self.mark_request_as_sent(r.request_id(), &response).await?; } - OutgoingRequests::KeysBackup(request) => { - let response = self.send_backup_request(request).await?; - self.mark_request_as_sent(r.request_id(), &response).await?; - } } Ok(()) } - async fn send_backup_request( - &self, - request: &matrix_sdk_base::crypto::KeysBackupRequest, - ) -> Result { - let request = ruma::api::client::backup::add_backup_keys::v3::Request::new( - request.version.clone(), - request.rooms.clone(), - ); - - Ok(self.send(request, None).await?) - } - pub(crate) async fn send_outgoing_requests(&self) -> Result<()> { const MAX_CONCURRENT_REQUESTS: usize = 20; From 9caec95c5e0a598dfbb5473cd4d5f3d4b972abfc Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 15 Mar 2024 13:58:56 +0000 Subject: [PATCH 49/74] crypto: Save private identity in the MemoryStore --- .../src/store/memorystore.rs | 30 +++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/store/memorystore.rs b/crates/matrix-sdk-crypto/src/store/memorystore.rs index 395b54bbc1e..43f97fc7df6 100644 --- a/crates/matrix-sdk-crypto/src/store/memorystore.rs +++ b/crates/matrix-sdk-crypto/src/store/memorystore.rs @@ -55,6 +55,7 @@ pub struct MemoryStore { sessions: SessionStore, inbound_group_sessions: GroupSessionStore, outbound_group_sessions: StdRwLock>, + private_identity: StdRwLock>, tracked_users: StdRwLock>, olm_hashes: StdRwLock>>, devices: DeviceStore, @@ -77,6 +78,7 @@ impl Default for MemoryStore { sessions: SessionStore::new(), inbound_group_sessions: GroupSessionStore::new(), outbound_group_sessions: Default::default(), + private_identity: Default::default(), tracked_users: Default::default(), olm_hashes: Default::default(), devices: DeviceStore::new(), @@ -130,6 +132,10 @@ impl MemoryStore { .unwrap() .extend(sessions.into_iter().map(|s| (s.room_id().to_owned(), s))); } + + fn save_private_identity(&self, private_identity: Option) { + *self.private_identity.write().unwrap() = private_identity; + } } type Result = std::result::Result; @@ -144,7 +150,7 @@ impl CryptoStore for MemoryStore { } async fn load_identity(&self) -> Result> { - Ok(None) + Ok(self.private_identity.read().unwrap().clone()) } async fn next_batch_token(&self) -> Result> { @@ -163,6 +169,7 @@ impl CryptoStore for MemoryStore { self.save_sessions(changes.sessions).await; self.save_inbound_group_sessions(changes.inbound_group_sessions); self.save_outbound_group_sessions(changes.outbound_group_sessions); + self.save_private_identity(changes.private_identity); self.save_devices(changes.devices.new); self.save_devices(changes.devices.changed); @@ -482,7 +489,10 @@ mod tests { use crate::{ identities::device::testing::get_device, - olm::{tests::get_account_and_session_test_helper, InboundGroupSession, OlmMessageHash}, + olm::{ + tests::get_account_and_session_test_helper, InboundGroupSession, OlmMessageHash, + PrivateCrossSigningIdentity, + }, store::{memorystore::MemoryStore, Changes, CryptoStore, PendingChanges}, }; @@ -568,6 +578,22 @@ mod tests { assert_eq!(loaded_tracked_users.len(), 2); } + #[async_test] + async fn test_private_identity_store() { + // Given a private identity + let private_identity = PrivateCrossSigningIdentity::empty(user_id!("@u:s")); + + // When we save it to the store + let store = MemoryStore::new(); + store.save_private_identity(Some(private_identity.clone())); + + // Then we can get it out again + let loaded_identity = + store.load_identity().await.expect("failed to load private identity").unwrap(); + + assert_eq!(loaded_identity.user_id(), user_id!("@u:s")); + } + #[async_test] async fn test_device_store() { let device = get_device(); From 3ac123db294d70b05d770f0e9176440ed31f0e2d Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 19 Mar 2024 15:59:35 +0100 Subject: [PATCH 50/74] crypto: fix BackedUpRoomKey Serialization --- .../src/olm/group_sessions/mod.rs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/crates/matrix-sdk-crypto/src/olm/group_sessions/mod.rs b/crates/matrix-sdk-crypto/src/olm/group_sessions/mod.rs index 998de0573a3..b6ef4bc1350 100644 --- a/crates/matrix-sdk-crypto/src/olm/group_sessions/mod.rs +++ b/crates/matrix-sdk-crypto/src/olm/group_sessions/mod.rs @@ -136,6 +136,11 @@ pub struct BackedUpRoomKey { /// Chain of Curve25519 keys through which this session was forwarded, via /// m.forwarded_room_key events. + #[serde( + default, + deserialize_with = "deserialize_curve_key_vec", + serialize_with = "serialize_curve_key_vec" + )] pub forwarding_curve25519_key_chain: Vec, } @@ -245,3 +250,33 @@ impl TryFrom for ExportedRoomKey { } } } + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::BackedUpRoomKey; + + #[test] + fn test_deserialize_backed_up_key() { + let data = json!({ + "algorithm": "m.megolm.v1.aes-sha2", + "room_id": "!room:id", + "sender_key": "FOvlmz18LLI3k/llCpqRoKT90+gFF8YhuL+v1YBXHlw", + "session_id": "/2K+V777vipCxPZ0gpY9qcpz1DYaXwuMRIu0UEP0Wa0", + "session_key": "AQAAAAAclzWVMeWBKH+B/WMowa3rb4ma3jEl6n5W4GCs9ue65CruzD3ihX+85pZ9hsV9Bf6fvhjp76WNRajoJYX0UIt7aosjmu0i+H+07hEQ0zqTKpVoSH0ykJ6stAMhdr6Q4uW5crBmdTTBIsqmoWsNJZKKoE2+ldYrZ1lrFeaJbjBIY/9ivle++74qQsT2dIKWPanKc9Q2Gl8LjESLtFBD9Fmt", + "sender_claimed_keys": { + "ed25519": "F4P7f1Z0RjbiZMgHk1xBCG3KC4/Ng9PmxLJ4hQ13sHA" + }, + "forwarding_curve25519_key_chain": ["DBPC2zr6c9qimo9YRFK3RVr0Two/I6ODb9mbsToZN3Q", "bBc/qzZFOOKshMMT+i4gjS/gWPDoKfGmETs9yfw9430"] + }); + + let backed_up_room_key: BackedUpRoomKey = serde_json::from_value(data) + .expect("We should be able to deserialize the backed up room key."); + assert_eq!( + backed_up_room_key.forwarding_curve25519_key_chain.len(), + 2, + "The number of forwarding Curve25519 chains should be two." + ); + } +} From a1c1b0e1576eed5b81feeadd96e4bedd436b4f97 Mon Sep 17 00:00:00 2001 From: Matthias Grandl <50196894+MatthiasGrandl@users.noreply.github.com> Date: Tue, 19 Mar 2024 18:30:10 +0100 Subject: [PATCH 51/74] timeline: make room() public (#3248) --- crates/matrix-sdk-ui/src/timeline/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index a56a51fdccb..66cb577d9a8 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -153,7 +153,8 @@ impl Timeline { TimelineBuilder::new(room) } - fn room(&self) -> &Room { + /// Returns the room for this timeline. + pub fn room(&self) -> &Room { self.inner.room() } From e2644829547207761e50115d132e8249956d0143 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Tue, 19 Mar 2024 20:22:08 +0100 Subject: [PATCH 52/74] feat(sdk): Rename `ItemPosition` to `Position`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The “position” can be placed inside a `Gap`, so naming it `Item…` is a non-sense. This patch removes the `Item` prefix. --- .../src/event_cache/linked_chunk.rs | 161 +++++++++--------- crates/matrix-sdk/src/event_cache/store.rs | 22 +-- 2 files changed, 91 insertions(+), 92 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 5f87e30c37a..49702fc0b16 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -122,14 +122,14 @@ impl LinkedChunk { pub fn insert_items_at( &mut self, items: I, - position: ItemPosition, + position: Position, ) -> Result<(), LinkedChunkError> where I: IntoIterator, I::IntoIter: ExactSizeIterator, { let chunk_identifier = position.chunk_identifier(); - let item_index = position.item_index(); + let item_index = position.index(); let chunk_identifier_generator = self.chunk_identifier_generator.clone(); @@ -185,11 +185,11 @@ impl LinkedChunk { /// `Result`. pub fn insert_gap_at( &mut self, - position: ItemPosition, content: Gap, + position: Position, ) -> Result<(), LinkedChunkError> { let chunk_identifier = position.chunk_identifier(); - let item_index = position.item_index(); + let item_index = position.index(); let chunk_identifier_generator = self.chunk_identifier_generator.clone(); @@ -343,7 +343,7 @@ impl LinkedChunk { } /// Search for an item, and return its position. - pub fn item_position<'a, P>(&'a self, mut predicate: P) -> Option + pub fn item_position<'a, P>(&'a self, mut predicate: P) -> Option where P: FnMut(&'a Item) -> bool, { @@ -395,18 +395,18 @@ impl LinkedChunk { /// Iterate over the items, backward. /// /// It iterates from the last to the first item. - pub fn ritems(&self) -> impl Iterator { - self.ritems_from(self.latest_chunk().last_item_position()) + pub fn ritems(&self) -> impl Iterator { + self.ritems_from(self.latest_chunk().last_position()) .expect("`ritems_from` cannot fail because at least one empty chunk must exist") } /// Iterate over the items, forward. /// /// It iterates from the first to the last item. - pub fn items(&self) -> impl Iterator { + pub fn items(&self) -> impl Iterator { let first_chunk = self.first_chunk(); - self.items_from(first_chunk.first_item_position()) + self.items_from(first_chunk.first_position()) .expect("`items` cannot fail because at least one empty chunk must exist") } @@ -415,8 +415,8 @@ impl LinkedChunk { /// It iterates from the item at `position` to the first item. pub fn ritems_from( &self, - position: ItemPosition, - ) -> Result, LinkedChunkError> { + position: Position, + ) -> Result, LinkedChunkError> { Ok(self .rchunks_from(position.chunk_identifier())? .filter_map(|chunk| match &chunk.content { @@ -424,16 +424,18 @@ impl LinkedChunk { ChunkContent::Items(items) => { let identifier = chunk.identifier(); - Some(items.iter().enumerate().rev().map(move |(item_index, item)| { - (ItemPosition(identifier, item_index), item) - })) + Some( + items.iter().enumerate().rev().map(move |(item_index, item)| { + (Position(identifier, item_index), item) + }), + ) } }) .flatten() .skip_while({ - let expected_index = position.item_index(); + let expected_index = position.index(); - move |(ItemPosition(_chunk_identifier, item_index), _item)| { + move |(Position(_chunk_identifier, item_index), _item)| { *item_index != expected_index } })) @@ -444,8 +446,8 @@ impl LinkedChunk { /// It iterates from the item at `position` to the last item. pub fn items_from( &self, - position: ItemPosition, - ) -> Result, LinkedChunkError> { + position: Position, + ) -> Result, LinkedChunkError> { Ok(self .chunks_from(position.chunk_identifier())? .filter_map(|chunk| match &chunk.content { @@ -453,13 +455,15 @@ impl LinkedChunk { ChunkContent::Items(items) => { let identifier = chunk.identifier(); - Some(items.iter().enumerate().map(move |(item_index, item)| { - (ItemPosition(identifier, item_index), item) - })) + Some( + items.iter().enumerate().map(move |(item_index, item)| { + (Position(identifier, item_index), item) + }), + ) } }) .flatten() - .skip(position.item_index())) + .skip(position.index())) } /// Get the first chunk, as an immutable reference. @@ -573,31 +577,24 @@ impl ChunkIdentifierGenerator { #[repr(transparent)] pub struct ChunkIdentifier(u64); -/// The position of an item in a [`LinkedChunk`]. +/// The position of something inside a [`Chunk`]. /// -/// It's a pair of a chunk position and an item index. `(…, 0)` represents -/// the last item in the chunk. +/// It's a pair of a chunk position and an item index. #[derive(Copy, Clone, Debug, PartialEq)] -pub struct ItemPosition(ChunkIdentifier, usize); +pub struct Position(ChunkIdentifier, usize); -impl ItemPosition { +impl Position { /// Get the chunk identifier of the item. pub fn chunk_identifier(&self) -> ChunkIdentifier { self.0 } - /// Get the item index inside its chunk. - pub fn item_index(&self) -> usize { + /// Get the index inside the chunk. + pub fn index(&self) -> usize { self.1 } } -impl From for ItemPosition { - fn from(chunk_identifier: ChunkIdentifier) -> Self { - Self(chunk_identifier, 0) - } -} - /// An iterator over a [`LinkedChunk`] that traverses the chunk in backward /// direction (i.e. it calls `previous` on each chunk to make progress). pub struct LinkedChunkIterBackward<'a, Item, Gap, const CAP: usize> { @@ -740,20 +737,22 @@ impl Chunk { &self.content } - /// Get the [`ItemPosition`] of the first item. - pub fn first_item_position(&self) -> ItemPosition { - ItemPosition(self.identifier(), 0) + /// Get the [`Position`] of the first item if any. + /// + /// If the `Chunk` is a `Gap`, it returns `0` for the index. + pub fn first_position(&self) -> Position { + Position(self.identifier(), 0) } - /// Get the [`ItemPosition`] of the last item. + /// Get the [`Position`] of the last item if any. /// - /// If it's a `Gap` chunk, the last part of the tuple will always be `0`. - pub fn last_item_position(&self) -> ItemPosition { + /// If the `Chunk` is a `Gap`, it returns `0` for the index. + pub fn last_position(&self) -> Position { let identifier = self.identifier(); match &self.content { - ChunkContent::Gap(..) => ItemPosition(identifier, 0), - ChunkContent::Items(items) => ItemPosition(identifier, items.len() - 1), + ChunkContent::Gap(..) => Position(identifier, 0), + ChunkContent::Items(items) => Position(identifier, items.len() - 1), } } @@ -937,8 +936,8 @@ mod tests { use assert_matches::assert_matches; use super::{ - Chunk, ChunkContent, ChunkIdentifier, ChunkIdentifierGenerator, ItemPosition, LinkedChunk, - LinkedChunkError, + Chunk, ChunkContent, ChunkIdentifier, ChunkIdentifierGenerator, LinkedChunk, + LinkedChunkError, Position, }; macro_rules! assert_items_eq { @@ -965,7 +964,7 @@ mod tests { $( assert_matches!( $iterator .next(), - Some((chunk_index, ItemPosition(chunk_identifier, item_index), & $item )) => { + Some((chunk_index, Position(chunk_identifier, item_index), & $item )) => { // Ensure the chunk index (from the enumeration) is correct. assert_eq!(chunk_index, $chunk_index); // Ensure the chunk identifier is the same for all items in this chunk. @@ -1005,7 +1004,7 @@ mod tests { let identifier = chunk.identifier(); Some(items.iter().enumerate().map(move |(item_index, item)| { - (chunk_index, ItemPosition(identifier, item_index), item) + (chunk_index, Position(identifier, item_index), item) })) } }) @@ -1096,7 +1095,7 @@ mod tests { assert_eq!(linked_chunk.chunk_identifier(Chunk::is_gap), Some(ChunkIdentifier(2))); assert_eq!( linked_chunk.item_position(|item| *item == 'e'), - Some(ItemPosition(ChunkIdentifier(1), 1)) + Some(Position(ChunkIdentifier(1), 1)) ); } @@ -1237,11 +1236,11 @@ mod tests { let mut iterator = linked_chunk.ritems(); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(3), 0), 'e'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'd'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'c'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'b'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'a'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(3), 0), 'e'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(2), 1), 'd'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(2), 0), 'c'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(0), 1), 'b'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(0), 0), 'a'))); assert_matches!(iterator.next(), None); } @@ -1254,11 +1253,11 @@ mod tests { let mut iterator = linked_chunk.items(); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'a'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'b'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'c'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'd'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(3), 0), 'e'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(0), 0), 'a'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(0), 1), 'b'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(2), 0), 'c'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(2), 1), 'd'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(3), 0), 'e'))); assert_matches!(iterator.next(), None); } @@ -1272,9 +1271,9 @@ mod tests { let mut iterator = linked_chunk.ritems_from(linked_chunk.item_position(|item| *item == 'c').unwrap())?; - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'c'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 1), 'b'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(0), 0), 'a'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(2), 0), 'c'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(0), 1), 'b'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(0), 0), 'a'))); assert_matches!(iterator.next(), None); Ok(()) @@ -1290,9 +1289,9 @@ mod tests { let mut iterator = linked_chunk.items_from(linked_chunk.item_position(|item| *item == 'c').unwrap())?; - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 0), 'c'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(2), 1), 'd'))); - assert_matches!(iterator.next(), Some((ItemPosition(ChunkIdentifier(3), 0), 'e'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(2), 0), 'c'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(2), 1), 'd'))); + assert_matches!(iterator.next(), Some((Position(ChunkIdentifier(3), 0), 'e'))); assert_matches!(iterator.next(), None); Ok(()) @@ -1346,7 +1345,7 @@ mod tests { // Insert in a chunk that does not exist. { assert_matches!( - linked_chunk.insert_items_at(['u', 'v'], ItemPosition(ChunkIdentifier(128), 0)), + linked_chunk.insert_items_at(['u', 'v'], Position(ChunkIdentifier(128), 0)), Err(LinkedChunkError::InvalidChunkIdentifier { identifier: ChunkIdentifier(128) }) ); } @@ -1354,7 +1353,7 @@ mod tests { // Insert in a chunk that exists, but at an item that does not exist. { assert_matches!( - linked_chunk.insert_items_at(['u', 'v'], ItemPosition(ChunkIdentifier(0), 128)), + linked_chunk.insert_items_at(['u', 'v'], Position(ChunkIdentifier(0), 128)), Err(LinkedChunkError::InvalidItemIndex { index: 128 }) ); } @@ -1369,7 +1368,7 @@ mod tests { ); assert_matches!( - linked_chunk.insert_items_at(['u', 'v'], ItemPosition(ChunkIdentifier(6), 0)), + linked_chunk.insert_items_at(['u', 'v'], Position(ChunkIdentifier(6), 0)), Err(LinkedChunkError::ChunkIsAGap { identifier: ChunkIdentifier(6) }) ); } @@ -1388,7 +1387,7 @@ mod tests { // Insert in the middle of a chunk. { let position_of_b = linked_chunk.item_position(|item| *item == 'b').unwrap(); - linked_chunk.insert_gap_at(position_of_b, ())?; + linked_chunk.insert_gap_at((), position_of_b)?; assert_items_eq!(linked_chunk, ['a'] [-] ['b', 'c'] ['d', 'e', 'f']); } @@ -1396,7 +1395,7 @@ mod tests { // Insert at the beginning of a chunk. { let position_of_a = linked_chunk.item_position(|item| *item == 'a').unwrap(); - linked_chunk.insert_gap_at(position_of_a, ())?; + linked_chunk.insert_gap_at((), position_of_a)?; assert_items_eq!(linked_chunk, [] [-] ['a'] [-] ['b', 'c'] ['d', 'e', 'f']); } @@ -1404,7 +1403,7 @@ mod tests { // Insert in a chunk that does not exist. { assert_matches!( - linked_chunk.insert_items_at(['u', 'v'], ItemPosition(ChunkIdentifier(128), 0)), + linked_chunk.insert_items_at(['u', 'v'], Position(ChunkIdentifier(128), 0)), Err(LinkedChunkError::InvalidChunkIdentifier { identifier: ChunkIdentifier(128) }) ); } @@ -1412,7 +1411,7 @@ mod tests { // Insert in a chunk that exists, but at an item that does not exist. { assert_matches!( - linked_chunk.insert_items_at(['u', 'v'], ItemPosition(ChunkIdentifier(0), 128)), + linked_chunk.insert_items_at(['u', 'v'], Position(ChunkIdentifier(0), 128)), Err(LinkedChunkError::InvalidItemIndex { index: 128 }) ); } @@ -1421,9 +1420,9 @@ mod tests { { // It is impossible to get the item position inside a gap. It's only possible if // the item position is crafted by hand or is outdated. - let position_of_a_gap = ItemPosition(ChunkIdentifier(4), 0); + let position_of_a_gap = Position(ChunkIdentifier(4), 0); assert_matches!( - linked_chunk.insert_gap_at(position_of_a_gap, ()), + linked_chunk.insert_gap_at((), position_of_a_gap), Err(LinkedChunkError::ChunkIsAGap { identifier: ChunkIdentifier(4) }) ); } @@ -1490,29 +1489,29 @@ mod tests { // First chunk. { let chunk = iterator.next().unwrap(); - assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(0), 0)); - assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(0), 2)); + assert_eq!(chunk.first_position(), Position(ChunkIdentifier(0), 0)); + assert_eq!(chunk.last_position(), Position(ChunkIdentifier(0), 2)); } // Second chunk. { let chunk = iterator.next().unwrap(); - assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(1), 0)); - assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(1), 1)); + assert_eq!(chunk.first_position(), Position(ChunkIdentifier(1), 0)); + assert_eq!(chunk.last_position(), Position(ChunkIdentifier(1), 1)); } // Gap. { let chunk = iterator.next().unwrap(); - assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(2), 0)); - assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(2), 0)); + assert_eq!(chunk.first_position(), Position(ChunkIdentifier(2), 0)); + assert_eq!(chunk.last_position(), Position(ChunkIdentifier(2), 0)); } // Last chunk. { let chunk = iterator.next().unwrap(); - assert_eq!(chunk.first_item_position(), ItemPosition(ChunkIdentifier(3), 0)); - assert_eq!(chunk.last_item_position(), ItemPosition(ChunkIdentifier(3), 0)); + assert_eq!(chunk.first_position(), Position(ChunkIdentifier(3), 0)); + assert_eq!(chunk.last_position(), Position(ChunkIdentifier(3), 0)); } } } diff --git a/crates/matrix-sdk/src/event_cache/store.rs b/crates/matrix-sdk/src/event_cache/store.rs index eede3c3c4d3..f5d12ae97e8 100644 --- a/crates/matrix-sdk/src/event_cache/store.rs +++ b/crates/matrix-sdk/src/event_cache/store.rs @@ -21,8 +21,8 @@ use tokio::sync::RwLock; use super::{ linked_chunk::{ - Chunk, ChunkIdentifier, ItemPosition, LinkedChunk, LinkedChunkError, LinkedChunkIter, - LinkedChunkIterBackward, + Chunk, ChunkIdentifier, LinkedChunk, LinkedChunkError, LinkedChunkIter, + LinkedChunkIterBackward, Position, }, Result, }; @@ -255,7 +255,7 @@ impl RoomEvents { pub fn insert_events_at( &mut self, events: I, - position: ItemPosition, + position: Position, ) -> StdResult<(), LinkedChunkError> where I: IntoIterator, @@ -267,10 +267,10 @@ impl RoomEvents { /// Insert a gap at a specified position. pub fn insert_gap_at( &mut self, - position: ItemPosition, gap: Gap, + position: Position, ) -> StdResult<(), LinkedChunkError> { - self.chunks.insert_gap_at(position, gap) + self.chunks.insert_gap_at(gap, position) } /// Search for a chunk, and return its identifier. @@ -282,7 +282,7 @@ impl RoomEvents { } /// Search for an item, and return its position. - pub fn event_position<'a, P>(&'a self, predicate: P) -> Option + pub fn event_position<'a, P>(&'a self, predicate: P) -> Option where P: FnMut(&'a SyncTimelineEvent) -> bool, { @@ -324,15 +324,15 @@ impl RoomEvents { /// Iterate over the events, backward. /// /// The most recent event comes first. - pub fn revents(&self) -> impl Iterator { + pub fn revents(&self) -> impl Iterator { self.chunks.ritems() } /// Iterate over the events, starting from `position`, backward. pub fn revents_from( &self, - position: ItemPosition, - ) -> StdResult, LinkedChunkError> { + position: Position, + ) -> StdResult, LinkedChunkError> { self.chunks.ritems_from(position) } @@ -340,8 +340,8 @@ impl RoomEvents { /// to the latest event. pub fn events_from( &self, - position: ItemPosition, - ) -> StdResult, LinkedChunkError> { + position: Position, + ) -> StdResult, LinkedChunkError> { self.chunks.items_from(position) } } From 40d96dbf27c6b988a949be31238e2d57696bde62 Mon Sep 17 00:00:00 2001 From: Andy Balaam Date: Fri, 15 Mar 2024 11:44:51 +0000 Subject: [PATCH 53/74] crypto: Refactor integration tests in preparation for them running against MemoryStore A couple of small changes that allow us to drop locks that would otherwise persist when running the tests over the MemoryStore, and some documentation comments for assertions to make it easier to spot which assertion failed, even though we are inside a macro. --- .../src/store/integration_tests.rs | 81 ++++++++++++------- 1 file changed, 50 insertions(+), 31 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/store/integration_tests.rs b/crates/matrix-sdk-crypto/src/store/integration_tests.rs index d0c1ab2d71d..1ed29ecbd11 100644 --- a/crates/matrix-sdk-crypto/src/store/integration_tests.rs +++ b/crates/matrix-sdk-crypto/src/store/integration_tests.rs @@ -187,26 +187,32 @@ macro_rules! cryptostore_integration_tests { #[async_test] async fn add_and_save_session() { let store_name = "add_and_save_session"; - let store = get_store(store_name, None).await; - let (account, session) = get_account_and_session().await; - let sender_key = session.sender_key.to_base64(); - let session_id = session.session_id().to_owned(); - store.save_pending_changes(PendingChanges { account: Some(account.deep_clone()), }).await.expect("Can't save account"); + // Given we created a session and saved it in the store + let (session_id, account, sender_key) = { + let store = get_store(store_name, None).await; + let (account, session) = get_account_and_session().await; + let sender_key = session.sender_key.to_base64(); + let session_id = session.session_id().to_owned(); - let changes = Changes { sessions: vec![session.clone()], ..Default::default() }; - store.save_changes(changes).await.unwrap(); + store.save_pending_changes(PendingChanges { account: Some(account.deep_clone()), }).await.expect("Can't save account"); - let sessions = store.get_sessions(&sender_key).await.unwrap().unwrap(); - let sessions_lock = sessions.lock().await; - let session = &sessions_lock[0]; + let changes = Changes { sessions: vec![session.clone()], ..Default::default() }; + store.save_changes(changes).await.unwrap(); - assert_eq!(session_id, session.session_id()); + let sessions = store.get_sessions(&sender_key).await.unwrap().unwrap(); + let sessions_lock = sessions.lock().await; + let session = &sessions_lock[0]; - drop(store); + assert_eq!(session_id, session.session_id()); + (session_id, account, sender_key) + }; + + // When we reload the store let store = get_store(store_name, None).await; + // Then the same account and session info was reloaded let loaded_account = store.load_account().await.unwrap().unwrap(); assert_eq!(account, loaded_account); @@ -220,36 +226,49 @@ macro_rules! cryptostore_integration_tests { #[async_test] async fn load_outbound_group_session() { 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_session(&room_id).await.unwrap().is_none()); - let (session, _) = account.create_group_session_pair_with_defaults(&room_id).await; + // Given we saved an outbound group session + { + let (account, store) = get_loaded_store(dir.clone()).await; + assert!( + store.get_outbound_group_session(&room_id).await.unwrap().is_none(), + "Initially there should be no outbound group session" + ); - let user_id = user_id!("@example:localhost"); - let request = ToDeviceRequest::new( - user_id, - DeviceIdOrAllDevices::AllDevices, - "m.dummy", - Raw::from_json(to_raw_value(&DummyEventContent::new()).unwrap()), - ); + let (session, _) = account.create_group_session_pair_with_defaults(&room_id).await; - session.add_request(TransactionId::new(), request.into(), Default::default()); + let user_id = user_id!("@example:localhost"); + let request = ToDeviceRequest::new( + user_id, + DeviceIdOrAllDevices::AllDevices, + "m.dummy", + Raw::from_json(to_raw_value(&DummyEventContent::new()).unwrap()), + ); - let changes = Changes { - outbound_group_sessions: vec![session.clone()], - ..Default::default() - }; + session.add_request(TransactionId::new(), request.into(), Default::default()); - store.save_changes(changes).await.expect("Can't save group session"); + let changes = Changes { + outbound_group_sessions: vec![session.clone()], + ..Default::default() + }; - drop(store); + store.save_changes(changes).await.expect("Can't save group session"); + assert!( + store.get_outbound_group_session(&room_id).await.unwrap().is_some(), + "Sanity: after we've saved one, there should be an outbound_group_session" + ); + } + // When we reload the account let store = get_store(dir, None).await; - store.load_account().await.unwrap(); - assert!(store.get_outbound_group_session(&room_id).await.unwrap().is_some()); + // Then the saved session is restored + assert!( + store.get_outbound_group_session(&room_id).await.unwrap().is_some(), + "The outbound_group_session should have been loaded" + ); } #[async_test] From 44443d3b46b6876f3fa721db26cb04a446b40ada Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Tue, 19 Mar 2024 18:55:44 +0100 Subject: [PATCH 54/74] crypto: Mac then decrypt in the PkDecryption compat module --- crates/matrix-sdk-crypto/src/backups/keys/compat.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/backups/keys/compat.rs b/crates/matrix-sdk-crypto/src/backups/keys/compat.rs index e5eb01612f8..59c32e6ad41 100644 --- a/crates/matrix-sdk-crypto/src/backups/keys/compat.rs +++ b/crates/matrix-sdk-crypto/src/backups/keys/compat.rs @@ -106,12 +106,12 @@ impl PkDecryption { let keys = Keys::new(shared_secret); - let cipher = Aes256CbcDec::new(keys.aes_key(), keys.iv()); - let decrypted = cipher.decrypt_padded_vec_mut::(&message.ciphertext)?; - let hmac = keys.hmac(); hmac.verify_truncated_left(&message.mac)?; + let cipher = Aes256CbcDec::new(keys.aes_key(), keys.iv()); + let decrypted = cipher.decrypt_padded_vec_mut::(&message.ciphertext)?; + Ok(decrypted) } From 3aa0a905b23e7da0bede174a5f4b74262793145e Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 20 Mar 2024 08:32:45 +0100 Subject: [PATCH 55/74] feat(sdk): `LinkedChunk::replace_gap_at` returns `&Chunk`. --- .../src/event_cache/linked_chunk.rs | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 49702fc0b16..3abc79adcf4 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -241,17 +241,21 @@ impl LinkedChunk { /// /// Because the `chunk_identifier` can represent non-gap chunk, this method /// returns a `Result`. + /// + /// The returned `Chunk` represents the newly created `Chunk` that contains + /// the first items. pub fn replace_gap_at( &mut self, items: I, chunk_identifier: ChunkIdentifier, - ) -> Result<(), LinkedChunkError> + ) -> Result<&Chunk, LinkedChunkError> where I: IntoIterator, I::IntoIter: ExactSizeIterator, { let chunk_identifier_generator = self.chunk_identifier_generator.clone(); let chunk_ptr; + let new_chunk_ptr; { let chunk = self @@ -269,8 +273,7 @@ impl LinkedChunk { // Insert a new items chunk… .insert_next(Chunk::new_items_leaked( chunk_identifier_generator.generate_next().unwrap(), - )) - // … and insert the items. + )) // … and insert the items. .push_items(items, &chunk_identifier_generator); ( @@ -283,6 +286,11 @@ impl LinkedChunk { } }; + new_chunk_ptr = chunk + .next + // SAFETY: A new `Chunk` has just been inserted, so it exists. + .unwrap(); + // Now that new items have been pushed, we can unlink the gap chunk. chunk.unlink(); @@ -305,7 +313,10 @@ impl LinkedChunk { // pointer, which is valid and well aligned. let _chunk_boxed = unsafe { Box::from_raw(chunk_ptr.as_ptr()) }; - Ok(()) + Ok( + // SAFETY: `new_chunk_ptr` is valid, non-null and well-aligned. + unsafe { new_chunk_ptr.as_ref() }, + ) } /// Get the chunk as a reference, from its identifier, if it exists. @@ -1445,7 +1456,9 @@ mod tests { let gap_identifier = linked_chunk.chunk_identifier(Chunk::is_gap).unwrap(); assert_eq!(gap_identifier, ChunkIdentifier(1)); - linked_chunk.replace_gap_at(['d', 'e', 'f', 'g', 'h'], gap_identifier)?; + let new_chunk = + linked_chunk.replace_gap_at(['d', 'e', 'f', 'g', 'h'], gap_identifier)?; + assert_eq!(new_chunk.identifier(), ChunkIdentifier(3)); assert_items_eq!( linked_chunk, ['a', 'b'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm'] @@ -1463,7 +1476,8 @@ mod tests { let gap_identifier = linked_chunk.chunk_identifier(Chunk::is_gap).unwrap(); assert_eq!(gap_identifier, ChunkIdentifier(5)); - linked_chunk.replace_gap_at(['w', 'x', 'y', 'z'], gap_identifier)?; + let new_chunk = linked_chunk.replace_gap_at(['w', 'x', 'y', 'z'], gap_identifier)?; + assert_eq!(new_chunk.identifier(), ChunkIdentifier(6)); assert_items_eq!( linked_chunk, ['a', 'b'] ['d', 'e', 'f'] ['g', 'h'] ['l', 'm'] ['w', 'x', 'y'] ['z'] From a248ec75e2ecdb05fc7a079a77b6d7ef55a48ec7 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 20 Mar 2024 12:01:39 +0100 Subject: [PATCH 56/74] feat(sdk): Optimise how `insert_gap_at` works when inserting at first position. Imagine we have this linked chunk: ```rust assert_items_eq!(linked_chunk, ['a'] [-] ['b', 'c'] ['d', 'e', 'f']); ``` Before the patch, when we were running: ```rust let position_of_d = linked_chunk.item_position(|item| *item == 'd').unwrap(); linked_chunk.insert_gap_at((), position_of_d)?; ``` it was taking `['d', 'e', 'f']` to split it at index 0, so `[]` + `['d', 'e', 'f']`, and then was inserting a gap in between, thus resulting into: ```rust assert_items_eq!(linked_chunk, ['a'] [-] ['b', 'c'] [] [-] ['d', 'e', 'f']); ``` The problem is that it creates a useless empty chunk. It's a waste of space, and rapidly, of computation. When used with `EventCache`, this problem occurs every time a backpagination occurs (because it executes a `replace_gap_at` to insert the new item, and then executes a `insert_gap_at` if the backpagination contains another `prev_batch` token). With this patch, the result of the operation is now: ```rust assert_items_eq!(linked_chunk, ['a'] [-] ['b', 'c'] [-] ['d', 'e', 'f']); ``` The `assert_items_eq!` macro has been updated to be able to assert empty chunks. The `test_insert_gap_at` has been updated to test all edge cases. --- .../src/event_cache/linked_chunk.rs | 131 ++++++++++++------ 1 file changed, 87 insertions(+), 44 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 3abc79adcf4..742e6aeab3f 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -197,6 +197,30 @@ impl LinkedChunk { .chunk_mut(chunk_identifier) .ok_or(LinkedChunkError::InvalidChunkIdentifier { identifier: chunk_identifier })?; + // If `item_index` is 0, we don't want to split the current items chunk to + // insert a new gap chunk, otherwise it would create an empty current items + // chunk. Let's handle this case in particular. + // + // Of course this optimisation applies if there is a previous chunk. Remember + // the invariant: a `Gap` cannot be the first chunk. + if item_index == 0 && chunk.is_items() && chunk.previous.is_some() { + let previous_chunk = chunk + .previous_mut() + // SAFETY: The `previous` chunk exists because we have tested + // `chunk.previous.is_some()` in the `if` statement. + .unwrap(); + + previous_chunk.insert_next(Chunk::new_gap_leaked( + chunk_identifier_generator.generate_next().unwrap(), + content, + )); + + // We don't need to update `self.last` because we have inserted a new chunk + // before `chunk`. + + return Ok(()); + } + let chunk = match &mut chunk.content { ChunkContent::Gap(..) => { return Err(LinkedChunkError::ChunkIsAGap { identifier: chunk_identifier }); @@ -345,7 +369,7 @@ impl LinkedChunk { } } - /// Search for a chunk, and return its identifier. + /// Search backward for a chunk, and return its identifier. pub fn chunk_identifier<'a, P>(&'a self, mut predicate: P) -> Option where P: FnMut(&'a Chunk) -> bool, @@ -353,7 +377,7 @@ impl LinkedChunk { self.rchunks().find_map(|chunk| predicate(chunk).then(|| chunk.identifier())) } - /// Search for an item, and return its position. + /// Search backward for an item, and return its position. pub fn item_position<'a, P>(&'a self, mut predicate: P) -> Option where P: FnMut(&'a Item) -> bool, @@ -952,75 +976,60 @@ mod tests { }; macro_rules! assert_items_eq { - ( @_ [ $iterator:ident, $chunk_index:ident, $item_index:ident ] { [-] $( $rest:tt )* } { $( $accumulator:tt )* } ) => { + ( @_ [ $iterator:ident ] { [-] $( $rest:tt )* } { $( $accumulator:tt )* } ) => { assert_items_eq!( @_ - [ $iterator, $chunk_index, $item_index ] + [ $iterator ] { $( $rest )* } { $( $accumulator )* - $chunk_index += 1; + { + let chunk = $iterator .next().expect("next chunk (expect gap)"); + assert!(chunk.is_gap(), "chunk "); + } } ) }; - ( @_ [ $iterator:ident, $chunk_index:ident, $item_index:ident ] { [ $( $item:expr ),* ] $( $rest:tt )* } { $( $accumulator:tt )* } ) => { + ( @_ [ $iterator:ident ] { [ $( $item:expr ),* ] $( $rest:tt )* } { $( $accumulator:tt )* } ) => { assert_items_eq!( @_ - [ $iterator, $chunk_index, $item_index ] + [ $iterator ] { $( $rest )* } { $( $accumulator )* - let _expected_chunk_identifier = $iterator .peek().unwrap().1.chunk_identifier(); - $( - assert_matches!( - $iterator .next(), - Some((chunk_index, Position(chunk_identifier, item_index), & $item )) => { - // Ensure the chunk index (from the enumeration) is correct. - assert_eq!(chunk_index, $chunk_index); - // Ensure the chunk identifier is the same for all items in this chunk. - assert_eq!(chunk_identifier, _expected_chunk_identifier); - // Ensure the item has the expected position. - assert_eq!(item_index, $item_index); - } - ); - $item_index += 1; - )* - $item_index = 0; - $chunk_index += 1; + { + let chunk = $iterator .next().expect("next chunk (expect items)"); + assert!(chunk.is_items()); + + let ChunkContent::Items(items) = chunk.content() else { unreachable!() }; + + let mut items_iterator = items.iter(); + + $( + assert_eq!(items_iterator.next(), Some(& $item )); + )* + + assert!(items_iterator.next().is_none(), "no more items"); + } } ) }; - ( @_ [ $iterator:ident, $chunk_index:ident, $item_index:ident ] {} { $( $accumulator:tt )* } ) => { + ( @_ [ $iterator:ident ] {} { $( $accumulator:tt )* } ) => { { - let mut $chunk_index = 0; - let mut $item_index = 0; $( $accumulator )* + assert!( $iterator .next().is_none(), "no more chunks"); } }; ( $linked_chunk:expr, $( $all:tt )* ) => { assert_items_eq!( @_ - [ iterator, _chunk_index, _item_index ] + [ iterator ] { $( $all )* } { - let mut iterator = $linked_chunk - .chunks() - .enumerate() - .filter_map(|(chunk_index, chunk)| match &chunk.content { - ChunkContent::Gap(..) => None, - ChunkContent::Items(items) => { - let identifier = chunk.identifier(); - - Some(items.iter().enumerate().map(move |(item_index, item)| { - (chunk_index, Position(identifier, item_index), item) - })) - } - }) - .flatten() - .peekable(); + let mut iterator = $linked_chunk.chunks(); } ) } @@ -1403,14 +1412,48 @@ mod tests { assert_items_eq!(linked_chunk, ['a'] [-] ['b', 'c'] ['d', 'e', 'f']); } - // Insert at the beginning of a chunk. + // Insert at the beginning of a chunk + it's the first chunk. { let position_of_a = linked_chunk.item_position(|item| *item == 'a').unwrap(); linked_chunk.insert_gap_at((), position_of_a)?; + // A new empty chunk is created as the first chunk. assert_items_eq!(linked_chunk, [] [-] ['a'] [-] ['b', 'c'] ['d', 'e', 'f']); } + // Insert at the beginning of a chunk. + { + let position_of_d = linked_chunk.item_position(|item| *item == 'd').unwrap(); + linked_chunk.insert_gap_at((), position_of_d)?; + + // A new empty chunk is NOT created, i.e. `['d', 'e', 'f']` is not + // split into `[]` + `['d', 'e', 'f']` because it's a waste of + // space. + assert_items_eq!(linked_chunk, [] [-] ['a'] [-] ['b', 'c'] [-] ['d', 'e', 'f']); + } + + // Insert in an empty chunk + it's the first chunk. + { + let position_of_first_empty_chunk = Position(ChunkIdentifier(0), 0); + assert_matches!( + linked_chunk.insert_gap_at((), position_of_first_empty_chunk), + Err(LinkedChunkError::InvalidItemIndex { index: 0 }) + ); + } + + // Insert in an empty chunk. + { + // Replace a gap by empty items. + let gap_identifier = linked_chunk.chunk_identifier(Chunk::is_gap).unwrap(); + let position = linked_chunk.replace_gap_at([], gap_identifier)?.first_position(); + + assert_items_eq!(linked_chunk, [] [-] ['a'] [-] ['b', 'c'] [] ['d', 'e', 'f']); + + linked_chunk.insert_gap_at((), position)?; + + assert_items_eq!(linked_chunk, [] [-] ['a'] [-] ['b', 'c'] [-] [] ['d', 'e', 'f']); + } + // Insert in a chunk that does not exist. { assert_matches!( From 0a02a41a14f8e85da9805828c76705976a5cf7d1 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 20 Mar 2024 15:28:03 +0100 Subject: [PATCH 57/74] doc(sdk): Fix mark up. Signed-off-by: Ivan Enderlin --- crates/matrix-sdk/src/room_directory_search.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index d8a367c0e72..493a1599522 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -90,7 +90,7 @@ impl SearchState { } } -/// RoomDirectorySearch allows searching the public room directory, with the +/// `RoomDirectorySearch` allows searching the public room directory, with the /// capability of using a filter and a batch_size. This struct is also /// responsible for keeping the current state of the search, and exposing an /// update of stream of the results, reset the search, or ask for the next page. From 96c7b3fc527d6d32e8e134e12a4be4a9d4e5c611 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 20 Mar 2024 15:28:24 +0100 Subject: [PATCH 58/74] doc(sdk): Add a warning. Signed-off-by: Ivan Enderlin --- crates/matrix-sdk/src/room_directory_search.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/room_directory_search.rs b/crates/matrix-sdk/src/room_directory_search.rs index 493a1599522..165ca52eae5 100644 --- a/crates/matrix-sdk/src/room_directory_search.rs +++ b/crates/matrix-sdk/src/room_directory_search.rs @@ -94,7 +94,8 @@ impl SearchState { /// capability of using a filter and a batch_size. This struct is also /// responsible for keeping the current state of the search, and exposing an /// update of stream of the results, reset the search, or ask for the next page. -/// Users must take great care when using the public room search since the +/// +/// ⚠️ Users must take great care when using the public room search since the /// results might contains NSFW content. /// /// # Example From 57b68614af71b81cf2ce8eea5fd0809ab5524811 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 20 Mar 2024 15:50:51 +0100 Subject: [PATCH 59/74] doc(sdk): US vs UK strike again. --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 742e6aeab3f..f0d617b6825 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -369,7 +369,7 @@ impl LinkedChunk { } } - /// Search backward for a chunk, and return its identifier. + /// Search backwards for a chunk, and return its identifier. pub fn chunk_identifier<'a, P>(&'a self, mut predicate: P) -> Option where P: FnMut(&'a Chunk) -> bool, @@ -377,7 +377,7 @@ impl LinkedChunk { self.rchunks().find_map(|chunk| predicate(chunk).then(|| chunk.identifier())) } - /// Search backward for an item, and return its position. + /// Search backwards for an item, and return its position. pub fn item_position<'a, P>(&'a self, mut predicate: P) -> Option where P: FnMut(&'a Item) -> bool, @@ -385,7 +385,7 @@ impl LinkedChunk { self.ritems().find_map(|(item_position, item)| predicate(item).then_some(item_position)) } - /// Iterate over the chunks, backward. + /// Iterate over the chunks, backwards. /// /// It iterates from the last to the first chunk. pub fn rchunks(&self) -> LinkedChunkIterBackward<'_, Item, Gap, CAP> { From 962c0bf4fda41eae404f7ff37c3715268d057b2c Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 20 Mar 2024 15:57:04 +0100 Subject: [PATCH 60/74] doc(sdk): Improve `SAFETY` paragraphs, and replace `unwrap`s by `expect`s. --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index f0d617b6825..0630d8b81f8 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -208,7 +208,7 @@ impl LinkedChunk { .previous_mut() // SAFETY: The `previous` chunk exists because we have tested // `chunk.previous.is_some()` in the `if` statement. - .unwrap(); + .expect("Previous chunk must be present"); previous_chunk.insert_next(Chunk::new_gap_leaked( chunk_identifier_generator.generate_next().unwrap(), @@ -266,8 +266,8 @@ impl LinkedChunk { /// Because the `chunk_identifier` can represent non-gap chunk, this method /// returns a `Result`. /// - /// The returned `Chunk` represents the newly created `Chunk` that contains - /// the first items. + /// This method returns a reference to the (first if many) newly created + /// `Chunk` that contains the `items`. pub fn replace_gap_at( &mut self, items: I, @@ -333,12 +333,14 @@ impl LinkedChunk { // Re-box the chunk, and let Rust does its job. // - // SAFETY: `chunk` is unlinked but it still exists in memory! We have its - // pointer, which is valid and well aligned. + // SAFETY: `chunk` is unlinked and not borrowed anymore. `LinkedChunk` doesn't + // use it anymore, it's a leak. It is time to re-`Box` it and drop it. let _chunk_boxed = unsafe { Box::from_raw(chunk_ptr.as_ptr()) }; Ok( - // SAFETY: `new_chunk_ptr` is valid, non-null and well-aligned. + // SAFETY: `new_chunk_ptr` is valid, non-null and well-aligned. It's taken from + // `chunk`, and that's how the entire `LinkedChunk` type works. Pointer construction + // safety is guaranteed by `Chunk::new_items_leaked` and `Chunk::new_gap_leaked`. unsafe { new_chunk_ptr.as_ref() }, ) } From ab2b5bfa23aff4dd5b9fe8c0ff36aad224b80aa2 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 20 Mar 2024 21:08:01 +0100 Subject: [PATCH 61/74] feat(sdk): Remove `AtomicU64::load` in `ChunkIdentifierGenerator`. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As suggested in https://github.com/matrix-org/matrix-rust-sdk/ pull/3251#discussion_r1532103818 by Poljar, it is possible that the value of the atomic changes between the `fetch_add` and the `load` (if and only if it is used in a concurrency model, which is not the case right now, but anyway… better being correct now!). The idea is not `load` but repeat the addition manually to compute the “current” value. --- crates/matrix-sdk/src/event_cache/linked_chunk.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 0630d8b81f8..f73df70f69b 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -593,15 +593,14 @@ impl ChunkIdentifierGenerator { /// In this case, `Result::Err` contains the previous unique identifier. pub fn generate_next(&self) -> Result { let previous = self.next.fetch_add(1, Ordering::Relaxed); - let current = self.next.load(Ordering::Relaxed); // Check for overflows. // unlikely — TODO: call `std::intrinsics::unlikely` once it's stable. - if current < previous { + if previous == u64::MAX { return Err(ChunkIdentifier(previous)); } - Ok(ChunkIdentifier(current)) + Ok(ChunkIdentifier(previous.saturating_add(1))) } } From 01c54129517d48e73bdee87c7c1e32bf53f23727 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 20 Mar 2024 21:14:51 +0100 Subject: [PATCH 62/74] feat(sdk): `ChunkIdentifierGenerator::generate_next` panics. This patch makes `ChunkIdentifierGenerator::generate_next` to panic if there is no more identifiers available. It was previously returning a `Result` but we were doing nothing with this `Result` except `unwrap`ping it. To simplify the API: let's panic. --- .../src/event_cache/linked_chunk.rs | 40 +++++++++---------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index f73df70f69b..0847b6d0d26 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -107,7 +107,7 @@ impl LinkedChunk { /// Push a gap at the end of the [`LinkedChunk`], i.e. after the last /// chunk. pub fn push_gap_back(&mut self, content: Gap) { - let next_identifier = self.chunk_identifier_generator.generate_next().unwrap(); + let next_identifier = self.chunk_identifier_generator.generate_next(); let last_chunk = self.latest_chunk_mut(); last_chunk.insert_next(Chunk::new_gap_leaked(next_identifier, content)); @@ -211,7 +211,7 @@ impl LinkedChunk { .expect("Previous chunk must be present"); previous_chunk.insert_next(Chunk::new_gap_leaked( - chunk_identifier_generator.generate_next().unwrap(), + chunk_identifier_generator.generate_next(), content, )); @@ -238,12 +238,12 @@ impl LinkedChunk { chunk // Insert a new gap chunk. .insert_next(Chunk::new_gap_leaked( - chunk_identifier_generator.generate_next().unwrap(), + chunk_identifier_generator.generate_next(), content, )) // Insert a new items chunk. .insert_next(Chunk::new_items_leaked( - chunk_identifier_generator.generate_next().unwrap(), + chunk_identifier_generator.generate_next(), )) // Finally, push the items that have been detached. .push_items(detached_items.into_iter(), &chunk_identifier_generator) @@ -296,7 +296,7 @@ impl LinkedChunk { let last_inserted_chunk = chunk // Insert a new items chunk… .insert_next(Chunk::new_items_leaked( - chunk_identifier_generator.generate_next().unwrap(), + chunk_identifier_generator.generate_next(), )) // … and insert the items. .push_items(items, &chunk_identifier_generator); @@ -590,17 +590,17 @@ impl ChunkIdentifierGenerator { /// Generate the next unique identifier. /// /// Note that it can fail if there is no more unique identifier available. - /// In this case, `Result::Err` contains the previous unique identifier. - pub fn generate_next(&self) -> Result { + /// In this case, this method will panic. + pub fn generate_next(&self) -> ChunkIdentifier { let previous = self.next.fetch_add(1, Ordering::Relaxed); // Check for overflows. // unlikely — TODO: call `std::intrinsics::unlikely` once it's stable. if previous == u64::MAX { - return Err(ChunkIdentifier(previous)); + panic!("No more chunk identifiers available. Congrats, you did it. 2^64 identifiers have been consumed.") } - Ok(ChunkIdentifier(previous.saturating_add(1))) + ChunkIdentifier(previous.saturating_add(1)) } } @@ -835,9 +835,7 @@ impl Chunk { ChunkContent::Gap(..) => { self // Insert a new items chunk. - .insert_next(Self::new_items_leaked( - chunk_identifier_generator.generate_next().unwrap(), - )) + .insert_next(Self::new_items_leaked(chunk_identifier_generator.generate_next())) // Now push the new items on the next chunk, and return the result of // `push_items`. .push_items(new_items, chunk_identifier_generator) @@ -862,7 +860,7 @@ impl Chunk { self // Insert a new items chunk. .insert_next(Self::new_items_leaked( - chunk_identifier_generator.generate_next().unwrap(), + chunk_identifier_generator.generate_next(), )) // Now push the rest of the new items on the next chunk, and return the // result of `push_items`. @@ -1040,18 +1038,18 @@ mod tests { fn test_chunk_identifier_generator() { let generator = ChunkIdentifierGenerator::new_from_scratch(); - assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(1))); - assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(2))); - assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(3))); - assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(4))); + assert_eq!(generator.generate_next(), ChunkIdentifier(1)); + assert_eq!(generator.generate_next(), ChunkIdentifier(2)); + assert_eq!(generator.generate_next(), ChunkIdentifier(3)); + assert_eq!(generator.generate_next(), ChunkIdentifier(4)); let generator = ChunkIdentifierGenerator::new_from_previous_chunk_identifier(ChunkIdentifier(42)); - assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(43))); - assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(44))); - assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(45))); - assert_eq!(generator.generate_next(), Ok(ChunkIdentifier(46))); + assert_eq!(generator.generate_next(), ChunkIdentifier(43)); + assert_eq!(generator.generate_next(), ChunkIdentifier(44)); + assert_eq!(generator.generate_next(), ChunkIdentifier(45)); + assert_eq!(generator.generate_next(), ChunkIdentifier(46)); } #[test] From ae170362a577191742a2abd92673a14b5f24e935 Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 21 Mar 2024 09:05:30 +0100 Subject: [PATCH 63/74] doc(crypto-ffi): `Device::first_time_seen_ts` has an incorrect unit. This patch changes seconds to milliseconds for the description of `Device::first_time_seen_ts`. --- bindings/matrix-sdk-crypto-ffi/src/device.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bindings/matrix-sdk-crypto-ffi/src/device.rs b/bindings/matrix-sdk-crypto-ffi/src/device.rs index 40f43a45e76..92102856780 100644 --- a/bindings/matrix-sdk-crypto-ffi/src/device.rs +++ b/bindings/matrix-sdk-crypto-ffi/src/device.rs @@ -25,8 +25,8 @@ pub struct Device { /// Is our cross signing identity trusted and does the identity trust the /// device. pub cross_signing_trusted: bool, - /// The first time this device was seen in local timestamp, seconds since - /// epoch. + /// The first time this device was seen in local timestamp, milliseconds + /// since epoch. pub first_time_seen_ts: u64, } From 5d5a3044c823af6b44bdda73652dddcf608ea62b Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 21 Mar 2024 09:14:56 +0100 Subject: [PATCH 64/74] chore: Avoid importing types redundantly. `TryFrom` and `TryInto` are imported redundantly. They are already defined in `core::prelude::rust_2021` which is automatically imported. This is generating warnings on my side. This patch fixes that. --- crates/matrix-sdk-crypto/src/identities/device.rs | 1 - crates/matrix-sdk-crypto/src/verification/event_enums.rs | 5 +---- crates/matrix-sdk-crypto/src/verification/machine.rs | 1 - 3 files changed, 1 insertion(+), 6 deletions(-) diff --git a/crates/matrix-sdk-crypto/src/identities/device.rs b/crates/matrix-sdk-crypto/src/identities/device.rs index 8fc0cc7639b..37b57835421 100644 --- a/crates/matrix-sdk-crypto/src/identities/device.rs +++ b/crates/matrix-sdk-crypto/src/identities/device.rs @@ -14,7 +14,6 @@ use std::{ collections::{BTreeMap, HashMap}, - convert::{TryFrom, TryInto}, ops::Deref, sync::{ atomic::{AtomicBool, Ordering}, diff --git a/crates/matrix-sdk-crypto/src/verification/event_enums.rs b/crates/matrix-sdk-crypto/src/verification/event_enums.rs index 146d5a9a6ba..103ce912582 100644 --- a/crates/matrix-sdk-crypto/src/verification/event_enums.rs +++ b/crates/matrix-sdk-crypto/src/verification/event_enums.rs @@ -12,10 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{ - collections::BTreeMap, - convert::{TryFrom, TryInto}, -}; +use std::collections::BTreeMap; use as_variant::as_variant; use ruma::{ diff --git a/crates/matrix-sdk-crypto/src/verification/machine.rs b/crates/matrix-sdk-crypto/src/verification/machine.rs index 5858a5d8367..46a15737619 100644 --- a/crates/matrix-sdk-crypto/src/verification/machine.rs +++ b/crates/matrix-sdk-crypto/src/verification/machine.rs @@ -14,7 +14,6 @@ use std::{ collections::HashMap, - convert::{TryFrom, TryInto}, sync::{Arc, RwLock as StdRwLock}, }; From b2e7ae431022ab37017728550e11f0354900abe7 Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Sun, 10 Mar 2024 17:14:51 +0100 Subject: [PATCH 65/74] sdk: add delete_pusher method Signed-off-by: hanadi92 --- crates/matrix-sdk/src/client/mod.rs | 34 +++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index 2242ced17af..4c09d6db350 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -48,7 +48,7 @@ use ruma::{ }, filter::{create_filter::v3::Request as FilterUploadRequest, FilterDefinition}, membership::{join_room_by_id, join_room_by_id_or_alias}, - push::{set_pusher, Pusher}, + push::{set_pusher, Pusher, PusherIds}, room::create_room, session::login::v3::DiscoveryInfo, sync::sync_events, @@ -2047,6 +2047,15 @@ impl Client { self.send(request, None).await } + /// Deletes a pusher by its ids + pub async fn delete_pusher( + &self, + pusher_ids: PusherIds, + ) -> HttpResult { + let request = set_pusher::v3::Request::delete(pusher_ids); + self.send(request, None).await + } + /// Get the notification settings of the current owner of the client. pub async fn notification_settings(&self) -> NotificationSettings { let ruleset = self.account().push_rules().await.unwrap_or_else(|_| Ruleset::new()); @@ -2110,7 +2119,11 @@ pub(crate) mod tests { #[cfg(target_arch = "wasm32")] wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser); - use ruma::{events::ignored_user_list::IgnoredUserListEventContent, UserId}; + use ruma::{ + api::client::push::PusherIds, events::ignored_user_list::IgnoredUserListEventContent, + UserId, + }; + use serde_json::json; use url::Url; use wiremock::{ matchers::{body_json, header, method, path}, @@ -2366,4 +2379,21 @@ pub(crate) mod tests { let msc4028_enabled = client.can_homeserver_push_encrypted_event_to_device().await.unwrap(); assert!(msc4028_enabled); } + + #[async_test] + async fn test_delete_pusher() { + let server = MockServer::start().await; + let client = logged_in_client(Some(server.uri())).await; + + Mock::given(method("POST")) + .and(path("_matrix/client/r0/pushers/set")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({}))) + .mount(&server) + .await; + + let pusher_ids = PusherIds::new("pushKey".to_owned(), "app_id".to_owned()); + let response = client.delete_pusher(pusher_ids).await; + + assert!(response.is_ok()); + } } From 7c4d180297c3fa43534a62a6a42cc0bfd5a99abe Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Sun, 10 Mar 2024 17:15:11 +0100 Subject: [PATCH 66/74] ffi: add delete_pusher method Signed-off-by: hanadi92 --- bindings/matrix-sdk-ffi/src/client.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index 9257c7e731f..6b2c211af65 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -633,6 +633,15 @@ impl Client { }) } + /// Deletes a pusher of given pusher ids + pub fn delete_pusher(&self, identifiers: PusherIdentifiers) -> Result<(), ClientError> { + RUNTIME.block_on(async move { + let ids = identifiers.into(); + self.inner.delete_pusher(ids).await?; + Ok(()) + }) + } + /// The homeserver this client is configured to use. pub fn homeserver(&self) -> String { self.inner.homeserver().to_string() From b83a644260277753800016ee014d8dc7bcc488e2 Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Mon, 11 Mar 2024 18:10:56 +0300 Subject: [PATCH 67/74] fix: use async instead of block on runtime Signed-off-by: hanadi92 --- bindings/matrix-sdk-ffi/src/client.rs | 37 ++++++++++++--------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index 6b2c211af65..879d837a6ab 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -608,7 +608,7 @@ impl Client { } /// Registers a pusher with given parameters - pub fn set_pusher( + pub async fn set_pusher( &self, identifiers: PusherIdentifiers, kind: PusherKind, @@ -617,29 +617,24 @@ impl Client { profile_tag: Option, lang: String, ) -> Result<(), ClientError> { - RUNTIME.block_on(async move { - let ids = identifiers.into(); - - let pusher_init = PusherInit { - ids, - kind: kind.try_into()?, - app_display_name, - device_display_name, - profile_tag, - lang, - }; - self.inner.set_pusher(pusher_init.into()).await?; - Ok(()) - }) + let ids = identifiers.into(); + + let pusher_init = PusherInit { + ids, + kind: kind.try_into()?, + app_display_name, + device_display_name, + profile_tag, + lang, + }; + self.inner.set_pusher(pusher_init.into()).await?; + Ok(()) } /// Deletes a pusher of given pusher ids - pub fn delete_pusher(&self, identifiers: PusherIdentifiers) -> Result<(), ClientError> { - RUNTIME.block_on(async move { - let ids = identifiers.into(); - self.inner.delete_pusher(ids).await?; - Ok(()) - }) + pub async fn delete_pusher(&self, identifiers: PusherIdentifiers) -> Result<(), ClientError> { + self.inner.delete_pusher(identifiers.into()).await?; + Ok(()) } /// The homeserver this client is configured to use. From 36c39b837afe42b128884f3e431a56b275ea5dba Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Tue, 12 Mar 2024 07:33:55 +0300 Subject: [PATCH 68/74] refactor: create a pusher manager to set and delete Signed-off-by: hanadi92 --- bindings/matrix-sdk-ffi/src/client.rs | 4 +- crates/matrix-sdk/src/client/mod.rs | 46 ++--------- crates/matrix-sdk/src/lib.rs | 2 + crates/matrix-sdk/src/pusher.rs | 107 ++++++++++++++++++++++++++ 4 files changed, 118 insertions(+), 41 deletions(-) create mode 100644 crates/matrix-sdk/src/pusher.rs diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index 879d837a6ab..701aba39a60 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -627,13 +627,13 @@ impl Client { profile_tag, lang, }; - self.inner.set_pusher(pusher_init.into()).await?; + self.inner.pusher().set(pusher_init.into()).await?; Ok(()) } /// Deletes a pusher of given pusher ids pub async fn delete_pusher(&self, identifiers: PusherIdentifiers) -> Result<(), ClientError> { - self.inner.delete_pusher(identifiers.into()).await?; + self.inner.pusher().delete(identifiers.into()).await?; Ok(()) } diff --git a/crates/matrix-sdk/src/client/mod.rs b/crates/matrix-sdk/src/client/mod.rs index 4c09d6db350..67d995fadbd 100644 --- a/crates/matrix-sdk/src/client/mod.rs +++ b/crates/matrix-sdk/src/client/mod.rs @@ -48,7 +48,6 @@ use ruma::{ }, filter::{create_filter::v3::Request as FilterUploadRequest, FilterDefinition}, membership::{join_room_by_id, join_room_by_id_or_alias}, - push::{set_pusher, Pusher, PusherIds}, room::create_room, session::login::v3::DiscoveryInfo, sync::sync_events, @@ -84,7 +83,7 @@ use crate::{ matrix_auth::MatrixAuth, notification_settings::NotificationSettings, sync::{RoomUpdate, SyncResponse}, - Account, AuthApi, AuthSession, Error, Media, RefreshTokenError, Result, Room, + Account, AuthApi, AuthSession, Error, Media, Pusher, RefreshTokenError, Result, Room, TransmissionProgress, }; #[cfg(feature = "e2e-encryption")] @@ -561,6 +560,11 @@ impl Client { Media::new(self.clone()) } + /// Get the pusher manager of the client. + pub fn pusher(&self) -> Pusher { + Pusher::new(self.clone()) + } + /// Access the OpenID Connect API of the client. #[cfg(feature = "experimental-oidc")] pub fn oidc(&self) -> Oidc { @@ -2041,21 +2045,6 @@ impl Client { Ok(()) } - /// Sets a given pusher - pub async fn set_pusher(&self, pusher: Pusher) -> HttpResult { - let request = set_pusher::v3::Request::post(pusher); - self.send(request, None).await - } - - /// Deletes a pusher by its ids - pub async fn delete_pusher( - &self, - pusher_ids: PusherIds, - ) -> HttpResult { - let request = set_pusher::v3::Request::delete(pusher_ids); - self.send(request, None).await - } - /// Get the notification settings of the current owner of the client. pub async fn notification_settings(&self) -> NotificationSettings { let ruleset = self.account().push_rules().await.unwrap_or_else(|_| Ruleset::new()); @@ -2119,11 +2108,7 @@ pub(crate) mod tests { #[cfg(target_arch = "wasm32")] wasm_bindgen_test::wasm_bindgen_test_configure!(run_in_browser); - use ruma::{ - api::client::push::PusherIds, events::ignored_user_list::IgnoredUserListEventContent, - UserId, - }; - use serde_json::json; + use ruma::{events::ignored_user_list::IgnoredUserListEventContent, UserId}; use url::Url; use wiremock::{ matchers::{body_json, header, method, path}, @@ -2379,21 +2364,4 @@ pub(crate) mod tests { let msc4028_enabled = client.can_homeserver_push_encrypted_event_to_device().await.unwrap(); assert!(msc4028_enabled); } - - #[async_test] - async fn test_delete_pusher() { - let server = MockServer::start().await; - let client = logged_in_client(Some(server.uri())).await; - - Mock::given(method("POST")) - .and(path("_matrix/client/r0/pushers/set")) - .respond_with(ResponseTemplate::new(200).set_body_json(json!({}))) - .mount(&server) - .await; - - let pusher_ids = PusherIds::new("pushKey".to_owned(), "app_id".to_owned()); - let response = client.delete_pusher(pusher_ids).await; - - assert!(response.is_ok()); - } } diff --git a/crates/matrix-sdk/src/lib.rs b/crates/matrix-sdk/src/lib.rs index b8cad7286b6..f05beacb4a9 100644 --- a/crates/matrix-sdk/src/lib.rs +++ b/crates/matrix-sdk/src/lib.rs @@ -47,6 +47,7 @@ pub mod media; pub mod notification_settings; #[cfg(feature = "experimental-oidc")] pub mod oidc; +pub mod pusher; pub mod room; pub mod room_directory_search; pub mod utils; @@ -78,6 +79,7 @@ pub use matrix_sdk_sqlite::SqliteCryptoStore; #[cfg(feature = "sqlite")] pub use matrix_sdk_sqlite::SqliteStateStore; pub use media::Media; +pub use pusher::Pusher; pub use room::Room; pub use ruma::{IdParseError, OwnedServerName, ServerName}; #[cfg(feature = "experimental-sliding-sync")] diff --git a/crates/matrix-sdk/src/pusher.rs b/crates/matrix-sdk/src/pusher.rs new file mode 100644 index 00000000000..8db9fa19cf0 --- /dev/null +++ b/crates/matrix-sdk/src/pusher.rs @@ -0,0 +1,107 @@ +// Copyright 2024 FIXME help with copyright wording +// +// 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. + +//! High-level pusher API. + +use ruma::api::client::push::{set_pusher, PusherIds}; + +use crate::{Client, Result}; + +/// A high-level API to interact with the pusher API. +/// +/// All the methods in this struct send a request to the homeserver. +#[derive(Debug, Clone)] +pub struct Pusher { + /// The underlying HTTP client. + client: Client, +} + +impl Pusher { + pub(crate) fn new(client: Client) -> Self { + Self { client } + } + + /// Sets a given pusher + pub async fn set(&self, pusher: ruma::api::client::push::Pusher) -> Result<()> { + let request = set_pusher::v3::Request::post(pusher); + self.client.send(request, None).await?; + Ok(()) + } + + /// Deletes a pusher by its ids + pub async fn delete(&self, pusher_ids: PusherIds) -> Result<()> { + let request = set_pusher::v3::Request::delete(pusher_ids); + self.client.send(request, None).await?; + Ok(()) + } +} + +// The http mocking library is not supported for wasm32 +#[cfg(all(test, not(target_arch = "wasm32")))] +mod tests { + use matrix_sdk_test::{async_test, test_json}; + use ruma::{ + api::client::push::{PusherIds, PusherInit, PusherKind}, + push::HttpPusherData, + }; + use wiremock::{ + matchers::{method, path}, + Mock, MockServer, ResponseTemplate, + }; + + use crate::test_utils::logged_in_client; + + async fn mock_api(server: MockServer) { + Mock::given(method("POST")) + .and(path("_matrix/client/r0/pushers/set")) + .respond_with(ResponseTemplate::new(200).set_body_json(&*test_json::EMPTY)) + .mount(&server) + .await; + } + + #[async_test] + async fn test_set_pusher() { + let server = MockServer::start().await; + let client = logged_in_client(Some(server.uri())).await; + mock_api(server).await; + + // prepare dummy pusher + let pusher = PusherInit { + ids: PusherIds::new("pushKey".to_owned(), "app_id".to_owned()), + app_display_name: "name".to_owned(), + kind: PusherKind::Http(HttpPusherData::new("dummy".to_owned())), + lang: "EN".to_owned(), + device_display_name: "name".to_owned(), + profile_tag: None, + }; + + let response = client.pusher().set(pusher.into()).await; + + assert!(response.is_ok()); + } + + #[async_test] + async fn test_delete_pusher() { + let server = MockServer::start().await; + let client = logged_in_client(Some(server.uri())).await; + mock_api(server).await; + + // prepare pusher ids + let pusher_ids = PusherIds::new("pushKey".to_owned(), "app_id".to_owned()); + + let response = client.pusher().delete(pusher_ids).await; + + assert!(response.is_ok()); + } +} From d2c9ca455db7c5c98e25a85ce3036171ca98b650 Mon Sep 17 00:00:00 2001 From: hanadi92 Date: Tue, 19 Mar 2024 12:37:05 +0300 Subject: [PATCH 69/74] docs: update copyright Signed-off-by: hanadi92 --- crates/matrix-sdk/src/pusher.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk/src/pusher.rs b/crates/matrix-sdk/src/pusher.rs index 8db9fa19cf0..36e21d9af6d 100644 --- a/crates/matrix-sdk/src/pusher.rs +++ b/crates/matrix-sdk/src/pusher.rs @@ -1,4 +1,5 @@ -// Copyright 2024 FIXME help with copyright wording +// Copyright 2024 The Matrix.org Foundation C.I.C. +// Copyright 2024 Hanadi Tamimi // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. From 17805cbcd89c8b2bd54a008e30413dea0f5a5056 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Thu, 21 Mar 2024 11:39:04 +0100 Subject: [PATCH 70/74] feat(bindings): added join room by id to ffi --- bindings/matrix-sdk-ffi/src/client.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index 701aba39a60..cb0b9ae52e3 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -751,6 +751,11 @@ impl Client { matrix_sdk::room_directory_search::RoomDirectorySearch::new((*self.inner).clone()), )) } + + pub async fn join_room_by_id(&self, room_id: String) -> Result, ClientError> { + let room = self.inner.join_room_by_id(room_id.into()).await?; + Ok(Arc::new(Room::new(room))) + } } #[uniffi::export(callback_interface)] From c13fb7e19f3be6d038cd7b9d82053bd9138758e7 Mon Sep 17 00:00:00 2001 From: Kegan Dougal <7190048+kegsay@users.noreply.github.com> Date: Thu, 21 Mar 2024 10:40:37 +0000 Subject: [PATCH 71/74] uniffi: wrap TaskHandle up in an Arc<> Up until uniffi 0.26 it was not possible to send objects across the boundary unless they were wrapped in an `Arc<>`, see https://github.com/mozilla/uniffi-rs/pull/1672 The bindings generator used in complement-crypto only supports up to uniffi 0.25, meaning having a function which returns objects ends up erroring with: ``` error[E0277]: the trait bound `TaskHandle: LowerReturn` is not satisfied --> bindings/matrix-sdk-ffi/src/room_directory_search.rs:109:10 | 109 | ) -> TaskHandle { | ^^^^^^^^^^ the trait `LowerReturn` is not implemented for `TaskHandle` | = help: the following other types implement trait `LowerReturn`: > > > > > > > > and 133 others error[E0277]: the trait bound `TaskHandle: LowerReturn<_>` is not satisfied --> bindings/matrix-sdk-ffi/src/room_directory_search.rs:82:1 | 82 | #[uniffi::export(async_runtime = "tokio")] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `LowerReturn<_>` is not implemented for `TaskHandle` | = help: the following other types implement trait `LowerReturn`: > > > > > > > > and 133 others ``` This PR wraps the offending function in an `Arc<>` to make it uniffi 0.25 compatible, which unbreaks complement crypto. --- bindings/matrix-sdk-ffi/src/room_directory_search.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/room_directory_search.rs b/bindings/matrix-sdk-ffi/src/room_directory_search.rs index 7036d7f5bf0..b3af0058b94 100644 --- a/bindings/matrix-sdk-ffi/src/room_directory_search.rs +++ b/bindings/matrix-sdk-ffi/src/room_directory_search.rs @@ -106,10 +106,10 @@ impl RoomDirectorySearch { pub async fn results( &self, listener: Box, - ) -> TaskHandle { + ) -> Arc { let (initial_values, mut stream) = self.inner.read().await.results(); - TaskHandle::new(RUNTIME.spawn(async move { + Arc::new(TaskHandle::new(RUNTIME.spawn(async move { listener.on_update(vec![RoomDirectorySearchEntryUpdate::Reset { values: initial_values.into_iter().map(Into::into).collect(), }]); @@ -117,7 +117,7 @@ impl RoomDirectorySearch { while let Some(diffs) = stream.next().await { listener.on_update(diffs.into_iter().map(|diff| diff.into()).collect()); } - })) + }))) } } From d447f63e33e816ef4c7e030ee320ae337be071c8 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Thu, 21 Mar 2024 11:53:44 +0100 Subject: [PATCH 72/74] fixed invalid parsing --- bindings/matrix-sdk-ffi/src/client.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bindings/matrix-sdk-ffi/src/client.rs b/bindings/matrix-sdk-ffi/src/client.rs index cb0b9ae52e3..dea353a8982 100644 --- a/bindings/matrix-sdk-ffi/src/client.rs +++ b/bindings/matrix-sdk-ffi/src/client.rs @@ -32,7 +32,7 @@ use matrix_sdk::{ AnyInitialStateEvent, AnyToDeviceEvent, InitialStateEvent, }, serde::Raw, - EventEncryptionAlgorithm, TransactionId, UInt, UserId, + EventEncryptionAlgorithm, RoomId, TransactionId, UInt, UserId, }, AuthApi, AuthSession, Client as MatrixClient, SessionChange, SessionTokens, }; @@ -753,7 +753,8 @@ impl Client { } pub async fn join_room_by_id(&self, room_id: String) -> Result, ClientError> { - let room = self.inner.join_room_by_id(room_id.into()).await?; + let room_id = RoomId::parse(room_id)?; + let room = self.inner.join_room_by_id(room_id.as_ref()).await?; Ok(Arc::new(Room::new(room))) } } From 199275ff89b94ac51af3ec0e44961e092c28951d Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Thu, 21 Mar 2024 12:39:18 +0100 Subject: [PATCH 73/74] feat(sdk): Rename `ChunkIdentifierGenerator::generate_next` to `next`. This patch renames `ChunkIdentifierGenerator::generate_next` to `next. This patch also simplifies a `.saturating_add(1)` to a simple `+ 1`, which is fine because we have checked for overflow just before. --- .../src/event_cache/linked_chunk.rs | 48 ++++++++----------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/crates/matrix-sdk/src/event_cache/linked_chunk.rs b/crates/matrix-sdk/src/event_cache/linked_chunk.rs index 0847b6d0d26..0435950bc40 100644 --- a/crates/matrix-sdk/src/event_cache/linked_chunk.rs +++ b/crates/matrix-sdk/src/event_cache/linked_chunk.rs @@ -107,7 +107,7 @@ impl LinkedChunk { /// Push a gap at the end of the [`LinkedChunk`], i.e. after the last /// chunk. pub fn push_gap_back(&mut self, content: Gap) { - let next_identifier = self.chunk_identifier_generator.generate_next(); + let next_identifier = self.chunk_identifier_generator.next(); let last_chunk = self.latest_chunk_mut(); last_chunk.insert_next(Chunk::new_gap_leaked(next_identifier, content)); @@ -210,10 +210,8 @@ impl LinkedChunk { // `chunk.previous.is_some()` in the `if` statement. .expect("Previous chunk must be present"); - previous_chunk.insert_next(Chunk::new_gap_leaked( - chunk_identifier_generator.generate_next(), - content, - )); + previous_chunk + .insert_next(Chunk::new_gap_leaked(chunk_identifier_generator.next(), content)); // We don't need to update `self.last` because we have inserted a new chunk // before `chunk`. @@ -237,14 +235,9 @@ impl LinkedChunk { chunk // Insert a new gap chunk. - .insert_next(Chunk::new_gap_leaked( - chunk_identifier_generator.generate_next(), - content, - )) + .insert_next(Chunk::new_gap_leaked(chunk_identifier_generator.next(), content)) // Insert a new items chunk. - .insert_next(Chunk::new_items_leaked( - chunk_identifier_generator.generate_next(), - )) + .insert_next(Chunk::new_items_leaked(chunk_identifier_generator.next())) // Finally, push the items that have been detached. .push_items(detached_items.into_iter(), &chunk_identifier_generator) } @@ -295,9 +288,8 @@ impl LinkedChunk { let last_inserted_chunk = chunk // Insert a new items chunk… - .insert_next(Chunk::new_items_leaked( - chunk_identifier_generator.generate_next(), - )) // … and insert the items. + .insert_next(Chunk::new_items_leaked(chunk_identifier_generator.next())) + // … and insert the items. .push_items(items, &chunk_identifier_generator); ( @@ -591,7 +583,7 @@ impl ChunkIdentifierGenerator { /// /// Note that it can fail if there is no more unique identifier available. /// In this case, this method will panic. - pub fn generate_next(&self) -> ChunkIdentifier { + pub fn next(&self) -> ChunkIdentifier { let previous = self.next.fetch_add(1, Ordering::Relaxed); // Check for overflows. @@ -600,7 +592,7 @@ impl ChunkIdentifierGenerator { panic!("No more chunk identifiers available. Congrats, you did it. 2^64 identifiers have been consumed.") } - ChunkIdentifier(previous.saturating_add(1)) + ChunkIdentifier(previous + 1) } } @@ -835,7 +827,7 @@ impl Chunk { ChunkContent::Gap(..) => { self // Insert a new items chunk. - .insert_next(Self::new_items_leaked(chunk_identifier_generator.generate_next())) + .insert_next(Self::new_items_leaked(chunk_identifier_generator.next())) // Now push the new items on the next chunk, and return the result of // `push_items`. .push_items(new_items, chunk_identifier_generator) @@ -859,9 +851,7 @@ impl Chunk { self // Insert a new items chunk. - .insert_next(Self::new_items_leaked( - chunk_identifier_generator.generate_next(), - )) + .insert_next(Self::new_items_leaked(chunk_identifier_generator.next())) // Now push the rest of the new items on the next chunk, and return the // result of `push_items`. .push_items(new_items, chunk_identifier_generator) @@ -1038,18 +1028,18 @@ mod tests { fn test_chunk_identifier_generator() { let generator = ChunkIdentifierGenerator::new_from_scratch(); - assert_eq!(generator.generate_next(), ChunkIdentifier(1)); - assert_eq!(generator.generate_next(), ChunkIdentifier(2)); - assert_eq!(generator.generate_next(), ChunkIdentifier(3)); - assert_eq!(generator.generate_next(), ChunkIdentifier(4)); + assert_eq!(generator.next(), ChunkIdentifier(1)); + assert_eq!(generator.next(), ChunkIdentifier(2)); + assert_eq!(generator.next(), ChunkIdentifier(3)); + assert_eq!(generator.next(), ChunkIdentifier(4)); let generator = ChunkIdentifierGenerator::new_from_previous_chunk_identifier(ChunkIdentifier(42)); - assert_eq!(generator.generate_next(), ChunkIdentifier(43)); - assert_eq!(generator.generate_next(), ChunkIdentifier(44)); - assert_eq!(generator.generate_next(), ChunkIdentifier(45)); - assert_eq!(generator.generate_next(), ChunkIdentifier(46)); + assert_eq!(generator.next(), ChunkIdentifier(43)); + assert_eq!(generator.next(), ChunkIdentifier(44)); + assert_eq!(generator.next(), ChunkIdentifier(45)); + assert_eq!(generator.next(), ChunkIdentifier(46)); } #[test] From 82bcf48c8890b3e1287e7f9f2d32783431741764 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 21 Mar 2024 16:22:26 +0000 Subject: [PATCH 74/74] Enable debuginfo for tarpaulin builds It appears that tarpaulin complains if the symbol information is stripped from the binary, and as of Rust 1.77, `debug=0` causes Cargo to strip all debug info. To fix this, set `debug=1`. --- .github/workflows/coverage.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 78531e7f7a8..d08fa00b071 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -105,7 +105,7 @@ jobs: --features experimental-widgets,testing env: CARGO_PROFILE_COV_INHERITS: 'dev' - CARGO_PROFILE_COV_DEBUG: 'false' + CARGO_PROFILE_COV_DEBUG: 1 HOMESERVER_URL: "http://localhost:8008" HOMESERVER_DOMAIN: "synapse" SLIDING_SYNC_PROXY_URL: "http://localhost:8118"