diff --git a/crates/bitwarden-core/src/platform/state_client.rs b/crates/bitwarden-core/src/platform/state_client.rs index d48d3ea108..1e755f3ccd 100644 --- a/crates/bitwarden-core/src/platform/state_client.rs +++ b/crates/bitwarden-core/src/platform/state_client.rs @@ -1,7 +1,7 @@ use std::sync::Arc; use bitwarden_state::{ - Key, Setting, SettingItem, SettingsError, + Key, Persist, Setting, SettingsError, registry::StateRegistryError, repository::{Repository, RepositoryItem}, }; @@ -61,8 +61,7 @@ impl StateClient { /// # Ok(()) /// # } /// ``` - pub fn setting(&self, key: Key) -> Result, SettingsError> { - let repository = self.client.internal.state_registry.get::()?; - Ok(Setting::new(repository, key)) + pub fn setting(&self, key: Key) -> Result, SettingsError> { + Ok(self.client.internal.state_registry.setting(key)?) } } diff --git a/crates/bitwarden-send/src/delete.rs b/crates/bitwarden-send/src/delete.rs index 0fb772d057..0d8289ef23 100644 --- a/crates/bitwarden-send/src/delete.rs +++ b/crates/bitwarden-send/src/delete.rs @@ -146,17 +146,14 @@ mod tests { use bitwarden_api_api::apis::ApiClient; use bitwarden_core::key_management::{KeySlotIds, SymmetricKeySlotId}; use bitwarden_crypto::{KeyStore, SymmetricKeyAlgorithm}; - use bitwarden_state::SettingItem; - use bitwarden_test::MemoryRepository; + use bitwarden_test::{MemoryRepository, MemorySetting}; use uuid::uuid; use super::*; use crate::{AuthType, Send, SendId, SendTextView, SendType, SendView}; fn make_pending_setting() -> Setting> { - let repository: Arc> = - Arc::new(MemoryRepository::::default()); - Setting::new(repository, PENDING_SEND_DELETIONS) + MemorySetting::create() } async fn make_store_with_send( diff --git a/crates/bitwarden-state/src/lib.rs b/crates/bitwarden-state/src/lib.rs index 52af0e60a1..2e55173076 100644 --- a/crates/bitwarden-state/src/lib.rs +++ b/crates/bitwarden-state/src/lib.rs @@ -9,11 +9,13 @@ pub mod repository; /// This module provides a registry for managing repositories of different types. pub mod registry; -/// Type-safe settings repository for storing application configuration and state. +/// Type-safe settings API for storing application configuration and state. pub mod settings; pub(crate) mod any_map; +pub(crate) mod persist; pub(crate) mod sdk_managed; +pub use persist::Persist; pub use sdk_managed::{DatabaseConfiguration, DatabaseError}; -pub use settings::{Key, Setting, SettingItem, SettingsError}; +pub use settings::{Key, Setting, SettingItem, SettingTrait, SettingsError}; diff --git a/crates/bitwarden-state/src/persist.rs b/crates/bitwarden-state/src/persist.rs new file mode 100644 index 0000000000..8c1c082ad7 --- /dev/null +++ b/crates/bitwarden-state/src/persist.rs @@ -0,0 +1,10 @@ +use serde::{Serialize, de::DeserializeOwned}; + +/// A value that can be persisted to SDK-managed storage. +/// +/// This exists purely as a shorthand for the bounds every stored value must satisfy, so they +/// don't have to be repeated at every use site. It carries no behavior and is implemented +/// automatically for any type that meets them. +pub trait Persist: Serialize + DeserializeOwned + Send + Sync + 'static {} + +impl Persist for T {} diff --git a/crates/bitwarden-state/src/registry.rs b/crates/bitwarden-state/src/registry.rs index 68c934d3ac..73b7d5ce01 100644 --- a/crates/bitwarden-state/src/registry.rs +++ b/crates/bitwarden-state/src/registry.rs @@ -5,9 +5,10 @@ use thiserror::Error; use crate::{ any_map::AnyMap, + persist::Persist, repository::{Repository, RepositoryItem, RepositoryMigrations}, sdk_managed::{Database, DatabaseConfiguration, DatabaseError, MemoryDatabase, SystemDatabase}, - settings::{Key, Setting, SettingItem}, + settings::{Key, Setting}, }; /// A registry that contains repositories for different types of items. @@ -56,9 +57,8 @@ impl StateRegistry { } /// Get a handle to a setting by its type-safe key. - pub fn setting(&self, key: Key) -> Result, StateRegistryError> { - let repo = self.get::()?; - Ok(Setting::new(repo, key)) + pub fn setting(&self, key: Key) -> Result, StateRegistryError> { + Ok(Setting::new(self.database.get_setting::(key.name))) } /// Registers a client-managed repository into the map, associating it with its type. @@ -301,6 +301,79 @@ mod tests { assert_eq!(setting.get().await.unwrap(), None::); } + #[tokio::test] + async fn test_settings_are_isolated_by_key() { + use crate::register_setting_key; + register_setting_key!(const THEME: String = "test_theme"); + register_setting_key!(const LOCALE: String = "test_locale"); + + let registry = StateRegistry::new_with_memory_db(); + let theme = registry.setting(THEME).unwrap(); + let locale = registry.setting(LOCALE).unwrap(); + + theme.update("dark".to_string()).await.unwrap(); + locale.update("en-US".to_string()).await.unwrap(); + + theme.delete().await.unwrap(); + assert_eq!(theme.get().await.unwrap(), None::); + assert_eq!(locale.get().await.unwrap(), Some("en-US".to_string())); + } + + #[tokio::test] + async fn test_setting_is_stored_in_the_setting_table_as_bare_json() { + use crate::{register_setting_key, settings::SettingItem}; + + #[derive(Debug, PartialEq, serde::Serialize, serde::Deserialize)] + struct Config { + theme: String, + } + register_setting_key!(const CONFIG: Config = "test_config"); + + let registry = StateRegistry::new_with_memory_db(); + let value = Config { + theme: "dark".to_string(), + }; + let expected = serde_json::to_value(&value).unwrap(); + registry + .setting(CONFIG) + .unwrap() + .update(value) + .await + .unwrap(); + + // Storage contract for existing databases: settings live in the `Setting` table, + // addressed by key name, holding the bare serialized value. + assert_eq!(SettingItem::NAME, "Setting"); + let raw: SettingItem = registry + .database + .get::("test_config") + .await + .unwrap() + .expect("setting is present"); + assert_eq!(raw.0, expected); + } + + #[tokio::test] + async fn test_setting_reports_closed_after_wipe() { + use crate::{register_setting_key, settings::SettingsError}; + register_setting_key!(const TEST_SETTING: String = "test_wiped_setting"); + + let registry = StateRegistry::new_with_memory_db(); + let setting = registry.setting(TEST_SETTING).unwrap(); + setting.update("hello".to_string()).await.unwrap(); + + registry.wipe().await.unwrap(); + + assert!(matches!( + setting.get().await, + Err(SettingsError::Database(DatabaseError::Closed)) + )); + assert!(matches!( + setting.update("bye".to_string()).await, + Err(SettingsError::Database(DatabaseError::Closed)) + )); + } + /// The concrete implementation is erased by the coercion to `Arc>`, so two /// implementations of the same item type share a slot and the later registration wins. #[tokio::test] diff --git a/crates/bitwarden-state/src/repository.rs b/crates/bitwarden-state/src/repository.rs index d7a6f055f2..4506f99aec 100644 --- a/crates/bitwarden-state/src/repository.rs +++ b/crates/bitwarden-state/src/repository.rs @@ -1,8 +1,6 @@ use std::{any::TypeId, sync::Arc}; -use serde::{Serialize, de::DeserializeOwned}; - -use crate::registry::StateRegistryError; +use crate::{persist::Persist, registry::StateRegistryError}; /// An error resulting from operations on a repository. #[derive(thiserror::Error, Debug)] @@ -72,9 +70,9 @@ pub trait Repository: Send + Sync { /// It should not be implemented manually; instead, users should /// use the [crate::register_repository_item] macro to register their item types. /// -/// All repository items must implement `Serialize` and `DeserializeOwned` to support -/// SDK-managed repositories that persist items to storage. -pub trait RepositoryItem: Internal + Serialize + DeserializeOwned + Send + Sync + 'static { +/// The [`Persist`] bound is what makes an item storable by SDK-managed repositories. It is +/// satisfied automatically by any type that is serializable and thread-safe. +pub trait RepositoryItem: Internal + Persist { /// The name of the type implementing this trait. const NAME: &'static str; diff --git a/crates/bitwarden-state/src/sdk_managed/mod.rs b/crates/bitwarden-state/src/sdk_managed/mod.rs index d2bd500981..4374a5ebb4 100644 --- a/crates/bitwarden-state/src/sdk_managed/mod.rs +++ b/crates/bitwarden-state/src/sdk_managed/mod.rs @@ -3,7 +3,11 @@ use std::sync::Arc; use bitwarden_error::bitwarden_error; use thiserror::Error; -use crate::repository::{Repository, RepositoryError, RepositoryItem, RepositoryMigrations}; +use crate::{ + persist::Persist, + repository::{Repository, RepositoryError, RepositoryItem, RepositoryMigrations}, + settings::{SettingItem, SettingTrait, SettingsError}, +}; mod configuration; pub use configuration::DatabaseConfiguration; @@ -242,6 +246,32 @@ impl Repository for DBRepository { } } +/// Stores a single setting in the `Setting` table, keyed by `name`, as the bare serialized value. +struct DBSetting { + database: SystemDatabase, + name: &'static str, + _marker: std::marker::PhantomData, +} + +#[async_trait::async_trait] +impl SettingTrait for DBSetting { + async fn get(&self) -> Result, SettingsError> { + match self.database.get::(self.name).await? { + Some(item) => Ok(Some(serde_json::from_value::(item.0)?)), + None => Ok(None), + } + } + + async fn set(&self, value: T) -> Result<(), SettingsError> { + let item = SettingItem(serde_json::to_value(&value)?); + Ok(self.database.set::(self.name, item).await?) + } + + async fn remove(&self) -> Result<(), SettingsError> { + Ok(self.database.remove::(self.name).await?) + } +} + impl SystemDatabase { pub(super) fn get_repository(&self) -> Arc> { Arc::new(DBRepository { @@ -249,4 +279,12 @@ impl SystemDatabase { _marker: std::marker::PhantomData, }) } + + pub(super) fn get_setting(&self, name: &'static str) -> Arc> { + Arc::new(DBSetting { + database: self.clone(), + name, + _marker: std::marker::PhantomData, + }) + } } diff --git a/crates/bitwarden-state/src/settings/mod.rs b/crates/bitwarden-state/src/settings/mod.rs index a6d908080a..f3ea86c4b4 100644 --- a/crates/bitwarden-state/src/settings/mod.rs +++ b/crates/bitwarden-state/src/settings/mod.rs @@ -1,7 +1,7 @@ -//! Type-safe settings repository for storing application configuration and state. +//! Type-safe settings API for storing application configuration and state. //! -//! This module provides a type-safe key-value API for storing settings, backed by -//! the SDK's repository pattern. +//! This module provides a type-safe key-value API for storing settings. Each setting resolves to +//! its own backend, defaulting to the SDK-managed database. //! //! # Usage //! @@ -44,4 +44,4 @@ mod key; mod setting; pub use key::Key; -pub use setting::{Setting, SettingItem, SettingsError}; +pub use setting::{Setting, SettingItem, SettingTrait, SettingsError}; diff --git a/crates/bitwarden-state/src/settings/setting.rs b/crates/bitwarden-state/src/settings/setting.rs index ba5bf50849..9a503993d2 100644 --- a/crates/bitwarden-state/src/settings/setting.rs +++ b/crates/bitwarden-state/src/settings/setting.rs @@ -5,13 +5,9 @@ use std::sync::Arc; use serde::{Deserialize, Serialize}; use thiserror::Error; -use super::Key; -use crate::{ - registry::StateRegistryError, - repository::{Repository, RepositoryError}, -}; +use crate::{persist::Persist, registry::StateRegistryError, sdk_managed::DatabaseError}; -/// Internal setting value stored in the settings repository. +/// Internal setting value as stored in the SDK-managed database. /// /// This type wraps a JSON value for flexible storage. Users should not work with /// this type directly - use the [`Setting`] handle via `StateClient::setting()` instead, @@ -20,9 +16,16 @@ use crate::{ #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct SettingItem(pub(crate) serde_json::Value); -// Register SettingItem for repository usage crate::register_repository_item!(String => SettingItem, "Setting"); +#[doc(hidden)] +#[async_trait::async_trait] +pub trait SettingTrait: Send + Sync { + async fn get(&self) -> Result, SettingsError>; + async fn set(&self, value: T) -> Result<(), SettingsError>; + async fn remove(&self) -> Result<(), SettingsError>; +} + /// A handle to a single setting value in storage. /// /// This type provides async methods to get, update, and delete the setting value. @@ -46,15 +49,16 @@ crate::register_repository_item!(String => SettingItem, "Setting"); /// setting.delete().await?; /// ``` #[derive(Clone)] -pub struct Setting { - repository: Arc>, - key: Key, +pub struct Setting { + backend: Arc>, } -impl Setting { - /// Create a new setting handle from a repository and key. - pub fn new(repository: Arc>, key: Key) -> Self { - Self { repository, key } +impl Setting { + /// Create a new setting handle from a backend. + /// + /// The backend is already bound to a single key, so it decides where the value is stored. + pub fn new(backend: Arc>) -> Self { + Self { backend } } /// Get the current value of this setting. @@ -67,34 +71,18 @@ impl Setting { /// - Schema evolution problems (type definition changed) /// - Data corruption /// - Type mismatch (wrong `Key` type for stored data) - pub async fn get(&self) -> Result, SettingsError> - where - T: for<'de> Deserialize<'de>, - { - match self.repository.get(self.key.name.to_string()).await? { - Some(item) => Ok(Some(serde_json::from_value::(item.0)?)), - None => Ok(None), - } + pub async fn get(&self) -> Result, SettingsError> { + self.backend.get().await } /// Update (or create) this setting with a new value. - pub async fn update(&self, value: T) -> Result<(), SettingsError> - where - T: Serialize, - { - let json_value = serde_json::to_value(&value)?; - let item = SettingItem(json_value); - - self.repository.set(self.key.name.to_string(), item).await?; - - Ok(()) + pub async fn update(&self, value: T) -> Result<(), SettingsError> { + self.backend.set(value).await } /// Delete this setting from storage. pub async fn delete(&self) -> Result<(), SettingsError> { - self.repository.remove(self.key.name.to_string()).await?; - - Ok(()) + self.backend.remove().await } } @@ -104,9 +92,9 @@ pub enum SettingsError { /// Failed to serialize/deserialize setting value #[error("Failed to serialize/deserialize setting: {0}")] Json(#[from] serde_json::Error), - /// Repository operation failed + /// Database operation failed #[error(transparent)] - Repository(#[from] RepositoryError), + Database(#[from] DatabaseError), /// State registry operation failed #[error(transparent)] Registry(#[from] StateRegistryError), diff --git a/crates/bitwarden-sync/src/sync_client.rs b/crates/bitwarden-sync/src/sync_client.rs index 306b1a983a..3111a2f3b5 100644 --- a/crates/bitwarden-sync/src/sync_client.rs +++ b/crates/bitwarden-sync/src/sync_client.rs @@ -323,17 +323,13 @@ mod tests { /// Helper to create a SyncClient with a state-backed last_sync setting. /// - /// If `stored_last_sync` is `Some`, it is written into the in-memory repository so + /// If `stored_last_sync` is `Some`, it is written into the in-memory setting so /// that subsequent calls to `needs_sync` see it. async fn test_client_with_last_sync( api_client: bitwarden_api_api::apis::ApiClient, stored_last_sync: Option>, ) -> SyncClient { - let repo: Arc> = - Arc::new(bitwarden_test::MemoryRepository::< - bitwarden_state::SettingItem, - >::default()); - let setting = bitwarden_state::Setting::new(repo, crate::state::LAST_SYNC); + let setting = bitwarden_test::MemorySetting::create(); if let Some(dt) = stored_last_sync { setting.update(dt).await.expect("pre-populate last_sync"); } @@ -614,11 +610,7 @@ mod tests { let error_log_clone = error_log.clone(); // Build Setting manually so we can inspect it after sync. - let repo: Arc> = - Arc::new(bitwarden_test::MemoryRepository::< - bitwarden_state::SettingItem, - >::default()); - let setting = bitwarden_state::Setting::new(repo, crate::state::LAST_SYNC); + let setting = bitwarden_test::MemorySetting::create(); setting .update(stored_last_sync) .await diff --git a/crates/bitwarden-test/src/lib.rs b/crates/bitwarden-test/src/lib.rs index 35f9134bba..a34f25eb12 100644 --- a/crates/bitwarden-test/src/lib.rs +++ b/crates/bitwarden-test/src/lib.rs @@ -6,4 +6,7 @@ pub use api::*; mod repository; pub use repository::*; +mod setting; +pub use setting::*; + pub mod play; diff --git a/crates/bitwarden-test/src/setting.rs b/crates/bitwarden-test/src/setting.rs new file mode 100644 index 0000000000..c98be5c8bc --- /dev/null +++ b/crates/bitwarden-test/src/setting.rs @@ -0,0 +1,37 @@ +use std::sync::{Arc, Mutex}; + +use bitwarden_state::{Persist, Setting, SettingTrait, SettingsError}; + +/// A simple in-memory setting backend. The data is only stored in memory and will not persist +/// beyond the lifetime of the backend instance. +/// +/// Primary use case is for unit and integration tests. +pub struct MemorySetting { + value: Mutex>, +} + +impl MemorySetting { + /// Create a setting handle backed by a fresh in-memory store. + pub fn create() -> Setting { + Setting::new(Arc::new(Self { + value: Mutex::new(None), + })) + } +} + +#[async_trait::async_trait] +impl SettingTrait for MemorySetting { + async fn get(&self) -> Result, SettingsError> { + Ok(self.value.lock().expect("Mutex is not poisoned").clone()) + } + + async fn set(&self, value: T) -> Result<(), SettingsError> { + *self.value.lock().expect("Mutex is not poisoned") = Some(value); + Ok(()) + } + + async fn remove(&self) -> Result<(), SettingsError> { + *self.value.lock().expect("Mutex is not poisoned") = None; + Ok(()) + } +}