fix(library): apply per-track artwork fix to second scan loop, suppress legitimate users

A code review of the previous commit surfaced a second scan loop in
legacy_compat.rs that had the same destructive update_album_group_artwork
pattern. Apply the same fix: per-folder cache + drop the destructive
call. Same intent and shape as the main loop fix.

The deprecated marker also surfaced three callers that are NOT bugs
and stay as-is, with #[allow(deprecated)] + a comment to make the
intent explicit:
- CUE ingestion: a CUE sheet is one physical album, so a single cover
  for all CUE tracks is semantically correct.
- v2_library_set_album_artwork: user-initiated set-cover-for-album, by
  design propagates to all tracks.
- Discogs backfill: only fires for albums with no existing artwork, so
  there is nothing per-track to destroy.

cc
This commit is contained in:
vicrodh committed 2026-05-06 16:38:14 -06:00
1 parent 7eba2749f5
commit 523cce46fd
2 files changed
+29 -10

No files matched your search

+23 -10
View File
@@ -1147,6 +1147,9 @@ async fn library_process_cue_file(
}
if let (Some(path), false) = (artwork_path.as_ref(), group_key.is_empty()) {
// CUE = single album; propagating one cover to all CUE tracks is
// semantically correct (they all share the same physical disc).
#[allow(deprecated)]
let _ = db.update_album_group_artwork(&group_key, path);
}
@@ -1602,6 +1605,11 @@ pub async fn v2_library_scan_folder(
})
.collect();
// Per-folder cache for find_folder_artwork (same pattern as the main
// scan loop above; replaces the destructive update_album_group_artwork
// shortcut that destroyed unique embedded covers).
let mut folder_artwork_cache: HashMap<PathBuf, Option<String>> = HashMap::new();
for audio_path in &scan_result.audio_files {
if state.scan_cancel.load(Ordering::Relaxed) {
let mut progress = state.scan_progress.lock().await;
@@ -1631,32 +1639,37 @@ pub async fn v2_library_scan_folder(
let mut artwork_path =
MetadataExtractor::extract_artwork(&canonical_path, &artwork_cache);
if artwork_path.is_none() {
if let Some(folder_art) = MetadataExtractor::find_folder_artwork(
canonical_path.as_path(),
Some(&track.album),
) {
let folder = canonical_path
.parent()
.map(|p| p.to_path_buf())
.unwrap_or_else(|| canonical_path.clone());
let cached = folder_artwork_cache
.entry(folder)
.or_insert_with(|| {
MetadataExtractor::find_folder_artwork(
canonical_path.as_path(),
Some(&track.album),
)
})
.clone();
if let Some(folder_art) = cached {
artwork_path = MetadataExtractor::cache_artwork_file(
std::path::Path::new(&folder_art),
&artwork_cache,
);
}
}
track.artwork_path = artwork_path.clone();
track.artwork_path = artwork_path;
let guard__ = state.db.lock().await;
let db = guard__
.as_ref()
.ok_or("No active session - please log in")?;
let group_key = track.album_group_key.clone();
if let Err(e) = db.insert_track(&track) {
all_errors.push(ScanError {
file_path: path_str,
error: e.to_string(),
});
} else if let Some(path) = artwork_path.as_ref() {
if !group_key.is_empty() {
let _ = db.update_album_group_artwork(&group_key, path);
}
}
}
Err(e) => {
+6
View File
@@ -1148,6 +1148,9 @@ pub async fn v2_library_set_album_artwork(
.ok_or_else(|| "Failed to cache artwork file".to_string())?;
let guard = state.db.lock().await;
let db = guard.as_ref().ok_or("No active session - please log in")?;
// User-initiated set-album-artwork command: writing one cover to all
// tracks in the group is the explicit user intent here.
#[allow(deprecated)]
db.update_album_group_artwork(&album_group_key, &cached_path)
.map_err(|e| e.to_string())?;
Ok(cached_path)
@@ -2742,6 +2745,9 @@ pub async fn v2_library_fetch_missing_artwork(
let db = guard__
.as_ref()
.ok_or("No active session - please log in")?;
// Backfill: only runs for albums with NO existing artwork, so there's
// nothing per-track to destroy. Group-level write is correct here.
#[allow(deprecated)]
if db
.update_album_group_artwork(&group_key, &artwork_path)
.is_ok()