Improve wasm repository and docs

Author: dani-garcia

crates/bitwarden-uniffi/src/platform/repository.rs

@@ -38,6 +38,8 @@ impl From<RepositoryError> for bitwarden_state::repository::RepositoryError {
     }
 }
 
+/// This macro creates a Uniffi repository trait and its implementation for the
+/// [bitwarden_state::repository::Repository] trait
 macro_rules! create_uniffi_repository {
     ($name:ident, $ty:ty) => {
         #[uniffi::export(with_foreign)]

crates/bitwarden-wasm-internal/src/platform/repository.rs

@@ -1,4 +1,84 @@
-use wasm_bindgen::prelude::wasm_bindgen;
+/*!
+* To support clients implementing the [Repository] trait in a [wasm_bindgen] environment,
+* we need to deal with an `extern "C"` interface, as that is what [wasm_bindgen] supports:
+*
+* This looks something like this:
+*
+* ```rust,ignore
+  #[wasm_bindgen]
+  extern "C" {
+       pub type CipherRepository;
+
+       #[wasm_bindgen(method, catch)]
+       async fn get(this: &CipherRepository, id: String) -> Result<JsValue, JsValue>;
+  }
+* ```
+*
+* As you can see, this has a few limitations:
+* - The type must be known at compile time, so we cannot use generics directly, which means we
+*   can't use the existing [Repository] trait directly.
+* - The return type must be [JsValue], so we need to convert our types to and from [JsValue].
+*
+* To facilitate this, we provide some utilities:
+* - [WasmRepository] trait, which defines the methods as we expect them to come from
+*   [wasm_bindgen], using [JsValue]. This is generic and should be implemented for each
+*   concrete repository we define, but the implementation should be very straightforward.
+* - [WasmRepositoryChannel] struct, which wraps a [WasmRepository] in a [ThreadBoundRunner] and
+*   implements the [Repository] trait. This has a few special considerations:
+*   - It uses [tsify_next::serde_wasm_bindgen] to convert between [JsValue] and our types, so we can use
+*     the existing [Repository] trait.
+*   - It runs the calls in a thread-bound manner, so we can safely call the [WasmRepository]
+*     methods from any thread.
+*/
+
+use std::{future::Future, marker::PhantomData, rc::Rc};
+
+use bitwarden_state::repository::{Repository, RepositoryError, RepositoryItem};
+use bitwarden_threading::ThreadBoundRunner;
+use serde::{de::DeserializeOwned, Serialize};
+use wasm_bindgen::{prelude::wasm_bindgen, JsValue};
+
+/// This trait defines the methods that a [wasm_bindgen] repository must implement.
+/// The trait itself exists to provide a generic way of handling the [wasm_bindgen] interface, which
+/// is !Send + !Sync, and only deals with [JsValue].
+pub(crate) trait WasmRepository<T> {
+    async fn get(&self, id: String) -> Result<JsValue, JsValue>;
+    async fn list(&self) -> Result<JsValue, JsValue>;
+    async fn set(&self, id: String, value: T) -> Result<JsValue, JsValue>;
+    async fn remove(&self, id: String) -> Result<JsValue, JsValue>;
+}
+
+/// This struct wraps a [WasmRepository] in a [ThreadBoundRunner] to allow it to be used as a
+/// [Repository] in a thread-safe manner. It implements the [Repository] trait directly, by
+/// converting the values as needed with [tsify_next::serde_wasm_bindgen].
+pub(crate) struct WasmRepositoryChannel<T, R: WasmRepository<T> + 'static>(
+    ThreadBoundRunner<R>,
+    PhantomData<T>,
+);
+
+impl<T, R: WasmRepository<T> + 'static> WasmRepositoryChannel<T, R> {
+    pub(crate) fn new(repository: R) -> Self {
+        Self(ThreadBoundRunner::new(repository), PhantomData)
+    }
+}
+
+#[async_trait::async_trait]
+impl<T: RepositoryItem + Serialize + DeserializeOwned, R: WasmRepository<T> + 'static> Repository<T>
+    for WasmRepositoryChannel<T, R>
+{
+    async fn get(&self, id: String) -> Result<Option<T>, RepositoryError> {
+        run_convert(&self.0, |s| async move { s.get(id).await }).await
+    }
+    async fn list(&self) -> Result<Vec<T>, RepositoryError> {
+        run_convert(&self.0, |s| async move { s.list().await }).await
+    }
+    async fn set(&self, id: String, value: T) -> Result<(), RepositoryError> {
+        run_convert(&self.0, |s| async move { s.set(id, value).await.and(UNIT) }).await
+    }
+    async fn remove(&self, id: String) -> Result<(), RepositoryError> {
+        run_convert(&self.0, |s| async move { s.remove(id).await.and(UNIT) }).await
+    }
+}
 
 #[wasm_bindgen(typescript_custom_section)]
 const REPOSITORY_CUSTOM_TS_TYPE: &'static str = r#"
@@ -10,6 +90,9 @@ export interface Repository<T> {
 }
 "#;
 
+/// This macro generates a [wasm_bindgen] interface for a repository type, and provides the
+/// implementation of [WasmRepository] and a way to convert it into something that implements
+/// the [Repository] trait.
 macro_rules! create_wasm_repository {
     ($name:ident, $ty:ty, $typescript_ty:literal) => {
         #[wasm_bindgen]
@@ -38,74 +121,71 @@ macro_rules! create_wasm_repository {
             ) -> Result<::wasm_bindgen::JsValue, ::wasm_bindgen::JsValue>;
         }
 
+        impl $crate::platform::repository::WasmRepository<$ty> for $name {
+            async fn get(
+                &self,
+                id: String,
+            ) -> Result<::wasm_bindgen::JsValue, ::wasm_bindgen::JsValue> {
+                self.get(id).await
+            }
+            async fn list(&self) -> Result<::wasm_bindgen::JsValue, ::wasm_bindgen::JsValue> {
+                self.list().await
+            }
+            async fn set(
+                &self,
+                id: String,
+                value: $ty,
+            ) -> Result<::wasm_bindgen::JsValue, ::wasm_bindgen::JsValue> {
+                self.set(id, value).await
+            }
+            async fn remove(
+                &self,
+                id: String,
+            ) -> Result<::wasm_bindgen::JsValue, ::wasm_bindgen::JsValue> {
+                self.remove(id).await
+            }
+        }
+
         impl $name {
             pub fn into_channel_impl(
                 self,
             ) -> ::std::sync::Arc<impl bitwarden_state::repository::Repository<$ty>> {
-                use ::bitwarden_state::repository::*;
-                use $crate::platform::repository::__macro_internal::*;
-
-                struct Store(::bitwarden_threading::ThreadBoundRunner<$name>);
-                let store = Store(::bitwarden_threading::ThreadBoundRunner::new(self));
-
-                #[async_trait::async_trait]
-                impl Repository<$ty> for Store {
-                    async fn get(&self, id: String) -> Result<Option<$ty>, RepositoryError> {
-                        run_convert(&self.0, |s| async move { s.get(id).await }).await
-                    }
-                    async fn list(&self) -> Result<Vec<$ty>, RepositoryError> {
-                        run_convert(&self.0, |s| async move { s.list().await }).await
-                    }
-                    async fn set(&self, id: String, value: $ty) -> Result<(), RepositoryError> {
-                        run_convert(&self.0, |s| async move { s.set(id, value).await.and(UNIT) })
-                            .await
-                    }
-                    async fn remove(&self, id: String) -> Result<(), RepositoryError> {
-                        run_convert(&self.0, |s| async move { s.remove(id).await.and(UNIT) }).await
-                    }
-                }
-
-                ::std::sync::Arc::new(store)
+                use $crate::platform::repository::WasmRepositoryChannel;
+                ::std::sync::Arc::new(WasmRepositoryChannel::new(self))
             }
         }
     };
 }
-pub(super) use create_wasm_repository;
-
-/// Some utilities to handle the conversion of JsValue to Rust types.
-/// They exist outside the macro to try to reduce code bloat in the generated code.
-#[doc(hidden)]
-pub mod __macro_internal {
-    use std::{future::Future, rc::Rc};
+pub(crate) use create_wasm_repository;
 
-    use bitwarden_state::repository::RepositoryError;
-    use wasm_bindgen::JsValue;
+const UNIT: Result<JsValue, JsValue> = Ok(JsValue::UNDEFINED);
 
-    pub const UNIT: Result<JsValue, JsValue> = Ok(JsValue::UNDEFINED);
-
-    pub async fn run_convert<T: 'static, Func, Fut, Ret>(
-        runner: &::bitwarden_threading::ThreadBoundRunner<T>,
-        f: Func,
-    ) -> Result<Ret, RepositoryError>
-    where
-        Func: FnOnce(Rc<T>) -> Fut + Send + 'static,
-        Fut: Future<Output = Result<JsValue, JsValue>>,
-        Ret: serde::de::DeserializeOwned + Send + Sync + 'static,
-    {
-        runner
-            .run_in_thread(|state| async move { convert_result(f(state).await) })
-            .await
-            .expect("Task should not panic")
-    }
+/// Utility function that runs a closure in a thread-bound manner, and converts the Result from
+/// [Result<JsValue, JsValue>] to a typed [Result<T, RepositoryError>].
+async fn run_convert<T: 'static, Func, Fut, Ret>(
+    runner: &::bitwarden_threading::ThreadBoundRunner<T>,
+    f: Func,
+) -> Result<Ret, RepositoryError>
+where
+    Func: FnOnce(Rc<T>) -> Fut + Send + 'static,
+    Fut: Future<Output = Result<JsValue, JsValue>>,
+    Ret: serde::de::DeserializeOwned + Send + Sync + 'static,
+{
+    runner
+        .run_in_thread(|state| async move { convert_result(f(state).await) })
+        .await
+        .expect("Task should not panic")
+}
 
-    fn convert_result<T: serde::de::DeserializeOwned>(
-        result: Result<JsValue, JsValue>,
-    ) -> Result<T, RepositoryError> {
-        result
-            .map_err(|e| RepositoryError::Internal(format!("{e:?}")))
-            .and_then(|value| {
-                ::tsify_next::serde_wasm_bindgen::from_value(value)
-                    .map_err(|e| RepositoryError::Internal(e.to_string()))
-            })
-    }
+/// Converts a [Result<JsValue, JsValue>] to a typed [Result<T, RepositoryError>] using
+/// [tsify_next::serde_wasm_bindgen]
+fn convert_result<T: serde::de::DeserializeOwned>(
+    result: Result<JsValue, JsValue>,
+) -> Result<T, RepositoryError> {
+    result
+        .map_err(|e| RepositoryError::Internal(format!("{e:?}")))
+        .and_then(|value| {
+            ::tsify_next::serde_wasm_bindgen::from_value(value)
+                .map_err(|e| RepositoryError::Internal(e.to_string()))
+        })
 }