diff --git a/benches/support/mod.rs b/benches/support/mod.rs index e068119..3590220 100644 --- a/benches/support/mod.rs +++ b/benches/support/mod.rs @@ -59,6 +59,7 @@ pub fn make_sounds(n: usize) -> Vec { } else { None }, + modified_ms: None, category: category.to_owned(), } }) diff --git a/docs/superpowers/specs/2026-07-20-issue-195-design.md b/docs/superpowers/specs/2026-07-20-issue-195-design.md new file mode 100644 index 0000000..ea80370 --- /dev/null +++ b/docs/superpowers/specs/2026-07-20-issue-195-design.md @@ -0,0 +1,75 @@ +# Issue #195 — Library Modified and First-Seen Timestamps + +Date: 2026-07-20 · Status: approved + +## Why + +The list-controls framework needs stable modified and date-added sort keys. Modified time comes +from the filesystem during the existing scan. Date added is HonkHonk state: it is stamped once +when a sound ID is first observed and survives later scans. + +The scan must also distinguish a complete view of the library from a partial one. A missing mount, +unreadable subtree, or other walk error must not look like intentional deletion and erase date-added +history. + +## Data Shapes + +- `SoundEntry.modified_ms: Option` stores filesystem modification time in Unix epoch + milliseconds. It is `None` when metadata, modification time, epoch conversion, or `u64` + conversion is unavailable. +- `LibraryScan` owns the scanned entries and a completeness flag. It replaces the ambiguous bare + `Vec` result at startup and rescan boundaries. +- `SoundMetaStore` becomes a named struct containing the existing custom metadata map plus + `added: BTreeMap`. +- The persisted file becomes an explicit versioned envelope containing custom metadata and added + timestamps. Loading retains compatibility with the legacy top-level sound-ID map. + +No new crate is needed; `std::fs::Metadata`, `std::time::SystemTime`, and the existing Serde stack +cover the feature. + +## Interfaces and Contracts + +- `Library::scan(dirs) -> Result` continues collecting accessible audio + entries. `complete` is false if a configured root is absent or any directory-walk/metadata + observation needed for membership fails. +- Timestamp conversion is a small pure helper returning `Option`; it never unwraps or truncates + `u128` milliseconds. +- `SoundMetaStore::added_ms(id) -> Option` exposes the persisted sort key. +- `SoundMetaStore::reconcile_added(ids, observed_at_ms, complete) -> bool` inserts missing IDs with + the single scan timestamp, never overwrites existing timestamps, and prunes absent IDs only when + `complete` is true. The boolean reports whether persistence is needed. +- Startup and `Message::RescanLibrary` reconcile immediately after scanning and save once when the + store changed. Save failures are logged with context and do not discard the usable in-memory + library. + +## Persistence Compatibility + +The loader first recognizes the new versioned envelope; otherwise it interprets the JSON object as +the legacy `HashMap`. Legacy customizations must survive the first new-version +save. Default `SoundMeta` entries remain pruned exactly as before; first-seen timestamps live only +in the dedicated map and do not create fake customization entries. + +## Invariants + +1. A successfully observed modification time is represented in epoch milliseconds without panic or + lossy integer casting. +2. Every observed sound ID receives one first-seen timestamp, shared from the scan boundary. +3. Re-observing an ID never changes its first-seen timestamp, including after persistence reload. +4. A complete scan prunes IDs no longer present; a partial scan never prunes unseen IDs. +5. Existing `sound_meta.json` customizations load unchanged and migrate on the next save. +6. Default custom metadata still occupies no entry in the customization map. + +## TDD and Change Map + +1. Add failing library tests for populated `modified_ms` and partial-scan completeness. +2. Add failing metadata-store tests for first stamp, stable restamp, complete-scan pruning, and + partial-scan preservation. +3. Add failing persistence tests for legacy loading and new-envelope round trips. +4. Introduce the data shapes and timestamp helpers, then make the focused state tests green. +5. Wire `LibraryScan` and reconciliation through startup and rescan, with app-boundary regression + tests. +6. Update existing `SoundEntry` fixtures mechanically with `modified_ms: None`, refactor, and run the + full Rust verification suite. + +Likely files: `src/state/library.rs`, `src/state/sound_meta.rs`, `src/state/mod.rs`, `src/main.rs`, +and narrow startup/rescan/test sites in `src/app/mod.rs`. diff --git a/src/app/filtering.rs b/src/app/filtering.rs index c049389..996bfbf 100644 --- a/src/app/filtering.rs +++ b/src/app/filtering.rs @@ -249,6 +249,7 @@ mod tests { format: AudioFormat::Wav, duration_ms: None, category: "Animals".into(), + modified_ms: None, }]; app.sound_meta .set_display_name("goose", Some("Angry Bird".into())); diff --git a/src/app/library_scan.rs b/src/app/library_scan.rs new file mode 100644 index 0000000..99ddacb --- /dev/null +++ b/src/app/library_scan.rs @@ -0,0 +1,147 @@ +use std::time::SystemTime; + +use crate::state::{LibraryScan, SoundEntry, SoundMetaStore}; + +use super::HonkHonk; + +pub(super) fn load_sound_meta(scan: &LibraryScan) -> SoundMetaStore { + let mut store = SoundMetaStore::load(); + reconcile_sound_meta(&mut store, &scan.entries, scan.complete, true); + store +} + +fn reconcile_sound_meta( + store: &mut SoundMetaStore, + sounds: &[SoundEntry], + complete: bool, + persist: bool, +) { + let Some(observed_at_ms) = crate::state::library::system_time_to_epoch_ms(SystemTime::now()) + else { + tracing::warn!("system clock is before Unix epoch; first-seen timestamps not reconciled"); + return; + }; + let changed = store.reconcile_added( + sounds.iter().map(|sound| &sound.id), + observed_at_ms, + complete, + ); + save_reconciled_meta(store, changed && persist); +} + +fn save_reconciled_meta(store: &SoundMetaStore, should_save: bool) { + if !should_save { + return; + } + if let Err(error) = store.save() { + tracing::warn!(error = %error, "failed to save reconciled sound metadata"); + } +} + +impl HonkHonk { + pub(super) fn apply_library_scan(&mut self, scan: LibraryScan) { + reconcile_sound_meta( + &mut self.sound_meta, + &scan.entries, + scan.complete, + self.persist, + ); + self.duration_scan_pairs = std::sync::Arc::new( + scan.entries + .iter() + .map(|sound| (sound.id.clone(), sound.path.clone())) + .collect(), + ); + self.sounds = scan.entries; + self.reconcile_playback_with_library(); + self.durations_loaded = false; + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::app::Message; + use crate::state::AudioFormat; + + fn sound(id: &str) -> SoundEntry { + SoundEntry { + id: id.to_owned(), + name: id.to_owned(), + path: format!("/sounds/{id}.wav").into(), + format: AudioFormat::Wav, + duration_ms: None, + modified_ms: None, + category: "Test".to_owned(), + } + } + + #[test] + fn applying_complete_scan_reconciles_first_seen_ids_and_prunes_stale_ids() { + let mut app = HonkHonk::new_for_test(); + app.sound_meta.reconcile_added(["stale"], 1, true); + + app.apply_library_scan(LibraryScan { + entries: vec![sound("current")], + complete: true, + }); + + assert!(app.sound_meta.added_ms("current").is_some()); + assert_eq!(app.sound_meta.added_ms("stale"), None); + } + + #[test] + fn applying_partial_scan_preserves_unseen_first_seen_ids() { + let mut app = HonkHonk::new_for_test(); + app.sound_meta.reconcile_added(["unseen"], 1, true); + + app.apply_library_scan(LibraryScan { + entries: vec![sound("current")], + complete: false, + }); + + assert_eq!(app.sound_meta.added_ms("unseen"), Some(1)); + assert!(app.sound_meta.added_ms("current").is_some()); + } + + #[test] + fn rescan_library_resets_durations_loaded() { + let mut app = HonkHonk::new_for_test(); + app.durations_loaded = true; + let _ = app.update(Message::RescanLibrary); + assert!(!app.durations_loaded); + } + + #[test] + fn remove_sound_directory_removes_path() { + let mut app = HonkHonk::new_for_test(); + let path = std::path::PathBuf::from("/tmp/hh_test_sounds"); + app.config.sound_directories.push(path.clone()); + + let _ = app.update(Message::RemoveSoundDirectory(path.clone())); + + assert!(!app.config.sound_directories.contains(&path)); + } + + #[test] + fn sound_directory_pick_some_appends_to_config() { + let mut app = HonkHonk::new_for_test(); + let path = std::path::PathBuf::from("/tmp/hh_new_sounds"); + let before = app.config.sound_directories.len(); + + let _ = app.update(Message::SoundDirectoryPickResult(Some(path.clone()))); + + assert_eq!(app.config.sound_directories.len(), before + 1); + assert!(app.config.sound_directories.contains(&path)); + } + + #[test] + fn sound_directory_pick_none_is_noop() { + let mut app = HonkHonk::new_for_test(); + let before = app.config.sound_directories.clone(); + + let _ = app.update(Message::SoundDirectoryPickResult(None)); + + assert_eq!(app.config.sound_directories, before); + } +} diff --git a/src/app/macros/tests.rs b/src/app/macros/tests.rs index 0b1030b..f951883 100644 --- a/src/app/macros/tests.rs +++ b/src/app/macros/tests.rs @@ -17,6 +17,7 @@ fn sound(id: &str, path: &str) -> SoundEntry { path: path.into(), format: AudioFormat::Wav, duration_ms: Some(100), + modified_ms: None, category: "Test".into(), } } diff --git a/src/app/mod.rs b/src/app/mod.rs index 6eaa27c..347719c 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -10,7 +10,7 @@ use crate::audio::effects::EffectSlot; use crate::audio::{AudioCommand, AudioEvent, AudioHandle, PlayMode}; use crate::shortcuts::ShortcutsStatus; use crate::state::config::{Density, OverlapMode}; -use crate::state::{AppConfig, SlotMap, SoundEntry, SoundMeta, SoundMetaStore}; +use crate::state::{AppConfig, LibraryScan, SlotMap, SoundEntry, SoundMeta, SoundMetaStore}; use crate::tray::{TrayEvent, TrayHandle}; use crate::ui::effects_panel::{self, EffectsUiState, PresetId}; use crate::ui::effects_panel_view; @@ -24,6 +24,7 @@ use notices::{Notice, NoticeId, NoticeQueue}; /// Play-dispatch coordination (`request_play` / `handle_decoded` / /// `start_playback`), extracted to keep this file from growing (#151). mod filtering; +mod library_scan; mod macros; #[cfg(test)] mod notice_tests; @@ -401,11 +402,13 @@ impl HonkHonk { pub fn new( mut tray: TrayHandle, audio: AudioHandle, - sounds: Vec, + scan: LibraryScan, config: AppConfig, slots: SlotMap, ) -> Self { let rx = tray.take_rx(); + let sound_meta = library_scan::load_sound_meta(&scan); + let sounds = scan.entries; let duration_scan_pairs = std::sync::Arc::new( sounds .iter() @@ -441,7 +444,7 @@ impl HonkHonk { input_devices: Vec::new(), shortcut_config: crate::shortcuts::config_ui::ShortcutConfigService::new(), notices: NoticeQueue::new(), - sound_meta: SoundMetaStore::load(), + sound_meta, persist: true, config_load_failed: false, editor_sound_id: None, @@ -968,21 +971,14 @@ impl HonkHonk { Task::none() } Message::RescanLibrary => { - let new_sounds = match crate::state::Library::scan(&self.config.sound_directories) { - Ok(sounds) => sounds, + let scan = match crate::state::Library::scan(&self.config.sound_directories) { + Ok(scan) => scan, Err(e) => { tracing::warn!(dirs = ?self.config.sound_directories, error = %e, "library rescan failed"); return Task::none(); } }; - let pairs: Vec<(String, std::path::PathBuf)> = new_sounds - .iter() - .map(|s| (s.id.clone(), s.path.clone())) - .collect(); - self.sounds = new_sounds; - self.reconcile_playback_with_library(); - self.duration_scan_pairs = std::sync::Arc::new(pairs); - self.durations_loaded = false; + self.apply_library_scan(scan); Task::none() } Message::AddSoundDirectory => Task::perform( @@ -1890,6 +1886,7 @@ mod tests { path: "/a.mp3".into(), format: crate::state::AudioFormat::Mp3, duration_ms: Some(1000), + modified_ms: None, category: "Honk".into(), }]; @@ -2155,6 +2152,7 @@ mod tests { path: wav_path, format: crate::state::AudioFormat::Wav, duration_ms: Some(100), + modified_ms: None, category: "Test".into(), }]; @@ -2247,6 +2245,7 @@ mod tests { path: "/a.mp3".into(), format: crate::state::AudioFormat::Mp3, duration_ms: Some(1000), + modified_ms: None, category: "Honk".into(), }, SoundEntry { @@ -2255,6 +2254,7 @@ mod tests { path: "/b.mp3".into(), format: crate::state::AudioFormat::Mp3, duration_ms: Some(1000), + modified_ms: None, category: "Memes".into(), }, ]; @@ -2272,6 +2272,7 @@ mod tests { path: "/a.mp3".into(), format: crate::state::AudioFormat::Mp3, duration_ms: Some(1000), + modified_ms: None, category: "Honk".into(), }]; let _ = app.update(Message::SearchChanged("GOOSE".into())); @@ -2288,6 +2289,7 @@ mod tests { path: "/a.mp3".into(), format: crate::state::AudioFormat::Mp3, duration_ms: Some(1000), + modified_ms: None, category: "Honk".into(), }, SoundEntry { @@ -2296,6 +2298,7 @@ mod tests { path: "/b.mp3".into(), format: crate::state::AudioFormat::Mp3, duration_ms: Some(1000), + modified_ms: None, category: "Memes".into(), }, ]; @@ -2376,6 +2379,7 @@ mod tests { path: path.clone(), format: crate::state::AudioFormat::Mp3, duration_ms: Some(500), + modified_ms: None, category: "Honk".into(), }]; let _ = app.update(Message::AssignSlot(0, path.clone())); @@ -2480,6 +2484,7 @@ mod tests { path: "/tmp/honk.wav".into(), format: crate::state::AudioFormat::Wav, duration_ms: None, + modified_ms: None, category: "Honk".into(), }]; let map = std::collections::HashMap::from([("abc123".to_string(), 1500u64)]); @@ -2497,6 +2502,7 @@ mod tests { path: "/tmp/honk.wav".into(), format: crate::state::AudioFormat::Wav, duration_ms: None, + modified_ms: None, category: "Honk".into(), }]; let map = std::collections::HashMap::from([("no-match".to_string(), 999u64)]); @@ -2533,44 +2539,6 @@ mod tests { assert!(matches!(app.view_mode, ViewMode::Main)); } - #[test] - fn rescan_library_resets_durations_loaded() { - let mut app = HonkHonk::new_for_test(); - app.durations_loaded = true; - let _ = app.update(Message::RescanLibrary); - assert!( - !app.durations_loaded, - "RescanLibrary must reset durations_loaded" - ); - } - - #[test] - fn remove_sound_directory_removes_path() { - let mut app = HonkHonk::new_for_test(); - let path = std::path::PathBuf::from("/tmp/hh_test_sounds"); - app.config.sound_directories.push(path.clone()); - let _ = app.update(Message::RemoveSoundDirectory(path.clone())); - assert!(!app.config.sound_directories.contains(&path)); - } - - #[test] - fn sound_directory_pick_some_appends_to_config() { - let mut app = HonkHonk::new_for_test(); - let path = std::path::PathBuf::from("/tmp/hh_new_sounds"); - let before = app.config.sound_directories.len(); - let _ = app.update(Message::SoundDirectoryPickResult(Some(path.clone()))); - assert_eq!(app.config.sound_directories.len(), before + 1); - assert!(app.config.sound_directories.contains(&path)); - } - - #[test] - fn sound_directory_pick_none_is_noop() { - let mut app = HonkHonk::new_for_test(); - let before = app.config.sound_directories.clone(); - let _ = app.update(Message::SoundDirectoryPickResult(None)); - assert_eq!(app.config.sound_directories, before); - } - #[test] fn theme_changed_updates_config() { let mut app = HonkHonk::new_for_test(); @@ -2910,6 +2878,7 @@ mod tests { path: "/a.mp3".into(), format: crate::state::AudioFormat::Mp3, duration_ms: None, + modified_ms: None, category: "General".into(), }]; let _ = app.update(Message::OpenSoundEditor("abc".into())); @@ -3020,6 +2989,7 @@ mod tests { path: "/fav.mp3".into(), format: crate::state::AudioFormat::Mp3, duration_ms: None, + modified_ms: None, category: "General".into(), }, SoundEntry { @@ -3028,6 +2998,7 @@ mod tests { path: "/nonfav.mp3".into(), format: crate::state::AudioFormat::Mp3, duration_ms: None, + modified_ms: None, category: "General".into(), }, ]; @@ -3048,6 +3019,7 @@ mod tests { path: "/a.mp3".into(), format: crate::state::AudioFormat::Mp3, duration_ms: None, + modified_ms: None, category: "X".into(), }, SoundEntry { @@ -3056,6 +3028,7 @@ mod tests { path: "/b.mp3".into(), format: crate::state::AudioFormat::Mp3, duration_ms: None, + modified_ms: None, category: "Y".into(), }, ]; @@ -3077,6 +3050,7 @@ mod tests { path: "/only.mp3".into(), format: crate::state::AudioFormat::Mp3, duration_ms: None, + modified_ms: None, category: "General".into(), }]; let _ = app.update(Message::ToggleFavorite("only".into())); @@ -3102,6 +3076,7 @@ mod tests { path: "/id1.wav".into(), format: crate::state::AudioFormat::Wav, duration_ms: None, + modified_ms: None, category: "Animals".into(), }]; // Rename the sound via the editor workflow @@ -3134,6 +3109,7 @@ mod tests { path: wav_path, format: crate::state::AudioFormat::Wav, duration_ms: Some(100), + modified_ms: None, category: "Test".into(), }]; diff --git a/src/app/playback/test_support.rs b/src/app/playback/test_support.rs index 0586e3e..f9c8d15 100644 --- a/src/app/playback/test_support.rs +++ b/src/app/playback/test_support.rs @@ -18,6 +18,7 @@ pub(super) fn sound(id: &str) -> SoundEntry { path: format!("/tmp/{id}.wav").into(), format: crate::state::AudioFormat::Wav, duration_ms: Some(100), + modified_ms: None, category: "Test".into(), } } diff --git a/src/app/recording.rs b/src/app/recording.rs index 794a189..bb5716e 100644 --- a/src/app/recording.rs +++ b/src/app/recording.rs @@ -100,6 +100,7 @@ mod tests { path: path.into(), format: AudioFormat::Wav, duration_ms: Some(100), + modified_ms: None, category: "Test".into(), } } diff --git a/src/main.rs b/src/main.rs index 25a0ede..c17ebb4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -55,11 +55,14 @@ fn main() -> iced::Result { pipewire::init(); - let sounds = match honkhonk::state::Library::scan(&config.sound_directories) { - Ok(s) => s, + let scan = match honkhonk::state::Library::scan(&config.sound_directories) { + Ok(scan) => scan, Err(e) => { tracing::warn!(error = %e, "failed to scan sound library"); - Vec::new() + honkhonk::state::LibraryScan { + entries: Vec::new(), + complete: false, + } } }; @@ -96,7 +99,7 @@ fn main() -> iced::Result { let tray_handle = std::sync::Mutex::new(Some(tray_handle)); let audio_handle = std::sync::Mutex::new(Some(audio_handle)); - let sounds = std::sync::Mutex::new(Some(sounds)); + let scan = std::sync::Mutex::new(Some(scan)); let config = std::sync::Mutex::new(Some(config)); let slots = std::sync::Mutex::new(Some(slots)); @@ -117,9 +120,9 @@ fn main() -> iced::Result { .expect("audio mutex poisoned") .take() .expect("boot called more than once"); - let sounds = sounds + let scan = scan .lock() - .expect("sounds mutex poisoned") + .expect("library scan mutex poisoned") .take() .expect("boot called more than once"); let config = config @@ -132,7 +135,7 @@ fn main() -> iced::Result { .expect("slots mutex poisoned") .take() .expect("boot called more than once"); - let mut app = honkhonk::app::HonkHonk::new(tray, audio, sounds, config, slots); + let mut app = honkhonk::app::HonkHonk::new(tray, audio, scan, config, slots); if config_load_failed { // A failed load means `config` is bare defaults: block the // quit-time save so it cannot clobber the user's real file. diff --git a/src/state/error.rs b/src/state/error.rs index 275785f..8e5692f 100644 --- a/src/state/error.rs +++ b/src/state/error.rs @@ -33,6 +33,9 @@ pub enum ConfigError { #[error("unable to determine XDG config directory")] NoConfigDir, + #[error("refusing to overwrite unreadable or unsupported sound metadata: {path}")] + UnsafeMetadataOverwrite { path: String }, + #[error("library scan error: {0}")] ScanEntry(String), } diff --git a/src/state/library.rs b/src/state/library.rs index 1a94a54..5441b91 100644 --- a/src/state/library.rs +++ b/src/state/library.rs @@ -1,7 +1,7 @@ -use std::collections::HashMap; -use std::collections::hash_map::DefaultHasher; +use std::collections::{HashMap, hash_map::DefaultHasher}; use std::hash::{Hash, Hasher}; use std::path::{Path, PathBuf}; +use std::time::{SystemTime, UNIX_EPOCH}; use lofty::prelude::AudioFile; use lofty::probe::Probe; @@ -13,6 +13,9 @@ use crate::state::error::ConfigError; const SUPPORTED_EXTENSIONS: &[&str] = &["mp3", "ogg", "flac", "wav", "aac", "m4a"]; +#[cfg(test)] +mod scan_tests; + #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub enum AudioFormat { Mp3, @@ -44,9 +47,17 @@ pub struct SoundEntry { pub path: PathBuf, pub format: AudioFormat, pub duration_ms: Option, + #[serde(default)] + pub modified_ms: Option, pub category: String, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct LibraryScan { + pub entries: Vec, + pub complete: bool, +} + /// Generates a deterministic hex ID from a file path. fn path_to_id(path: &Path) -> String { let mut hasher = DefaultHasher::new(); @@ -61,6 +72,19 @@ fn is_audio_extension(ext: &str) -> bool { .any(|&supported| supported.eq_ignore_ascii_case(ext)) } +pub(crate) fn system_time_to_epoch_ms(time: SystemTime) -> Option { + time.duration_since(UNIX_EPOCH) + .ok()? + .as_millis() + .try_into() + .ok() +} + +fn modified_ms(path: &Path) -> Option { + let modified = std::fs::metadata(path).ok()?.modified().ok()?; + system_time_to_epoch_ms(modified) +} + /// Builds a SoundEntry from a validated audio file path. fn entry_from_path(path: &Path) -> Option { let ext = path.extension()?.to_str()?; @@ -85,6 +109,7 @@ fn entry_from_path(path: &Path) -> Option { path: path.to_path_buf(), format: AudioFormat::from_extension(ext), duration_ms: None, + modified_ms: modified_ms(path), category, }) } @@ -121,17 +146,22 @@ pub struct Library; impl Library { /// Recursively scans directories for audio files and returns /// a list of SoundEntry items. - pub fn scan(dirs: &[PathBuf]) -> Result, ConfigError> { + pub fn scan(dirs: &[PathBuf]) -> Result { let mut entries = Vec::new(); + let mut complete = true; for dir in dirs { - if !dir.exists() { + if !dir.is_dir() { + complete = false; continue; } + // An existing empty directory is complete: without compositor- or + // mount-specific APIs it is indistinguishable from an intentionally empty library. let walker = WalkDir::new(dir).follow_links(true); for result in walker { let Ok(dir_entry) = result else { + complete = false; continue; }; @@ -145,7 +175,7 @@ impl Library { } } - Ok(entries) + Ok(LibraryScan { entries, complete }) } } @@ -236,8 +266,9 @@ mod tests { fs::write(dir.path().join("quack.ogg"), b"fake ogg").unwrap(); fs::write(dir.path().join("boom.flac"), b"fake flac").unwrap(); - let entries = Library::scan(&[dir.path().to_path_buf()]).unwrap(); - assert_eq!(entries.len(), 3); + let scan = Library::scan(&[dir.path().to_path_buf()]).unwrap(); + assert_eq!(scan.entries.len(), 3); + assert!(scan.complete); } #[test] @@ -247,23 +278,24 @@ mod tests { fs::write(dir.path().join("image.png"), b"not audio").unwrap(); fs::write(dir.path().join("sound.mp3"), b"audio").unwrap(); - let entries = Library::scan(&[dir.path().to_path_buf()]).unwrap(); - assert_eq!(entries.len(), 1); - assert_eq!(entries[0].name, "sound"); - assert_eq!(entries[0].format, AudioFormat::Mp3); + let scan = Library::scan(&[dir.path().to_path_buf()]).unwrap(); + assert_eq!(scan.entries.len(), 1); + assert_eq!(scan.entries[0].name, "sound"); + assert_eq!(scan.entries[0].format, AudioFormat::Mp3); } #[test] fn scan_handles_empty_directory() { let dir = tempfile::tempdir().unwrap(); - let entries = Library::scan(&[dir.path().to_path_buf()]).unwrap(); - assert!(entries.is_empty()); + let scan = Library::scan(&[dir.path().to_path_buf()]).unwrap(); + assert!(scan.entries.is_empty()); + assert!(scan.complete); } #[test] fn scan_handles_nonexistent_directory() { - let entries = Library::scan(&[PathBuf::from("/nonexistent/path/12345")]).unwrap(); - assert!(entries.is_empty()); + let scan = Library::scan(&[PathBuf::from("/nonexistent/path/12345")]).unwrap(); + assert!(scan.entries.is_empty()); } #[test] @@ -274,8 +306,8 @@ mod tests { fs::write(dir.path().join("top.wav"), b"top").unwrap(); fs::write(sub.join("nested.aac"), b"nested").unwrap(); - let entries = Library::scan(&[dir.path().to_path_buf()]).unwrap(); - assert_eq!(entries.len(), 2); + let scan = Library::scan(&[dir.path().to_path_buf()]).unwrap(); + assert_eq!(scan.entries.len(), 2); } #[test] @@ -285,9 +317,8 @@ mod tests { fs::write(dir1.path().join("a.mp3"), b"a").unwrap(); fs::write(dir2.path().join("b.flac"), b"b").unwrap(); - let entries = - Library::scan(&[dir1.path().to_path_buf(), dir2.path().to_path_buf()]).unwrap(); - assert_eq!(entries.len(), 2); + let scan = Library::scan(&[dir1.path().to_path_buf(), dir2.path().to_path_buf()]).unwrap(); + assert_eq!(scan.entries.len(), 2); } #[test] @@ -296,10 +327,10 @@ mod tests { let file_path = dir.path().join("my_sound.wav"); fs::write(&file_path, b"wav data").unwrap(); - let entries = Library::scan(&[dir.path().to_path_buf()]).unwrap(); - assert_eq!(entries.len(), 1); + let scan = Library::scan(&[dir.path().to_path_buf()]).unwrap(); + assert_eq!(scan.entries.len(), 1); - let entry = &entries[0]; + let entry = &scan.entries[0]; assert_eq!(entry.name, "my_sound"); assert_eq!(entry.path, file_path); assert_eq!(entry.format, AudioFormat::Wav); @@ -315,9 +346,9 @@ mod tests { fs::create_dir(&memes).unwrap(); fs::write(memes.join("honk.mp3"), b"data").unwrap(); - let entries = Library::scan(&[dir.path().to_path_buf()]).unwrap(); - assert_eq!(entries.len(), 1); - assert_eq!(entries[0].category, "Memes"); + let scan = Library::scan(&[dir.path().to_path_buf()]).unwrap(); + assert_eq!(scan.entries.len(), 1); + assert_eq!(scan.entries[0].category, "Memes"); } #[test] @@ -325,8 +356,8 @@ mod tests { let dir = tempfile::tempdir().unwrap(); fs::write(dir.path().join("toplevel.mp3"), b"data").unwrap(); - let entries = Library::scan(&[dir.path().to_path_buf()]).unwrap(); - assert_eq!(entries.len(), 1); + let scan = Library::scan(&[dir.path().to_path_buf()]).unwrap(); + assert_eq!(scan.entries.len(), 1); // A file at the root of the scan dir has no subdirectory parent, // so category falls back to the scan directory's own name. @@ -336,6 +367,6 @@ mod tests { .unwrap() .to_string_lossy() .into_owned(); - assert_eq!(entries[0].category, expected); + assert_eq!(scan.entries[0].category, expected); } } diff --git a/src/state/library/scan_tests.rs b/src/state/library/scan_tests.rs new file mode 100644 index 0000000..c5aea1d --- /dev/null +++ b/src/state/library/scan_tests.rs @@ -0,0 +1,48 @@ +use std::fs; +use std::path::PathBuf; + +use super::*; + +#[test] +fn scan_records_file_modification_time_in_epoch_milliseconds() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("timestamped.wav"); + fs::write(&path, b"data").unwrap(); + + let scan = Library::scan(&[dir.path().to_path_buf()]).unwrap(); + + assert!(scan.entries[0].modified_ms.is_some()); +} + +#[test] +fn scan_marks_missing_root_as_incomplete() { + let scan = Library::scan(&[PathBuf::from("/nonexistent/path/12345")]).unwrap(); + + assert!(scan.entries.is_empty()); + assert!(!scan.complete); +} + +#[test] +fn scan_marks_non_directory_root_as_incomplete() { + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("not-a-directory"); + fs::write(&file, b"data").unwrap(); + + let scan = Library::scan(&[file]).unwrap(); + + assert!(scan.entries.is_empty()); + assert!(!scan.complete); +} + +#[test] +fn time_before_epoch_has_no_timestamp() { + let before_epoch = std::time::UNIX_EPOCH - std::time::Duration::from_millis(1); + + assert_eq!(system_time_to_epoch_ms(before_epoch), None); +} + +#[test] +fn epoch_milliseconds_are_converted_without_loss() { + let time = std::time::UNIX_EPOCH + std::time::Duration::from_millis(42); + assert_eq!(system_time_to_epoch_ms(time), Some(42)); +} diff --git a/src/state/mod.rs b/src/state/mod.rs index fefc028..8481de4 100644 --- a/src/state/mod.rs +++ b/src/state/mod.rs @@ -7,7 +7,7 @@ pub mod sound_meta; pub use config::{AppConfig, Density, OverlapMode, Renderer}; pub use error::ConfigError; -pub use library::{AudioFormat, Library, SoundEntry}; +pub use library::{AudioFormat, Library, LibraryScan, SoundEntry}; pub use macros::{Macro, MacroStore, Step}; pub use slots::SlotMap; pub use sound_meta::{SoundMeta, SoundMetaStore}; diff --git a/src/state/sound_meta.rs b/src/state/sound_meta.rs index a2953d5..cf23b10 100644 --- a/src/state/sound_meta.rs +++ b/src/state/sound_meta.rs @@ -1,12 +1,12 @@ -use std::collections::HashMap; -use std::path::Path; +use std::collections::{BTreeMap, BTreeSet, HashMap}; use serde::{Deserialize, Serialize}; -use crate::state::error::ConfigError; +mod persistence; const META_FILE_NAME: &str = "sound_meta.json"; const CONFIG_DIR_NAME: &str = "honkhonk"; +const META_FORMAT_VERSION: u32 = 1; /// Per-sound user customisations persisted independently of library scan. /// Keyed by sound ID (deterministic hex hash of file path). @@ -45,26 +45,47 @@ impl SoundMeta { } /// In-memory store for all sound metadata, backed by a JSON file. -#[derive(Debug, Clone, PartialEq, Default)] -pub struct SoundMetaStore(HashMap); +#[derive(Debug, Clone, PartialEq)] +pub struct SoundMetaStore { + custom: HashMap, + added: BTreeMap, + writable: bool, +} + +impl Default for SoundMetaStore { + fn default() -> Self { + Self { + custom: HashMap::new(), + added: BTreeMap::new(), + writable: true, + } + } +} impl SoundMetaStore { + fn read_protected() -> Self { + Self { + writable: false, + ..Self::default() + } + } + /// Returns metadata for a sound, falling back to default if not set. pub fn get(&self, id: &str) -> SoundMeta { - self.0.get(id).cloned().unwrap_or_default() + self.custom.get(id).cloned().unwrap_or_default() } /// Returns a reference to the metadata if it exists. pub fn get_ref(&self, id: &str) -> Option<&SoundMeta> { - self.0.get(id) + self.custom.get(id) } /// Upserts metadata for a sound. Removes the entry if it becomes default. pub fn set(&mut self, id: String, meta: SoundMeta) { if meta.is_default() { - self.0.remove(&id); + self.custom.remove(&id); } else { - self.0.insert(id, meta); + self.custom.insert(id, meta); } } @@ -93,62 +114,40 @@ impl SoundMetaStore { /// Returns `true` if the sound is a favorite. pub fn is_favorite(&self, id: &str) -> bool { - self.0.get(id).map(|m| m.favorite).unwrap_or(false) + self.custom.get(id).map(|m| m.favorite).unwrap_or(false) } /// Returns the per-sound volume multiplier (defaults to 1.0). pub fn volume_for(&self, id: &str) -> f32 { - self.0.get(id).map(|m| m.volume).unwrap_or(1.0) - } - - fn meta_path() -> Result { - let proj = directories::ProjectDirs::from("", "", CONFIG_DIR_NAME) - .ok_or(ConfigError::NoConfigDir)?; - Ok(proj.config_dir().join(META_FILE_NAME)) + self.custom.get(id).map(|m| m.volume).unwrap_or(1.0) } - /// Loads the store from the default XDG path, returning an empty store on - /// any error (missing file, corrupt JSON). - pub fn load() -> Self { - Self::meta_path() - .ok() - .and_then(|p| std::fs::read_to_string(p).ok()) - .and_then(|s| serde_json::from_str::>(&s).ok()) - .map(SoundMetaStore) - .unwrap_or_default() + pub fn added_ms(&self, id: &str) -> Option { + self.added.get(id).copied() } - /// Persists the store to the default XDG path. - pub fn save(&self) -> Result<(), ConfigError> { - self.save_to(&Self::meta_path()?) - } + pub fn reconcile_added(&mut self, ids: I, observed_at_ms: u64, complete: bool) -> bool + where + I: IntoIterator, + S: AsRef, + { + let observed: BTreeSet = ids.into_iter().map(|id| id.as_ref().to_owned()).collect(); + let mut changed = false; + + for id in &observed { + if !self.added.contains_key(id) { + self.added.insert(id.clone(), observed_at_ms); + changed = true; + } + } - /// Persists the store to an arbitrary path (used in tests). - pub fn save_to(&self, path: &Path) -> Result<(), ConfigError> { - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent).map_err(|e| ConfigError::DirectoryCreation { - path: parent.display().to_string(), - source: e, - })?; + if complete { + let previous_len = self.added.len(); + self.added.retain(|id, _| observed.contains(id)); + changed |= self.added.len() != previous_len; } - let json = serde_json::to_string_pretty(&self.0).map_err(|e| ConfigError::Serialize { - path: path.display().to_string(), - source: e, - })?; - std::fs::write(path, json).map_err(|e| ConfigError::Io { - path: path.display().to_string(), - source: e, - })?; - Ok(()) - } - - /// Loads from an arbitrary path (used in tests). - pub fn load_from(path: &Path) -> Self { - std::fs::read_to_string(path) - .ok() - .and_then(|s| serde_json::from_str::>(&s).ok()) - .map(SoundMetaStore) - .unwrap_or_default() + + changed } } @@ -201,7 +200,10 @@ mod tests { store.set_volume("id1", 1.5); // Reset to default store.set("id1".to_owned(), SoundMeta::default()); - assert!(store.0.is_empty(), "default meta should be pruned from map"); + assert!( + store.custom.is_empty(), + "default meta should be pruned from map" + ); } #[test] @@ -252,6 +254,41 @@ mod tests { assert!(!store.is_favorite("any")); } + #[test] + fn corrupt_file_cannot_be_overwritten() { + let dir = tempdir().unwrap(); + let path = dir.path().join("bad.json"); + std::fs::write(&path, b"not json!!!").unwrap(); + let store = SoundMetaStore::load_from(&path); + + assert!(store.save_to(&path).is_err()); + assert_eq!(std::fs::read(&path).unwrap(), b"not json!!!"); + } + + #[test] + fn future_version_file_cannot_be_downgraded() { + let dir = tempdir().unwrap(); + let path = dir.path().join("future.json"); + let future = r#"{"version":999,"custom":{},"added":{"abc":42}}"#; + std::fs::write(&path, future).unwrap(); + let store = SoundMetaStore::load_from(&path); + + assert!(store.save_to(&path).is_err()); + assert_eq!(std::fs::read_to_string(&path).unwrap(), future); + } + + #[test] + fn malformed_envelope_without_version_cannot_be_overwritten() { + let dir = tempdir().unwrap(); + let path = dir.path().join("malformed-envelope.json"); + let malformed = r#"{"custom":{"abc":{"favorite":true}},"added":{"abc":42}}"#; + std::fs::write(&path, malformed).unwrap(); + let store = SoundMetaStore::load_from(&path); + + assert!(store.save_to(&path).is_err()); + assert_eq!(std::fs::read_to_string(&path).unwrap(), malformed); + } + #[test] fn is_default_detects_all_fields_at_default() { assert!(SoundMeta::default().is_default()); @@ -263,4 +300,81 @@ mod tests { .is_default() ); } + + #[test] + fn reconcile_added_stamps_new_ids_once() { + let mut store = SoundMetaStore::default(); + + assert!(store.reconcile_added(["first", "second"], 1_000, true)); + assert_eq!(store.added_ms("first"), Some(1_000)); + assert_eq!(store.added_ms("second"), Some(1_000)); + assert!(!store.reconcile_added(["first", "second"], 2_000, true)); + assert_eq!(store.added_ms("first"), Some(1_000)); + } + + #[test] + fn complete_reconcile_prunes_unseen_ids() { + let mut store = SoundMetaStore::default(); + store.reconcile_added(["kept", "removed"], 1_000, true); + + assert!(store.reconcile_added(["kept"], 2_000, true)); + assert_eq!(store.added_ms("kept"), Some(1_000)); + assert_eq!(store.added_ms("removed"), None); + } + + #[test] + fn partial_reconcile_preserves_unseen_ids() { + let mut store = SoundMetaStore::default(); + store.reconcile_added(["observed", "temporarily-missing"], 1_000, true); + + assert!(!store.reconcile_added(["observed"], 2_000, false)); + assert_eq!(store.added_ms("temporarily-missing"), Some(1_000)); + } + + #[test] + fn load_from_accepts_legacy_top_level_metadata_map() { + let dir = tempdir().unwrap(); + let path = dir.path().join("legacy.json"); + std::fs::write( + &path, + r#"{"abc":{"favorite":true,"volume":1.25,"display_name":"Honk"}}"#, + ) + .unwrap(); + + let store = SoundMetaStore::load_from(&path); + + assert!(store.is_favorite("abc")); + assert_eq!(store.get("abc").display_name.as_deref(), Some("Honk")); + assert_eq!(store.added_ms("abc"), None); + store.save_to(&path).unwrap(); + assert!(SoundMetaStore::load_from(&path).is_favorite("abc")); + } + + #[test] + fn versioned_store_round_trips_added_timestamps() { + let dir = tempdir().unwrap(); + let path = dir.path().join("meta.json"); + let mut store = SoundMetaStore::default(); + store.toggle_favorite("abc"); + store.reconcile_added(["abc"], 4_242, true); + + store.save_to(&path).unwrap(); + let loaded = SoundMetaStore::load_from(&path); + + assert!(loaded.is_favorite("abc")); + assert_eq!(loaded.added_ms("abc"), Some(4_242)); + let json = std::fs::read_to_string(path).unwrap(); + assert!(json.contains("\"version\"")); + assert!(json.contains("\"custom\"")); + assert!(json.contains("\"added\"")); + } + + #[test] + fn first_seen_timestamps_do_not_create_custom_metadata() { + let mut store = SoundMetaStore::default(); + + store.reconcile_added(["abc"], 1_000, true); + + assert!(store.get_ref("abc").is_none()); + } } diff --git a/src/state/sound_meta/persistence.rs b/src/state/sound_meta/persistence.rs new file mode 100644 index 0000000..33ef804 --- /dev/null +++ b/src/state/sound_meta/persistence.rs @@ -0,0 +1,103 @@ +use std::collections::{BTreeMap, HashMap}; +use std::path::{Path, PathBuf}; + +use serde::{Deserialize, Serialize}; + +use super::{CONFIG_DIR_NAME, META_FILE_NAME, META_FORMAT_VERSION, SoundMeta, SoundMetaStore}; +use crate::state::error::ConfigError; + +#[derive(Debug, Serialize, Deserialize)] +struct PersistedSoundMeta { + version: u32, + #[serde(default)] + custom: HashMap, + #[serde(default)] + added: BTreeMap, +} + +impl SoundMetaStore { + fn meta_path() -> Result { + let project_dirs = directories::ProjectDirs::from("", "", CONFIG_DIR_NAME) + .ok_or(ConfigError::NoConfigDir)?; + Ok(project_dirs.config_dir().join(META_FILE_NAME)) + } + + /// Loads from the default XDG path. Unreadable or unsupported data yields + /// an empty, write-protected store so startup cannot destroy the source. + pub fn load() -> Self { + let Ok(path) = Self::meta_path() else { + return Self::read_protected(); + }; + Self::load_from(&path) + } + + /// Persists the store to the default XDG path. + pub fn save(&self) -> Result<(), ConfigError> { + self.save_to(&Self::meta_path()?) + } + + /// Persists the store to an arbitrary path (used in tests). + pub fn save_to(&self, path: &Path) -> Result<(), ConfigError> { + if !self.writable { + return Err(ConfigError::UnsafeMetadataOverwrite { + path: path.display().to_string(), + }); + } + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).map_err(|source| ConfigError::DirectoryCreation { + path: parent.display().to_string(), + source, + })?; + } + let persisted = PersistedSoundMeta { + version: META_FORMAT_VERSION, + custom: self.custom.clone(), + added: self.added.clone(), + }; + let json = + serde_json::to_string_pretty(&persisted).map_err(|source| ConfigError::Serialize { + path: path.display().to_string(), + source, + })?; + std::fs::write(path, json).map_err(|source| ConfigError::Io { + path: path.display().to_string(), + source, + })?; + Ok(()) + } + + /// Loads from an arbitrary path (used in tests). + pub fn load_from(path: &Path) -> Self { + match std::fs::read_to_string(path) { + Ok(json) => Self::deserialize(&json).unwrap_or_else(Self::read_protected), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Self::default(), + Err(_) => Self::read_protected(), + } + } + + fn deserialize(json: &str) -> Option { + let value: serde_json::Value = serde_json::from_str(json).ok()?; + if value.get("version").is_some() { + return Self::from_versioned(value); + } + if value.get("custom").is_some() || value.get("added").is_some() { + return None; + } + + let custom = serde_json::from_value(value).ok()?; + Some(Self { + custom, + added: BTreeMap::new(), + writable: true, + }) + } + + fn from_versioned(value: serde_json::Value) -> Option { + let persisted: PersistedSoundMeta = serde_json::from_value(value).ok()?; + (persisted.version == META_FORMAT_VERSION).then_some(Self { + custom: persisted.custom, + added: persisted.added, + writable: true, + }) + } +}