From e7ac2a895b328164d8350d57ec8bbe21b4d76d5a Mon Sep 17 00:00:00 2001 From: Ivan Enderlin Date: Wed, 13 Nov 2024 08:22:39 +0100 Subject: [PATCH] feat(base): Make `ObservableMap::stream` works on `wasm32-unknown-unknown`. This patch updates `eyeball-im` and `eyeball-im-util` to integrate https://github.com/jplatte/eyeball/pull/63/. With this new feature, we can have a single implementation of `ObservableMap` (instead of 2: one for all targets, one for `wasm32-u-u`). It makes it possible to get `Client::rooms_stream` available on all targets now. --- Cargo.lock | 5 +- crates/matrix-sdk-base/src/client.rs | 3 - crates/matrix-sdk-base/src/store/mod.rs | 3 - .../src/store/observable_map.rs | 288 +++++++----------- 4 files changed, 108 insertions(+), 191 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index be3ee36dc0b..1fc6bea3bed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1642,14 +1642,13 @@ dependencies = [ [[package]] name = "eyeball-im" -version = "0.5.0" +version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2ae8c5165c9770f3ec7cccce12f4c5d70f01fa8bf84cf30cfbfd5a1c6f8901d5" +checksum = "a1c02432230060cae0621e15803e073976d22974e0f013c9cb28a4ea1b484629" dependencies = [ "futures-core", "imbl", "tokio", - "tokio-util", "tracing", ] diff --git a/crates/matrix-sdk-base/src/client.rs b/crates/matrix-sdk-base/src/client.rs index f36178b56fa..8fec8251daf 100644 --- a/crates/matrix-sdk-base/src/client.rs +++ b/crates/matrix-sdk-base/src/client.rs @@ -22,9 +22,7 @@ use std::{ }; use eyeball::{SharedObservable, Subscriber}; -#[cfg(not(target_arch = "wasm32"))] use eyeball_im::{Vector, VectorDiff}; -#[cfg(not(target_arch = "wasm32"))] use futures_util::Stream; #[cfg(feature = "e2e-encryption")] use matrix_sdk_crypto::{ @@ -236,7 +234,6 @@ impl BaseClient { /// Get a stream of all the rooms changes, in addition to the existing /// rooms. - #[cfg(not(target_arch = "wasm32"))] pub fn rooms_stream(&self) -> (Vector, impl Stream>>) { self.store.rooms_stream() } diff --git a/crates/matrix-sdk-base/src/store/mod.rs b/crates/matrix-sdk-base/src/store/mod.rs index cfea522adda..1bb92c27c3c 100644 --- a/crates/matrix-sdk-base/src/store/mod.rs +++ b/crates/matrix-sdk-base/src/store/mod.rs @@ -29,9 +29,7 @@ use std::{ sync::{Arc, RwLock as StdRwLock}, }; -#[cfg(not(target_arch = "wasm32"))] use eyeball_im::{Vector, VectorDiff}; -#[cfg(not(target_arch = "wasm32"))] use futures_util::Stream; use once_cell::sync::OnceCell; @@ -267,7 +265,6 @@ impl Store { /// Get a stream of all the rooms changes, in addition to the existing /// rooms. - #[cfg(not(target_arch = "wasm32"))] pub fn rooms_stream(&self) -> (Vector, impl Stream>>) { self.rooms.read().unwrap().stream() } diff --git a/crates/matrix-sdk-base/src/store/observable_map.rs b/crates/matrix-sdk-base/src/store/observable_map.rs index 1650d08a160..3392dd6510f 100644 --- a/crates/matrix-sdk-base/src/store/observable_map.rs +++ b/crates/matrix-sdk-base/src/store/observable_map.rs @@ -14,212 +14,137 @@ //! An [`ObservableMap`] implementation. -#[cfg(not(target_arch = "wasm32"))] -mod impl_non_wasm32 { - use std::{borrow::Borrow, collections::HashMap, hash::Hash}; - - use eyeball_im::{ObservableVector, Vector, VectorDiff}; - use futures_util::Stream; - - /// An observable map. - /// - /// This is an “observable map” naive implementation. Just like regular - /// hashmap, we have a redirection from a key to a position, and from a - /// position to a value. The (key, position) tuples are stored in an - /// [`HashMap`]. The (position, value) tuples are stored in an - /// [`ObservableVector`]. The (key, position) tuple is only provided for - /// fast _reading_ implementations, like `Self::get` and - /// `Self::get_or_create`. The (position, value) tuples are observable, - /// this is what interests us the most here. - /// - /// Why not implementing a new `ObservableMap` type in `eyeball-im` instead - /// of this custom implementation? Because we want to continue providing - /// `VectorDiff` when observing the changes, so that the rest of the API in - /// the Matrix Rust SDK aren't broken. Indeed, an `ObservableMap` must - /// produce `MapDiff`, which would be quite different. - /// Plus, we would like to re-use all our existing code, test, stream - /// adapters and so on. - /// - /// This is a trade-off. This implementation is simple enough for the - /// moment, and basically does the job. - #[derive(Debug)] - pub(crate) struct ObservableMap - where - V: Clone + Send + Sync + 'static, - { - /// The (key, position) tuples. - mapping: HashMap, +use std::{borrow::Borrow, collections::HashMap, hash::Hash}; + +use eyeball_im::{ObservableVector, Vector, VectorDiff}; +use futures_util::Stream; + +/// An observable map. +/// +/// This is an “observable map” naive implementation. Just like regular +/// hashmap, we have a redirection from a key to a position, and from a +/// position to a value. The (key, position) tuples are stored in an +/// [`HashMap`]. The (position, value) tuples are stored in an +/// [`ObservableVector`]. The (key, position) tuple is only provided for +/// fast _reading_ implementations, like `Self::get` and +/// `Self::get_or_create`. The (position, value) tuples are observable, +/// this is what interests us the most here. +/// +/// Why not implementing a new `ObservableMap` type in `eyeball-im` instead +/// of this custom implementation? Because we want to continue providing +/// `VectorDiff` when observing the changes, so that the rest of the API in +/// the Matrix Rust SDK aren't broken. Indeed, an `ObservableMap` must +/// produce `MapDiff`, which would be quite different. +/// Plus, we would like to re-use all our existing code, test, stream +/// adapters and so on. +/// +/// This is a trade-off. This implementation is simple enough for the +/// moment, and basically does the job. +#[derive(Debug)] +pub(crate) struct ObservableMap +where + V: Clone + 'static, +{ + /// The (key, position) tuples. + mapping: HashMap, + + /// The values where the indices are the `position` part of + /// `Self::mapping`. + values: ObservableVector, +} - /// The values where the indices are the `position` part of - /// `Self::mapping`. - values: ObservableVector, +impl ObservableMap +where + K: Hash + Eq, + V: Clone + 'static, +{ + /// Create a new `Self`. + pub(crate) fn new() -> Self { + Self { mapping: HashMap::new(), values: ObservableVector::new() } } - impl ObservableMap - where - K: Hash + Eq, - V: Clone + Send + Sync + 'static, - { - /// Create a new `Self`. - pub(crate) fn new() -> Self { - Self { mapping: HashMap::new(), values: ObservableVector::new() } - } - - /// Insert a new `V` in the collection. - /// - /// If the `V` value already exists, it will be updated to the new one. - pub(crate) fn insert(&mut self, key: K, value: V) -> usize { - match self.mapping.get(&key) { - Some(position) => { - self.values.set(*position, value); - - *position - } - None => { - let position = self.values.len(); - - self.values.push_back(value); - self.mapping.insert(key, position); + /// Insert a new `V` in the collection. + /// + /// If the `V` value already exists, it will be updated to the new one. + pub(crate) fn insert(&mut self, key: K, value: V) -> usize { + match self.mapping.get(&key) { + Some(position) => { + self.values.set(*position, value); - position - } + *position } - } + None => { + let position = self.values.len(); - /// Reading one `V` value based on their ID, if it exists. - pub(crate) fn get(&self, key: &L) -> Option<&V> - where - K: Borrow, - L: Hash + Eq + ?Sized, - { - self.mapping.get(key).and_then(|position| self.values.get(*position)) - } + self.values.push_back(value); + self.mapping.insert(key, position); - /// Reading one `V` value based on their ID, or create a new one (by - /// using `default`). - pub(crate) fn get_or_create(&mut self, key: &L, default: F) -> &V - where - K: Borrow, - L: Hash + Eq + ?Sized + ToOwned, - F: FnOnce() -> V, - { - let position = match self.mapping.get(key) { - Some(position) => *position, - None => { - let value = default(); - let position = self.values.len(); - - self.values.push_back(value); - self.mapping.insert(key.to_owned(), position); - - position - } - }; - - self.values - .get(position) - .expect("Value should be present or has just been inserted, but it's missing") - } - - /// Return an iterator over the existing values. - pub(crate) fn iter(&self) -> impl Iterator { - self.values.iter() - } - - /// Get a [`Stream`] of the values. - pub(crate) fn stream(&self) -> (Vector, impl Stream>>) { - self.values.subscribe().into_values_and_batched_stream() - } - - /// Remove a `V` value based on their ID, if it exists. - /// - /// Returns the removed value. - pub(crate) fn remove(&mut self, key: &L) -> Option - where - K: Borrow, - L: Hash + Eq + ?Sized, - { - let position = self.mapping.remove(key)?; - Some(self.values.remove(position)) + position + } } } -} -#[cfg(target_arch = "wasm32")] -mod impl_wasm32 { - use std::{borrow::Borrow, collections::BTreeMap, hash::Hash}; - - /// An observable map for Wasm. It's a simple wrapper around `BTreeMap`. - #[derive(Debug)] - pub(crate) struct ObservableMap(BTreeMap) + /// Reading one `V` value based on their ID, if it exists. + pub(crate) fn get(&self, key: &L) -> Option<&V> where - V: Clone + 'static; + K: Borrow, + L: Hash + Eq + ?Sized, + { + self.mapping.get(key).and_then(|position| self.values.get(*position)) + } - impl ObservableMap + /// Reading one `V` value based on their ID, or create a new one (by + /// using `default`). + pub(crate) fn get_or_create(&mut self, key: &L, default: F) -> &V where - K: Hash + Eq + Ord, - V: Clone + 'static, + K: Borrow, + L: Hash + Eq + ?Sized + ToOwned, + F: FnOnce() -> V, { - /// Create a new `Self`. - pub(crate) fn new() -> Self { - Self(BTreeMap::new()) - } + let position = match self.mapping.get(key) { + Some(position) => *position, + None => { + let value = default(); + let position = self.values.len(); - /// Insert a new `V` in the collection. - /// - /// If the `V` value already exists, it will be updated to the new one. - pub(crate) fn insert(&mut self, key: K, value: V) { - self.0.insert(key, value); - } + self.values.push_back(value); + self.mapping.insert(key.to_owned(), position); - /// Reading one `V` value based on their ID, if it exists. - pub(crate) fn get(&self, key: &L) -> Option<&V> - where - K: Borrow, - L: Hash + Eq + Ord + ?Sized, - { - self.0.get(key) - } + position + } + }; - /// Reading one `V` value based on their ID, or create a new one (by - /// using `default`). - pub(crate) fn get_or_create(&mut self, key: &L, default: F) -> &V - where - K: Borrow, - L: Hash + Eq + ?Sized + ToOwned, - F: FnOnce() -> V, - { - self.0.entry(key.to_owned()).or_insert_with(default) - } + self.values + .get(position) + .expect("Value should be present or has just been inserted, but it's missing") + } - /// Return an iterator over the existing values. - pub(crate) fn iter(&self) -> impl Iterator { - self.0.values() - } + /// Return an iterator over the existing values. + pub(crate) fn iter(&self) -> impl Iterator { + self.values.iter() + } - /// Remove a `V` value based on their ID, if it exists. - /// - /// Returns the removed value. - pub(crate) fn remove(&mut self, key: &L) -> Option - where - K: Borrow, - L: Hash + Eq + Ord + ?Sized, - { - self.0.remove(key) - } + /// Get a [`Stream`] of the values. + pub(crate) fn stream(&self) -> (Vector, impl Stream>>) { + self.values.subscribe().into_values_and_batched_stream() } -} -#[cfg(not(target_arch = "wasm32"))] -pub(crate) use impl_non_wasm32::ObservableMap; -#[cfg(target_arch = "wasm32")] -pub(crate) use impl_wasm32::ObservableMap; + /// Remove a `V` value based on their ID, if it exists. + /// + /// Returns the removed value. + pub(crate) fn remove(&mut self, key: &L) -> Option + where + K: Borrow, + L: Hash + Eq + ?Sized, + { + let position = self.mapping.remove(key)?; + Some(self.values.remove(position)) + } +} #[cfg(test)] mod tests { - #[cfg(not(target_arch = "wasm32"))] use eyeball_im::VectorDiff; - #[cfg(not(target_arch = "wasm32"))] use stream_assert::{assert_closed, assert_next_eq, assert_pending}; use super::ObservableMap; @@ -314,7 +239,6 @@ mod tests { ); } - #[cfg(not(target_arch = "wasm32"))] #[test] fn test_stream() { let mut map = ObservableMap::::new();