max / audiofiles
5 files changed,
+230 insertions,
-80 deletions
| @@ -470,6 +470,7 @@ dependencies = [ | |||
| 470 | 470 | "sha2 0.11.0", | |
| 471 | 471 | "stratum-dsp", | |
| 472 | 472 | "symphonia 0.6.0", | |
| 473 | + | "synckit-config", | |
| 473 | 474 | "tagtree", | |
| 474 | 475 | "tempfile", | |
| 475 | 476 | "thiserror 2.0.19", | |
| @@ -5363,6 +5364,7 @@ dependencies = [ | |||
| 5363 | 5364 | "serde", | |
| 5364 | 5365 | "serde_json", | |
| 5365 | 5366 | "sha2 0.11.0", | |
| 5367 | + | "synckit-config", | |
| 5366 | 5368 | "thiserror 2.0.19", | |
| 5367 | 5369 | "tokio", | |
| 5368 | 5370 | "tokio-stream", | |
| @@ -5375,6 +5377,14 @@ dependencies = [ | |||
| 5375 | 5377 | ] | |
| 5376 | 5378 | ||
| 5377 | 5379 | [[package]] | |
| 5380 | + | name = "synckit-config" | |
| 5381 | + | version = "0.1.2" | |
| 5382 | + | dependencies = [ | |
| 5383 | + | "rusqlite", | |
| 5384 | + | "thiserror 2.0.19", | |
| 5385 | + | ] | |
| 5386 | + | ||
| 5387 | + | [[package]] | |
| 5378 | 5388 | name = "synstructure" | |
| 5379 | 5389 | version = "0.13.2" | |
| 5380 | 5390 | source = "registry+https://github.com/rust-lang/crates.io-index" |
| @@ -101,34 +101,24 @@ impl ConfigBackend for DirectBackend { | |||
| 101 | 101 | ||
| 102 | 102 | fn get_config(&self, key: ConfigKey) -> BackendResult<Option<String>> { | |
| 103 | 103 | let db = self.db.lock(); | |
| 104 | - | let result = db | |
| 105 | - | .conn() | |
| 106 | - | .query_row( | |
| 107 | - | "SELECT value FROM user_config WHERE key = ?1", | |
| 108 | - | [key.as_str()], | |
| 109 | - | |row| row.get::<_, String>(0), | |
| 110 | - | ) | |
| 111 | - | .ok(); | |
| 112 | - | Ok(result) | |
| 104 | + | // A read error reads as "unset" the way the old direct query did, so a | |
| 105 | + | // transient DB hiccup falls back to the caller's default rather than | |
| 106 | + | // surfacing an error on a settings read. | |
| 107 | + | Ok(db.get_config(key).ok().flatten()) | |
| 113 | 108 | } | |
| 114 | 109 | ||
| 115 | 110 | #[instrument(skip(self, value))] | |
| 116 | 111 | fn set_config(&self, key: ConfigKey, value: &str) -> BackendResult<()> { | |
| 117 | 112 | let db = self.db.lock(); | |
| 118 | - | db.conn() | |
| 119 | - | .execute( | |
| 120 | - | "INSERT OR REPLACE INTO user_config (key, value) VALUES (?1, ?2)", | |
| 121 | - | rusqlite::params![key.as_str(), value], | |
| 122 | - | ) | |
| 123 | - | .map_err(audiofiles_core::error::CoreError::Db)?; | |
| 113 | + | db.set_config(key, value) | |
| 114 | + | .map_err(audiofiles_core::error::CoreError::from)?; | |
| 124 | 115 | Ok(()) | |
| 125 | 116 | } | |
| 126 | 117 | ||
| 127 | 118 | fn delete_config(&self, key: ConfigKey) -> BackendResult<()> { | |
| 128 | 119 | let db = self.db.lock(); | |
| 129 | - | db.conn() | |
| 130 | - | .execute("DELETE FROM user_config WHERE key = ?1", [key.as_str()]) | |
| 131 | - | .map_err(audiofiles_core::error::CoreError::Db)?; | |
| 120 | + | db.delete_config(key) | |
| 121 | + | .map_err(audiofiles_core::error::CoreError::from)?; | |
| 132 | 122 | Ok(()) | |
| 133 | 123 | } | |
| 134 | 124 |
| @@ -24,6 +24,7 @@ hound = { workspace = true } | |||
| 24 | 24 | tracing = { workspace = true } | |
| 25 | 25 | rayon = { workspace = true } | |
| 26 | 26 | tagtree = { workspace = true } | |
| 27 | + | synckit-config = { path = "../../../../synckit/synckit-config" } | |
| 27 | 28 | ||
| 28 | 29 | [dev-dependencies] | |
| 29 | 30 | tempfile = "3.25.0" |
| @@ -1,52 +1,53 @@ | |||
| 1 | 1 | //! Single source of truth for `user_config` keys and their sync posture. | |
| 2 | 2 | //! | |
| 3 | 3 | //! Every persisted `user_config` key is declared exactly once, here, together | |
| 4 | - | //! with whether it may cross the sync boundary. Two filters are generated from | |
| 5 | - | //! this one registry: | |
| 4 | + | //! with whether it may cross the sync boundary. One [`ConfigSpec`] is derived | |
| 5 | + | //! from this registry ([`CONFIG`]) and drives everything downstream: | |
| 6 | 6 | //! | |
| 7 | 7 | //! - the Rust import filter ([`key_excluded_from_sync`]), used when applying a | |
| 8 | 8 | //! remote pull batch, and | |
| 9 | - | //! - the SQL export filter (the `config_key_policy` table seeded by | |
| 10 | - | //! [`crate::db::Database`] and joined by the `user_config` sync triggers). | |
| 9 | + | //! - the SQL export filter (the `config_key_policy` table seeded from | |
| 10 | + | //! [`CONFIG`] by [`crate::db::Database`] and joined by the `user_config` sync | |
| 11 | + | //! triggers). | |
| 11 | 12 | //! | |
| 12 | - | //! The two hand-maintained denylists this replaces, a SQL trigger predicate in | |
| 13 | + | //! The two hand-maintained denylists this replaced, a SQL trigger predicate in | |
| 13 | 14 | //! `db.rs` and a Rust predicate in `audiofiles-sync`, had drifted apart and | |
| 14 | 15 | //! twice failed to cover device-local keys (`mirror_path`, `mirror_enabled`, | |
| 15 | 16 | //! `import_preflight_disabled`), letting a hostile server steer a local | |
| 16 | 17 | //! filesystem write root across the sync boundary (fuzz-2026-07-06 #2, | |
| 17 | 18 | //! fuzz-2026-07-20 #1, fuzz-2026-07-21 #3 CHRONIC). | |
| 18 | 19 | //! | |
| 19 | - | //! Because keys are declared through the [`config_keys!`] macro, a new key | |
| 20 | - | //! cannot be added without stating its posture, and a caller cannot name a key | |
| 21 | - | //! that is not in the registry: [`ConfigKey`] is the type the config accessors | |
| 22 | - | //! take, so an unclassified key does not compile. | |
| 23 | - | ||
| 24 | - | /// Whether a `user_config` key may be replicated to other devices. | |
| 25 | - | #[derive(Debug, Clone, Copy, PartialEq, Eq)] | |
| 26 | - | pub enum SyncPosture { | |
| 27 | - | /// Safe to sync, a user preference shared across the user's devices. | |
| 28 | - | Replicated, | |
| 29 | - | /// Must never leave the device, names a local filesystem path or gates a | |
| 30 | - | /// local safety check. A remote peer must not be able to set it. | |
| 31 | - | DeviceLocal, | |
| 32 | - | } | |
| 20 | + | //! Posture is [`synckit_config::Posture`], the family's shared vocabulary | |
| 21 | + | //! ([`Synced`]/[`Local`], the generalization of this crate's old | |
| 22 | + | //! `Replicated`/`DeviceLocal`). Storage and the posture policy come from the | |
| 23 | + | //! shared config store rather than being audiofiles' own; what stays local is | |
| 24 | + | //! the typed key façade, because the compile-time guarantee is worth keeping: | |
| 25 | + | //! keys are declared through the [`config_keys!`] macro, so a new key cannot be | |
| 26 | + | //! added without stating its posture, and a caller cannot name a key that is | |
| 27 | + | //! not in the registry. [`ConfigKey`] is the type the config accessors take, so | |
| 28 | + | //! an unclassified key does not compile, a stronger promise than the spec's | |
| 29 | + | //! runtime fail-closed default, kept on top of it. | |
| 30 | + | //! | |
| 31 | + | //! <!-- wiki: af-config --> | |
| 32 | + | ||
| 33 | + | use synckit_config::{ConfigSpec, Posture}; | |
| 33 | 34 | ||
| 34 | 35 | /// Declare the full set of `user_config` keys in one place. Each line binds an | |
| 35 | - | /// enum variant, its on-disk key string, and its [`SyncPosture`]; the macro | |
| 36 | - | /// derives the variant list, the string mapping (both directions), and the | |
| 37 | - | /// posture lookup so the three can never disagree. | |
| 36 | + | /// enum variant, its on-disk key string, and its [`Posture`]; the macro derives | |
| 37 | + | /// the variant list, the string mapping (both directions), the posture lookup, | |
| 38 | + | /// and the [`ConfigSpec`] so they can never disagree. | |
| 38 | 39 | macro_rules! config_keys { | |
| 39 | 40 | ($( $(#[$m:meta])* $variant:ident => $key:literal : $posture:ident ),+ $(,)?) => { | |
| 40 | 41 | /// A known `user_config` key. The config accessors take this rather than | |
| 41 | 42 | /// a `&str`, so adding a new key means adding a variant here, which in | |
| 42 | - | /// turn forces declaring its [`SyncPosture`]. | |
| 43 | + | /// turn forces declaring its [`Posture`]. | |
| 43 | 44 | #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] | |
| 44 | 45 | pub enum ConfigKey { | |
| 45 | 46 | $( $(#[$m])* $variant ),+ | |
| 46 | 47 | } | |
| 47 | 48 | ||
| 48 | 49 | impl ConfigKey { | |
| 49 | - | /// Every declared key, for seeding the SQL policy table. | |
| 50 | + | /// Every declared key, in declaration order. | |
| 50 | 51 | pub const ALL: &'static [ConfigKey] = &[ $( ConfigKey::$variant ),+ ]; | |
| 51 | 52 | ||
| 52 | 53 | /// The on-disk `user_config.key` string. | |
| @@ -57,8 +58,8 @@ macro_rules! config_keys { | |||
| 57 | 58 | ||
| 58 | 59 | /// This key's sync posture. | |
| 59 | 60 | #[must_use] | |
| 60 | - | pub const fn posture(self) -> SyncPosture { | |
| 61 | - | match self { $( ConfigKey::$variant => SyncPosture::$posture ),+ } | |
| 61 | + | pub const fn posture(self) -> Posture { | |
| 62 | + | match self { $( ConfigKey::$variant => Posture::$posture ),+ } | |
| 62 | 63 | } | |
| 63 | 64 | ||
| 64 | 65 | /// Resolve a raw key string (e.g. from a remote pull batch) to a | |
| @@ -71,46 +72,56 @@ macro_rules! config_keys { | |||
| 71 | 72 | } | |
| 72 | 73 | } | |
| 73 | 74 | } | |
| 75 | + | ||
| 76 | + | /// The config declaration the shared store reads: table name plus every | |
| 77 | + | /// key's posture. Derived from the same macro rows as [`ConfigKey`], so | |
| 78 | + | /// the typed façade and the spec cannot describe different key sets. | |
| 79 | + | /// | |
| 80 | + | /// The table is `user_config`, audiofiles' existing settings table; the | |
| 81 | + | /// store attaches to it rather than creating its own (see | |
| 82 | + | /// [`ConfigStore::attached`](synckit_config::ConfigStore::attached)). | |
| 83 | + | pub const CONFIG: ConfigSpec = | |
| 84 | + | ConfigSpec::new("user_config", &[ $( ($key, Posture::$posture) ),+ ]); | |
| 74 | 85 | }; | |
| 75 | 86 | } | |
| 76 | 87 | ||
| 77 | 88 | config_keys! { | |
| 78 | - | // --- Replicated: user preferences shared across the user's devices. --- | |
| 79 | - | Theme => "theme": Replicated, | |
| 80 | - | PreviewLoop => "preview_loop": Replicated, | |
| 81 | - | PreviewAutoplay => "preview_autoplay": Replicated, | |
| 82 | - | EditResultMode => "edit_result_mode": Replicated, | |
| 83 | - | ForgeAutoTrimOvershoot => "forge.auto_trim_overshoot": Replicated, | |
| 84 | - | CurrentVfsId => "current_vfs_id": Replicated, | |
| 85 | - | SidebarVisible => "sidebar_visible": Replicated, | |
| 86 | - | DetailVisible => "detail_visible": Replicated, | |
| 87 | - | FilterPanelOpen => "filter_panel_open": Replicated, | |
| 88 | - | VfsExplained => "vfs_explained": Replicated, | |
| 89 | - | HintsDismissed => "hints_dismissed": Replicated, | |
| 90 | - | SyncIntroDismissed => "sync_intro_dismissed": Replicated, | |
| 91 | - | RowHeight => "row_height": Replicated, | |
| 92 | - | SuggestionsDismissed => "suggestions.dismissed": Replicated, | |
| 93 | - | ColumnConfig => "column_config": Replicated, | |
| 94 | - | SampleTombstoneRetainDays => "sample_tombstone_retain_days": Replicated, | |
| 95 | - | ||
| 96 | - | // --- DeviceLocal: local paths and safety gates. Never sync these. --- | |
| 89 | + | // --- Synced: user preferences shared across the user's devices. --- | |
| 90 | + | Theme => "theme": Synced, | |
| 91 | + | PreviewLoop => "preview_loop": Synced, | |
| 92 | + | PreviewAutoplay => "preview_autoplay": Synced, | |
| 93 | + | EditResultMode => "edit_result_mode": Synced, | |
| 94 | + | ForgeAutoTrimOvershoot => "forge.auto_trim_overshoot": Synced, | |
| 95 | + | CurrentVfsId => "current_vfs_id": Synced, | |
| 96 | + | SidebarVisible => "sidebar_visible": Synced, | |
| 97 | + | DetailVisible => "detail_visible": Synced, | |
| 98 | + | FilterPanelOpen => "filter_panel_open": Synced, | |
| 99 | + | VfsExplained => "vfs_explained": Synced, | |
| 100 | + | HintsDismissed => "hints_dismissed": Synced, | |
| 101 | + | SyncIntroDismissed => "sync_intro_dismissed": Synced, | |
| 102 | + | RowHeight => "row_height": Synced, | |
| 103 | + | SuggestionsDismissed => "suggestions.dismissed": Synced, | |
| 104 | + | ColumnConfig => "column_config": Synced, | |
| 105 | + | SampleTombstoneRetainDays => "sample_tombstone_retain_days": Synced, | |
| 106 | + | ||
| 107 | + | // --- Local: local paths and safety gates. Never sync these. --- | |
| 97 | 108 | /// Reference-in-place vault mode. Local safety gate. | |
| 98 | - | LooseFiles => "loose_files": DeviceLocal, | |
| 109 | + | LooseFiles => "loose_files": Local, | |
| 99 | 110 | /// Legacy alias of `loose_files`, read once during migration. | |
| 100 | - | UnsafeMode => "unsafe_mode": DeviceLocal, | |
| 111 | + | UnsafeMode => "unsafe_mode": Local, | |
| 101 | 112 | /// Whether the on-disk mirror is enabled. Local. | |
| 102 | - | MirrorEnabled => "mirror_enabled": DeviceLocal, | |
| 113 | + | MirrorEnabled => "mirror_enabled": Local, | |
| 103 | 114 | /// Absolute local path the sample library is mirrored to. Local. | |
| 104 | - | MirrorPath => "mirror_path": DeviceLocal, | |
| 115 | + | MirrorPath => "mirror_path": Local, | |
| 105 | 116 | /// Suppresses the local import safety preflight. Local safety gate. | |
| 106 | - | ImportPreflightDisabled => "import_preflight_disabled": DeviceLocal, | |
| 117 | + | ImportPreflightDisabled => "import_preflight_disabled": Local, | |
| 107 | 118 | } | |
| 108 | 119 | ||
| 109 | 120 | impl ConfigKey { | |
| 110 | 121 | /// True when this key must not cross the sync boundary. | |
| 111 | 122 | #[must_use] | |
| 112 | 123 | pub const fn is_excluded_from_sync(self) -> bool { | |
| 113 | - | matches!(self.posture(), SyncPosture::DeviceLocal) | |
| 124 | + | !self.posture().replicated() | |
| 114 | 125 | } | |
| 115 | 126 | } | |
| 116 | 127 | ||
| @@ -166,4 +177,34 @@ mod tests { | |||
| 166 | 177 | assert!(key_excluded_from_sync("sync_cursor")); | |
| 167 | 178 | assert!(key_excluded_from_sync("totally_unknown_key")); | |
| 168 | 179 | } | |
| 180 | + | ||
| 181 | + | // The derived spec and the typed registry must describe the same key set | |
| 182 | + | // with the same postures. If they drifted, the SQL export filter (seeded | |
| 183 | + | // from the spec) and the Rust import filter (driven off the registry) would | |
| 184 | + | // disagree, which is the exact class of bug the single registry exists to | |
| 185 | + | // rule out. | |
| 186 | + | #[test] | |
| 187 | + | fn the_spec_matches_the_registry() { | |
| 188 | + | let spec_keys: Vec<&str> = CONFIG.keys().map(|(k, _)| k).collect(); | |
| 189 | + | let registry_keys: Vec<&str> = ConfigKey::ALL.iter().map(|k| k.as_str()).collect(); | |
| 190 | + | assert_eq!(spec_keys, registry_keys, "same keys, same order"); | |
| 191 | + | ||
| 192 | + | for &key in ConfigKey::ALL { | |
| 193 | + | assert_eq!( | |
| 194 | + | CONFIG.is_synced(key.as_str()), | |
| 195 | + | !key.is_excluded_from_sync(), | |
| 196 | + | "posture disagreement for {key:?}", | |
| 197 | + | ); | |
| 198 | + | } | |
| 199 | + | } | |
| 200 | + | ||
| 201 | + | // Posture is synckit's shared vocabulary now; the mapping from the old names | |
| 202 | + | // is the security-load-bearing half, so pin it: a sample of each kind. | |
| 203 | + | #[test] | |
| 204 | + | fn postures_map_to_the_shared_vocabulary() { | |
| 205 | + | assert_eq!(ConfigKey::Theme.posture(), Posture::Synced); | |
| 206 | + | assert_eq!(ConfigKey::MirrorPath.posture(), Posture::Local); | |
| 207 | + | assert!(ConfigKey::MirrorPath.is_excluded_from_sync()); | |
| 208 | + | assert!(!ConfigKey::Theme.is_excluded_from_sync()); | |
| 209 | + | } | |
| 169 | 210 | } |
| @@ -4,19 +4,36 @@ use std::path::Path; | |||
| 4 | 4 | ||
| 5 | 5 | use rusqlite::{Connection, functions::FunctionFlags}; | |
| 6 | 6 | use sha2::{Digest, Sha256}; | |
| 7 | + | use synckit_config::{ConfigError, ConfigStore}; | |
| 7 | 8 | use thiserror::Error; | |
| 8 | 9 | use tracing::instrument; | |
| 9 | 10 | ||
| 11 | + | use crate::config_key::{CONFIG, ConfigKey}; | |
| 12 | + | ||
| 10 | 13 | #[derive(Error, Debug)] | |
| 11 | 14 | pub enum DbError { | |
| 12 | 15 | #[error("SQLite error: {0}")] | |
| 13 | 16 | Sqlite(#[from] rusqlite::Error), | |
| 14 | 17 | } | |
| 15 | 18 | ||
| 19 | + | /// The config store wraps rusqlite; its one failure mode is the database, so it | |
| 20 | + | /// folds into [`DbError::Sqlite`] rather than carrying a second SQL error type. | |
| 21 | + | impl From<ConfigError> for DbError { | |
| 22 | + | fn from(error: ConfigError) -> Self { | |
| 23 | + | match error { | |
| 24 | + | ConfigError::Db(error) => DbError::Sqlite(error), | |
| 25 | + | } | |
| 26 | + | } | |
| 27 | + | } | |
| 28 | + | ||
| 16 | 29 | /// Core database wrapper. All access is synchronous, no async runtime needed, | |
| 17 | 30 | /// safe to use from a CLAP plugin host thread. | |
| 18 | 31 | pub struct Database { | |
| 19 | 32 | conn: Connection, | |
| 33 | + | /// The shared config store over `user_config`. Attached, not opened: the | |
| 34 | + | /// table and its sync triggers are stood up by this crate's migrations, so | |
| 35 | + | /// the store drives the existing table rather than creating its own. | |
| 36 | + | config: ConfigStore, | |
| 20 | 37 | } | |
| 21 | 38 | ||
| 22 | 39 | const MIGRATION_001: &str = r" | |
| @@ -1668,7 +1685,10 @@ impl Database { | |||
| 1668 | 1685 | PRAGMA wal_checkpoint(TRUNCATE);", | |
| 1669 | 1686 | )?; | |
| 1670 | 1687 | register_hash_row_id(&conn)?; | |
| 1671 | - | let mut db = Self { conn }; | |
| 1688 | + | let mut db = Self { | |
| 1689 | + | conn, | |
| 1690 | + | config: ConfigStore::attached(&CONFIG), | |
| 1691 | + | }; | |
| 1672 | 1692 | db.migrate()?; | |
| 1673 | 1693 | db.seed_config_key_policy()?; | |
| 1674 | 1694 | Ok(db) | |
| @@ -1689,30 +1709,57 @@ impl Database { | |||
| 1689 | 1709 | let conn = Connection::open_in_memory()?; | |
| 1690 | 1710 | conn.execute_batch("PRAGMA foreign_keys=ON;")?; | |
| 1691 | 1711 | register_hash_row_id(&conn)?; | |
| 1692 | - | let mut db = Self { conn }; | |
| 1712 | + | let mut db = Self { | |
| 1713 | + | conn, | |
| 1714 | + | config: ConfigStore::attached(&CONFIG), | |
| 1715 | + | }; | |
| 1693 | 1716 | db.migrate()?; | |
| 1694 | 1717 | db.seed_config_key_policy()?; | |
| 1695 | 1718 | Ok(db) | |
| 1696 | 1719 | } | |
| 1697 | 1720 | ||
| 1698 | - | /// Seed `config_key_policy` from the [`ConfigKey`](crate::config_key::ConfigKey) | |
| 1699 | - | /// registry, the single source of truth for which `user_config` keys may | |
| 1700 | - | /// sync. Run at every open so the SQL export triggers always reflect the | |
| 1701 | - | /// current registry: adding a key in Rust is enough, no migration needed. | |
| 1702 | - | /// Idempotent (clear + reinsert the closed set). | |
| 1721 | + | /// Seed `config_key_policy` from the [`CONFIG`] spec, the single source of | |
| 1722 | + | /// truth for which `user_config` keys may sync. Run at every open so the SQL | |
| 1723 | + | /// export triggers always reflect the current registry: adding a key in Rust | |
| 1724 | + | /// is enough, no migration needed. Idempotent (clear + reinsert the closed | |
| 1725 | + | /// set); the rows are the spec's [`policy_rows`](synckit_config::ConfigSpec::policy_rows), | |
| 1726 | + | /// so an undeclared key has no row and the export filter cannot admit it. | |
| 1703 | 1727 | fn seed_config_key_policy(&self) -> Result<(), DbError> { | |
| 1704 | - | use crate::config_key::{ConfigKey, SyncPosture}; | |
| 1705 | 1728 | self.conn.execute("DELETE FROM config_key_policy", [])?; | |
| 1706 | 1729 | let mut stmt = self | |
| 1707 | 1730 | .conn | |
| 1708 | 1731 | .prepare("INSERT INTO config_key_policy (key, replicated) VALUES (?1, ?2)")?; | |
| 1709 | - | for &key in ConfigKey::ALL { | |
| 1710 | - | let replicated = i64::from(matches!(key.posture(), SyncPosture::Replicated)); | |
| 1711 | - | stmt.execute(rusqlite::params![key.as_str(), replicated])?; | |
| 1732 | + | for row in CONFIG.policy_rows() { | |
| 1733 | + | stmt.execute(rusqlite::params![row.key, i64::from(row.replicated)])?; | |
| 1712 | 1734 | } | |
| 1713 | 1735 | Ok(()) | |
| 1714 | 1736 | } | |
| 1715 | 1737 | ||
| 1738 | + | /// Read a `user_config` value through the shared config store, `None` when | |
| 1739 | + | /// unset. The store is attached to the `user_config` table this crate's | |
| 1740 | + | /// migrations own. | |
| 1741 | + | pub fn get_config(&self, key: ConfigKey) -> Result<Option<String>, DbError> { | |
| 1742 | + | Ok(self.config.get(&self.conn, key.as_str())?) | |
| 1743 | + | } | |
| 1744 | + | ||
| 1745 | + | /// Write a `user_config` value through the shared config store. | |
| 1746 | + | /// | |
| 1747 | + | /// An upsert on the key: writing a key that already exists fires the table's | |
| 1748 | + | /// UPDATE trigger once, not a DELETE followed by an INSERT the way the old | |
| 1749 | + | /// `INSERT OR REPLACE` did, so a synced key enqueues one changelog row per | |
| 1750 | + | /// edit rather than a spurious delete-then-insert pair. | |
| 1751 | + | pub fn set_config(&self, key: ConfigKey, value: &str) -> Result<(), DbError> { | |
| 1752 | + | self.config.set(&self.conn, key.as_str(), value)?; | |
| 1753 | + | Ok(()) | |
| 1754 | + | } | |
| 1755 | + | ||
| 1756 | + | /// Remove a `user_config` key through the shared config store. Absent | |
| 1757 | + | /// already is not an error. | |
| 1758 | + | pub fn delete_config(&self, key: ConfigKey) -> Result<(), DbError> { | |
| 1759 | + | self.config.unset(&self.conn, key.as_str())?; | |
| 1760 | + | Ok(()) | |
| 1761 | + | } | |
| 1762 | + | ||
| 1716 | 1763 | /// Apply pending migrations using PRAGMA user_version as the version tracker. | |
| 1717 | 1764 | /// | |
| 1718 | 1765 | /// Each migration step runs inside a transaction so the schema change and | |
| @@ -2109,6 +2156,67 @@ mod tests { | |||
| 2109 | 2156 | assert_eq!(changelog_rows("theme"), 1, "theme must be exported"); | |
| 2110 | 2157 | } | |
| 2111 | 2158 | ||
| 2159 | + | // The same export boundary, exercised through the real write path the app | |
| 2160 | + | // uses now: `Database::set_config` over the shared `ConfigStore`, not a raw | |
| 2161 | + | // INSERT. The store's upsert and the spec-seeded policy must still keep a | |
| 2162 | + | // device-local key off the changelog while a replicated key lands. | |
| 2163 | + | #[test] | |
| 2164 | + | fn set_config_respects_the_export_boundary() { | |
| 2165 | + | let db = Database::open_in_memory().unwrap(); | |
| 2166 | + | let changelog_rows = |key: &str| -> i64 { | |
| 2167 | + | db.conn() | |
| 2168 | + | .query_row( | |
| 2169 | + | "SELECT COUNT(*) FROM sync_changelog WHERE table_name = 'user_config' AND row_id = ?1", | |
| 2170 | + | [key], | |
| 2171 | + | |r| r.get(0), | |
| 2172 | + | ) | |
| 2173 | + | .unwrap() | |
| 2174 | + | }; | |
| 2175 | + | ||
| 2176 | + | db.set_config(ConfigKey::MirrorPath, "/mnt/samples") | |
| 2177 | + | .unwrap(); | |
| 2178 | + | assert_eq!( | |
| 2179 | + | db.get_config(ConfigKey::MirrorPath).unwrap().as_deref(), | |
| 2180 | + | Some("/mnt/samples"), | |
| 2181 | + | "a device-local key is still stored locally", | |
| 2182 | + | ); | |
| 2183 | + | assert_eq!( | |
| 2184 | + | changelog_rows("mirror_path"), | |
| 2185 | + | 0, | |
| 2186 | + | "a device-local key must never be exported", | |
| 2187 | + | ); | |
| 2188 | + | ||
| 2189 | + | db.set_config(ConfigKey::Theme, "dark").unwrap(); | |
| 2190 | + | assert_eq!(changelog_rows("theme"), 1, "a replicated key is exported"); | |
| 2191 | + | } | |
| 2192 | + | ||
| 2193 | + | // The reason the store upserts instead of `INSERT OR REPLACE`: rewriting a | |
| 2194 | + | // replicated key is one UPDATE, not a DELETE-then-INSERT pair. The old | |
| 2195 | + | // path enqueued a spurious delete for every re-save of a synced setting. | |
| 2196 | + | #[test] | |
| 2197 | + | fn rewriting_a_synced_key_enqueues_an_update_not_a_delete() { | |
| 2198 | + | let db = Database::open_in_memory().unwrap(); | |
| 2199 | + | db.set_config(ConfigKey::Theme, "light").unwrap(); | |
| 2200 | + | db.set_config(ConfigKey::Theme, "dark").unwrap(); | |
| 2201 | + | ||
| 2202 | + | let ops: Vec<String> = db | |
| 2203 | + | .conn() | |
| 2204 | + | .prepare( | |
| 2205 | + | "SELECT op FROM sync_changelog WHERE table_name = 'user_config' AND row_id = 'theme' ORDER BY id", | |
| 2206 | + | ) | |
| 2207 | + | .unwrap() | |
| 2208 | + | .query_map([], |r| r.get(0)) | |
| 2209 | + | .unwrap() | |
| 2210 | + | .collect::<Result<_, _>>() | |
| 2211 | + | .unwrap(); | |
| 2212 | + | ||
| 2213 | + | assert_eq!(ops, vec!["INSERT", "UPDATE"], "one insert then one update"); | |
| 2214 | + | assert!( | |
| 2215 | + | !ops.iter().any(|op| op == "DELETE"), | |
| 2216 | + | "no spurious delete from a re-save", | |
| 2217 | + | ); | |
| 2218 | + | } | |
| 2219 | + | ||
| 2112 | 2220 | /// Open a fresh file-backed DB, close, reopen. The second open re-enters | |
| 2113 | 2221 | /// `migrate()`; with `user_version=17` no migration body runs, but the | |
| 2114 | 2222 | /// shape verifies our open/close cycle is clean (no locks, no WAL leak). |