Skip to content

Fix: delete stale Download objects when clearing cache#663

Open
GeiserX wants to merge 2 commits into
BLeeEZ:masterfrom
GeiserX:fix/delete-cache-stale-downloads
Open

Fix: delete stale Download objects when clearing cache#663
GeiserX wants to merge 2 commits into
BLeeEZ:masterfrom
GeiserX:fix/delete-cache-stale-downloads

Conversation

@GeiserX
Copy link
Copy Markdown

@GeiserX GeiserX commented Apr 1, 2026

Summary

Root Cause

Two issues combine to produce the bug:

1. Stale DownloadMO after cache clear (preventive fix — commit 1)

deleteCacheFinalStep (called by "Delete downloaded songs and podcast episodes") sets relFilePath = nil but leaves the associated DownloadMO Core Data relationship intact. This follows the same pattern already used when resolving duplicate songs (line 185-186) and podcast episodes (line 217-218), which already delete the associated DownloadMO via context.delete(download).

2. Overly restrictive fetch predicate (self-healing fix — commit 2)

getSongsForCompleteLibraryDownload requires both predicates to match:

NSPredicate(format: "%K == nil", #keyPath(SongMO.relFilePath)),  // ✅ cleared by cache delete
NSPredicate(format: "%K == nil", #keyPath(SongMO.download)),      // ❌ stale DownloadMO still present

The download == nil predicate is redundant with DownloadRequestManager.addLowPrio, which already correctly handles existing downloads: if the song is not cached (!object.isCached), it resets the download and re-queues it. Songs that are already cached are skipped. Removing this predicate allows addLowPrio's own deduplication logic to handle both fresh and stale state.

Why both commits

  • Commit 1 (preventive): Deletes DownloadMO during cache clear so the stale state is never created going forward.
  • Commit 2 (migratory): Removes the redundant download == nil predicate so users upgrading from older builds self-heal on next "Download all" without manual cleanup.

Test Plan

  • Fresh account: sync library → "Download all songs in library" → downloads start normally
  • Cache clear cycle: let some downloads complete → "Delete downloaded songs and podcast episodes" → "Download all songs in library" → second run queues songs successfully
  • Already-broken account from older build: upgrade to patched build → "Download all songs in library" → stale state self-heals and downloads resume
  • Normal download flow (without cache clear) still works as before
  • Songs currently being downloaded are not disrupted by a concurrent "Download all"

GeiserX added 2 commits April 1, 2026 15:34
When "Delete downloaded songs and podcast episodes" clears the cache,
`deleteCacheFinalStep` sets `relFilePath` to nil but leaves the
associated `DownloadMO` relationship intact.

`getSongsForCompleteLibraryDownload` requires BOTH `relFilePath == nil`
AND `download == nil` to consider a song eligible for download. The
stale `DownloadMO` causes every song to be filtered out, making
"Download all songs in library" silently do nothing after a cache clear.

This follows the same pattern used when deleting songs (lines 185-186)
and podcast episodes (lines 217-218), which already delete the
associated `DownloadMO` via `context.delete(download)`.

Fixes BLeeEZ#662
The previous commit prevents new stale DownloadMOs from being
created during cache clear. However, users upgrading from older
builds may already have stale DownloadMOs (download != nil but
relFilePath == nil), which still block getSongsForCompleteLibraryDownload.

Remove the download == nil predicate from the fetch. This is safe
because DownloadRequestManager.addLowPrio already handles existing
downloads correctly: if the song is not cached (!object.isCached),
it resets the download and re-queues it (line 106). Songs that are
already cached are skipped (line 111). So the predicate was
redundant with addLowPrio's own deduplication logic.

This makes the fix both preventive (commit 1) and migratory
(this commit) — existing broken accounts self-heal on next
"Download all songs in library" without requiring manual cleanup.
@GeiserX
Copy link
Copy Markdown
Author

GeiserX commented Apr 1, 2026

Validation Results

Tested both commits on macOS (patched build from this PR branch) against Navidrome with ALAC library (~1636 songs).

Repro matrix

# Account URL Cache state before test Action Expected Result
1 192.168.99.99:4534 (LAN IP) Fresh account, no stale DownloadMOs "Download all songs" Downloads start PASS (worked even before fix — clean bucket)
2 my.tail-scale.ts.net:4534 (hostname) 1655 stale DownloadMOs (download exists, relFilePath == nil) "Download all songs" Downloads start (self-healing) PASS — all 1655 songs re-queued and streaming
3 Either account After cache clear + re-download "Download all songs" Downloads start PASS — commit 1 prevents stale state from forming

Key observations

  • Account 1 (hostname) had 1655 DownloadMO objects with no associated cached file — classic stale state from the old deleteCacheFinalStep not cleaning up downloads.
  • Before the fix, "Download all songs" returned 0 candidates because getSongsForCompleteLibraryDownload required download == nil.
  • After the fix (commit 2), addLowPrio correctly detected !object.isCached on the existing stale downloads, reset them, and re-queued — confirmed by Navidrome streaming logs showing all songs downloading in sequence.
  • Commit 1 ensures this stale state won't recur for future cache clears.

Navidrome log excerpt (hostname account, post-fix)

Streaming file artist=U3 format=raw originalFormat=m4a title=""40""
Streaming file artist="The Rolling Mones" format=raw originalFormat=m4a title="(I Can't Get No) Satisfaction"
Streaming file artist="David Mowie" format=raw originalFormat=m4a title="\"Heroes\""
Streaming file artist="Blue Pinkster Cult" format=raw originalFormat=m4a title="(Don't Fear) The Reaper"
Streaming file artist=Pandiohead format=raw originalFormat=m4a title="15 Step"
... (1655 songs total)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] "Download all songs in library" silently does nothing after switching download format or clearing cache

1 participant