fix(downloads): queue the whole album, and make every queued track findable offline
An album download put a handful of its tracks on the device while the button reported the album as downloaded. Two independent gaps, one shared cause. - `download_album` read its track list from `items WHERE album_id = ?` — the local catalog cache. Jellyfin does not return `AlbumId` on every listing endpoint, so tracks cached from one of those sit in `items` with a NULL `album_id` and are invisible to that query. On the reported database three whole albums (18, 12 and 9 tracks) had it NULL on every track; a partially linked album queued only the linked subset. - The frontend then resolved one stream URL per track from its own list and paired it with the returned row ids by position. The ids came back in the backend's `index_number` order over a different set of rows, so a row could be handed another track's URL and any track past the end of the shorter list was never started. On Android that loop also stopped wherever the webview was suspended. - `album_id` is what `OfflineRepository::get_items` joins a track to its album on, so a track that did download stayed invisible under its album offline — the same missing link seen from the other side. The operation now belongs to Rust end to end: - `HybridRepository::get_album_tracks` asks the server what the album contains. Cache-first `get_items` is right for browsing and wrong for deciding what to download; it errors offline so the caller falls back to the ungated local catalog, keeping the queue-while-offline flow. - `queue_album_tracks` writes the album link onto every track it queues, and creates an `items` row for tracks the cache has never seen. - Stream URLs resolve here, through the existing reconnect resolver, now scoped to the rows just queued so one album cannot start every unrelated pending row. Only the album id crosses the IPC boundary. - `album_file_names` gives each track its own file. A title repeated inside one album (deluxe edition, two discs) mapped to one path, so those downloads overwrote each other. Re-tapping download on a broken album heals it: missing tracks are queued and the tracks already on disk get their link. `download_series`/`download_season` still derive their episode lists from the cache the same way and want the same treatment. DR-173, UT-170..172. Rust 673 tests, frontend 975 tests, svelte-check and check:boundary clean. Note: this tree is shared with a concurrent session. Only the files above are committed; docs/traceability.md is left to be regenerated once that work lands.
This commit is contained in:
@@ -327,6 +327,7 @@ Internal architecture, components, and application logic.
|
|||||||
| DR-167 | Each downloaded library shows only its own media. Cached items carry no link back to their library — `library_id` and `parent_id` are NULL on every row ([[offline-libraries-never-cached]]) — so `get_downloaded_items` matched the library branch with `EXISTS (SELECT 1 FROM libraries l WHERE l.id = ?)`, which asserts only that the requested library *exists* and never constrains the item to it. Opening any downloaded library therefore listed every downloaded top-level item on the server: films under Music, albums under TV. The sibling query that decides which libraries *appear* already carried the right rule — a `collection_type` ↔ `item_type` mapping — so the two disagreed about the same question. That mapping is now the named constant `LIBRARY_HOLDS_ITEM`, used by both, and a library of unknown collection type still keeps everything rather than being emptied by a rule that cannot classify it. The taxonomy stays in Rust, never the frontend | Downloads | UR-055 | Done |
|
| DR-167 | Each downloaded library shows only its own media. Cached items carry no link back to their library — `library_id` and `parent_id` are NULL on every row ([[offline-libraries-never-cached]]) — so `get_downloaded_items` matched the library branch with `EXISTS (SELECT 1 FROM libraries l WHERE l.id = ?)`, which asserts only that the requested library *exists* and never constrains the item to it. Opening any downloaded library therefore listed every downloaded top-level item on the server: films under Music, albums under TV. The sibling query that decides which libraries *appear* already carried the right rule — a `collection_type` ↔ `item_type` mapping — so the two disagreed about the same question. That mapping is now the named constant `LIBRARY_HOLDS_ITEM`, used by both, and a library of unknown collection type still keeps everything rather than being emptied by a rule that cannot classify it. The taxonomy stays in Rust, never the frontend | Downloads | UR-055 | Done |
|
||||||
| DR-168 | Pause and resume actually stop and restart the bytes. `pause_download` wrote `status = 'paused'` and did nothing else, and no cancellation existed anywhere in the download stack — no token, no flag, no abort — so the streaming task ran on, kept writing, and overwrote the row with `completed`/`failed` when it finished: the row flicked to "paused" and undid itself. `resume_download` had the mirror defect, flipping the row to `pending` without calling `pump_download_queue`; the pump runs when something calls it rather than polling, so a resumed download sat untouched until an unrelated event happened to pump the queue. A per-download stop flag (`download::stop`) is the missing half — a module-level registry because the two sides never meet, the command holding Tauri state and the worker running detached in `async_runtime::spawn`. The worker reads it between chunks and on retry (so a pause is not swallowed by a 45-second backoff), flushes, and returns `Stopped`, which is deliberately **not** retryable and **not** recorded as a failure: the `.part` file is left intact because that is exactly what the resume's Range request continues from. Registering returns a *fresh* flag, or a resumed download would inherit the pause that stopped it and halt instantly. Cancel and `clear_stale_downloads` signal it too, so neither deletes a file still being written | Downloads | UR-055 | Done |
|
| DR-168 | Pause and resume actually stop and restart the bytes. `pause_download` wrote `status = 'paused'` and did nothing else, and no cancellation existed anywhere in the download stack — no token, no flag, no abort — so the streaming task ran on, kept writing, and overwrote the row with `completed`/`failed` when it finished: the row flicked to "paused" and undid itself. `resume_download` had the mirror defect, flipping the row to `pending` without calling `pump_download_queue`; the pump runs when something calls it rather than polling, so a resumed download sat untouched until an unrelated event happened to pump the queue. A per-download stop flag (`download::stop`) is the missing half — a module-level registry because the two sides never meet, the command holding Tauri state and the worker running detached in `async_runtime::spawn`. The worker reads it between chunks and on retry (so a pause is not swallowed by a 45-second backoff), flushes, and returns `Stopped`, which is deliberately **not** retryable and **not** recorded as a failure: the `.part` file is left intact because that is exactly what the resume's Range request continues from. Registering returns a *fresh* flag, or a resumed download would inherit the pause that stopped it and halt instantly. Cancel and `clear_stale_downloads` signal it too, so neither deletes a file still being written | Downloads | UR-055 | Done |
|
||||||
| DR-169 | Partial files are actually reaped. The worker named its sidecar with `Path::with_extension("part")`, which *replaces* the extension — `movie.mp4` became `movie.part` — while every cleanup path deleted `"{file_path}.part"`, i.e. `movie.mp4.part`. The two never matched, so the partial file of every cancelled or failed download stayed on disk indefinitely, invisible to the disk-usage totals because no `downloads` row pointed at it. `partial_path` appends instead, is the single definition both the writer and the cleaners use, and incidentally removes a collision the old form had, where `movie.mp4` and `movie.mkv` mapped to one `movie.part` | Downloads | UR-055 | Done |
|
| DR-169 | Partial files are actually reaped. The worker named its sidecar with `Path::with_extension("part")`, which *replaces* the extension — `movie.mp4` became `movie.part` — while every cleanup path deleted `"{file_path}.part"`, i.e. `movie.mp4.part`. The two never matched, so the partial file of every cancelled or failed download stayed on disk indefinitely, invisible to the disk-usage totals because no `downloads` row pointed at it. `partial_path` appends instead, is the single definition both the writer and the cleaners use, and incidentally removes a collision the old form had, where `movie.mp4` and `movie.mkv` mapped to one `movie.part` | Downloads | UR-055 | Done |
|
||||||
|
| DR-173 | Downloading an album queues the **whole** album, and every track it queued is findable offline afterwards. Two independent gaps left an album with a handful of its tracks on the device while the button reported the album as downloaded. First, `download_album` took its track list from `items WHERE album_id = ?` — the local catalog cache. Jellyfin does not return `AlbumId` on every listing endpoint, so tracks cached by one of those endpoints sit in `items` with a NULL `album_id` and are invisible to that query; on the reporter's database three whole albums (18, 12 and 9 tracks) had it NULL on *every* track, so "download album" would have queued nothing for them, and a partially-linked album queued only the linked subset. Second, the frontend then resolved one stream URL per track from its own list and paired it with the returned row ids **by position** — a pairing with no basis, since the ids came back in the backend's `index_number` order over a different set of rows, so a row could be handed another track's URL and any track past the end of the shorter list was never started at all; on Android that loop also stopped wherever the webview was suspended. The same `album_id` is what `OfflineRepository::get_items` joins a track to its album on, so a track that did download stayed invisible under its album offline — the two halves of the same missing link. The operation now belongs to Rust end to end: `HybridRepository::get_album_tracks` asks the **server** what the album contains (cache-first `get_items` is right for browsing and wrong for deciding what to download) and errors offline so the caller falls back to the ungated local catalog, keeping the queue-while-offline flow; `queue_album_tracks` writes the album link onto every track it queues — queuing a track *is* the statement that it belongs to the album, rather than something to hope a listing endpoint recorded — and the stream URLs are resolved here through the existing reconnect resolver, now scoped to the rows just queued so one album cannot start every unrelated pending row. Nothing crosses the IPC boundary but the album id. Re-queuing a broken album heals it: the missing tracks are added and the tracks already on disk get their link. `download_series`/`download_season` still derive their episode lists from the cache the same way and want the same treatment | Downloads | UR-018, UR-055 | Done |
|
||||||
| DR-170 | Downloads at a chosen bitrate are no longer corrupted by their own retries. Only the `original` preset asks for `Static=true`; every other rung requests a **transcode**, which Jellyfin serves chunked, with no `Content-Length`, and cannot byte-seek — so it ignores `Range` and answers `200` with the whole stream from the beginning rather than `206` with the requested tail. The worker sent the Range header whenever a `.part` existed and appended the body unconditionally, so each retry and each resume concatenated a fresh copy of the entire transcode onto the bytes already on disk: the file grew past its real size and would not play, which is why "downloads for different bitrates" stayed broken after the `videoBitRate` casing fix (DR-adc460f3) corrected the *request*. `resume_offset` makes the response decide — append only on a `206`, otherwise truncate and take the stream from the top — and the total size is computed from that offset rather than from a partial length the server never agreed to | Downloads | UR-071 | Done |
|
| DR-170 | Downloads at a chosen bitrate are no longer corrupted by their own retries. Only the `original` preset asks for `Static=true`; every other rung requests a **transcode**, which Jellyfin serves chunked, with no `Content-Length`, and cannot byte-seek — so it ignores `Range` and answers `200` with the whole stream from the beginning rather than `206` with the requested tail. The worker sent the Range header whenever a `.part` existed and appended the body unconditionally, so each retry and each resume concatenated a fresh copy of the entire transcode onto the bytes already on disk: the file grew past its real size and would not play, which is why "downloads for different bitrates" stayed broken after the `videoBitRate` casing fix (DR-adc460f3) corrected the *request*. `resume_offset` makes the response decide — append only on a `206`, otherwise truncate and take the stream from the top — and the total size is computed from that offset rather than from a partial length the server never agreed to | Downloads | UR-071 | Done |
|
||||||
| DR-172 | Native Android video is opt-in again, because as a default it shipped as **audio with no picture**. DR-161 flipped `experimentalNativeVideo` on so picture-in-picture could shrink a real video surface; on a device that produced sound and a blank screen. The decode path was never the problem — logcat showed ExoPlayer running (`Position update` ticks) and feeding a live `SurfaceView` with an active BufferQueue. The compositing was: the SurfaceView sits *behind* the WebView, and the step that clears the opaque layers above it never took effect, with `WebView transparent = false` logged and `= true` never appearing. So the video rendered correctly the whole time, behind an opaque page. This is exactly the defect the flag existed to contain — `VideoPlayer.scrubRegression.test.ts` had recorded that "the native SurfaceView has never been visible through the webview" — and enabling it by default shipped a verified decode path on top of an unverified display path. Reverting costs nothing that matters: PiP does not depend on it (DR-160 drives PiP from the WebView `<video>`), and working video outranks PiP showing a native surface. The flag stays available in Settings, now described as incomplete rather than as a performance win, and the scrub-regression mocks that were made explicit under DR-161 are kept explicit so those tests state which path they guard rather than inheriting a default that has now moved twice. Fixing the compositing is the prerequisite for trying this default again | UI | UR-003, UR-004, UR-041 | Done |
|
| DR-172 | Native Android video is opt-in again, because as a default it shipped as **audio with no picture**. DR-161 flipped `experimentalNativeVideo` on so picture-in-picture could shrink a real video surface; on a device that produced sound and a blank screen. The decode path was never the problem — logcat showed ExoPlayer running (`Position update` ticks) and feeding a live `SurfaceView` with an active BufferQueue. The compositing was: the SurfaceView sits *behind* the WebView, and the step that clears the opaque layers above it never took effect, with `WebView transparent = false` logged and `= true` never appearing. So the video rendered correctly the whole time, behind an opaque page. This is exactly the defect the flag existed to contain — `VideoPlayer.scrubRegression.test.ts` had recorded that "the native SurfaceView has never been visible through the webview" — and enabling it by default shipped a verified decode path on top of an unverified display path. Reverting costs nothing that matters: PiP does not depend on it (DR-160 drives PiP from the WebView `<video>`), and working video outranks PiP showing a native surface. The flag stays available in Settings, now described as incomplete rather than as a performance win, and the scrub-regression mocks that were made explicit under DR-161 are kept explicit so those tests state which path they guard rather than inheriting a default that has now moved twice. Fixing the compositing is the prerequisite for trying this default again | UI | UR-003, UR-004, UR-041 | Done |
|
||||||
| DR-171 | A downloaded video keeps audio the device can actually decode. `original` quality asked for `Static=true`, which hands back the source file byte-for-byte — E-AC-3/AC-3/DTS/TrueHD track included — and video is rendered on both platforms by the webview `<video>` element, which decodes none of them. Streaming already knew this: DR-149 judges the track the server would serve against `WEBVIEW_AUDIO_CODECS` and forces a transcode over Jellyfin's own direct-play offer, because 10.11.5 honours a `DirectPlayProfile`'s container and video codec but ignores its audio codec. The download path never consulted that policy, so the *same film* had sound when streamed and played as picture in silence once downloaded — and offline a download is the only source a video has, so there was no working path left to fall back to. The rule is now one rule: `served_audio_codec` picks the track the server will serve (the default, or the first when none is marked) and both callers judge it, the streaming verdict staying a bool and the download path needing the codec itself so it can say what to re-encode. Only the audio is re-encoded — `allowVideoStreamCopy=true` keeps an h264 source's picture byte-for-byte and no bitrate or resolution cap is added, so `original` still means original quality; a source the webview could not have rendered anyway (HEVC) becomes h264 as a side effect, which is the only form of it that would have played. The decision is per item rather than blanket because the transcode costs the byte-range resumability `Static=true` gives the download worker (see DR-170 for what a chunked, length-less response does to a resume), so a file whose audio already plays keeps the direct copy. An unknown codec — item not fetchable, or the server named none — changes nothing: the policy only ever *adds* a transcode, so it cannot make a working download worse. The codec set judged against is the **webview's**, not the platform's, even though DR-161 made ExoPlayer the Android default: `experimentalNativeVideo` is a user setting, a downloaded file outlives whatever it was set to when the file arrived, and the narrow list is the only one that holds on both sides of it — at the cost of a Dolby-licensed device re-encoding a track its ExoPlayer could have played. `resolve_video_download_url` is the single entrance for all three resolution sites (the frontend's per-item command, the bulk series/season enqueue, and the offline-queued resume), since the pure builder cannot look a codec up and a caller that forgets to is exactly how the silent downloads shipped. **Files already downloaded stay silent** — the bytes on disk are the wrong bytes and only a re-download replaces them | Downloads | UR-071, UR-004 | Done |
|
| DR-171 | A downloaded video keeps audio the device can actually decode. `original` quality asked for `Static=true`, which hands back the source file byte-for-byte — E-AC-3/AC-3/DTS/TrueHD track included — and video is rendered on both platforms by the webview `<video>` element, which decodes none of them. Streaming already knew this: DR-149 judges the track the server would serve against `WEBVIEW_AUDIO_CODECS` and forces a transcode over Jellyfin's own direct-play offer, because 10.11.5 honours a `DirectPlayProfile`'s container and video codec but ignores its audio codec. The download path never consulted that policy, so the *same film* had sound when streamed and played as picture in silence once downloaded — and offline a download is the only source a video has, so there was no working path left to fall back to. The rule is now one rule: `served_audio_codec` picks the track the server will serve (the default, or the first when none is marked) and both callers judge it, the streaming verdict staying a bool and the download path needing the codec itself so it can say what to re-encode. Only the audio is re-encoded — `allowVideoStreamCopy=true` keeps an h264 source's picture byte-for-byte and no bitrate or resolution cap is added, so `original` still means original quality; a source the webview could not have rendered anyway (HEVC) becomes h264 as a side effect, which is the only form of it that would have played. The decision is per item rather than blanket because the transcode costs the byte-range resumability `Static=true` gives the download worker (see DR-170 for what a chunked, length-less response does to a resume), so a file whose audio already plays keeps the direct copy. An unknown codec — item not fetchable, or the server named none — changes nothing: the policy only ever *adds* a transcode, so it cannot make a working download worse. The codec set judged against is the **webview's**, not the platform's, even though DR-161 made ExoPlayer the Android default: `experimentalNativeVideo` is a user setting, a downloaded file outlives whatever it was set to when the file arrived, and the narrow list is the only one that holds on both sides of it — at the cost of a Dolby-licensed device re-encoding a track its ExoPlayer could have played. `resolve_video_download_url` is the single entrance for all three resolution sites (the frontend's per-item command, the bulk series/season enqueue, and the offline-queued resume), since the pure builder cannot look a codec up and a caller that forgets to is exactly how the silent downloads shipped. **Files already downloaded stay silent** — the bytes on disk are the wrong bytes and only a re-download replaces them | Downloads | UR-071, UR-004 | Done |
|
||||||
@@ -369,7 +370,7 @@ Internal architecture, components, and application logic.
|
|||||||
| UR-015 | - | DR-005, DR-020 |
|
| UR-015 | - | DR-005, DR-020 |
|
||||||
| UR-016 | - | - |
|
| UR-016 | - | - |
|
||||||
| UR-017 | - | DR-014, DR-021 |
|
| UR-017 | - | DR-014, DR-021 |
|
||||||
| UR-018 | IR-013 | DR-015, DR-018 |
|
| UR-018 | IR-013 | DR-015, DR-018, DR-173 |
|
||||||
| UR-019 | IR-015 | DR-022 |
|
| UR-019 | IR-015 | DR-022 |
|
||||||
| UR-020 | IR-016, IR-018 | DR-023 |
|
| UR-020 | IR-016, IR-018 | DR-023 |
|
||||||
| UR-021 | IR-016, IR-019 | DR-024 |
|
| UR-021 | IR-016, IR-019 | DR-024 |
|
||||||
@@ -406,7 +407,7 @@ Internal architecture, components, and application logic.
|
|||||||
| UR-052 | IR-027 | DR-078, DR-079, DR-080, DR-143 |
|
| UR-052 | IR-027 | DR-078, DR-079, DR-080, DR-143 |
|
||||||
| UR-053 | IR-029 | DR-074 |
|
| UR-053 | IR-029 | DR-074 |
|
||||||
| UR-054 | - | DR-075, DR-076, DR-077, DR-147 |
|
| UR-054 | - | DR-075, DR-076, DR-077, DR-147 |
|
||||||
| UR-055 | - | DR-081, DR-082, DR-083, DR-084, DR-167, DR-168, DR-169 |
|
| UR-055 | - | DR-081, DR-082, DR-083, DR-084, DR-167, DR-168, DR-169, DR-173 |
|
||||||
| UR-056 | - | DR-085 |
|
| UR-056 | - | DR-085 |
|
||||||
| UR-057 | - | DR-086 |
|
| UR-057 | - | DR-086 |
|
||||||
| UR-058 | - | DR-087, DR-142 |
|
| UR-058 | - | DR-087, DR-142 |
|
||||||
@@ -578,6 +579,9 @@ Internal architecture, components, and application logic.
|
|||||||
| UT-152 | Caching a server result mirrors its watch position locally — including for an item carrying a position but no favourite flag — without inventing a row for an item the server reported no user data for, and without pulling a still-unsynced local position backwards | DR-155 | Done |
|
| UT-152 | Caching a server result mirrors its watch position locally — including for an item carrying a position but no favourite flag — without inventing a row for an item the server reported no user data for, and without pulling a still-unsynced local position backwards | DR-155 | Done |
|
||||||
| UT-162 | Each downloaded library lists only its own media: the music library shows the album and neither the film nor the series, the movie library only the film, the TV library only the series | DR-163 | Done |
|
| UT-162 | Each downloaded library lists only its own media: the music library shows the album and neither the film nor the series, the movie library only the film, the TV library only the series | DR-163 | Done |
|
||||||
| UT-163 | `partial_path` appends rather than replacing the extension, so it matches what the cleanup paths delete, keeps two sources for one title apart, and still produces a sidecar for an extension-less target | DR-165 | Done |
|
| UT-163 | `partial_path` appends rather than replacing the extension, so it matches what the cleanup paths delete, keeps two sources for one title apart, and still produces a sidecar for an extension-less target | DR-165 | Done |
|
||||||
|
| UT-170 | `queue_album_tracks` queues a row for every track of the album — including tracks the cache holds without an `album_id` and tracks it has never seen at all — links each one to its album so offline browsing can find it, returns the row ids in track order, and is idempotent: re-queuing fills the gaps without duplicating rows or resetting a completed track. `cached_album_tracks` (the offline fallback) finds tracks by either album link and does not sweep in another album's | DR-173 | Done |
|
||||||
|
| UT-171 | `resolve_pending_download_urls` restricted to a set of row ids resolves only those rows and leaves other pending rows untouched, and an empty id set resolves nothing rather than sweeping everything | DR-173 | Done |
|
||||||
|
| UT-172 | `album_file_names` gives every track of an album its own file: a title repeated within the album (deluxe edition, two discs) is disambiguated by track number and item id instead of the second download overwriting the first, an unambiguous title keeps its own name, and path separators in a title are sanitised so a track cannot escape the album directory | DR-173 | Done |
|
||||||
| UT-164 | `resume_offset` appends only when the server answered `206`; a `200` after a Range request restarts the file, because that body is the whole stream | DR-166 | Done |
|
| UT-164 | `resume_offset` appends only when the server answered `206`; a `200` after a Range request restarts the file, because that body is the whole stream | DR-166 | Done |
|
||||||
| UT-165 | A registered download starts unflagged, `signal` sets the flag its worker reads, signalling an unregistered id reports not-in-flight, `clear` forgets it, and re-registering drops a previous stop so a resumed download does not halt instantly | DR-164 | Done |
|
| UT-165 | A registered download starts unflagged, `signal` sets the flag its worker reads, signalling an unregistered id reports not-in-flight, `clear` forgets it, and re-registering drops a previous stop so a resumed download does not halt instantly | DR-164 | Done |
|
||||||
| UT-166 | `original` quality re-encodes audio the webview cannot decode (E-AC-3/AC-3/DTS/TrueHD) to AAC without capping bitrate or resolution, keeps the `Static=true` direct copy for audio that plays here (AAC/MP3/Opus/Vorbis/FLAC) and for an unknown codec, leaves the explicit quality presets untouched, and picks the served track by the same default-or-first rule the streaming verdict uses | DR-171 | Done |
|
| UT-166 | `original` quality re-encodes audio the webview cannot decode (E-AC-3/AC-3/DTS/TrueHD) to AAC without capping bitrate or resolution, keeps the `Static=true` direct copy for audio that plays here (AAC/MP3/Opus/Vorbis/FLAC) and for an unknown codec, leaves the explicit quality presets untouched, and picks the served track by the same default-or-first rule the streaming verdict uses | DR-171 | Done |
|
||||||
|
|||||||
@@ -175,8 +175,8 @@ describe("live requirements.md", () => {
|
|||||||
|
|
||||||
expect(defined.UR).toBe(75);
|
expect(defined.UR).toBe(75);
|
||||||
expect(defined.IR).toBe(32);
|
expect(defined.IR).toBe(32);
|
||||||
expect(defined.DR).toBe(165);
|
expect(defined.DR).toBe(166);
|
||||||
expect(defined.JA).toBe(35);
|
expect(defined.JA).toBe(35);
|
||||||
expect(defined.total).toBe(307);
|
expect(defined.total).toBe(308);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -520,15 +520,27 @@ pub(crate) async fn requeue_mistyped_video_downloads(
|
|||||||
/// `pending`/`stream_url IS NULL` row, resolve each via `resolve` (returning
|
/// `pending`/`stream_url IS NULL` row, resolve each via `resolve` (returning
|
||||||
/// `None` leaves the row pending), and heal the row so the pump can start it.
|
/// `None` leaves the row pending), and heal the row so the pump can start it.
|
||||||
/// The `resolve` closure receives `(item_id, media_type, quality_preset)`.
|
/// The `resolve` closure receives `(item_id, media_type, quality_preset)`.
|
||||||
|
///
|
||||||
|
/// `only_ids` restricts the sweep to specific download rows. Reconnect passes
|
||||||
|
/// `None` and heals everything; a bulk enqueue (an album, say) passes the rows
|
||||||
|
/// it just created, so clicking download on one album cannot also start every
|
||||||
|
/// unrelated row that has been sitting pending.
|
||||||
pub(crate) async fn resolve_pending_download_urls<F, Fut>(
|
pub(crate) async fn resolve_pending_download_urls<F, Fut>(
|
||||||
db_service: &Arc<crate::storage::db_service::RusqliteService>,
|
db_service: &Arc<crate::storage::db_service::RusqliteService>,
|
||||||
target_dir: &str,
|
target_dir: &str,
|
||||||
|
only_ids: Option<&[i64]>,
|
||||||
resolve: F,
|
resolve: F,
|
||||||
) -> Result<ResumeQueuedResult, String>
|
) -> Result<ResumeQueuedResult, String>
|
||||||
where
|
where
|
||||||
F: Fn(String, String, String) -> Fut,
|
F: Fn(String, String, String) -> Fut,
|
||||||
Fut: std::future::Future<Output = Option<String>>,
|
Fut: std::future::Future<Output = Option<String>>,
|
||||||
{
|
{
|
||||||
|
if only_ids.is_some_and(|ids| ids.is_empty()) {
|
||||||
|
return Ok(ResumeQueuedResult {
|
||||||
|
resolved: 0,
|
||||||
|
failed: 0,
|
||||||
|
});
|
||||||
|
}
|
||||||
// A row's own media_type wins; otherwise the *item's* type decides. Rows
|
// A row's own media_type wins; otherwise the *item's* type decides. Rows
|
||||||
// queued from a media card never carry one (`download_item` does not record
|
// queued from a media card never carry one (`download_item` does not record
|
||||||
// it), and defaulting that NULL to 'audio' resolved movies against
|
// it), and defaulting that NULL to 'audio' resolved movies against
|
||||||
@@ -541,6 +553,16 @@ where
|
|||||||
.map(|t| format!("'{t}'"))
|
.map(|t| format!("'{t}'"))
|
||||||
.collect::<Vec<_>>()
|
.collect::<Vec<_>>()
|
||||||
.join(", ");
|
.join(", ");
|
||||||
|
let id_filter = match only_ids {
|
||||||
|
Some(ids) => format!(
|
||||||
|
" AND d.id IN ({})",
|
||||||
|
ids.iter()
|
||||||
|
.map(|id| id.to_string())
|
||||||
|
.collect::<Vec<_>>()
|
||||||
|
.join(", ")
|
||||||
|
),
|
||||||
|
None => String::new(),
|
||||||
|
};
|
||||||
let rows_query = Query::new(&format!(
|
let rows_query = Query::new(&format!(
|
||||||
"SELECT d.id, d.item_id,
|
"SELECT d.id, d.item_id,
|
||||||
COALESCE(
|
COALESCE(
|
||||||
@@ -552,7 +574,7 @@ where
|
|||||||
COALESCE(d.quality_preset, 'original')
|
COALESCE(d.quality_preset, 'original')
|
||||||
FROM downloads d
|
FROM downloads d
|
||||||
LEFT JOIN items i ON i.id = d.item_id
|
LEFT JOIN items i ON i.id = d.item_id
|
||||||
WHERE d.status = 'pending' AND d.stream_url IS NULL"
|
WHERE d.status = 'pending' AND d.stream_url IS NULL{id_filter}"
|
||||||
));
|
));
|
||||||
let rows: Vec<(i64, String, String, String)> = db_service
|
let rows: Vec<(i64, String, String, String)> = db_service
|
||||||
.query_many(rows_query, |row| {
|
.query_many(rows_query, |row| {
|
||||||
@@ -676,6 +698,7 @@ pub async fn resume_queued_downloads(
|
|||||||
let outcome = resolve_pending_download_urls(
|
let outcome = resolve_pending_download_urls(
|
||||||
&db_service,
|
&db_service,
|
||||||
&target_dir,
|
&target_dir,
|
||||||
|
None,
|
||||||
move |item_id: String, media_type: String, quality: String| {
|
move |item_id: String, media_type: String, quality: String| {
|
||||||
let repo = Arc::clone(&repo_for_resolve);
|
let repo = Arc::clone(&repo_for_resolve);
|
||||||
async move {
|
async move {
|
||||||
@@ -861,10 +884,12 @@ mod tests {
|
|||||||
// A completed row: irrelevant.
|
// A completed row: irrelevant.
|
||||||
insert_download(&db, "done", "completed", Some("http://done/url"), None).await;
|
insert_download(&db, "done", "completed", Some("http://done/url"), None).await;
|
||||||
|
|
||||||
let out =
|
let out = resolve_pending_download_urls(
|
||||||
resolve_pending_download_urls(&db, "/data/downloads", |item_id, _mt, _q| async move {
|
&db,
|
||||||
Some(format!("http://resolved/{item_id}"))
|
"/data/downloads",
|
||||||
})
|
None,
|
||||||
|
|item_id, _mt, _q| async move { Some(format!("http://resolved/{item_id}")) },
|
||||||
|
)
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap();
|
||||||
|
|
||||||
@@ -882,13 +907,77 @@ mod tests {
|
|||||||
assert_eq!(url2.as_deref(), Some("http://existing/url"));
|
assert_eq!(url2.as_deref(), Some("http://existing/url"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// A bulk enqueue resolves only the rows it just created. Downloading one
|
||||||
|
/// album must not also start every unrelated row that has been sitting
|
||||||
|
/// pending with no URL (the smart cache leaves plenty of those).
|
||||||
|
///
|
||||||
|
/// TRACES: UR-018, UR-055 | DR-173 | UT-171
|
||||||
|
#[tokio::test]
|
||||||
|
async fn only_ids_restricts_the_sweep_to_the_given_rows() {
|
||||||
|
let db = test_db();
|
||||||
|
insert_download(&db, "mine", "pending", None, Some("audio")).await;
|
||||||
|
insert_download(&db, "someone-elses", "pending", None, Some("audio")).await;
|
||||||
|
|
||||||
|
let mine: i64 = db
|
||||||
|
.query_one(
|
||||||
|
Query::new("SELECT id FROM downloads WHERE item_id = 'mine'"),
|
||||||
|
|row| row.get(0),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
let out = resolve_pending_download_urls(
|
||||||
|
&db,
|
||||||
|
"/data",
|
||||||
|
Some(&[mine]),
|
||||||
|
|item_id, _mt, _q| async move { Some(format!("http://resolved/{item_id}")) },
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert_eq!(out.resolved, 1);
|
||||||
|
assert_eq!(out.failed, 0);
|
||||||
|
|
||||||
|
let (_s, url, _t) = get_row(&db, "mine").await;
|
||||||
|
assert_eq!(url.as_deref(), Some("http://resolved/mine"));
|
||||||
|
|
||||||
|
let (status, other_url, _t) = get_row(&db, "someone-elses").await;
|
||||||
|
assert_eq!(status, "pending");
|
||||||
|
assert_eq!(
|
||||||
|
other_url, None,
|
||||||
|
"a scoped resolve must leave unrelated pending rows alone"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// An empty id list resolves nothing — it must not fall through to "sweep
|
||||||
|
/// everything", which is what an unguarded `IN ()` would amount to.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-018, UR-055 | DR-173 | UT-171
|
||||||
|
#[tokio::test]
|
||||||
|
async fn an_empty_id_list_resolves_nothing() {
|
||||||
|
let db = test_db();
|
||||||
|
insert_download(&db, "untouched", "pending", None, Some("audio")).await;
|
||||||
|
|
||||||
|
let out =
|
||||||
|
resolve_pending_download_urls(&db, "/data", Some(&[]), |item_id, _mt, _q| async move {
|
||||||
|
Some(format!("http://resolved/{item_id}"))
|
||||||
|
})
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert_eq!(out.resolved, 0);
|
||||||
|
let (_s, url, _t) = get_row(&db, "untouched").await;
|
||||||
|
assert_eq!(url, None);
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn counts_unresolvable_rows_as_failed_and_leaves_them_pending() {
|
async fn counts_unresolvable_rows_as_failed_and_leaves_them_pending() {
|
||||||
let db = test_db();
|
let db = test_db();
|
||||||
insert_download(&db, "bad", "pending", None, None).await;
|
insert_download(&db, "bad", "pending", None, None).await;
|
||||||
|
|
||||||
// Resolver returns None (e.g. server lookup failed).
|
// Resolver returns None (e.g. server lookup failed).
|
||||||
let out = resolve_pending_download_urls(&db, "/data", |_id, _mt, _q| async move { None })
|
let out =
|
||||||
|
resolve_pending_download_urls(&db, "/data", None, |_id, _mt, _q| async move { None })
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap();
|
||||||
|
|
||||||
@@ -920,7 +1009,7 @@ mod tests {
|
|||||||
|
|
||||||
let seen = Arc::new(Mutex::new(Vec::new()));
|
let seen = Arc::new(Mutex::new(Vec::new()));
|
||||||
let seen_c = Arc::clone(&seen);
|
let seen_c = Arc::clone(&seen);
|
||||||
resolve_pending_download_urls(&db, "/data", move |item_id, media_type, _q| {
|
resolve_pending_download_urls(&db, "/data", None, move |item_id, media_type, _q| {
|
||||||
let seen = Arc::clone(&seen_c);
|
let seen = Arc::clone(&seen_c);
|
||||||
async move {
|
async move {
|
||||||
seen.lock().unwrap().push((item_id.clone(), media_type));
|
seen.lock().unwrap().push((item_id.clone(), media_type));
|
||||||
@@ -953,7 +1042,7 @@ mod tests {
|
|||||||
|
|
||||||
let seen = Arc::new(Mutex::new(String::new()));
|
let seen = Arc::new(Mutex::new(String::new()));
|
||||||
let seen_c = Arc::clone(&seen);
|
let seen_c = Arc::clone(&seen);
|
||||||
resolve_pending_download_urls(&db, "/data", move |_id, media_type, _q| {
|
resolve_pending_download_urls(&db, "/data", None, move |_id, media_type, _q| {
|
||||||
let seen = Arc::clone(&seen_c);
|
let seen = Arc::clone(&seen_c);
|
||||||
async move {
|
async move {
|
||||||
*seen.lock().unwrap() = media_type;
|
*seen.lock().unwrap() = media_type;
|
||||||
@@ -977,7 +1066,7 @@ mod tests {
|
|||||||
|
|
||||||
let seen = Arc::new(Mutex::new(String::new()));
|
let seen = Arc::new(Mutex::new(String::new()));
|
||||||
let seen_c = Arc::clone(&seen);
|
let seen_c = Arc::clone(&seen);
|
||||||
resolve_pending_download_urls(&db, "/data", move |_id, media_type, _q| {
|
resolve_pending_download_urls(&db, "/data", None, move |_id, media_type, _q| {
|
||||||
let seen = Arc::clone(&seen_c);
|
let seen = Arc::clone(&seen_c);
|
||||||
async move {
|
async move {
|
||||||
*seen.lock().unwrap() = media_type;
|
*seen.lock().unwrap() = media_type;
|
||||||
@@ -1037,11 +1126,15 @@ mod tests {
|
|||||||
let db = test_db();
|
let db = test_db();
|
||||||
insert_download(&db, "vid-1", "pending", None, Some("video")).await;
|
insert_download(&db, "vid-1", "pending", None, Some("video")).await;
|
||||||
|
|
||||||
let out =
|
let out = resolve_pending_download_urls(
|
||||||
resolve_pending_download_urls(&db, "/data", |item_id, media_type, _q| async move {
|
&db,
|
||||||
|
"/data",
|
||||||
|
None,
|
||||||
|
|item_id, media_type, _q| async move {
|
||||||
assert_eq!(media_type, "video");
|
assert_eq!(media_type, "video");
|
||||||
Some(format!("http://transcode/{item_id}"))
|
Some(format!("http://transcode/{item_id}"))
|
||||||
})
|
},
|
||||||
|
)
|
||||||
.await
|
.await
|
||||||
.unwrap();
|
.unwrap();
|
||||||
|
|
||||||
|
|||||||
@@ -350,57 +350,209 @@ pub async fn download_item(
|
|||||||
Ok(download_id)
|
Ok(download_id)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Queue an entire album for download
|
/// One track of an album, as the album-download path queues it.
|
||||||
#[tauri::command]
|
///
|
||||||
#[specta::specta]
|
/// `artist_name` carries whatever the catalog holds for the track's artists (a
|
||||||
pub async fn download_album(
|
/// JSON array, as stored on `items.artists`); it is display metadata for the
|
||||||
db: State<'_, DatabaseWrapper>,
|
/// downloads list, not a lookup key.
|
||||||
album_id: String,
|
///
|
||||||
user_id: String,
|
/// TRACES: UR-018, UR-055 | DR-173
|
||||||
base_path: String,
|
#[derive(Debug, Clone, PartialEq)]
|
||||||
) -> Result<Vec<i64>, String> {
|
pub(crate) struct AlbumTrack {
|
||||||
let db_service = {
|
pub id: String,
|
||||||
let database = db.0.lock().map_err(|e| e.to_string())?;
|
pub name: String,
|
||||||
Arc::new(database.service())
|
pub artist_name: Option<String>,
|
||||||
};
|
pub album_name: Option<String>,
|
||||||
|
pub index_number: Option<i32>,
|
||||||
|
}
|
||||||
|
|
||||||
// Get all tracks in the album with metadata
|
impl From<&crate::repository::types::MediaItem> for AlbumTrack {
|
||||||
|
fn from(item: &crate::repository::types::MediaItem) -> Self {
|
||||||
|
Self {
|
||||||
|
id: item.id.clone(),
|
||||||
|
name: item.name.clone(),
|
||||||
|
artist_name: item
|
||||||
|
.artists
|
||||||
|
.as_ref()
|
||||||
|
.and_then(|a| serde_json::to_string(a).ok()),
|
||||||
|
album_name: item.album_name.clone(),
|
||||||
|
index_number: item.index_number,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The album's tracks as the local catalog cache knows them.
|
||||||
|
///
|
||||||
|
/// Only a fallback for [`download_album`]: the cache links a track to its album
|
||||||
|
/// through `items.album_id`, which Jellyfin does not populate on every listing
|
||||||
|
/// endpoint, so this can legitimately return fewer tracks than the album has.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-018, UR-055 | DR-173
|
||||||
|
pub(crate) async fn cached_album_tracks(
|
||||||
|
db_service: &Arc<crate::storage::db_service::RusqliteService>,
|
||||||
|
album_id: &str,
|
||||||
|
) -> Result<Vec<AlbumTrack>, String> {
|
||||||
let tracks_query = Query::with_params(
|
let tracks_query = Query::with_params(
|
||||||
"SELECT id, name, artists, album_name FROM items
|
"SELECT id, name, artists, album_name, index_number FROM items
|
||||||
WHERE album_id = ? AND item_type = 'Audio'
|
WHERE (album_id = ? OR parent_id = ?) AND item_type = 'Audio'
|
||||||
ORDER BY index_number",
|
ORDER BY index_number",
|
||||||
vec![QueryParam::String(album_id)],
|
vec![
|
||||||
|
QueryParam::String(album_id.to_string()),
|
||||||
|
QueryParam::String(album_id.to_string()),
|
||||||
|
],
|
||||||
);
|
);
|
||||||
|
|
||||||
let tracks: Vec<(String, String, Option<String>, Option<String>)> = db_service
|
db_service
|
||||||
.query_many(tracks_query, |row| {
|
.query_many(tracks_query, |row| {
|
||||||
Ok((row.get(0)?, row.get(1)?, row.get(2)?, row.get(3)?))
|
Ok(AlbumTrack {
|
||||||
|
id: row.get(0)?,
|
||||||
|
name: row.get(1)?,
|
||||||
|
artist_name: row.get(2)?,
|
||||||
|
album_name: row.get(3)?,
|
||||||
|
index_number: row.get(4)?,
|
||||||
})
|
})
|
||||||
|
})
|
||||||
|
.await
|
||||||
|
.map_err(|e| e.to_string())
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Queue one download row per track and link every track to its album.
|
||||||
|
///
|
||||||
|
/// The linkage is the half that is easy to miss: offline browsing joins a track
|
||||||
|
/// to its album on `items.album_id` (see `OfflineRepository::get_items`), so a
|
||||||
|
/// track whose cached row lacks it stays invisible under the album even after
|
||||||
|
/// its file is on disk. Queuing a track *is* the statement that it belongs to
|
||||||
|
/// this album, so the link is written here rather than hoped for from whichever
|
||||||
|
/// listing endpoint happened to cache the row.
|
||||||
|
///
|
||||||
|
/// Idempotent: re-queuing an album fills in what is missing and returns the same
|
||||||
|
/// row ids, in the order the tracks were given.
|
||||||
|
///
|
||||||
|
/// A file name per track, unique within the album.
|
||||||
|
///
|
||||||
|
/// A title is not a unique name inside its own album: a deluxe edition carries
|
||||||
|
/// the album version and a demo of the same song, and a two-disc set repeats
|
||||||
|
/// titles across discs. Naming files after the title alone gave those tracks one
|
||||||
|
/// path, and each download overwrote the previous one — an album that quietly
|
||||||
|
/// ends up short by however many titles it repeats. The track number
|
||||||
|
/// disambiguates the ordinary case; anything still colliding falls back to the
|
||||||
|
/// item id, which is unique by construction.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-018, UR-055 | DR-173 | UT-172
|
||||||
|
pub(crate) fn album_file_names(tracks: &[AlbumTrack]) -> Vec<String> {
|
||||||
|
let mut counts: std::collections::HashMap<String, usize> = std::collections::HashMap::new();
|
||||||
|
for track in tracks {
|
||||||
|
*counts.entry(track.name.to_lowercase()).or_default() += 1;
|
||||||
|
}
|
||||||
|
|
||||||
|
tracks
|
||||||
|
.iter()
|
||||||
|
.map(|track| {
|
||||||
|
let title = sanitize_filename(&track.name);
|
||||||
|
if counts.get(&track.name.to_lowercase()).copied().unwrap_or(0) <= 1 {
|
||||||
|
return format!("{}.mp3", title);
|
||||||
|
}
|
||||||
|
match track.index_number {
|
||||||
|
Some(n) => format!("{:02} - {} [{}].mp3", n, title, track.id),
|
||||||
|
None => format!("{} [{}].mp3", title, track.id),
|
||||||
|
}
|
||||||
|
})
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// TRACES: UR-018, UR-055 | DR-173 | UT-170
|
||||||
|
pub(crate) async fn queue_album_tracks(
|
||||||
|
db_service: &Arc<crate::storage::db_service::RusqliteService>,
|
||||||
|
album_id: &str,
|
||||||
|
tracks: &[AlbumTrack],
|
||||||
|
user_id: &str,
|
||||||
|
base_path: &str,
|
||||||
|
) -> Result<Vec<i64>, String> {
|
||||||
|
let mut download_ids = Vec::with_capacity(tracks.len());
|
||||||
|
let file_names = album_file_names(tracks);
|
||||||
|
|
||||||
|
for (track, file_name) in tracks.iter().zip(file_names) {
|
||||||
|
// Cache a row for a track the catalog has never seen, borrowing the
|
||||||
|
// album's server. Nothing is inserted when the album itself is unknown,
|
||||||
|
// which also keeps the parent_id foreign key satisfiable.
|
||||||
|
let cache_query = Query::with_params(
|
||||||
|
"INSERT OR IGNORE INTO items
|
||||||
|
(id, server_id, parent_id, name, item_type, album_id, album_name, artists, index_number)
|
||||||
|
SELECT ?, a.server_id, a.id, ?, 'Audio', a.id, ?, ?, ?
|
||||||
|
FROM items a WHERE a.id = ?",
|
||||||
|
vec![
|
||||||
|
QueryParam::String(track.id.clone()),
|
||||||
|
QueryParam::String(track.name.clone()),
|
||||||
|
track
|
||||||
|
.album_name
|
||||||
|
.clone()
|
||||||
|
.map(QueryParam::String)
|
||||||
|
.unwrap_or(QueryParam::Null),
|
||||||
|
track
|
||||||
|
.artist_name
|
||||||
|
.clone()
|
||||||
|
.map(QueryParam::String)
|
||||||
|
.unwrap_or(QueryParam::Null),
|
||||||
|
track
|
||||||
|
.index_number
|
||||||
|
.map(QueryParam::Int)
|
||||||
|
.unwrap_or(QueryParam::Null),
|
||||||
|
QueryParam::String(album_id.to_string()),
|
||||||
|
],
|
||||||
|
);
|
||||||
|
db_service
|
||||||
|
.execute(cache_query)
|
||||||
.await
|
.await
|
||||||
.map_err(|e| e.to_string())?;
|
.map_err(|e| e.to_string())?;
|
||||||
|
|
||||||
let mut download_ids = Vec::new();
|
// Link an already-cached track to the album. The parent_id subquery
|
||||||
|
// resolves to NULL when the album is not cached, so the foreign key
|
||||||
|
// holds either way.
|
||||||
|
let link_query = Query::with_params(
|
||||||
|
"UPDATE items
|
||||||
|
SET album_id = ?,
|
||||||
|
parent_id = COALESCE(parent_id, (SELECT id FROM items WHERE id = ?))
|
||||||
|
WHERE id = ?",
|
||||||
|
vec![
|
||||||
|
QueryParam::String(album_id.to_string()),
|
||||||
|
QueryParam::String(album_id.to_string()),
|
||||||
|
QueryParam::String(track.id.clone()),
|
||||||
|
],
|
||||||
|
);
|
||||||
|
db_service
|
||||||
|
.execute(link_query)
|
||||||
|
.await
|
||||||
|
.map_err(|e| e.to_string())?;
|
||||||
|
|
||||||
// Queue each track with album priority (100) and metadata
|
let file_path = format!("{}/{}", base_path, file_name);
|
||||||
for (track_id, track_name, artist_name, album_name) in tracks {
|
|
||||||
let file_path = format!("{}/{}.mp3", base_path, sanitize_filename(&track_name));
|
|
||||||
|
|
||||||
|
// Queue at album priority (100). A track already downloaded stays
|
||||||
|
// completed — re-queuing an album must fill the gaps, not re-fetch it.
|
||||||
let insert_query = Query::with_params(
|
let insert_query = Query::with_params(
|
||||||
"INSERT INTO downloads (item_id, user_id, file_path, status, priority, queued_at, item_name, artist_name, album_name)
|
"INSERT INTO downloads (item_id, user_id, file_path, status, priority, queued_at, item_name, artist_name, album_name, media_type)
|
||||||
VALUES (?, ?, ?, 'pending', 100, CURRENT_TIMESTAMP, ?, ?, ?)
|
VALUES (?, ?, ?, 'pending', 100, CURRENT_TIMESTAMP, ?, ?, ?, 'audio')
|
||||||
ON CONFLICT(item_id, user_id) DO UPDATE SET
|
ON CONFLICT(item_id, user_id) DO UPDATE SET
|
||||||
priority = 100,
|
priority = 100,
|
||||||
status = 'pending',
|
status = CASE WHEN downloads.status = 'completed' THEN 'completed' ELSE 'pending' END,
|
||||||
|
media_type = 'audio',
|
||||||
item_name = COALESCE(excluded.item_name, downloads.item_name),
|
item_name = COALESCE(excluded.item_name, downloads.item_name),
|
||||||
artist_name = COALESCE(excluded.artist_name, downloads.artist_name),
|
artist_name = COALESCE(excluded.artist_name, downloads.artist_name),
|
||||||
album_name = COALESCE(excluded.album_name, downloads.album_name)",
|
album_name = COALESCE(excluded.album_name, downloads.album_name)",
|
||||||
vec![
|
vec![
|
||||||
QueryParam::String(track_id.clone()),
|
QueryParam::String(track.id.clone()),
|
||||||
QueryParam::String(user_id.clone()),
|
QueryParam::String(user_id.to_string()),
|
||||||
QueryParam::String(file_path),
|
QueryParam::String(file_path),
|
||||||
QueryParam::String(track_name),
|
QueryParam::String(track.name.clone()),
|
||||||
artist_name.map(QueryParam::String).unwrap_or(QueryParam::Null),
|
track
|
||||||
album_name.map(QueryParam::String).unwrap_or(QueryParam::Null),
|
.artist_name
|
||||||
|
.clone()
|
||||||
|
.map(QueryParam::String)
|
||||||
|
.unwrap_or(QueryParam::Null),
|
||||||
|
track
|
||||||
|
.album_name
|
||||||
|
.clone()
|
||||||
|
.map(QueryParam::String)
|
||||||
|
.unwrap_or(QueryParam::Null),
|
||||||
],
|
],
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -413,8 +565,8 @@ pub async fn download_album(
|
|||||||
let id_query = Query::with_params(
|
let id_query = Query::with_params(
|
||||||
"SELECT id FROM downloads WHERE item_id = ? AND user_id = ?",
|
"SELECT id FROM downloads WHERE item_id = ? AND user_id = ?",
|
||||||
vec![
|
vec![
|
||||||
QueryParam::String(track_id),
|
QueryParam::String(track.id.clone()),
|
||||||
QueryParam::String(user_id.clone()),
|
QueryParam::String(user_id.to_string()),
|
||||||
],
|
],
|
||||||
);
|
);
|
||||||
|
|
||||||
@@ -428,6 +580,129 @@ pub async fn download_album(
|
|||||||
Ok(download_ids)
|
Ok(download_ids)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Queue an entire album for download.
|
||||||
|
///
|
||||||
|
/// Owns the whole operation: the album's track list comes from the server (the
|
||||||
|
/// only place that knows all of it), every track is queued and linked to its
|
||||||
|
/// album, each row's stream URL is resolved here, and the queue is pumped.
|
||||||
|
///
|
||||||
|
/// The frontend used to do the second half — resolve one URL per track and pair
|
||||||
|
/// it with the returned ids **by position**. That pairing had no basis: the ids
|
||||||
|
/// came back in the backend's own order over a different set of rows, so
|
||||||
|
/// whenever the two lists disagreed a row was handed another track's URL, and
|
||||||
|
/// any track past the end of the shorter list was never started at all. Nothing
|
||||||
|
/// crosses the boundary now except the album id.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-018, UR-055 | DR-173 | UT-170
|
||||||
|
#[tauri::command]
|
||||||
|
#[specta::specta]
|
||||||
|
pub async fn download_album(
|
||||||
|
db: State<'_, DatabaseWrapper>,
|
||||||
|
repository: State<'_, crate::commands::repository::RepositoryManagerWrapper>,
|
||||||
|
download_manager: State<'_, DownloadManagerWrapper>,
|
||||||
|
app: tauri::AppHandle,
|
||||||
|
handle: String,
|
||||||
|
album_id: String,
|
||||||
|
user_id: String,
|
||||||
|
base_path: String,
|
||||||
|
) -> Result<Vec<i64>, String> {
|
||||||
|
let db_service = {
|
||||||
|
let database = db.0.lock().map_err(|e| e.to_string())?;
|
||||||
|
Arc::new(database.service())
|
||||||
|
};
|
||||||
|
|
||||||
|
let repo = repository.0.get(&handle);
|
||||||
|
|
||||||
|
// Ask the server what the album contains; the cache is only a fallback for
|
||||||
|
// when it cannot answer.
|
||||||
|
let tracks: Vec<AlbumTrack> = match &repo {
|
||||||
|
Some(repo) => match repo.get_album_tracks(&album_id).await {
|
||||||
|
Ok(items) if !items.is_empty() => items.iter().map(AlbumTrack::from).collect(),
|
||||||
|
Ok(_) => cached_album_tracks(&db_service, &album_id).await?,
|
||||||
|
Err(e) => {
|
||||||
|
warn!(
|
||||||
|
"[download_album] Could not list album {} from the repository ({:?}); \
|
||||||
|
falling back to the cached track list",
|
||||||
|
album_id, e
|
||||||
|
);
|
||||||
|
cached_album_tracks(&db_service, &album_id).await?
|
||||||
|
}
|
||||||
|
},
|
||||||
|
None => cached_album_tracks(&db_service, &album_id).await?,
|
||||||
|
};
|
||||||
|
|
||||||
|
if tracks.is_empty() {
|
||||||
|
warn!("[download_album] No tracks found for album {}", album_id);
|
||||||
|
return Ok(Vec::new());
|
||||||
|
}
|
||||||
|
|
||||||
|
let download_ids =
|
||||||
|
queue_album_tracks(&db_service, &album_id, &tracks, &user_id, &base_path).await?;
|
||||||
|
|
||||||
|
info!(
|
||||||
|
"[download_album] Queued {} track(s) for album {}",
|
||||||
|
download_ids.len(),
|
||||||
|
album_id
|
||||||
|
);
|
||||||
|
|
||||||
|
// Resolve each queued row's stream URL here, then pump. Without a
|
||||||
|
// repository (or while offline) the rows stay pending with no URL and
|
||||||
|
// `resume_queued_downloads` picks them up on reconnect.
|
||||||
|
let Some(repo) = repo else {
|
||||||
|
return Ok(download_ids);
|
||||||
|
};
|
||||||
|
|
||||||
|
let target_dir = {
|
||||||
|
let database = db.0.lock().map_err(|e| e.to_string())?;
|
||||||
|
database
|
||||||
|
.path()
|
||||||
|
.parent()
|
||||||
|
.ok_or_else(|| "Database path has no parent directory".to_string())?
|
||||||
|
.to_string_lossy()
|
||||||
|
.to_string()
|
||||||
|
};
|
||||||
|
|
||||||
|
let repo_for_resolve = Arc::clone(&repo);
|
||||||
|
let outcome = crate::commands::catalog::resolve_pending_download_urls(
|
||||||
|
&db_service,
|
||||||
|
&target_dir,
|
||||||
|
Some(&download_ids),
|
||||||
|
move |item_id: String, _media_type: String, _quality: String| {
|
||||||
|
let repo = Arc::clone(&repo_for_resolve);
|
||||||
|
async move {
|
||||||
|
use crate::repository::MediaRepository;
|
||||||
|
match repo.get_audio_stream_url(&item_id).await {
|
||||||
|
Ok(url) => Some(url),
|
||||||
|
Err(e) => {
|
||||||
|
warn!(
|
||||||
|
"[download_album] Failed to resolve stream URL for {}: {:?}",
|
||||||
|
item_id, e
|
||||||
|
);
|
||||||
|
None
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
},
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
|
||||||
|
if outcome.failed > 0 {
|
||||||
|
warn!(
|
||||||
|
"[download_album] {} track(s) could not be resolved and stay queued for the next \
|
||||||
|
reconnect",
|
||||||
|
outcome.failed
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
let active_downloads = {
|
||||||
|
let manager = download_manager.0.lock().map_err(|e| e.to_string())?;
|
||||||
|
manager.get_active_downloads()
|
||||||
|
};
|
||||||
|
pump_download_queue(app, db_service, active_downloads).await;
|
||||||
|
|
||||||
|
Ok(download_ids)
|
||||||
|
}
|
||||||
|
|
||||||
/// Queue a video item (movie or episode) for download with quality preset
|
/// Queue a video item (movie or episode) for download with quality preset
|
||||||
#[tauri::command]
|
#[tauri::command]
|
||||||
#[specta::specta]
|
#[specta::specta]
|
||||||
@@ -2658,4 +2933,294 @@ mod tests {
|
|||||||
download_source: "user".to_string(),
|
download_source: "user".to_string(),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ===== Album download: track sourcing and album linkage =====
|
||||||
|
|
||||||
|
/// A database with just the tables the album-download path touches.
|
||||||
|
fn album_test_db() -> Arc<crate::storage::db_service::RusqliteService> {
|
||||||
|
let conn = rusqlite::Connection::open_in_memory().unwrap();
|
||||||
|
conn.execute_batch(
|
||||||
|
r#"
|
||||||
|
CREATE TABLE items (
|
||||||
|
id TEXT PRIMARY KEY,
|
||||||
|
server_id TEXT NOT NULL,
|
||||||
|
parent_id TEXT,
|
||||||
|
name TEXT NOT NULL,
|
||||||
|
item_type TEXT NOT NULL,
|
||||||
|
album_id TEXT,
|
||||||
|
album_name TEXT,
|
||||||
|
album_artist TEXT,
|
||||||
|
artists TEXT,
|
||||||
|
index_number INTEGER
|
||||||
|
);
|
||||||
|
CREATE TABLE downloads (
|
||||||
|
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||||
|
item_id TEXT NOT NULL,
|
||||||
|
user_id TEXT NOT NULL,
|
||||||
|
file_path TEXT NOT NULL,
|
||||||
|
status TEXT DEFAULT 'pending',
|
||||||
|
priority INTEGER DEFAULT 0,
|
||||||
|
progress REAL DEFAULT 0,
|
||||||
|
queued_at TEXT,
|
||||||
|
item_name TEXT,
|
||||||
|
artist_name TEXT,
|
||||||
|
album_name TEXT,
|
||||||
|
media_type TEXT,
|
||||||
|
stream_url TEXT,
|
||||||
|
target_dir TEXT,
|
||||||
|
UNIQUE(item_id, user_id)
|
||||||
|
);
|
||||||
|
INSERT INTO items (id, server_id, name, item_type)
|
||||||
|
VALUES ('album1', 'server1', 'The Golden Age', 'MusicAlbum');
|
||||||
|
"#,
|
||||||
|
)
|
||||||
|
.unwrap();
|
||||||
|
Arc::new(crate::storage::db_service::RusqliteService::new(Arc::new(
|
||||||
|
Mutex::new(conn),
|
||||||
|
)))
|
||||||
|
}
|
||||||
|
|
||||||
|
fn album_track(id: &str, name: &str, index: i32) -> AlbumTrack {
|
||||||
|
AlbumTrack {
|
||||||
|
id: id.to_string(),
|
||||||
|
name: name.to_string(),
|
||||||
|
artist_name: Some("Woodkid".to_string()),
|
||||||
|
album_name: Some("The Golden Age".to_string()),
|
||||||
|
index_number: Some(index),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The album-download regression: every track the album actually has must be
|
||||||
|
/// queued, and each queued track must be linked to its album.
|
||||||
|
///
|
||||||
|
/// `download_album` used to take its track list from
|
||||||
|
/// `items WHERE album_id = ?`. Jellyfin does not return `AlbumId` on every
|
||||||
|
/// listing endpoint, so tracks cached from those endpoints sit in `items`
|
||||||
|
/// with a NULL `album_id` — invisible to that query. "Download album" then
|
||||||
|
/// silently queued only the subset that happened to carry the link, which is
|
||||||
|
/// the reported "only 4-5 songs downloaded". The same column is what offline
|
||||||
|
/// browsing joins tracks to their album on (`i.album_id = ?` in
|
||||||
|
/// `OfflineRepository::get_items`), so even a track that did download stayed
|
||||||
|
/// invisible under its album offline.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-018, UR-055 | DR-173 | UT-170
|
||||||
|
#[tokio::test]
|
||||||
|
async fn test_queue_album_tracks_queues_every_track_and_links_it_to_the_album() {
|
||||||
|
let db = album_test_db();
|
||||||
|
|
||||||
|
// The cache holds all three tracks, but only one carries `album_id` —
|
||||||
|
// exactly the state the bug report's database is in.
|
||||||
|
for sql in [
|
||||||
|
"INSERT INTO items (id, server_id, name, item_type, album_id) \
|
||||||
|
VALUES ('t1', 'server1', 'Run Boy Run', 'Audio', 'album1')",
|
||||||
|
"INSERT INTO items (id, server_id, name, item_type, album_id) \
|
||||||
|
VALUES ('t2', 'server1', 'The Great Escape', 'Audio', NULL)",
|
||||||
|
"INSERT INTO items (id, server_id, name, item_type, album_id) \
|
||||||
|
VALUES ('t3', 'server1', 'Boat Song', 'Audio', NULL)",
|
||||||
|
] {
|
||||||
|
db.execute(Query::new(sql)).await.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
let tracks = vec![
|
||||||
|
album_track("t1", "Run Boy Run", 1),
|
||||||
|
album_track("t2", "The Great Escape", 2),
|
||||||
|
album_track("t3", "Boat Song", 3),
|
||||||
|
];
|
||||||
|
|
||||||
|
let ids = queue_album_tracks(&db, "album1", &tracks, "user1", "albums/album1")
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
ids.len(),
|
||||||
|
3,
|
||||||
|
"every track of the album must get a download row"
|
||||||
|
);
|
||||||
|
|
||||||
|
let queued: i64 = db
|
||||||
|
.query_one(
|
||||||
|
Query::new("SELECT COUNT(*) FROM downloads WHERE status = 'pending'"),
|
||||||
|
|row| row.get(0),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(queued, 3);
|
||||||
|
|
||||||
|
// Each track is now linked to its album, so the offline album page can
|
||||||
|
// find it once the download completes.
|
||||||
|
let linked: i64 = db
|
||||||
|
.query_one(
|
||||||
|
Query::new("SELECT COUNT(*) FROM items WHERE album_id = 'album1'"),
|
||||||
|
|row| row.get(0),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(
|
||||||
|
linked, 3,
|
||||||
|
"queued tracks must be linked to their album; offline browsing joins on album_id"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The returned ids must line up with the tracks that were passed in. The
|
||||||
|
/// frontend used to pair `downloadIds[i]` with its own `tracks[i]`, which is
|
||||||
|
/// only sound if both lists agree — they did not, because the backend
|
||||||
|
/// ordered by `index_number` over a different set of rows. Resolving URLs in
|
||||||
|
/// Rust removes the pairing entirely, but the order is still the contract
|
||||||
|
/// for anything that reads the ids back.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-018, UR-055 | DR-173 | UT-170
|
||||||
|
#[tokio::test]
|
||||||
|
async fn test_queue_album_tracks_returns_ids_in_track_order() {
|
||||||
|
let db = album_test_db();
|
||||||
|
let tracks = vec![
|
||||||
|
album_track("t1", "Run Boy Run", 1),
|
||||||
|
album_track("t2", "The Great Escape", 2),
|
||||||
|
];
|
||||||
|
|
||||||
|
let ids = queue_album_tracks(&db, "album1", &tracks, "user1", "albums/album1")
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
for (id, track) in ids.iter().zip(tracks.iter()) {
|
||||||
|
let item_id: String = db
|
||||||
|
.query_one(
|
||||||
|
Query::with_params(
|
||||||
|
"SELECT item_id FROM downloads WHERE id = ?",
|
||||||
|
vec![QueryParam::Int64(*id)],
|
||||||
|
),
|
||||||
|
|row| row.get(0),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(&item_id, &track.id, "id {} must be {}'s row", id, track.id);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Re-queueing an album already partly downloaded must not duplicate rows or
|
||||||
|
/// reset a completed track — it fills in what is missing.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-018, UR-055 | DR-173 | UT-170
|
||||||
|
#[tokio::test]
|
||||||
|
async fn test_queue_album_tracks_is_idempotent() {
|
||||||
|
let db = album_test_db();
|
||||||
|
let tracks = vec![
|
||||||
|
album_track("t1", "Run Boy Run", 1),
|
||||||
|
album_track("t2", "The Great Escape", 2),
|
||||||
|
];
|
||||||
|
|
||||||
|
let first = queue_album_tracks(&db, "album1", &tracks, "user1", "albums/album1")
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
let second = queue_album_tracks(&db, "album1", &tracks, "user1", "albums/album1")
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
assert_eq!(first, second, "the same tracks must map to the same rows");
|
||||||
|
|
||||||
|
let rows: i64 = db
|
||||||
|
.query_one(Query::new("SELECT COUNT(*) FROM downloads"), |row| {
|
||||||
|
row.get(0)
|
||||||
|
})
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(rows, 2, "re-queueing must not duplicate download rows");
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Two tracks of one album can share a title — a deluxe edition carrying the
|
||||||
|
/// album version and a demo of the same song, or the same song on two discs.
|
||||||
|
/// Naming the file after the title alone gave them one path, so the second
|
||||||
|
/// download overwrote the first and the album ended up short however many
|
||||||
|
/// duplicates it had.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-018, UR-055 | DR-173 | UT-172
|
||||||
|
#[test]
|
||||||
|
fn test_album_file_names_are_unique_within_the_album() {
|
||||||
|
let tracks = vec![
|
||||||
|
album_track("t1", "Crucified Again", 5),
|
||||||
|
album_track("t2", "Crucified Again", 5),
|
||||||
|
album_track("t3", "Get Right", 7),
|
||||||
|
];
|
||||||
|
|
||||||
|
let names = album_file_names(&tracks);
|
||||||
|
|
||||||
|
assert_eq!(names.len(), 3);
|
||||||
|
let unique: std::collections::HashSet<_> = names.iter().collect();
|
||||||
|
assert_eq!(
|
||||||
|
unique.len(),
|
||||||
|
3,
|
||||||
|
"every track of an album needs its own file: {:?}",
|
||||||
|
names
|
||||||
|
);
|
||||||
|
assert!(names.iter().all(|n| n.ends_with(".mp3")), "{:?}", names);
|
||||||
|
assert!(
|
||||||
|
names[2].contains("Get Right"),
|
||||||
|
"an unambiguous title keeps its name: {}",
|
||||||
|
names[2]
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Path separators in a track title must not escape the album directory.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-018, UR-055 | DR-173 | UT-172
|
||||||
|
#[test]
|
||||||
|
fn test_album_file_names_sanitize_the_title() {
|
||||||
|
let names = album_file_names(&[album_track("t1", "AC/DC: Live?", 1)]);
|
||||||
|
assert!(!names[0].contains('/'), "{}", names[0]);
|
||||||
|
assert!(!names[0].contains(':'), "{}", names[0]);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// The offline fallback reads the catalog directly, not through the
|
||||||
|
/// availability-gated offline listing: queueing an album while the server is
|
||||||
|
/// unreachable is a supported flow (the rows resolve on reconnect), and
|
||||||
|
/// gating it on what is already downloaded would queue only the tracks the
|
||||||
|
/// device already has.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-018, UR-055 | DR-173 | UT-170
|
||||||
|
#[tokio::test]
|
||||||
|
async fn test_cached_album_tracks_finds_tracks_by_either_album_link() {
|
||||||
|
let db = album_test_db();
|
||||||
|
for sql in [
|
||||||
|
"INSERT INTO items (id, server_id, name, item_type, album_id, index_number) \
|
||||||
|
VALUES ('t1', 'server1', 'Run Boy Run', 'Audio', 'album1', 1)",
|
||||||
|
// Linked by parent_id only — how a track cached from a folder
|
||||||
|
// listing lands in the catalog.
|
||||||
|
"INSERT INTO items (id, server_id, name, item_type, parent_id, index_number) \
|
||||||
|
VALUES ('t2', 'server1', 'The Great Escape', 'Audio', 'album1', 2)",
|
||||||
|
// A different album's track must not be swept in.
|
||||||
|
"INSERT INTO items (id, server_id, name, item_type, album_id) \
|
||||||
|
VALUES ('other', 'server1', 'Iron', 'Audio', 'album2')",
|
||||||
|
] {
|
||||||
|
db.execute(Query::new(sql)).await.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
let tracks = cached_album_tracks(&db, "album1").await.unwrap();
|
||||||
|
let ids: Vec<_> = tracks.iter().map(|t| t.id.as_str()).collect();
|
||||||
|
assert_eq!(ids, vec!["t1", "t2"]);
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Tracks the cache has never seen still get queued: the row is created and
|
||||||
|
/// an `items` row is written for it, so the download is both startable and
|
||||||
|
/// visible offline afterwards.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-018, UR-055 | DR-173 | UT-170
|
||||||
|
#[tokio::test]
|
||||||
|
async fn test_queue_album_tracks_handles_tracks_absent_from_the_cache() {
|
||||||
|
let db = album_test_db();
|
||||||
|
let tracks = vec![album_track("never-cached", "Iron", 1)];
|
||||||
|
|
||||||
|
let ids = queue_album_tracks(&db, "album1", &tracks, "user1", "albums/album1")
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(ids.len(), 1);
|
||||||
|
|
||||||
|
let (item_type, album_id): (String, Option<String>) = db
|
||||||
|
.query_one(
|
||||||
|
Query::new("SELECT item_type, album_id FROM items WHERE id = 'never-cached'"),
|
||||||
|
|row| Ok((row.get(0)?, row.get(1)?)),
|
||||||
|
)
|
||||||
|
.await
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(item_type, "Audio");
|
||||||
|
assert_eq!(album_id.as_deref(), Some("album1"));
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -119,6 +119,40 @@ impl HybridRepository {
|
|||||||
.await
|
.await
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Every track of an album, asked of the **server** rather than the cache.
|
||||||
|
///
|
||||||
|
/// Deliberately not `get_items`, which is cache-first: it answers from SQLite
|
||||||
|
/// the moment the cache has any content. That is right for browsing and wrong
|
||||||
|
/// for deciding what to download, because a partial or unlinked cache then
|
||||||
|
/// decides how much of the album gets queued while the user is told the whole
|
||||||
|
/// album is downloading. Downloading is the one operation that must know the
|
||||||
|
/// album's *complete* contents.
|
||||||
|
///
|
||||||
|
/// Errors when the server cannot answer (offline); the caller falls back to
|
||||||
|
/// the local catalog and the rows are queued either way, resolving on
|
||||||
|
/// reconnect. Server results are written back to the cache, so browsing
|
||||||
|
/// benefits from the round trip too.
|
||||||
|
///
|
||||||
|
/// TRACES: UR-018, UR-055 | DR-173
|
||||||
|
pub async fn get_album_tracks(&self, album_id: &str) -> Result<Vec<MediaItem>, RepoError> {
|
||||||
|
let options = Some(GetItemsOptions {
|
||||||
|
include_item_types: Some(vec!["Audio".to_string()]),
|
||||||
|
sort_by: Some("ParentIndexNumber,IndexNumber,SortName".to_string()),
|
||||||
|
limit: Some(1000),
|
||||||
|
..Default::default()
|
||||||
|
});
|
||||||
|
|
||||||
|
let result = self.online.get_items(album_id, options).await?;
|
||||||
|
|
||||||
|
if !result.items.is_empty() {
|
||||||
|
if let Err(e) = self.offline.save_to_cache(album_id, &result.items).await {
|
||||||
|
warn!("[HybridRepo] Failed to cache album tracks: {:?}", e);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
Ok(result.items)
|
||||||
|
}
|
||||||
|
|
||||||
/// Search only the local SQLite cache (downloaded content).
|
/// Search only the local SQLite cache (downloaded content).
|
||||||
///
|
///
|
||||||
/// Fast (100ms timeout) — used to render instant results before the server
|
/// Fast (100ms timeout) — used to render instant results before the server
|
||||||
|
|||||||
+16
-3
@@ -840,10 +840,23 @@ async downloadItemAndStart(request: DownloadItemAndStartRequest) : Promise<numbe
|
|||||||
return await TAURI_INVOKE("download_item_and_start", { request });
|
return await TAURI_INVOKE("download_item_and_start", { request });
|
||||||
},
|
},
|
||||||
/**
|
/**
|
||||||
* Queue an entire album for download
|
* Queue an entire album for download.
|
||||||
|
*
|
||||||
|
* Owns the whole operation: the album's track list comes from the server (the
|
||||||
|
* only place that knows all of it), every track is queued and linked to its
|
||||||
|
* album, each row's stream URL is resolved here, and the queue is pumped.
|
||||||
|
*
|
||||||
|
* The frontend used to do the second half — resolve one URL per track and pair
|
||||||
|
* it with the returned ids **by position**. That pairing had no basis: the ids
|
||||||
|
* came back in the backend's own order over a different set of rows, so
|
||||||
|
* whenever the two lists disagreed a row was handed another track's URL, and
|
||||||
|
* any track past the end of the shorter list was never started at all. Nothing
|
||||||
|
* crosses the boundary now except the album id.
|
||||||
|
*
|
||||||
|
* TRACES: UR-018, UR-055 | DR-173 | UT-170
|
||||||
*/
|
*/
|
||||||
async downloadAlbum(albumId: string, userId: string, basePath: string) : Promise<number[]> {
|
async downloadAlbum(handle: string, albumId: string, userId: string, basePath: string) : Promise<number[]> {
|
||||||
return await TAURI_INVOKE("download_album", { albumId, userId, basePath });
|
return await TAURI_INVOKE("download_album", { handle, albumId, userId, basePath });
|
||||||
},
|
},
|
||||||
/**
|
/**
|
||||||
* Queue a video item (movie or episode) for download with quality preset
|
* Queue a video item (movie or episode) for download with quality preset
|
||||||
|
|||||||
@@ -1,7 +1,6 @@
|
|||||||
<script lang="ts">
|
<script lang="ts">
|
||||||
import { downloads } from "$lib/stores/downloads";
|
import { downloads } from "$lib/stores/downloads";
|
||||||
import { auth } from "$lib/stores/auth";
|
import { auth } from "$lib/stores/auth";
|
||||||
import { commands } from "$lib/api/bindings";
|
|
||||||
import type { MediaItem } from "$lib/api/types";
|
import type { MediaItem } from "$lib/api/types";
|
||||||
|
|
||||||
interface Props {
|
interface Props {
|
||||||
@@ -83,28 +82,14 @@
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// Download the album: queue all tracks, then start each one
|
// Download the album. One call: the backend lists the album's tracks
|
||||||
|
// from the server, queues every one of them, resolves each stream URL
|
||||||
|
// and pumps the queue. This page's `tracks` are what the user is
|
||||||
|
// looking at, not the download list — pairing them against the returned
|
||||||
|
// ids by position is what used to leave most of an album unqueued.
|
||||||
const repo = auth.getRepository();
|
const repo = auth.getRepository();
|
||||||
const basePath = `albums/${albumId}`;
|
const basePath = `albums/${albumId}`;
|
||||||
const downloadIds = await downloads.downloadAlbum(albumId, userId, basePath);
|
await downloads.downloadAlbum(repo.getHandle(), albumId, userId, basePath);
|
||||||
|
|
||||||
// Get target directory for downloads
|
|
||||||
const targetDir = await commands.storageGetPath();
|
|
||||||
|
|
||||||
// Enqueue each track with its resolved stream URL. The backend queue
|
|
||||||
// pump starts up to max_concurrent at a time and advances through the
|
|
||||||
// rest automatically as slots free up — so we never hit (and silently
|
|
||||||
// drop) the concurrency limit the way startDownload did.
|
|
||||||
for (let i = 0; i < tracks.length && i < downloadIds.length; i++) {
|
|
||||||
try {
|
|
||||||
const streamUrl = await repo.getAudioStreamUrl(tracks[i].id);
|
|
||||||
if (streamUrl) {
|
|
||||||
await commands.enqueueDownload(downloadIds[i], streamUrl, targetDir);
|
|
||||||
}
|
|
||||||
} catch (e) {
|
|
||||||
console.error(`Failed to enqueue download for track ${tracks[i].id}:`, e);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Refresh to get updated statuses
|
// Refresh to get updated statuses
|
||||||
await downloads.refresh(userId);
|
await downloads.refresh(userId);
|
||||||
|
|||||||
@@ -158,9 +158,10 @@ describe("downloads store", () => {
|
|||||||
},
|
},
|
||||||
}); // get_downloads
|
}); // get_downloads
|
||||||
|
|
||||||
const ids = await downloads.downloadAlbum("album-1", "user-1", "/base/path");
|
const ids = await downloads.downloadAlbum("handle-1", "album-1", "user-1", "/base/path");
|
||||||
|
|
||||||
expect(mockInvoke).toHaveBeenCalledWith("download_album", {
|
expect(mockInvoke).toHaveBeenCalledWith("download_album", {
|
||||||
|
handle: "handle-1",
|
||||||
albumId: "album-1",
|
albumId: "album-1",
|
||||||
userId: "user-1",
|
userId: "user-1",
|
||||||
basePath: "/base/path",
|
basePath: "/base/path",
|
||||||
|
|||||||
@@ -191,12 +191,23 @@ function createDownloadsStore() {
|
|||||||
},
|
},
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Queue an entire album for download
|
* Queue an entire album for download.
|
||||||
|
*
|
||||||
|
* The backend does all of it — listing the album's tracks, queueing them,
|
||||||
|
* resolving each stream URL and starting the queue. It returns the queued
|
||||||
|
* row ids for reporting only; nothing here pairs them back to tracks.
|
||||||
|
*
|
||||||
|
* TRACES: UR-018, UR-055 | DR-173
|
||||||
*/
|
*/
|
||||||
async downloadAlbum(albumId: string, userId: string, basePath: string): Promise<number[]> {
|
async downloadAlbum(
|
||||||
|
handle: string,
|
||||||
|
albumId: string,
|
||||||
|
userId: string,
|
||||||
|
basePath: string
|
||||||
|
): Promise<number[]> {
|
||||||
try {
|
try {
|
||||||
console.log('📥 downloadAlbum called:', { albumId, userId, basePath });
|
console.log('📥 downloadAlbum called:', { albumId, userId, basePath });
|
||||||
const downloadIds = await commands.downloadAlbum(albumId, userId, basePath);
|
const downloadIds = await commands.downloadAlbum(handle, albumId, userId, basePath);
|
||||||
console.log(' Got download IDs from backend:', downloadIds);
|
console.log(' Got download IDs from backend:', downloadIds);
|
||||||
|
|
||||||
// Refresh downloads
|
// Refresh downloads
|
||||||
|
|||||||
Reference in New Issue
Block a user