From ed5682c88736d9aa01875dbefd273592cd5906d1 Mon Sep 17 00:00:00 2001 From: Artur Meski Date: Mon, 23 Mar 2026 19:34:11 +0000 Subject: [PATCH] Various issues including cover issues with MPD (corrupted/incomplete files) --- src/db.rs | 146 +++++++++++++++++++++++++++++++++++ src/enrich.rs | 203 +++++++++++++++++++++++++++++++++++++++++++++---- src/main.rs | 56 +++++++++++++- src/mpd.rs | 96 +++++++++++++++++++++-- src/watcher.rs | 50 +++++++++++- 5 files changed, 526 insertions(+), 25 deletions(-) diff --git a/src/db.rs b/src/db.rs index 9ae5ef1..c43cc31 100644 --- a/src/db.rs +++ b/src/db.rs @@ -747,6 +747,21 @@ pub fn reset_missing_cover_timestamps(conn: &Connection) -> Result { Ok(count) } +/// Reset `fetched_at` to NULL for cover-missing albums for one artist only. +/// +/// Matching is case-insensitive (`LOWER(artist) = LOWER(?1)`), so callers can +/// pass user input without worrying about exact casing. +pub fn reset_missing_cover_timestamps_for_artist(conn: &Connection, artist: &str) -> Result { + let count = conn.execute( + "UPDATE album_cache + SET fetched_at = NULL + WHERE cover_url IS NULL + AND LOWER(artist) = LOWER(?1)", + params![artist], + )?; + Ok(count) +} + pub fn uncached_albums(conn: &Connection) -> Result> { // Retry incomplete cache entries at most once per 7 days. let retry_before = (chrono::Local::now().naive_local() - chrono::Duration::days(7)) @@ -776,6 +791,27 @@ pub fn uncached_albums(conn: &Connection) -> Result> { rows.collect() } +/// Return all distinct albums with scrobbles for a specific artist. +/// +/// Matching is case-insensitive. Empty album names are excluded. +pub fn albums_for_artist(conn: &Connection, artist: &str) -> Result> { + let mut stmt = conn.prepare( + "SELECT DISTINCT s.artist, s.album + FROM scrobbles s + WHERE s.album != '' + AND LOWER(s.artist) = LOWER(?1) + ORDER BY s.artist, s.album", + )?; + + let rows = stmt.query_map(params![artist], |row| { + Ok(UncachedAlbum { + artist: row.get(0)?, + album: row.get(1)?, + }) + })?; + rows.collect() +} + /// Returns true if `genre` should rank below more specific genres. /// /// Single-word genres ("rock", "electronic", "ambient") are broad descriptors @@ -1847,4 +1883,114 @@ mod tests { "CAA cover should replace the local cover" ); } + + #[test] + fn test_albums_for_artist_case_insensitive() { + let conn = open_memory_db().unwrap(); + + insert_scrobble( + &conn, + &NewScrobble { + artist: "Pink Floyd".into(), + album: "The Division Bell".into(), + title: "Marooned".into(), + track_duration_secs: Some(329), + played_duration_secs: 200, + scrobbled_at: today_at("10:00:00"), + source: "MPRIS".into(), + }, + ) + .unwrap(); + insert_scrobble( + &conn, + &NewScrobble { + artist: "Pink Floyd".into(), + album: "The Wall".into(), + title: "Mother".into(), + track_duration_secs: Some(345), + played_duration_secs: 210, + scrobbled_at: today_at("10:05:00"), + source: "MPRIS".into(), + }, + ) + .unwrap(); + insert_scrobble( + &conn, + &NewScrobble { + artist: "Deftones".into(), + album: "White Pony".into(), + title: "Digital Bath".into(), + track_duration_secs: Some(291), + played_duration_secs: 200, + scrobbled_at: today_at("10:10:00"), + source: "MPRIS".into(), + }, + ) + .unwrap(); + + let albums = albums_for_artist(&conn, "pInK fLoYd").unwrap(); + assert_eq!(albums.len(), 2); + assert!( + albums + .iter() + .any(|a| a.artist == "Pink Floyd" && a.album == "The Division Bell") + ); + assert!( + albums + .iter() + .any(|a| a.artist == "Pink Floyd" && a.album == "The Wall") + ); + } + + #[test] + fn test_reset_missing_cover_timestamps_for_artist_only() { + let conn = open_memory_db().unwrap(); + + upsert_album_cache( + &conn, + &AlbumCacheEntry { + artist: "Pink Floyd".into(), + album: "The Division Bell".into(), + musicbrainz_id: Some("mbid-1".into()), + cover_url: None, + genre: None, + fetched_at: today_at("12:00:00"), + }, + ) + .unwrap(); + upsert_album_cache( + &conn, + &AlbumCacheEntry { + artist: "Deftones".into(), + album: "White Pony".into(), + musicbrainz_id: Some("mbid-2".into()), + cover_url: None, + genre: None, + fetched_at: today_at("12:01:00"), + }, + ) + .unwrap(); + + let updated = reset_missing_cover_timestamps_for_artist(&conn, "pink floyd").unwrap(); + assert_eq!(updated, 1); + + let pink_fetched: Option = conn + .query_row( + "SELECT fetched_at FROM album_cache WHERE artist = 'Pink Floyd' AND album = 'The Division Bell'", + [], + |r| r.get(0), + ) + .unwrap(); + assert!(pink_fetched.is_none()); + + let deftones_fetched: Option = conn + .query_row( + "SELECT fetched_at FROM album_cache WHERE artist = 'Deftones' AND album = 'White Pony'", + [], + |r| r.get(0), + ) + .unwrap(); + let expected = today_at("12:01:00"); + assert_eq!(deftones_fetched.as_deref(), Some(expected.as_str())); + } } diff --git a/src/enrich.rs b/src/enrich.rs index 8af313d..596379b 100644 --- a/src/enrich.rs +++ b/src/enrich.rs @@ -716,13 +716,14 @@ fn download_cover_with_fallback( mbid: &str, release_group_id: Option<&str>, covers_dir: &Path, + force_redownload: bool, ) -> Option { // Use the MBID as the filename regardless of which endpoint succeeds, // so we have a consistent cache key. let dest = covers_dir.join(format!("{}.jpg", mbid)); - // Skip download if we already have the file from a previous run. - if dest.exists() { + // Reuse cached file unless force mode explicitly asks for a refresh. + if should_reuse_existing_cover(dest.exists(), force_redownload) { return Some(dest.to_string_lossy().to_string()); } @@ -746,6 +747,11 @@ fn download_cover_with_fallback( None } +/// Decide whether an existing cover file should be reused as-is. +fn should_reuse_existing_cover(dest_exists: bool, force_redownload: bool) -> bool { + dest_exists && !force_redownload +} + /// Decode image bytes, downscale if either dimension exceeds `MAX_COVER_DIM`, /// and re-encode as JPEG at `COVER_JPEG_QUALITY`. /// @@ -794,6 +800,76 @@ fn itunes_cover_stem(artist: &str, album: &str) -> String { format!("itunes_{:016x}", hash) } +/// Build a relaxed normalised key for fuzzy text matching. +/// +/// Lowercases and keeps only alphanumeric characters and spaces, then +/// collapses whitespace. This makes matching resilient to punctuation, +/// symbols, and repeated spacing differences. +fn normalise_match_key(s: &str) -> String { + let mapped: String = s + .chars() + .map(|c| { + if c.is_alphanumeric() || c.is_whitespace() { + c + } else { + ' ' + } + }) + .collect(); + normalise_spaces(&mapped.to_ascii_lowercase()) +} + +/// Return true when two strings match exactly or one contains the other, +/// after relaxed normalisation. +fn loosely_matches(query: &str, candidate: &str) -> bool { + let q = normalise_match_key(query); + let c = normalise_match_key(candidate); + if q.is_empty() || c.is_empty() { + return false; + } + q == c || q.contains(&c) || c.contains(&q) +} + +/// Score an iTunes result for the requested artist/album. +/// +/// The score prefers exact artist/album matches, but still accepts useful +/// near-matches (for example "The Division Bell - Single" for album field). +fn score_itunes_result(result: &serde_json::Value, artist: &str, album: &str) -> Option { + let result_artist = result.get("artistName")?.as_str()?; + let result_album = result.get("collectionName")?.as_str()?; + let _ = result.get("artworkUrl100")?.as_str()?; + + if !loosely_matches(artist, result_artist) || !loosely_matches(album, result_album) { + return None; + } + + let artist_q = normalise_match_key(artist); + let artist_r = normalise_match_key(result_artist); + let album_q = normalise_match_key(album); + let album_r = normalise_match_key(result_album); + + let artist_score = if artist_q == artist_r { 4 } else { 2 }; + let album_score = if album_q == album_r { 4 } else { 2 }; + Some(artist_score + album_score) +} + +/// Pick the best-matching iTunes artwork URL for the requested artist/album. +fn pick_best_itunes_cover_url( + results: &[serde_json::Value], + artist: &str, + album: &str, +) -> Option { + let best = results + .iter() + .filter_map(|r| score_itunes_result(r, artist, album).map(|score| (score, r))) + .max_by_key(|(score, _)| *score)?; + + best.1 + .get("artworkUrl100")? + .as_str() + .map(|s| s.replace("100x100bb", "600x600bb")) +} + /// Search the iTunes Store for the front cover of an album. /// /// Uses Apple's public search API — no authentication required. Searches @@ -818,13 +894,9 @@ fn fetch_itunes_cover_url(client: &Client, artist: &str, album: &str) -> Option< let json: serde_json::Value = response.json().ok()?; let results = json.get("results")?.as_array()?; - // Take the first result's artwork URL and bump the resolution. - // Apple's URLs end in "100x100bb.jpg" — replace with "600x600bb". - results.iter().find_map(|r| { - r.get("artworkUrl100")? - .as_str() - .map(|s| s.replace("100x100bb", "600x600bb")) - }) + // Prefer a result whose artist and album actually match the query, so + // unrelated top hits don't produce obviously wrong covers. + pick_best_itunes_cover_url(results, artist, album) } /// Attempt to download an image from the given URL and save it to `dest`. @@ -889,6 +961,42 @@ pub fn run_enrich_targeted( quiet: bool, no_itunes: bool, ) { + run_enrich_targeted_with_options(conn, needed, quiet, no_itunes, false); +} + +/// Like [`run_enrich_targeted`], but supports force refresh for the provided +/// subset. +pub fn run_enrich_targeted_with_options( + conn: &Connection, + needed: &std::collections::HashSet<(String, String)>, + quiet: bool, + no_itunes: bool, + force: bool, +) { + if needed.is_empty() { + if !quiet { + eprintln!("No matching albums to enrich."); + } + return; + } + + if force { + let mut albums: Vec = needed + .iter() + .map(|(artist, album)| db::UncachedAlbum { + artist: artist.clone(), + album: album.clone(), + }) + .collect(); + albums.sort_by(|a, b| a.artist.cmp(&b.artist).then_with(|| a.album.cmp(&b.album))); + eprintln!( + "Force mode enabled: refreshing {} selected album(s).", + albums.len() + ); + run_enrich_albums(conn, albums, quiet, no_itunes, true); + return; + } + let all_uncached = match db::uncached_albums(conn) { Ok(v) => v, Err(e) => { @@ -901,7 +1009,7 @@ pub fn run_enrich_targeted( .into_iter() .filter(|a| needed.contains(&(a.artist.clone(), a.album.clone()))) .collect(); - run_enrich_albums(conn, albums, quiet, no_itunes); + run_enrich_albums(conn, albums, quiet, no_itunes, false); } /// Run the enrichment process: find all uncached albums and fetch their @@ -921,9 +1029,10 @@ pub fn run_enrich(conn: &Connection, force: bool, quiet: bool, no_itunes: bool) conn.execute("DELETE FROM album_cache", []) .expect("Failed to clear album_cache"); eprintln!("Cleared album cache (force mode)."); + eprintln!("Force mode enabled: refreshing existing cover files when re-fetching."); } let albums = db::uncached_albums(conn).expect("Failed to query uncached albums"); - run_enrich_albums(conn, albums, quiet, no_itunes); + run_enrich_albums(conn, albums, quiet, no_itunes, force); } /// Inner enrichment loop shared by `run_enrich` and `run_enrich_targeted`. @@ -932,6 +1041,7 @@ fn run_enrich_albums( albums: Vec, quiet: bool, no_itunes: bool, + force_redownload: bool, ) { if albums.is_empty() { if !quiet { @@ -1047,6 +1157,7 @@ fn run_enrich_albums( primary_mbid, release_group_id.as_deref(), &covers, + force_redownload, ); // If still nothing, try alternate candidate releases from the search. @@ -1151,8 +1262,13 @@ pub fn enrich_by_mbid( ); return; } - let result = - download_cover_with_fallback(&client, mbid, release_group_id.as_deref(), &covers); + let result = download_cover_with_fallback( + &client, + mbid, + release_group_id.as_deref(), + &covers, + false, + ); if result.is_some() { eprintln!(" Cover downloaded."); } else { @@ -1254,6 +1370,67 @@ mod tests { ); } + #[test] + fn test_pick_best_itunes_cover_url_prefers_matching_artist_album() { + let results = vec![ + serde_json::json!({ + "artistName": "Amiteve", + "collectionName": "The Division Bell - Single", + "artworkUrl100": "https://example.com/amiteve/100x100bb.jpg" + }), + serde_json::json!({ + "artistName": "Pink Floyd", + "collectionName": "The Division Bell", + "artworkUrl100": "https://example.com/pinkfloyd/100x100bb.jpg" + }), + ]; + + let chosen = pick_best_itunes_cover_url(&results, "Pink Floyd", "The Division Bell"); + assert_eq!( + chosen.as_deref(), + Some("https://example.com/pinkfloyd/600x600bb.jpg") + ); + } + + #[test] + fn test_pick_best_itunes_cover_url_rejects_wrong_artist() { + let results = vec![serde_json::json!({ + "artistName": "Amiteve", + "collectionName": "The Division Bell - Single", + "artworkUrl100": "https://example.com/amiteve/100x100bb.jpg" + })]; + + let chosen = pick_best_itunes_cover_url(&results, "Pink Floyd", "The Division Bell"); + assert!(chosen.is_none()); + } + + #[test] + fn test_pick_best_itunes_cover_url_accepts_album_suffix_variant() { + let results = vec![serde_json::json!({ + "artistName": "Pink Floyd", + "collectionName": "The Division Bell - Single", + "artworkUrl100": "https://example.com/pinkfloyd-single/100x100bb.jpg" + })]; + + let chosen = pick_best_itunes_cover_url(&results, "Pink Floyd", "The Division Bell"); + assert_eq!( + chosen.as_deref(), + Some("https://example.com/pinkfloyd-single/600x600bb.jpg") + ); + } + + #[test] + fn test_should_reuse_existing_cover_default_mode() { + assert!(should_reuse_existing_cover(true, false)); + assert!(!should_reuse_existing_cover(false, false)); + } + + #[test] + fn test_should_reuse_existing_cover_force_mode() { + assert!(!should_reuse_existing_cover(true, true)); + assert!(!should_reuse_existing_cover(false, true)); + } + #[test] fn test_pick_genre_string_prefers_genres_then_tags() { let genre = pick_genre_string( diff --git a/src/main.rs b/src/main.rs index be28c50..24cceb4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -189,6 +189,14 @@ enum Commands { #[arg(long)] retry_covers: bool, + /// Limit enrichment to albums by this artist (case-insensitive match). + /// + /// Useful when fixing one artist's covers/metadata without touching + /// the rest of the library. With `--force`, only this artist's albums + /// are re-fetched. + #[arg(long)] + artist: Option, + /// Skip the iTunes Search API when fetching cover art. Falls back /// directly to the Cover Art Archive (CAA). Useful if you prefer /// open-data sources or iTunes results are poor for your library. @@ -719,6 +727,7 @@ fn main() { online, force, retry_covers, + artist, no_itunes, no_mpd_covers, mpd_host, @@ -745,6 +754,31 @@ fn main() { } }; + let artist_needed = if let Some(ref artist_name) = artist { + match db::albums_for_artist(&conn, artist_name) { + Ok(v) => { + if v.is_empty() { + eprintln!("No scrobbled albums found for artist \"{}\".", artist_name); + return; + } + Some( + v.into_iter() + .map(|a| (a.artist, a.album)) + .collect::>(), + ) + } + Err(e) => { + eprintln!( + "[error] Failed to query albums for artist \"{}\": {}", + artist_name, e + ); + std::process::exit(1); + } + } + } else { + None + }; + // Run MPD cover extraction first (fast, local, no rate limits). // Pre-populating cover_url means the online enrichment that follows // will skip fetching covers for albums that already have one. @@ -753,7 +787,11 @@ fn main() { host: mpd_host, port: mpd_port, }; - mpd::run_mpd_cover_enrich(&mpd_cfg, &conn); + if let Some(ref needed) = artist_needed { + mpd::run_mpd_cover_enrich_targeted(&mpd_cfg, &conn, needed); + } else { + mpd::run_mpd_cover_enrich(&mpd_cfg, &conn); + } } // Online enrichment: MusicBrainz lookup for MBID + genre, Cover Art @@ -761,14 +799,26 @@ fn main() { // Reset the cooldown for cover-missing albums before the online // pass so they are picked up by uncached_albums. if retry_covers { - match db::reset_missing_cover_timestamps(&conn) { + let reset = if let Some(ref artist_name) = artist { + db::reset_missing_cover_timestamps_for_artist(&conn, artist_name) + } else { + db::reset_missing_cover_timestamps(&conn) + }; + + match reset { Ok(n) => eprintln!("Reset cooldown for {} album(s) missing covers.", n), Err(e) => eprintln!("[warn] Failed to reset cover timestamps: {}", e), } } if online { - enrich::run_enrich(&conn, force, false, no_itunes); + if let Some(ref needed) = artist_needed { + enrich::run_enrich_targeted_with_options( + &conn, needed, false, no_itunes, force, + ); + } else { + enrich::run_enrich(&conn, force, false, no_itunes); + } } } Commands::LastScrobble { db_path } => { diff --git a/src/mpd.rs b/src/mpd.rs index 3ba25f3..8e0b289 100644 --- a/src/mpd.rs +++ b/src/mpd.rs @@ -311,10 +311,34 @@ fn read_exact_bytes(conn: &mut MpdConn, n: u64) -> Option> { Some(buf) } -/// Read the single `OK\n` line that terminates a binary chunk response. -fn read_ok_line(conn: &mut MpdConn) { - let mut line = String::new(); - let _ = conn.reader.read_line(&mut line); +/// Consume the terminator after a binary chunk (`\nOK\n`). +/// +/// MPD's binary responses place a newline byte immediately after the raw +/// chunk payload, then an `OK` line. That means the first `read_line` after +/// `read_exact(binary_len)` often returns an empty line, not `OK`. +/// +/// Returns `Some(())` only when an `OK` line is observed; returns `None` on +/// EOF, protocol errors, or I/O errors. +fn read_chunk_terminator(conn: &mut MpdConn) -> Option<()> { + loop { + let mut line = String::new(); + conn.reader.read_line(&mut line).ok()?; + + let line = line.trim_end_matches('\n').trim_end_matches('\r'); + if line.is_empty() { + // Expected: this is the newline that follows the raw binary bytes. + continue; + } + if line == "OK" { + return Some(()); + } + if line.starts_with("ACK") { + return None; + } + + // Any other non-empty line here means the stream got out of sync. + return None; + } } // --------------------------------------------------------------------------- @@ -458,6 +482,7 @@ fn escape_mpd_string(s: &str) -> String { fn read_picture(conn: &mut MpdConn, file_uri: &str) -> Option> { let mut all_bytes: Vec = Vec::new(); let mut offset: u64 = 0; + let mut expected_total: Option = None; loop { // Request the next chunk starting at `offset`. @@ -467,6 +492,14 @@ fn read_picture(conn: &mut MpdConn, file_uri: &str) -> Option> { // Parse the `size`, `type`, and `binary` header fields. let (total_size, chunk_size) = read_picture_header(conn)?; + if total_size > 0 { + match expected_total { + None => expected_total = Some(total_size), + Some(prev) if prev != total_size => return None, + Some(_) => {} + } + } + // `binary: 0` means no embedded picture (or we've read everything). if chunk_size == 0 { // If we have already accumulated bytes, we're done; otherwise there @@ -479,7 +512,7 @@ fn read_picture(conn: &mut MpdConn, file_uri: &str) -> Option> { all_bytes.extend_from_slice(&chunk); // Each chunk response is terminated by a bare `OK` line. - read_ok_line(conn); + read_chunk_terminator(conn)?; offset += chunk_size; @@ -489,6 +522,13 @@ fn read_picture(conn: &mut MpdConn, file_uri: &str) -> Option> { } } + // If MPD declared a total byte size, ensure we received all of it. + if let Some(total) = expected_total + && all_bytes.len() as u64 != total + { + return None; + } + if all_bytes.is_empty() { None } else { @@ -1101,8 +1141,28 @@ fn run_mpd_cover_enrich_albums( mod tests { use super::*; use std::cell::RefCell; + use std::io::Cursor; use std::rc::Rc; + struct NullWriter; + + impl Write for NullWriter { + fn write(&mut self, buf: &[u8]) -> io::Result { + Ok(buf.len()) + } + + fn flush(&mut self) -> io::Result<()> { + Ok(()) + } + } + + fn fake_conn(input: &[u8]) -> MpdConn { + MpdConn { + reader: BufReader::new(Box::new(Cursor::new(input.to_vec()))), + writer: Box::new(NullWriter), + } + } + fn state( file: &str, artist: &str, @@ -1215,6 +1275,32 @@ mod tests { assert_eq!(parse_mpd_state("unknown"), PlayerStatus::Stopped); } + // ----------------------------------------------------------------------- + // readpicture protocol handling + // ----------------------------------------------------------------------- + + #[test] + fn test_read_chunk_terminator_consumes_blank_then_ok() { + let mut conn = fake_conn(b"\nOK\n"); + assert!(read_chunk_terminator(&mut conn).is_some()); + } + + #[test] + fn test_read_picture_reads_multiple_chunks_fully() { + // Two chunks: "hello" + " world!" = 12 bytes total. + let mut conn = + fake_conn(b"size: 12\nbinary: 5\nhello\nOK\nsize: 12\nbinary: 7\n world!\nOK\n"); + let bytes = read_picture(&mut conn, "music/test.flac").unwrap(); + assert_eq!(bytes, b"hello world!"); + } + + #[test] + fn test_read_picture_rejects_truncated_transfer() { + // MPD says total is 12, but only 5 bytes arrive before binary: 0. + let mut conn = fake_conn(b"size: 12\nbinary: 5\nhello\nOK\nsize: 12\nbinary: 0\nOK\n"); + assert!(read_picture(&mut conn, "music/test.flac").is_none()); + } + // ----------------------------------------------------------------------- // dispatch_events // ----------------------------------------------------------------------- diff --git a/src/watcher.rs b/src/watcher.rs index 6c6fddf..17c4bff 100644 --- a/src/watcher.rs +++ b/src/watcher.rs @@ -127,6 +127,9 @@ impl CurrentTrack { /// track, accumulated across multiple play/pause cycles pub struct ScrobbleTracker { current_track: Option, + /// Local timestamp captured when tracking for the current track starts. + /// Used for report period attribution instead of evaluation time. + track_started_at: Option, is_playing: bool, has_seen_status: bool, playing_since: Option, @@ -140,6 +143,7 @@ impl ScrobbleTracker { pub fn new(scrobble_fn: F, source: String) -> Self { Self { current_track: None, + track_started_at: None, is_playing: false, has_seen_status: false, playing_since: None, @@ -205,6 +209,7 @@ impl ScrobbleTracker { title, duration_us, }); + self.track_started_at = Some(chrono::Local::now().naive_local()); self.accumulated_secs = 0.0; // Do not blindly assume metadata implies Playing. If we've @@ -284,8 +289,10 @@ impl ScrobbleTracker { // Only scrobble if the user listened for at least the threshold duration. if self.accumulated_secs >= threshold { - let now = chrono::Local::now() - .naive_local() + let attribution_time = self + .track_started_at + .take() + .unwrap_or_else(|| chrono::Local::now().naive_local()) .format("%Y-%m-%dT%H:%M:%S") .to_string(); @@ -298,11 +305,13 @@ impl ScrobbleTracker { title: track.title, track_duration_secs: track_dur, played_duration_secs: self.accumulated_secs.round() as i64, - scrobbled_at: now, + scrobbled_at: attribution_time, source: self.source.clone(), }; (self.scrobble_fn)(scrobble); } + + self.track_started_at = None; } } } @@ -446,6 +455,8 @@ pub fn create_db_tracker( #[cfg(test)] pub struct TestableTracker { current_track: Option, + /// Simulated timestamp for when current track tracking started. + track_started_at_secs: Option, is_playing: bool, has_seen_status: bool, /// Simulated timestamp (in seconds) when the current Playing stretch began. @@ -465,6 +476,7 @@ impl TestableTracker { pub fn new() -> Self { Self { current_track: None, + track_started_at_secs: None, is_playing: false, has_seen_status: false, playing_since_secs: None, @@ -499,17 +511,20 @@ impl TestableTracker { let threshold = track.threshold_secs(); if self.accumulated_secs >= threshold { let track_dur = track.duration_secs(); + let attribution_secs = self.track_started_at_secs.take().unwrap_or(self.clock_secs); let scrobble = NewScrobble { artist: track.artist, album: track.album, title: track.title, track_duration_secs: track_dur, played_duration_secs: self.accumulated_secs.round() as i64, - scrobbled_at: format!("test-time-{}", self.clock_secs), + scrobbled_at: format!("test-time-{}", attribution_secs), source: self.source.clone(), }; self.scrobbled.push(scrobble); } + + self.track_started_at_secs = None; } } @@ -541,6 +556,7 @@ impl TestableTracker { title, duration_us, }); + self.track_started_at_secs = Some(self.clock_secs); self.accumulated_secs = 0.0; if self.has_seen_status { self.playing_since_secs = if self.is_playing { @@ -1014,6 +1030,32 @@ mod tests { assert_eq!(tracker.scrobbled.len(), 0); } + #[test] + fn test_scrobble_timestamp_uses_track_start_time() { + // Attribute the scrobble to when tracking started, not when the + // outgoing track was evaluated. + let mut tracker = TestableTracker::new(); + + tracker.advance_time(1_000.0); + tracker.handle_event(Event::Metadata { + artist: "Burial".into(), + album: "Untrue".into(), + title: "Untrue".into(), + duration_us: Some(60_000_000), // threshold = 30s + }); + + tracker.advance_time(35.0); + tracker.handle_event(Event::Metadata { + artist: "Deftones".into(), + album: "White Pony".into(), + title: "Digital Bath".into(), + duration_us: Some(291_000_000), + }); + + assert_eq!(tracker.scrobbled.len(), 1); + assert_eq!(tracker.scrobbled[0].scrobbled_at, "test-time-1000"); + } + #[test] fn test_stop_below_threshold_no_scrobble() { // Play a track for 3 seconds then stop — should NOT be scrobbled,