Remove inventory registration, doesn't work on wasm :sadface:

Author: dani-garcia

Cargo.lock

@@ -1,9 +1,6 @@
 use std::sync::Arc;
 
-use bitwarden_state::{
-    registry::StateRegistryError,
-    repository::{Repository, RepositoryItem},
-};
+use bitwarden_state::repository::{Repository, RepositoryItem};
 
 use crate::Client;
 
@@ -17,7 +14,7 @@ impl StateClient {
     pub fn register_client_managed<T: 'static + Repository<V>, V: RepositoryItem>(
         &self,
         store: Arc<T>,
-    ) -> Result<(), StateRegistryError> {
+    ) {
         self.client
             .internal
             .repository_map

crates/bitwarden-core/src/platform/state_client.rs

@@ -16,7 +16,6 @@ wasm = []
 [dependencies]
 async-trait = { workspace = true }
 bitwarden-error = { workspace = true }
-inventory = ">=0.3.20, <0.4"
 log = { workspace = true }
 thiserror = { workspace = true }
 

crates/bitwarden-state/Cargo.toml

@@ -4,10 +4,6 @@ use std::{
     sync::{Arc, RwLock},
 };
 
-use bitwarden_error::bitwarden_error;
-use thiserror::Error;
-
-use super::repository::RepositoryItemRegistration;
 use crate::repository::{Repository, RepositoryItem};
 
 /// A registry that contains repositories for different types of items.
@@ -22,17 +18,6 @@ impl std::fmt::Debug for StateRegistry {
     }
 }
 
-#[allow(missing_docs)]
-#[bitwarden_error(flat)]
-#[derive(Debug, Error)]
-pub enum StateRegistryError {
-    #[error("Repository for type {0} is not registered as client-managed")]
-    RepositoryNotClientManaged(&'static str),
-
-    #[error("Not all client-managed repositories are registered")]
-    NotAllRepositoriesRegistered,
-}
-
 impl StateRegistry {
     /// Creates a new empty `StateRegistry`.
     #[allow(clippy::new_without_default)]
@@ -43,29 +28,11 @@ impl StateRegistry {
     }
 
     /// Registers a client-managed repository into the map, associating it with its type.
-    pub fn register_client_managed<T: RepositoryItem>(
-        &self,
-        value: Arc<dyn Repository<T>>,
-    ) -> Result<(), StateRegistryError> {
-        let mut possible_registrations = RepositoryItemRegistration::iter();
-        match possible_registrations.find(|reg| reg.type_id() == TypeId::of::<T>()) {
-            Some(reg) => {
-                if !reg.rtype.is_client_managed() {
-                    return Err(StateRegistryError::RepositoryNotClientManaged(reg.name));
-                }
-            }
-            // This should never happen, as we have tests to ensure all repositories are registered.
-            _ => {
-                return Err(StateRegistryError::NotAllRepositoriesRegistered);
-            }
-        }
-
+    pub fn register_client_managed<T: RepositoryItem>(&self, value: Arc<dyn Repository<T>>) {
         self.client_managed
             .write()
             .expect("RwLock should not be poisoned")
             .insert(TypeId::of::<T>(), Box::new(value));
-
-        Ok(())
     }
 
     /// Retrieves a client-managed repository from the map given its type.
@@ -77,34 +44,6 @@ impl StateRegistry {
             .and_then(|boxed| boxed.downcast_ref::<Arc<dyn Repository<T>>>())
             .map(Arc::clone)
     }
-
-    /// Validates that all repositories registered in the client-managed state registry.
-    /// This should only be called after all the repositories have been registered by the clients.
-    pub fn validate_repositories(&self) -> Result<(), StateRegistryError> {
-        let possible_registrations = RepositoryItemRegistration::iter();
-        let mut missing_repository = false;
-
-        let client_managed = self
-            .client_managed
-            .read()
-            .expect("RwLock should not be poisoned");
-
-        for reg in possible_registrations {
-            if reg.rtype.is_client_managed() && !client_managed.contains_key(&reg.type_id()) {
-                log::error!(
-                    "Repository for type {} is not registered in the client-managed state registry",
-                    reg.name
-                );
-                missing_repository = true;
-            }
-        }
-
-        if missing_repository {
-            return Err(StateRegistryError::NotAllRepositoriesRegistered);
-        }
-
-        Ok(())
-    }
 }
 
 #[cfg(test)]
@@ -144,9 +83,9 @@ mod tests {
     #[derive(PartialEq, Eq, Debug)]
     struct TestItem<T>(T);
 
-    register_repository_item!(TestItem<usize>, "TestItem<usize>", ClientManaged);
-    register_repository_item!(TestItem<String>, "TestItem<String>", ClientManaged);
-    register_repository_item!(TestItem<Vec<u8>>, "TestItem<Vec<u8>>", ClientManaged);
+    register_repository_item!(TestItem<usize>, "TestItem<usize>");
+    register_repository_item!(TestItem<String>, "TestItem<String>");
+    register_repository_item!(TestItem<Vec<u8>>, "TestItem<Vec<u8>>");
 
     impl_repository!(TestA, TestItem<usize>);
     impl_repository!(TestB, TestItem<String>);
@@ -172,17 +111,17 @@ mod tests {
         assert!(map.get_client_managed::<TestItem<String>>().is_none());
         assert!(map.get_client_managed::<TestItem<Vec<u8>>>().is_none());
 
-        map.register_client_managed(a.clone()).unwrap();
+        map.register_client_managed(a.clone());
         assert_eq!(get(&map).await, Some(TestItem(a.0)));
         assert!(map.get_client_managed::<TestItem<String>>().is_none());
         assert!(map.get_client_managed::<TestItem<Vec<u8>>>().is_none());
 
-        map.register_client_managed(b.clone()).unwrap();
+        map.register_client_managed(b.clone());
         assert_eq!(get(&map).await, Some(TestItem(a.0)));
         assert_eq!(get(&map).await, Some(TestItem(b.0.clone())));
         assert!(map.get_client_managed::<TestItem<Vec<u8>>>().is_none());
 
-        map.register_client_managed(c.clone()).unwrap();
+        map.register_client_managed(c.clone());
         assert_eq!(get(&map).await, Some(TestItem(a.0)));
         assert_eq!(get(&map).await, Some(TestItem(b.0.clone())));
         assert_eq!(get(&map).await, Some(TestItem(c.0.clone())));

crates/bitwarden-state/src/registry.rs

@@ -35,128 +35,25 @@ pub trait RepositoryItem: Internal + Send + Sync + 'static {
 }
 
 /// Register a type for use in a repository. The type must only be registered once in the crate
-/// where it's defined. The provided name must be unique and not be changed. Through the use of the
-/// `inventory` crate, this macro will register the type globally and
-/// [test_utils::verify_no_duplicate_registrations] will ensure that no duplicate registrations
-/// exist.
+/// where it's defined. The provided name must be unique and not be changed.
 #[macro_export]
 macro_rules! register_repository_item {
-    ($ty:ty, $name:literal, $type:expr) => {
+    ($ty:ty, $name:literal) => {
         const _: () = {
             impl $crate::repository::___internal::Internal for $ty {}
             impl $crate::repository::RepositoryItem for $ty {
                 const NAME: &'static str = $name;
             }
-            use $crate::repository::___internal::RepositoryItemRegistrationType::*;
-            $crate::repository::___internal::submit! {
-                $crate::repository::___internal::RepositoryItemRegistration::new::<$ty>($name, $type)
-            }
         };
     };
 }
 
-/// This code is used to register types that can be stored in a repository.
-/// It's not meant to be used directly, users of this crate should use the
+/// This code is not meant to be used directly, users of this crate should use the
 /// [register_repository_item] macro to register their types.
 #[doc(hidden)]
 pub mod ___internal {
-    use super::*;
 
     // This trait is just to try to discourage users from implementing `RepositoryItem` directly.
     pub trait Internal {}
-
-    /// This enum indicated what kind of repository registration is being performed.
-    #[derive(Debug, Clone, Copy, PartialEq, Eq)]
-    pub enum RepositoryItemRegistrationType {
-        ClientManaged,
-        SdkManaged,
-        Both,
-    }
-
-    impl RepositoryItemRegistrationType {
-        pub fn is_client_managed(&self) -> bool {
-            matches!(self, Self::ClientManaged | Self::Both)
-        }
-        pub fn is_sdk_managed(&self) -> bool {
-            matches!(self, Self::SdkManaged | Self::Both)
-        }
-    }
-
-    #[derive(Debug)]
-    pub struct RepositoryItemRegistration {
-        pub(crate) name: &'static str,
-        /// The type identifier of the repository item. We're using a function pointer because
-        /// [TypeId::of] is not const stable.
-        pub(crate) type_id: fn() -> TypeId,
-        pub(crate) rtype: RepositoryItemRegistrationType,
-    }
-
-    impl RepositoryItemRegistration {
-        /// Creates a new `RepositoryRegistration` for the given type.
-        pub const fn new<T: RepositoryItem>(
-            name: &'static str,
-            rtype: RepositoryItemRegistrationType,
-        ) -> Self {
-            Self {
-                name,
-                type_id: || T::type_id(),
-                rtype,
-            }
-        }
-
-        pub(crate) fn iter() -> impl Iterator<Item = &'static RepositoryItemRegistration> {
-            inventory::iter::<RepositoryItemRegistration>()
-        }
-    }
-
-    inventory::collect!(RepositoryItemRegistration);
-    pub use inventory::submit;
-}
-pub(crate) use ___internal::{Internal, RepositoryItemRegistration};
-
-#[cfg(test)]
-mod tests {
-    use super::*;
-
-    struct TestItem;
-
-    register_repository_item!(TestItem, "TestItem", SdkManaged);
-
-    #[test]
-    fn test_repository_item_registration() {
-        let registered: Vec<_> = RepositoryItemRegistration::iter().collect();
-        // We can't really test the exact number, as they might be registered in different crates.
-        assert!(!registered.is_empty(), "No repository items registered");
-    }
-
-    #[test]
-    pub fn verify_no_duplicate_registrations() {
-        crate::repository::test_utils::verify_no_duplicate_registrations();
-    }
-}
-
-#[doc(hidden)]
-pub mod test_utils {
-    /// Verify that no duplicate repository item registrations exist.
-    /// This needs to be called in the final SDK crates (WASM, UniFFI, CLI, ...) to ensure that all
-    /// registrations are checked.
-    pub fn verify_no_duplicate_registrations() {
-        use crate::repository::___internal::RepositoryItemRegistration;
-
-        let mut seen_names = std::collections::HashSet::new();
-        let mut seen_ids = std::collections::HashSet::new();
-
-        for reg in RepositoryItemRegistration::iter() {
-            assert!(
-                seen_names.insert(reg.name),
-                "Duplicate repository registration name: {}",
-                reg.name
-            );
-            assert!(
-                seen_ids.insert((reg.type_id)()),
-                "Duplicate repository registration id: {}",
-                reg.name
-            );
-        }
-    }
 }
+pub(crate) use ___internal::Internal;

crates/bitwarden-state/src/repository.rs

@@ -64,9 +64,6 @@ pub enum Error {
     #[error(transparent)]
     Crypto(#[from] bitwarden_crypto::CryptoError),
 
-    #[error(transparent)]
-    StateRegistry(#[from] bitwarden_state::registry::StateRegistryError),
-
     // Generators
     #[error(transparent)]
     Username(#[from] UsernameError),

crates/bitwarden-uniffi/src/error.rs

@@ -56,13 +56,11 @@ repository::create_uniffi_repository!(CipherRepository, Cipher);
 
 #[uniffi::export]
 impl StateClient {
-    pub fn register_cipher_repository(&self, store: Arc<dyn CipherRepository>) -> Result<()> {
+    pub fn register_cipher_repository(&self, store: Arc<dyn CipherRepository>) {
         let store_internal = UniffiRepositoryBridge::new(store);
         self.0
             .platform()
             .state()
             .register_client_managed(store_internal)
-            .map_err(Error::StateRegistry)?;
-        Ok(())
     }
 }

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

@@ -100,11 +100,3 @@ macro_rules! create_uniffi_repository {
     };
 }
 pub(super) use create_uniffi_repository;
-
-#[cfg(test)]
-mod tests {
-    #[test]
-    pub fn verify_no_duplicate_registrations() {
-        bitwarden_state::repository::test_utils::verify_no_duplicate_registrations();
-    }
-}

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

@@ -121,7 +121,7 @@ pub struct Cipher {
     pub revision_date: DateTime<Utc>,
 }
 
-bitwarden_state::register_repository_item!(Cipher, "Cipher", ClientManaged);
+bitwarden_state::register_repository_item!(Cipher, "Cipher");
 
 #[allow(missing_docs)]
 #[derive(Serialize, Deserialize, Debug, Clone)]

crates/bitwarden-vault/src/cipher/cipher.rs

@@ -26,19 +26,6 @@ pub fn set_log_level(level: LogLevel) {
     set_max_level(log_level.to_level_filter());
 }
 
-// This is needed to ensure the linker doesn't optimize away the constructors used by the inventory
-// crate. https://docs.rs/inventory/0.3.20/inventory/index.html#webassembly-and-constructors
-unsafe extern "C" {
-    fn __wasm_call_ctors();
-}
-
-#[wasm_bindgen(start)]
-fn start() {
-    unsafe {
-        __wasm_call_ctors();
-    }
-}
-
 #[allow(missing_docs)]
 #[wasm_bindgen]
 pub fn init_sdk(log_level: Option<LogLevel>) {