From b6cf06ff2a6dd03f9b32cc818b2eef4686d25087 Mon Sep 17 00:00:00 2001 From: vicrodh <103399283+vicrodh@users.noreply.github.com> Date: Thu, 30 Jul 2026 17:14:40 -0600 Subject: [PATCH] feat(source): wire My QBZ through the seam, and land the owner's container rules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stage 2. The measured win: opening a 34-item collection now costs **2** library.db opens instead of 23-34 — the local source owns a pooled handle and hands out no guard, only `with(|db| ...)`, so a `!Send` `&LibraryDatabase` cannot cross an await either. Bug 1 from the seam design is dead by construction, not by care. The owner's rules, decided today, now live in one place instead of eight: - **Collection** takes albums, singles and EPs from ANY source — qobuz, folders, Plex, and whatever is added next. **Mixtape** takes albums, playlists and tracks. This was a single boolean (`restrict_to_mixtape`) that could express neither release types nor "collections accept local". - An unsupported combination must never be VISIBLE: the picker stops offering the impossible action, and the error stays a log-only safety net. Owner: "el bloqueo no debería ser visible nunca, más bien esto hay que ajustarlo en la UI". - **Ephemeral honours its name** — playable, queueable, and offerable to nothing else. - A row's source comes from the ROW. Eight QML sites built the add-to-mixtape payload with `"source": "qobuz"` written as a literal, so a Plex or local track was stored as Qobuz and afterwards could not play at all: the app asked the Qobuz API for an id that never existed there. `QobuzSource` reads the client through a lens on every call instead of holding a clone. That is not style: `qbz-core` REPLACES its client (core.rs:346 and :384, both on paths that run before `set_session`), so a cached clone goes stale and fails silently — the hazard the crate's own doc comment described. The lens is installed in `on_boot`, first of everything, and finding out why is the lesson of this commit. The gate was green — build, 142 tests, both QML audits, a clean offscreen smoke — and the feature was visibly broken: every Qobuz row showed "No results found", an empty Quality cell and a wrong "SINGLE" type, while the Plex rows resolved fine. The registry is a OnceLock, so whoever touches it first BUILDS it, and one built without the lens leaves Qobuz permanently detached. Only driving the real app surfaced it; the crate's error string named the cause exactly, which is the one thing that made it a ten-minute fix instead of an afternoon. cc --- crates/Cargo.lock | 1 + crates/qbz-qt/Cargo.toml | 1 + .../qbz-qt/qml/controls/AddToMixtapeModal.qml | 52 +- .../qbz-qt/qml/shell/NowPlayingBarSmall.qml | 34 +- crates/qbz-qt/qml/shell/PlayerBar.qml | 51 +- crates/qbz-qt/qml/views/AlbumView.qml | 10 + crates/qbz-qt/qml/views/LabelView.qml | 8 + crates/qbz-qt/qml/views/LibraryView.qml | 22 +- crates/qbz-qt/qml/views/MixView.qml | 11 + crates/qbz-qt/qml/views/PlaylistView.qml | 10 + crates/qbz-qt/qml/views/SearchView.qml | 11 + crates/qbz-qt/src/main.rs | 22 + crates/qbz-qt/src/myqbz_add_qt.rs | 430 ++++++++++++- crates/qbz-qt/src/myqbz_detail_qt.rs | 69 ++- crates/qbz-qt/src/myqbz_play_qt.rs | 138 +++-- crates/qbz-source/src/acceptance.rs | 575 ++++++++++++++++++ crates/qbz-source/src/lib.rs | 34 +- crates/qbz-source/src/registry.rs | 50 +- crates/qbz-source/src/sources/mod.rs | 2 +- crates/qbz-source/src/sources/qobuz.rs | 215 +++++-- 20 files changed, 1574 insertions(+), 172 deletions(-) create mode 100644 crates/qbz-source/src/acceptance.rs diff --git a/crates/Cargo.lock b/crates/Cargo.lock index cf1a2ff0b..efa99e34d 100644 --- a/crates/Cargo.lock +++ b/crates/Cargo.lock @@ -6638,6 +6638,7 @@ dependencies = [ "qbz-offline-cache", "qbz-plex", "qbz-qobuz", + "qbz-source", "qbz-text-utils", "qbz-theme", "rand 0.10.1", diff --git a/crates/qbz-qt/Cargo.toml b/crates/qbz-qt/Cargo.toml index 05c3db900..d2105788b 100644 --- a/crates/qbz-qt/Cargo.toml +++ b/crates/qbz-qt/Cargo.toml @@ -37,6 +37,7 @@ qbz-library = { path = "../qbz-library" } # Slint app uses (crates/qbz/Cargo.toml:96), so the two frontends share # library.db's mixtape tables rather than each growing their own. qbz-mixtape = { path = "../qbz-mixtape" } +qbz-source = { path = "../qbz-source" } # Cast: Chromecast + DLNA discovery, the two device families and the local # media server (src/cast_qt.rs drives it — nothing is reimplemented). qbz-cast = { path = "../qbz-cast" } diff --git a/crates/qbz-qt/qml/controls/AddToMixtapeModal.qml b/crates/qbz-qt/qml/controls/AddToMixtapeModal.qml index 16bcde9c3..21daf1c75 100644 --- a/crates/qbz-qt/qml/controls/AddToMixtapeModal.qml +++ b/crates/qbz-qt/qml/controls/AddToMixtapeModal.qml @@ -23,6 +23,17 @@ // title+subtitle; bulk -> "{count} items" + "{first} + N more"), so this file // never inspects the pending items. // +// WHAT MAY GO WHERE (R1) IS ALSO RUST'S ANSWER, not a rule this file re-derives +// — see myqbz_add_qt.rs `payload_accepts`. The picker LIST arrives already +// filtered (a container that cannot take the payload is never a row), and the +// two creatable kinds arrive as `allowMixtape` / `allowCollection`, which gate +// the footer chips AND the create-panel radios. That second half matters: with +// the old single `restrictToMixtape` flag the chips narrowed to one but the +// create panel still showed BOTH radios, so a track payload could still be +// flipped to "Collections" and written into one. Nothing here ever renders an +// action the payload cannot perform, which is the whole point — the backend +// block is a safety net that must stay invisible. +// // The name typed into the create panel is QML-LOCAL: the bridge takes it as an // argument (`createAndAdd(kind, name)`) and has no name property. Same for the // create KIND, which the two radios flip locally over the doc's value — there @@ -53,7 +64,11 @@ Item { readonly property bool loading: root.doc.loading === true readonly property bool creating: root.doc.creating === true readonly property bool createBusy: root.doc.createBusy === true - readonly property bool restrictToMixtape: root.doc.restrictToMixtape === true + // R1, as Rust computed it for the pending payload. Both may be true, one + // may be true; both false cannot reach a visible modal (Rust refuses to + // open the picker when nothing accepts the payload). + readonly property bool allowMixtape: root.doc.allowMixtape === true + readonly property bool allowCollection: root.doc.allowCollection === true readonly property string busyId: root.doc.busyId || "" readonly property string search: root.doc.search || "" readonly property var rows: root.doc.rows || [] @@ -65,8 +80,14 @@ Item { readonly property string createKind: root.createKindOverride !== "" ? root.createKindOverride : root.docCreateKind readonly property bool createMixtape: root.createKind === "mixtape" + // R1 for the kind the panel is currently on. The forbidden radio is not + // rendered, so this can only go false on a stale frame — but Create & Add + // must not be pressable in that frame either. + readonly property bool createKindAllowed: root.createKind === "collection" + ? root.allowCollection : root.allowMixtape // :207 — the raw non-empty check, exactly as Slint gates it (Rust trims). readonly property bool canCreate: root.createName !== "" && !root.createBusy + && root.createKindAllowed // A fresh panel starts empty and follows the kind Rust was asked for; a // closed modal must not keep a half-typed name for the next caller. @@ -266,21 +287,29 @@ Item { anchors.margins: 12 spacing: 8 // Both chips carry `horizontal-stretch: 1`, so one fills - // the row and two split it. - readonly property int chips: root.restrictToMixtape ? 1 : 2 - readonly property real chipW: (width - (chips - 1) * 8) / chips + // the row and two split it. R1 decides how many there are: + // one chip per CREATABLE container, never zero (a payload + // nothing accepts never opened this modal). The guard on + // `chips > 0` is only so a pre-publish frame cannot divide + // by zero. + readonly property int chips: + (root.allowMixtape ? 1 : 0) + (root.allowCollection ? 1 : 0) + readonly property real chipW: + chips > 0 ? (width - (chips - 1) * 8) / chips : 0 FooterIconButton { anchors.verticalCenter: parent.verticalCenter - width: parent.chipW + visible: root.allowMixtape + width: visible ? parent.chipW : 0 label: root.t("Mixtapes") onClicked: QbzMyQbzAdd.showCreate("mixtape") } FooterIconButton { - // Hidden for track/playlist payloads: collections hold - // whole albums only (:423). + // Offered only when a Collection can take the payload: + // albums / singles / EPs, from any source. Tracks and + // playlists have no Collection slot (R1). anchors.verticalCenter: parent.verticalCenter - visible: !root.restrictToMixtape + visible: root.allowCollection width: visible ? parent.chipW : 0 label: root.t("Collections") onClicked: QbzMyQbzAdd.showCreate("collection") @@ -426,14 +455,21 @@ Item { font.weight: theme.weightSemibold font.letterSpacing: 1.5 } + // R1 again, and this is the half the old flag missed: a + // radio for a kind the payload cannot go into is NOT + // rendered. A Row skips invisible children, so the single + // remaining (and already selected) radio simply reads as + // the kind being created. Row { spacing: 16 QbzRadioOption { + visible: root.allowMixtape label: root.t("Mixtapes") selected: root.createMixtape onClicked: if (!root.createBusy) root.createKindOverride = "mixtape" } QbzRadioOption { + visible: root.allowCollection label: root.t("Collections") selected: !root.createMixtape onClicked: if (!root.createBusy) root.createKindOverride = "collection" diff --git a/crates/qbz-qt/qml/shell/NowPlayingBarSmall.qml b/crates/qbz-qt/qml/shell/NowPlayingBarSmall.qml index 1f4e0f5b8..753e92cad 100644 --- a/crates/qbz-qt/qml/shell/NowPlayingBarSmall.qml +++ b/crates/qbz-qt/qml/shell/NowPlayingBarSmall.qml @@ -89,6 +89,25 @@ Rectangle { } } + // SOURCE of the now-playing track for the MyQBZ AddItem payload + // ("qobuz" | "local"), read off the queue document's `current` row — the + // same row the heart above reads — and never written as a literal. The + // full bar carries the long-form rationale (PlayerBar.qml `npSource`); the + // short version: `QbzPlayer` publishes no source word, `isLocal === false` + // proves a Qobuz catalog id, and `isLocal === true` conflates four id + // namespaces (local row / Plex / ephemeral / offline-cached Qobuz) that + // this bar cannot separate without sniffing the id. "" = unknown, and the + // menu then does not offer "Add to mixtape" at all. + readonly property string npSource: { + try { + var d = JSON.parse(QbzQueue.queueJson) + if (d && d.current && d.current.id === QbzPlayer.npTrackId) + return d.current.isLocal === true ? "" : "qobuz" + } catch (e) { + } + return "" + } + function fmt(secs) { var m = Math.floor(secs / 60) var s = Math.floor(secs % 60) @@ -480,8 +499,15 @@ Rectangle { { "label": QbzSession.tr("Add to queue", QbzSession.trRev), "icon": "list-end", "action": "queue" }, { "label": QbzSession.tr("Play later", QbzSession.trRev), "icon": "list-plus", "action": "later" }, { "label": QbzSession.tr("Play next", QbzSession.trRev), "icon": "list-start", "action": "next" }, - { "label": QbzSession.tr("Add to mixtape", QbzSession.trRev), "icon": "cassette-tape", "action": "mixtape" }, ] + // Only when the track's source is KNOWN (see `npSource`). + if (root.npSource !== "") { + m.push({ + "label": QbzSession.tr("Add to mixtape", QbzSession.trRev), + "icon": "cassette-tape", + "action": "mixtape" + }) + } if (QbzPlayer.npAlbumId !== "") { m.push({ "label": QbzSession.tr("Add album to collection", QbzSession.trRev), @@ -504,9 +530,11 @@ Rectangle { // MyQBZ AddItem, built here from the now-playing state: // `npArtworkPath` is a file:// CACHE path, so it is NOT the // artworkUrl — the store would keep a dead local path. - if (id !== "") + // Source from the queue's current ROW (`npSource`), not a + // literal; the entry is absent when that row cannot answer. + if (id !== "" && root.npSource !== "") QbzMyQbzAdd.open(JSON.stringify([{ - "itemType": "track", "source": "qobuz", + "itemType": "track", "source": root.npSource, "sourceItemId": id, "title": QbzPlayer.npTitle, "subtitle": QbzPlayer.npArtist, "artworkUrl": "", "year": null, "trackCount": null diff --git a/crates/qbz-qt/qml/shell/PlayerBar.qml b/crates/qbz-qt/qml/shell/PlayerBar.qml index fa06def69..d491ef466 100644 --- a/crates/qbz-qt/qml/shell/PlayerBar.qml +++ b/crates/qbz-qt/qml/shell/PlayerBar.qml @@ -134,6 +134,40 @@ Rectangle { } } + // SOURCE of the now-playing track — the word the MyQBZ AddItem payload + // needs ("qobuz" | "local"), NEVER a literal. + // + // This bar's "row" is the queue document's `current` row, the same row the + // heart above reads. `QbzPlayer` publishes no source word at all + // (player_bridge.rs:31-114 — there is no `np_source`), so the only + // published fact about the current track's provenance is that row's + // `isLocal` (queue_qt.rs:46-50, straight off `QueueTrack::is_local`). + // + // `isLocal === false` is unambiguous: the queue id IS a Qobuz catalog id. + // `isLocal === true` is NOT one thing — it covers a `library.db` row id, a + // `PLEX_TRACK_ID_FLOOR`-namespaced id, an `EPHEMERAL_ID_FLOOR` id, and the + // offline cache, whose queue id is a QOBUZ id (local_playback.rs:53-58). + // Those four need four different payloads and this bar cannot tell them + // apart without sniffing the id shape, which is exactly the scattered + // knowledge the source seam exists to end. So it answers "" = UNKNOWN, and + // the menu below drops "Add to mixtape" instead of stamping a guess: an + // action that cannot work must not be offered, and an ephemeral track must + // leave no trace outside the queue. + // + // The id guard is not paranoia-for-free: it is what keeps a stale queue + // document from lending the PREVIOUS track's provenance to this one. Every + // track edge republishes both (playback_qt.rs:1119-1120 and siblings), so + // it holds in steady state. + readonly property string npSource: { + try { + var d = JSON.parse(QbzQueue.queueJson) + if (d && d.current && d.current.id === QbzPlayer.npTrackId) + return d.current.isLocal === true ? "" : "qobuz" + } catch (e) { + } + return "" + } + // Audio settings — the button OPENS A FLYOUT (Normalization + Gapless), // it is not itself a toggle: `PlayerBar.slint:666-706`. Its `active` // colour does mirror normalization here (`:677 @@ -531,8 +565,16 @@ Rectangle { { "label": QbzSession.tr("Add to queue", QbzSession.trRev), "icon": "list-end", "action": "queue" }, { "label": QbzSession.tr("Play later", QbzSession.trRev), "icon": "list-plus", "action": "later" }, { "label": QbzSession.tr("Play next", QbzSession.trRev), "icon": "list-start", "action": "next" }, - { "label": QbzSession.tr("Add to mixtape", QbzSession.trRev), "icon": "cassette-tape", "action": "mixtape" }, ] + // Only when the track's source is KNOWN (see `npSource`): a row we + // cannot address is not offered. + if (root.npSource !== "") { + m.push({ + "label": QbzSession.tr("Add to mixtape", QbzSession.trRev), + "icon": "cassette-tape", + "action": "mixtape" + }) + } if (QbzPlayer.npAlbumId !== "") { m.push({ "label": QbzSession.tr("Add album to collection", QbzSession.trRev), @@ -555,9 +597,12 @@ Rectangle { // MyQBZ AddItem, built here from the now-playing state: // `npArtworkPath` is a file:// CACHE path, so it is NOT the // artworkUrl — the store would keep a dead local path. - if (id !== "") + // The source comes from the queue's current ROW (`npSource`), + // never from a literal; the entry is not even in the menu when + // that row cannot answer. + if (id !== "" && root.npSource !== "") QbzMyQbzAdd.open(JSON.stringify([{ - "itemType": "track", "source": "qobuz", + "itemType": "track", "source": root.npSource, "sourceItemId": id, "title": QbzPlayer.npTitle, "subtitle": QbzPlayer.npArtist, "artworkUrl": "", "year": null, "trackCount": null diff --git a/crates/qbz-qt/qml/views/AlbumView.qml b/crates/qbz-qt/qml/views/AlbumView.qml index 393dc9d51..81fa38691 100644 --- a/crates/qbz-qt/qml/views/AlbumView.qml +++ b/crates/qbz-qt/qml/views/AlbumView.qml @@ -962,6 +962,16 @@ Rectangle { // MyQBZ "Add to mixtape" — the HOST builds the // AddItem array (TrackRow does not know // itemType/source). + // + // SOURCE: this view never shows a local album. + // `open_album` (main.rs:538-548) routes any id + // that `is_local_feed_id("album", …)` accepts + // — a Plex `plex:` key, a group key, a path — + // to the LocalAlbum view instead, so what + // reaches `album_qt::TrackRow` here is always + // a `/album/get` response. `item.id` is a + // Qobuz catalog id by construction of the + // route, not by assumption at this call site. onMixtapeRequested: QbzMyQbzAdd.open(JSON.stringify([{ "itemType": "track", "source": "qobuz", "sourceItemId": item.id, "title": item.title || "", diff --git a/crates/qbz-qt/qml/views/LabelView.qml b/crates/qbz-qt/qml/views/LabelView.qml index 1d1ae8625..2c8f87d62 100644 --- a/crates/qbz-qt/qml/views/LabelView.qml +++ b/crates/qbz-qt/qml/views/LabelView.qml @@ -525,6 +525,14 @@ Rectangle { // MyQBZ "Add to mixtape" — the HOST builds the // AddItem array (TrackRow does not know // itemType/source). + // + // SOURCE: a label page is a Qobuz catalog surface. + // These rows are the `/label/page` response's + // `top_tracks` mapped to track rows + // (label_qt.rs:280-287) — there is no local label + // library and no non-Qobuz row can reach this + // Repeater, so `modelData.id` is a Qobuz catalog + // id by construction of the document. onMixtapeRequested: QbzMyQbzAdd.open(JSON.stringify([{ "itemType": "track", "source": "qobuz", "sourceItemId": modelData.id, diff --git a/crates/qbz-qt/qml/views/LibraryView.qml b/crates/qbz-qt/qml/views/LibraryView.qml index 96a3fc89c..e1f449ff2 100644 --- a/crates/qbz-qt/qml/views/LibraryView.qml +++ b/crates/qbz-qt/qml/views/LibraryView.qml @@ -1466,8 +1466,28 @@ Rectangle { // MyQBZ "Add to mixtape" — the HOST builds the // AddItem array (TrackRow does not know // itemType/source). + // + // The source comes off the ROW: every feed row + // carries its own word (library_qt.rs:54 — + // "qobuz" | "local" | "plex"), so a Plex or local + // row can never be stored as a Qobuz id again. + // "plex" is folded into "local" because the store + // has no Plex source: `AddItem.source` is + // "qobuz" | "local" and `source_from_str` + // (myqbz_add_qt.rs:85-90) maps everything that is + // not "local" to Qobuz — passing "plex" verbatim + // would silently become "qobuz" again. + // + // Today this list only ever holds Qobuz rows (the + // Tracks tab keeps `kind === "track" && group === + // "favorites"` and the local layer lands in group + // "local"), so this is a no-op until that filter + // changes — which is the point: it stays right + // when it does. onMixtapeRequested: QbzMyQbzAdd.open(JSON.stringify([{ - "itemType": "track", "source": "qobuz", + "itemType": "track", + "source": item.source === "local" || item.source === "plex" + ? "local" : "qobuz", "sourceItemId": item.id, "title": item.title || "", "subtitle": item.artist || "", "artworkUrl": "", "year": null, "trackCount": null diff --git a/crates/qbz-qt/qml/views/MixView.qml b/crates/qbz-qt/qml/views/MixView.qml index be958b8e7..4762bd9f1 100644 --- a/crates/qbz-qt/qml/views/MixView.qml +++ b/crates/qbz-qt/qml/views/MixView.qml @@ -285,6 +285,17 @@ Rectangle { } // MyQBZ "Add to mixtape" — the HOST builds the AddItem // array (TrackRow does not know itemType/source). + // + // SOURCE: these rows carry no source field, and they do + // not need one — every one of the four mixes is built + // from `qbz_models::Track` values that came back from the + // Qobuz API (foryou_qt.rs:855-873: dynamic/suggest for + // daily+weekly, `get_favorites("tracks")` for fav, + // `get_playlist` for top), and the DailyQ/WeeklyQ seed + // explicitly drops local / Plex / ephemeral recents + // (foryou_qt.rs:774-776). `modelData.id` is therefore a + // Qobuz catalog id by construction of the document, not + // by assumption at this call site. onMixtapeRequested: QbzMyQbzAdd.open(JSON.stringify([{ "itemType": "track", "source": "qobuz", "sourceItemId": modelData.id, diff --git a/crates/qbz-qt/qml/views/PlaylistView.qml b/crates/qbz-qt/qml/views/PlaylistView.qml index f883b4413..2e6fc08a8 100644 --- a/crates/qbz-qt/qml/views/PlaylistView.qml +++ b/crates/qbz-qt/qml/views/PlaylistView.qml @@ -647,6 +647,16 @@ Rectangle { onRemoveRequested: QbzBridge.playlistRemoveTrack(item.playlistTrackId) // MyQBZ "Add to mixtape" — the HOST builds the AddItem // array (TrackRow does not know itemType/source). + // + // SOURCE: `PlaylistTrackRow` (playlist_qt.rs:47-78) has no + // source field because this whole view has one source — + // LOCAL playlists are out of scope everywhere in it + // (playlist_qt.rs:23-25), the document is mapped from + // Qobuz `Track` values, and `playlistTrackId` is the + // Qobuz membership row id. `item.id` is a Qobuz catalog + // id by construction of the view, not by assumption here. + // If local playlists are ever ported, the row must start + // carrying its source and this must read it. onMixtapeRequested: QbzMyQbzAdd.open(JSON.stringify([{ "itemType": "track", "source": "qobuz", "sourceItemId": item.id, "title": item.title || "", diff --git a/crates/qbz-qt/qml/views/SearchView.qml b/crates/qbz-qt/qml/views/SearchView.qml index 55fb60166..8a23625f7 100644 --- a/crates/qbz-qt/qml/views/SearchView.qml +++ b/crates/qbz-qt/qml/views/SearchView.qml @@ -705,6 +705,17 @@ Rectangle { // MyQBZ "Add to mixtape" — the HOST builds the // AddItem array (TrackRow does not know // itemType/source). + // + // SOURCE: the results page has ONE source. + // `search_qt::TrackRow` (:171) carries no + // source word because every row is mapped from + // a Qobuz `Track` (`map_track`, :320-336) and + // the local "on this device" sections are not + // ported (search_qt.rs:24-27) — the cortinilla + // rows, which CAN be local, carry their own + // `source` field and are a different document. + // `item.id` is a Qobuz catalog id by + // construction, not by assumption here. onMixtapeRequested: QbzMyQbzAdd.open(JSON.stringify([{ "itemType": "track", "source": "qobuz", "sourceItemId": item.id, "title": item.title || "", diff --git a/crates/qbz-qt/src/main.rs b/crates/qbz-qt/src/main.rs index 52240567a..44407afd0 100644 --- a/crates/qbz-qt/src/main.rs +++ b/crates/qbz-qt/src/main.rs @@ -185,6 +185,28 @@ pub(crate) fn ui(f: impl FnOnce(Pin<&mut QbzBridge>) + Send + 'static) { /// Component.onCompleted — the Qt-thread handle is registered first, so /// every `ui()` hop below lands). pub(crate) fn on_boot() { + // The source registry, FIRST of all — before anything can call + // `qbz_source::registry()`. It is a OnceLock, so whoever touches it first + // BUILDS it, and a registry built without the client lens leaves the Qobuz + // source permanently detached: every catalog call then answers + // `NotConfigured`, loudly but uselessly. Measured exactly that way on the + // first wiring pass — the local and Plex rows resolved while all 26 Qobuz + // albums came back "no client lens installed". + // + // A LENS, not a cached clone: `qbz-core` REPLACES its client (core.rs:346 + // and :384, both from paths that run before `set_session`), so anything + // holding a clone goes stale and fails silently. Reading through on every + // call is what makes that unrepresentable. The read guard lives inside the + // returned future and is dropped with it, which is the same discipline + // `myqbz_play_qt` documents for its own clone-then-drop. + qbz_source::init_registry(std::sync::Arc::new(|| { + Box::pin(async { + let lock = app().core().client(); + let guard = lock.read().await; + guard.as_ref().cloned() + }) + })); + // Offline guardrails FIRST (before the login screen can show): engine + // connectivity actor, then the live status forwarder. Both need the // tokio runtime context for their spawns. diff --git a/crates/qbz-qt/src/myqbz_add_qt.rs b/crates/qbz-qt/src/myqbz_add_qt.rs index de4e1a0c1..92a60f6fa 100644 --- a/crates/qbz-qt/src/myqbz_add_qt.rs +++ b/crates/qbz-qt/src/myqbz_add_qt.rs @@ -5,10 +5,17 @@ //! of [`AddItem`]s (from QML as a JSON array through `QbzMyQbzAdd.open`, or //! directly from a Rust seam such as the Local Library bulk bar). The batch is //! held in a process-global PENDING store; the picker list is loaded -//! kind-restricted + recency-sorted + `item_exists`-resolved, and on pick / +//! R1-filtered + recency-sorted + `item_exists`-resolved, and on pick / //! create-and-add the items are written into the chosen collection through the //! shared `qbz_mixtape::repo`. //! +//! WHAT MAY GO WHERE is decided ONCE, by [`payload_accepts`] (the R1 rule +//! below). The picker never renders a container that cannot accept the payload, +//! never offers a create chip or a create radio for one, and refuses to open at +//! all when NOTHING can accept it. Every refusal is a `log::warn!` and nothing +//! else: an impossible combination must not be reachable, so the user never +//! sees a block — the block only exists as a safety net behind the UI (R2). +//! //! Dedup is the backend's job: `add_item_with(allow_duplicate = false)` returns //! `Ok(false)` for an exact `(collection_id, source, source_item_id)` duplicate //! — that is NOT an error. We count added vs skipped and surface the net result @@ -89,6 +96,170 @@ fn source_from_str(s: &str) -> AlbumSource { } } +// --------------------------------------------------------------------------- +// R1 — which container accepts which item. THE ONE PLACE. +// --------------------------------------------------------------------------- +// +// >>> TEMPORARY HOME. This rule MUST move to `qbz-source` <<< +// +// It is a property of the ITEM (its kind and its source), not of the MyQBZ +// picker, and the picker is only its first caller: the per-row flyouts +// (TrackRow, PlayerBar, NowPlayingBarSmall, the Local Library bulk bars) have +// to ask the same question to decide whether to render "Add to mixtape" at all. +// The moment a second call site re-derives it by hand, the rule has been +// scattered and the class of bug this replaces is back. When `qbz-source` +// grows the predicate, delete `Accepts` / `accepts_item` / `payload_accepts` +// here and forward to it; NOTHING else in this file reads the rule directly. + +/// The set of container kinds that may accept a payload. +/// +/// Read it as three independent yes/no answers rather than a "restriction +/// level": the old `restrict_to_mixtape` boolean could only express "mixtapes +/// only" vs "everything", which cannot say that a LOCAL album belongs in a +/// Collection while a LOCAL playlist belongs nowhere. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct Accepts { + pub mixtape: bool, + pub collection: bool, + /// A built discography. It is a Collection in every way that matters here + /// (it holds whole albums), it is just not creatable from this picker — + /// hence a third flag rather than reusing `collection`. + pub artist_collection: bool, +} + +impl Accepts { + const NONE: Accepts = Accepts { + mixtape: false, + collection: false, + artist_collection: false, + }; + const ALL: Accepts = Accepts { + mixtape: true, + collection: true, + artist_collection: true, + }; + const MIXTAPE_ONLY: Accepts = Accepts { + mixtape: true, + collection: false, + artist_collection: false, + }; + + /// Anything at all can take this payload. + fn any(self) -> bool { + self.mixtape || self.collection || self.artist_collection + } + + /// A batch is offerable to a container only if EVERY item is. + fn intersect(self, other: Accepts) -> Accepts { + Accepts { + mixtape: self.mixtape && other.mixtape, + collection: self.collection && other.collection, + artist_collection: self.artist_collection && other.artist_collection, + } + } + + /// The picker's row filter. + fn allows(self, kind: CollectionKind) -> bool { + match kind { + CollectionKind::Mixtape => self.mixtape, + CollectionKind::Collection => self.collection, + CollectionKind::ArtistCollection => self.artist_collection, + } + } + + /// The create sub-panel's filter. `"collection"` maps to `Collection`, + /// anything else to `Mixtape` — the same mapping [`create_collection`] + /// applies, so the gate and the write can never disagree. + fn allows_create(self, kind: &str) -> bool { + if kind == "collection" { + self.collection + } else { + self.mixtape + } + } + + /// The kind the create panel should open on when `wanted` is not offered: + /// the one that IS. `None` = nothing is creatable for this payload. + fn create_fallback(self) -> Option<&'static str> { + if self.mixtape { + Some("mixtape") + } else if self.collection { + Some("collection") + } else { + None + } + } +} + +/// R3 — an ephemeral row leaves NO trace: it can be played and queued, never +/// stored. Ephemeral ids are synthetic and high (`>= 2^48`, +/// `local_ephemeral::EPHEMERAL_ID_FLOOR`), and neither a `local_tracks` row id +/// (`< 2^40`, below even `local_plex::PLEX_TRACK_ID_FLOOR`) nor a Qobuz track +/// id ever reaches that floor. +/// +/// The test is on the ID ALONE, deliberately — NOT on `it.source`. Several +/// call sites stamp a literal `"source": "qobuz"` onto whatever is playing +/// (PlayerBar.qml:559, NowPlayingBarSmall.qml:508), so a source-gated check +/// would wave an ephemeral now-playing track straight through. +fn is_ephemeral_item(it: &AddItem) -> bool { + it.source_item_id + .trim() + .parse::() + .is_ok_and(crate::local_ephemeral::is_ephemeral_id) +} + +/// R1 for ONE item. +/// +/// - **Collection** (and artist collection): albums, singles and EPs — which +/// in this payload vocabulary are all one thing, `itemType == "album"`; there +/// is no release-type field and R1 draws no line between them. From ANY +/// source: `"local"` covers folders, Plex and Qobuz downloads alike +/// (`qbz-models/src/mixtape.rs` — `AlbumSource::Local` is an umbrella), so +/// the source is NOT consulted for an album. +/// - **Mixtape**: albums, playlists and tracks. +/// - **A LOCAL playlist belongs nowhere.** `qbz_mixtape::enqueue:: +/// resolve_local_item` (`enqueue.rs:404,417-422`) answers `(Playlist, Local)` +/// with the hard error `"local playlists not supported in this release"` — +/// there is no local-only playlist id to resolve against. That error is THE +/// unsupported combination R2 talks about; it stays where it is, as the +/// safety net, and this arm is what makes it unreachable from the UI. +/// - **Ephemeral: nothing** (R3). +/// +/// Kind and source are read through `item_type_from_str` / `source_from_str`, +/// the same two functions [`add_items`] writes with, so the gate can never +/// classify an item differently from the row that would be inserted. +fn accepts_item(it: &AddItem) -> Accepts { + if is_ephemeral_item(it) { + return Accepts::NONE; + } + match item_type_from_str(&it.item_type) { + ItemType::Album => Accepts::ALL, + ItemType::Track => Accepts::MIXTAPE_ONLY, + ItemType::Playlist => match source_from_str(&it.source) { + AlbumSource::Qobuz => Accepts::MIXTAPE_ONLY, + AlbumSource::Local => Accepts::NONE, + }, + } +} + +/// R1 for a BATCH: a container is offered only when it accepts every item. +/// An empty payload accepts nothing (the picker has nothing to add). +pub(crate) fn payload_accepts(items: &[AddItem]) -> Accepts { + if items.is_empty() { + return Accepts::NONE; + } + items + .iter() + .fold(Accepts::ALL, |acc, it| acc.intersect(accepts_item(it))) +} + +/// The rule applied to whatever is currently pending. Used by the mutating +/// handlers so they re-derive from the payload rather than trusting a document +/// flag a stale QML frame could still be holding. +fn pending_accepts() -> Accepts { + payload_accepts(&pending_snapshot()) +} + // --------------------------------------------------------------------------- // Document (spec 02 §5.3) // --------------------------------------------------------------------------- @@ -121,10 +292,16 @@ struct AddDoc { header_subtitle: String, #[serde(rename = "bulkMode")] bulk_mode: bool, - /// True when ANY pending item is not an album (collections hold whole - /// albums only, so the list narrows to mixtapes). - #[serde(rename = "restrictToMixtape")] - restrict_to_mixtape: bool, + /// R1, projected for the create surfaces: whether a NEW mixtape / a NEW + /// collection may be created for this payload. They are the create chips' + /// and the create radios' visibility, and they are NOT a "restriction + /// level" — both can be true, both can be false (in which case the picker + /// never opened at all). The ROW list is filtered in Rust, so QML has no + /// second copy of the rule to keep in sync. + #[serde(rename = "allowMixtape")] + allow_mixtape: bool, + #[serde(rename = "allowCollection")] + allow_collection: bool, search: String, /// The collection currently being written to ("" = idle). #[serde(rename = "busyId")] @@ -146,7 +323,10 @@ impl Default for AddDoc { header_title: String::new(), header_subtitle: String::new(), bulk_mode: false, - restrict_to_mixtape: false, + // A closed document offers nothing; `open_items` seeds the real + // answer before `open` ever becomes true. + allow_mixtape: false, + allow_collection: false, search: String::new(), busy_id: String::new(), creating: false, @@ -187,18 +367,18 @@ pub struct LoadedRow { pub already_has: bool, } -/// Load the collections offered as targets, kind-restricted + recency-sorted + +/// Load the collections offered as targets, R1-filtered + recency-sorted + /// `item_exists`-resolved. BLOCKING (DB) — call from `spawn_blocking`. /// -/// - `restrict_to_mixtape` keeps `kind == mixtape` only (which also excludes -/// artist_collections); -/// - otherwise there is NO kind restriction at all: an album may be added to a -/// Collection AND to an artist_collection, so the user can augment a built -/// discography (`myqbz_add.rs:159-167`); +/// - `accepts` is [`payload_accepts`] for this batch and is the ONLY row +/// filter. A container the payload cannot go into is dropped here, in Rust, +/// so it never reaches the model and cannot be clicked (R2). An album batch +/// still sees artist_collections, so a built discography can be augmented +/// (`myqbz_add.rs:159-167`); /// - sort = `last_played_at ?? updated_at` DESC; /// - `already_has` = every pending item's `(source, source_item_id)` is already /// in that collection. -pub(crate) fn load_rows(restrict_to_mixtape: bool, items: &[AddItem]) -> Vec { +pub(crate) fn load_rows(accepts: Accepts, items: &[AddItem]) -> Vec { crate::library_db_qt::with_db(false, |db| { Ok(db.with_connection(|conn| { let mut cols: Vec = @@ -207,9 +387,7 @@ pub(crate) fn load_rows(restrict_to_mixtape: bool, items: &[AddItem]) -> Vec) { if items.is_empty() { return; } let bulk = items.len() > 1; - // Restrict to mixtapes when ANY pending item is a track/playlist. - let restrict = items.iter().any(|it| it.item_type != "album"); + let accepts = payload_accepts(&items); + if !accepts.any() { + log::warn!( + "[qbz-qt] myqbz_add: no container accepts this payload, picker not opened \ + ({} item(s), first = {}/{} id {}) — the caller should not be offering \ + \"Add to Mixtape/Collection\" for it", + items.len(), + items[0].item_type, + items[0].source, + items[0].source_item_id + ); + return; + } let first_title = items[0].title.clone(); let first_subtitle = items[0].subtitle.clone().unwrap_or_default(); @@ -333,11 +528,14 @@ pub(crate) fn open_items(items: Vec) { doc.header_title = header_title; doc.header_subtitle = header_subtitle; doc.bulk_mode = bulk; - doc.restrict_to_mixtape = restrict; + doc.allow_mixtape = accepts.mixtape; + doc.allow_collection = accepts.collection; doc.search = String::new(); doc.busy_id = String::new(); doc.creating = false; - doc.create_kind = "mixtape".to_string(); + // Never preset the panel to a kind this payload cannot create; + // `accepts.any()` above guarantees the fallback exists. + doc.create_kind = accepts.create_fallback().unwrap_or("mixtape").to_string(); doc.create_busy = false; doc.loading = true; doc.open = true; @@ -346,7 +544,7 @@ pub(crate) fn open_items(items: Vec) { let items = pending_snapshot(); crate::spawn(async move { - let rows = tokio::task::spawn_blocking(move || load_rows(restrict, &items)) + let rows = tokio::task::spawn_blocking(move || load_rows(accepts, &items)) .await .unwrap_or_default(); *ROWS_CACHE.lock().unwrap_or_else(|e| e.into_inner()) = rows; @@ -396,13 +594,32 @@ pub(crate) fn search(query: &str) { } /// Open the create sub-panel preset to a kind ("mixtape" | "collection"). +/// +/// R1-clamped: a kind the pending payload cannot go into is replaced by one it +/// can, and if it can go nowhere the panel does not open. The chip that would +/// ask for a forbidden kind is not rendered in the first place — this is the +/// safety net behind it, and like every other one here it is log-only. pub(crate) fn show_create(kind: &str) { + let accepts = pending_accepts(); + let wanted = if kind == "collection" { + "collection" + } else { + "mixtape" + }; + let resolved = if accepts.allows_create(wanted) { + Some(wanted) + } else { + accepts.create_fallback() + }; + let Some(resolved) = resolved else { + log::warn!("[qbz-qt] myqbz_add: create '{kind}' refused — nothing is creatable for this payload"); + return; + }; + if resolved != wanted { + log::warn!("[qbz-qt] myqbz_add: create '{wanted}' is not offered for this payload, using '{resolved}'"); + } with_doc(|doc| { - doc.create_kind = if kind == "collection" { - "collection".to_string() - } else { - "mixtape".to_string() - }; + doc.create_kind = resolved.to_string(); doc.create_busy = false; doc.creating = true; }); @@ -485,20 +702,33 @@ fn toast_outcome(name: &str, outcome: &AddOutcome) { crate::toast_qt::success(msg); } -/// The chosen collection's display name, for the toast. -fn row_name(collection_id: &str) -> String { +/// The chosen collection's display name, for the toast. `None` when the id is +/// not one of the rows this picker loaded. +fn row_name(collection_id: &str) -> Option { ROWS_CACHE .lock() .unwrap_or_else(|e| e.into_inner()) .iter() .find(|r| r.id == collection_id) .map(|r| r.name.clone()) - .unwrap_or_default() } /// Pick a target: add every pending item, toast the outcome, close. /// Re-entrant clicks while a write is in flight are ignored. +/// +/// The id MUST be one of the loaded rows, and those are already R1-filtered by +/// [`load_rows`] — so this one lookup is also the R2 net on the write path: an +/// invokable called with any other collection id (a stale QML frame, a row that +/// was filtered out) writes nothing. Log-only, per R2. pub(crate) fn pick(collection_id: &str) { + let Some(name) = row_name(collection_id) else { + log::warn!( + "[qbz-qt] myqbz_add: pick('{collection_id}') is not an offered row for this \ + payload, ignored" + ); + return; + }; + let accepted = with_doc(|doc| { if !doc.busy_id.is_empty() { return false; @@ -511,7 +741,6 @@ pub(crate) fn pick(collection_id: &str) { } publish(); - let name = row_name(collection_id); let items = pending_snapshot(); let cid = collection_id.to_string(); crate::spawn(async move { @@ -548,6 +777,19 @@ pub(crate) fn create_collection(kind: &str, name: &str) -> Option<(String, Strin /// raw English literal the reference uses (`main.rs:6009`, absent from all /// eight catalogs, kept 1:1). pub(crate) fn create_and_add(kind: &str, name: &str) { + // R1 net on the create path: the radio for a forbidden kind is not + // rendered, so reaching here means a stale frame or a direct invokable + // call. Log-only (R2) — never a toast. + if !pending_accepts().allows_create(kind) { + log::warn!( + "[qbz-qt] myqbz_add: createAndAdd('{kind}') refused — this payload cannot go \ + into a {kind}" + ); + with_doc(|doc| doc.create_busy = false); + publish(); + return; + } + let trimmed = name.trim().to_string(); let accepted = with_doc(|doc| { if trimmed.is_empty() || doc.create_busy { @@ -662,6 +904,132 @@ mod tests { assert_eq!(item_type_from_str("playlist"), ItemType::Playlist); } + fn add(item_type: &str, source: &str, id: &str) -> AddItem { + AddItem { + item_type: item_type.into(), + source: source.into(), + source_item_id: id.into(), + title: "t".into(), + subtitle: None, + artwork_url: None, + year: None, + track_count: None, + } + } + + #[test] + fn r1_albums_go_everywhere_from_every_source() { + // "Collection: albums, singles, EPs. From ANY source — qobuz, local + // (folders AND Plex AND anything added later)." + for source in ["qobuz", "local"] { + let a = payload_accepts(&[add("album", source, "12345")]); + assert_eq!(a, Accepts::ALL, "album/{source} must reach every container"); + } + // A Plex album key is a `local` album — the id shape changes nothing. + assert_eq!( + payload_accepts(&[add("album", "local", "plex:5677211365378243606")]), + Accepts::ALL + ); + } + + #[test] + fn r1_tracks_are_mixtape_only() { + for source in ["qobuz", "local"] { + assert_eq!( + payload_accepts(&[add("track", source, "42")]), + Accepts::MIXTAPE_ONLY + ); + } + } + + #[test] + fn r1_qobuz_playlists_are_mixtape_only_and_local_playlists_go_nowhere() { + assert_eq!( + payload_accepts(&[add("playlist", "qobuz", "998877")]), + Accepts::MIXTAPE_ONLY + ); + // R2: `qbz_mixtape::enqueue::resolve_local_item` hard-errors on + // (Playlist, Local). Offering ANY container for it is what makes that + // error reachable, so nothing is offered and the picker never opens. + let local_playlist = payload_accepts(&[add("playlist", "local", "whatever")]); + assert_eq!(local_playlist, Accepts::NONE); + assert!(!local_playlist.any()); + } + + #[test] + fn r3_ephemeral_items_are_accepted_by_nothing() { + let eph = (1i64 << 48) + 7; + let a = payload_accepts(&[add("track", "local", &eph.to_string())]); + assert_eq!(a, Accepts::NONE, "an ephemeral row leaves no trace"); + // And a caller that stamps a literal "qobuz" source onto whatever is + // playing (PlayerBar.qml:559) must not slip past — the ID decides. + assert_eq!( + payload_accepts(&[add("track", "qobuz", &eph.to_string())]), + Accepts::NONE + ); + // An ephemeral ALBUM is still ephemeral, despite being album-shaped. + assert_eq!( + payload_accepts(&[add("album", "local", &eph.to_string())]), + Accepts::NONE + ); + } + + #[test] + fn ids_below_the_ephemeral_floor_are_untouched() { + // A Plex track id is namespaced at 2^40 — well under 2^48, so it stays + // a normal local track. + let plex = (1i64 << 40) | 4444; + assert_eq!( + payload_accepts(&[add("track", "local", &plex.to_string())]), + Accepts::MIXTAPE_ONLY + ); + // A non-numeric id is not an ephemeral id. + assert_eq!( + payload_accepts(&[add("album", "qobuz", "e94no900otyrz")]), + Accepts::ALL + ); + } + + #[test] + fn a_batch_is_offered_only_what_every_item_accepts() { + // Album + track -> mixtape only (the album's Collection slot loses). + assert_eq!( + payload_accepts(&[add("album", "qobuz", "1"), add("track", "qobuz", "2")]), + Accepts::MIXTAPE_ONLY + ); + // One local playlist poisons the batch — exactly the item that would + // hard-error at resolve time. + assert_eq!( + payload_accepts(&[add("album", "qobuz", "1"), add("playlist", "local", "p")]), + Accepts::NONE + ); + // Empty is nothing, not everything. + assert_eq!(payload_accepts(&[]), Accepts::NONE); + } + + #[test] + fn accepts_drives_rows_and_create_consistently() { + let tracks = Accepts::MIXTAPE_ONLY; + assert!(tracks.allows(CollectionKind::Mixtape)); + assert!(!tracks.allows(CollectionKind::Collection)); + assert!( + !tracks.allows(CollectionKind::ArtistCollection), + "an artist collection is a discography — albums only" + ); + assert!(tracks.allows_create("mixtape")); + assert!(!tracks.allows_create("collection")); + assert_eq!(tracks.create_fallback(), Some("mixtape")); + + let albums = Accepts::ALL; + assert!(albums.allows(CollectionKind::ArtistCollection)); + assert!(albums.allows_create("collection")); + + assert_eq!(Accepts::NONE.create_fallback(), None); + // `allows_create` uses create_collection's own mapping: unknown = mixtape. + assert!(albums.allows_create("nonsense")); + assert!(!Accepts::NONE.allows_create("nonsense")); + } + #[test] fn kind_icons_match_the_reference() { assert_eq!(kind_icon(CollectionKind::Mixtape), "cassette"); diff --git a/crates/qbz-qt/src/myqbz_detail_qt.rs b/crates/qbz-qt/src/myqbz_detail_qt.rs index 46b5ee978..457beff67 100644 --- a/crates/qbz-qt/src/myqbz_detail_qt.rs +++ b/crates/qbz-qt/src/myqbz_detail_qt.rs @@ -83,7 +83,11 @@ pub struct InlineTrackRow { /// Always "" — the rows are mounted with `showArtwork: false`. #[serde(rename = "artPath")] pub art_path: String, - /// "qobuz" | "local" | "plex" — the delegate's `draggable` gate. + /// "qobuz" | "local" | "offline" | "plex" — the delegate's `draggable` + /// gate, and a `SourceIcon.qml` kind. `"offline"` is the badge of a Qobuz + /// download/purchase, which is a local FILE with a cloud glyph; it appears + /// here because the resolved rows now carry the source's real word instead + /// of a flattened `"local"` (see `source_kind_of`). pub source: String, } @@ -107,8 +111,11 @@ pub struct DetailRow { /// local/Plex items, whose tracks carry none). #[serde(rename = "artistId")] pub artist_id: String, - /// RESOLVED source ("qobuz" | "plex" | "local") — this is how a Plex item - /// stored as `AlbumSource::Local` finally reads "plex". + /// RESOLVED source ("qobuz" | "plex" | "local" | "offline") — this is how a + /// Plex item stored as `AlbumSource::Local` finally reads "plex", and a + /// Qobuz download stored the same way reads "offline". Both come from the + /// registry's claim on the resolved row (`source_kind_of`), never from a + /// literal. #[serde(rename = "sourceKind")] pub source_kind: String, #[serde(rename = "typeLabel")] @@ -575,6 +582,35 @@ fn build_rows(items: &[MixtapeCollectionItem]) -> Vec { .collect() } +/// The source word a RESOLVED track renders as — the glyph kind in +/// `SourceIcon.qml` and the `sourceKind` field of a detail row. +/// +/// It comes from the ROW, never from a literal. What it replaces (twice) was +/// +/// ```ignore +/// track.source.clone().unwrap_or_else(|| if track.is_local { "local" } else { "qobuz" }) +/// ``` +/// +/// — a hand-written fallback that guessed a source out of one boolean, and +/// which drops every word that is not exactly `"local"` or `"qobuz"`. +/// `SourceRegistry::claim` applies the ONE source-word table +/// (`qbz_source::SourceId::from_word`) to all six words that really occur, and +/// for a LOCAL row the BADGE keeps the distinction the glyph needs: a Qobuz +/// download is a local FILE (source `local`) but must draw the cloud glyph +/// (badge `offline`), which the source id alone collapses. +/// +/// A row nothing claims (`Unclaimed` / `Ambiguous`) falls back to its badge, +/// which is `"local"` for an unknown word — never `"qobuz"`, because guessing +/// Qobuz from "not otherwise recognised" is exactly the mis-attribution the +/// seam deletes. +fn source_kind_of(track: &QueueTrack) -> &'static str { + let raw = qbz_source::RawRef::from_queue_track(track); + match qbz_source::registry().claim(&raw).map(|m| m.source()) { + Ok(id) if id != qbz_source::SourceId::LOCAL => id.as_str(), + _ => raw.badge.as_str(), + } +} + /// One inline track row from a resolved `QueueTrack` (spec 02 §5.2). fn track_to_item(track: &QueueTrack) -> InlineTrackRow { let quality_tier = match track.bit_depth { @@ -588,13 +624,7 @@ fn track_to_item(track: &QueueTrack) -> InlineTrackRow { } else { crate::home_qt::quality_detail_from_parts(track.bit_depth, track.sample_rate) }; - let source = track.source.clone().unwrap_or_else(|| { - if track.is_local { - "local".to_string() - } else { - "qobuz".to_string() - } - }); + let source = source_kind_of(track).to_string(); InlineTrackRow { id: track.id.to_string(), @@ -1012,14 +1042,10 @@ fn resolve_from_tracks(item: &MixtapeCollectionItem, tracks: &[QueueTrack]) -> R let stored = source_str(item.source); let first = tracks.first(); + // The RESOLVED source, from the row itself (`source_kind_of`). An item that + // resolved to nothing keeps its STORED source — there is no row to ask. let source_kind = match first { - Some(t) => t.source.clone().unwrap_or_else(|| { - if t.is_local { - "local".to_string() - } else { - "qobuz".to_string() - } - }), + Some(t) => source_kind_of(t).to_string(), None => stored.to_string(), }; @@ -1122,6 +1148,15 @@ fn apply_resolved(item: &MixtapeCollectionItem, resolved: ResolvedItem, collecti /// Resolve every not-yet-cached item's tracks and hydrate its row. /// Fire-and-forget: a failure leaves the stored-source defaults in place. +/// +/// Since the `qbz-source` wiring, this pass NO LONGER NEEDS A QOBUZ CLIENT: +/// `fetch_item_tracks` used to return early with an empty vec when there was +/// none, so a local or Plex row rendered a grey square, an empty quality +/// skeleton and no inline tracks for the whole session whenever it was offline +/// or logged out. Each item now resolves against its own source, and only the +/// Qobuz ones fail when the catalog is unreachable. `runtime` survives as the +/// per-task handle the resolve tasks are spawned with; the resolver itself is +/// the process-wide registry. fn resolve_items(runtime: Arc>) { let pending: Vec = { let items = FULL_ITEMS.lock().unwrap(); diff --git a/crates/qbz-qt/src/myqbz_play_qt.rs b/crates/qbz-qt/src/myqbz_play_qt.rs index 5094b3330..37d1b2410 100644 --- a/crates/qbz-qt/src/myqbz_play_qt.rs +++ b/crates/qbz-qt/src/myqbz_play_qt.rs @@ -7,6 +7,31 @@ //! `qbz-mixtape` enqueue resolver, then drives the queue through this port's //! stamping seam (`playback_qt::set_queue_stamped` / `stamped`). //! +//! RESOLUTION GOES THROUGH `qbz-source` (design 02 §9 stage 2). Every path +//! here builds ONE `qbz_source::RegistryResolver` over the process-wide +//! registry instead of a per-call `ProdItemResolver { qobuz client, local +//! closure }`. Three things follow, and all three are the point: +//! - the hand-rolled `resolve_local` closure is GONE, and with it the +//! `library_db_qt::with_db` open it performed once per item (23-34 opens for +//! one collection); `LocalSource` owns one pooled handle. +//! - NO path is gated on a Qobuz client any more. A collection's local / Plex +//! items RESOLVE — quality, artwork, tracks, and into the queue — offline, +//! logged out or during an API hiccup, which they did not before. +//! - a Qobuz item with no client now fails PER ITEM (logged + skipped by +//! `resolve_collection_tracks`) instead of emptying the whole resolve. +//! +//! No source is branched on by hand here: `SourceRegistry::claim` is the one +//! normalisation point, and the source of a row always comes from the ROW. +//! +//! STILL OPEN, and NOT this stage's job: every `Play` arm below starts the +//! first track with `core().play_track_resolved`, which is the Qobuz-only +//! entry (`qbz-core/src/core.rs:730-736` errors "No Qobuz client available"). +//! A local or Plex first track therefore still fails AT THE PLAYER even though +//! it now resolves. That router is design 02 §9 stage 3 +//! (`SourceRegistry::playback` → `PlaybackTicket`), which owns both copies of +//! `play_audible` as well; nothing here should grow a source branch to +//! anticipate it. +//! //! Contract, all load-bearing and 1:1 with the reference: //! - "Resolve all" uses the collection's persisted `play_mode`; the hero //! Shuffle forces `AlbumShuffle`. @@ -35,9 +60,10 @@ use std::sync::Arc; use qbz_app::shell::AppRuntime; use qbz_core::LoggingAdapter; -use qbz_mixtape::enqueue::{resolve_collection_tracks, ItemResolver, ProdItemResolver}; +use qbz_mixtape::enqueue::{resolve_collection_tracks, ItemResolver}; use qbz_models::mixtape::{CollectionPlayMode, MixtapeCollection, MixtapeCollectionItem}; use qbz_models::QueueTrack; +use qbz_source::{RawRef, RegistryResolver, SourceId}; type Runtime = Arc>; @@ -100,40 +126,37 @@ impl InlineTrackMode { // Resolver construction // --------------------------------------------------------------------------- -/// The SYNCHRONOUS local-item resolver handed to `ProdItemResolver`. +/// THE resolver for every path in this file. /// -/// A plain `fn` item, which satisfies the resolver's `Send + Sync` bound. -/// `with_db` is synchronous, so `&LibraryDatabase` never crosses an `.await` -/// (T1 — this is exactly why `qbz-mixtape` split the local resolver into a sync -/// free fn). A DB-open failure becomes its own `Err` so the item is SKIPPED, not -/// treated as an empty success. -fn resolve_local(item: &MixtapeCollectionItem) -> Result, String> { - crate::library_db_qt::with_db(false, |db| { - Ok(qbz_mixtape::enqueue::resolve_local_item(db, item)) - }) - .unwrap_or_else(|| Err("library database unavailable".to_string())) -} - -/// Snapshot the shared Qobuz client. The CLONE is what keeps the `&` handed to -/// `ProdItemResolver` alive across every Qobuz `.await`; holding the -/// `RwLockReadGuard` instead deadlocks against the playback path (T3). -async fn qobuz_client(runtime: &Runtime, site: &str) -> Option { - let client_lock = runtime.core().client(); - let client = { - let guard = client_lock.read().await; - guard.as_ref().cloned() - }; - if client.is_none() { - log::warn!("[qbz-qt] myqbz_play: no Qobuz client; cannot {site}"); - } - client +/// It replaces `ProdItemResolver::new(&client, resolve_local)` at all five +/// former construction sites. Two functions died with them: +/// +/// - `resolve_local` — the synchronous local closure, whose body was +/// `library_db_qt::with_db(false, |db| resolve_local_item(db, item))`. It ran +/// ONCE PER ITEM, and `with_db` was `LibraryDatabase::open` per call: 23-34 +/// opens for one collection, 200 for a 200-item one. `LocalSource` pools the +/// handle, so the same sequential resolve loop (`enqueue.rs:50-68`) now +/// reuses ONE. +/// - `qobuz_client` — the per-call client snapshot, and the four `let-else` +/// guards it fed. `RegistryResolver` needs no client: `QobuzSource` holds the +/// shared one (published by the auth path) and only the QOBUZ arm needs it. +/// +/// `&LibraryDatabase` still never crosses an `.await` — `LocalSource::tracks` +/// has no await in its body and the pool hands out no guard, so the T1 +/// constraint is now enforced by the type system instead of by convention. +fn registry_resolver() -> RegistryResolver { + RegistryResolver::new(qbz_source::registry()) } /// Resolve a whole collection's items into a flat queue. `force_shuffle` /// overrides the persisted mode with `AlbumShuffle` (the hero Shuffle CTA). -/// No client ⇒ empty (the caller toasts). +/// +/// Items that fail are logged and SKIPPED by `resolve_collection_tracks`, so a +/// collection whose Qobuz items cannot resolve (offline, logged out) still +/// plays its local and Plex ones. `_runtime` is kept in the signature because +/// `myqbz_mix_qt::…` calls this and the registry is process-wide. pub(crate) async fn resolve_collection( - runtime: &Runtime, + _runtime: &Runtime, collection: &MixtapeCollection, force_shuffle: bool, ) -> Vec { @@ -142,21 +165,15 @@ pub(crate) async fn resolve_collection( } else { collection.play_mode }; - let Some(client) = qobuz_client(runtime, "resolve collection").await else { - return Vec::new(); - }; - let resolver = ProdItemResolver::new(&client, resolve_local); + let resolver = registry_resolver(); resolve_collection_tracks(collection.items.clone(), play_mode, &resolver).await } /// Resolve a SINGLE item (per-row actions). This path bypasses /// `resolve_collection_tracks`, so it must stamp `source_item_id_hint` INLINE — /// missing it silently breaks item-boundary detection. -async fn resolve_single_item(runtime: &Runtime, item: &MixtapeCollectionItem) -> Vec { - let Some(client) = qobuz_client(runtime, "resolve item").await else { - return Vec::new(); - }; - let resolver = ProdItemResolver::new(&client, resolve_local); +async fn resolve_single_item(item: &MixtapeCollectionItem) -> Vec { + let resolver = registry_resolver(); match resolver.resolve(item).await { Ok(mut tracks) => { let hint = item.source_item_id.clone(); @@ -183,14 +200,18 @@ async fn resolve_single_item(runtime: &Runtime, item: &MixtapeCollectionItem) -> /// The offline check does NOT short-circuit anything: the resolve is attempted /// and the check only picks the LOG LEVEL of the failure, because the offline /// gate refuses the Qobuz arm by design (spec 02 §1.1). +/// +/// THE CLIENT GATE IS GONE, and that is a behaviour change, not a cleanup. This +/// function used to return early with an empty vec when there was no Qobuz +/// client, so every LOCAL and PLEX row of a collection showed no quality, no +/// artwork and no tracks — and refused to play — whenever the session was +/// offline, pre-login or mid-hiccup. The local arm never needed that client; +/// only `QobuzSource` does, and it now reports its own absence per item. pub(crate) async fn fetch_item_tracks( - runtime: &Runtime, + _runtime: &Runtime, item: &MixtapeCollectionItem, ) -> Vec { - let Some(client) = qobuz_client(runtime, "fetch item tracks").await else { - return Vec::new(); - }; - let resolver = ProdItemResolver::new(&client, resolve_local); + let resolver = registry_resolver(); match resolver.resolve(item).await { Ok(tracks) => tracks, Err(e) => { @@ -226,21 +247,27 @@ pub(crate) async fn fetch_item_tracks( /// verbatim so wiring the picker is a one-line call, not a re-port. #[allow(dead_code)] pub(crate) async fn resolve_bulk_qobuz_track_ids( - runtime: &Runtime, + _runtime: &Runtime, items: &[MixtapeCollectionItem], ) -> Vec { - let Some(client) = qobuz_client(runtime, "resolve bulk ids").await else { - return Vec::new(); - }; - let resolver = ProdItemResolver::new(&client, resolve_local); + let resolver = registry_resolver(); + let registry = qbz_source::registry(); let mut ids: Vec = Vec::new(); for item in items { match resolver.resolve(item).await { Ok(tracks) => { for t in tracks { - let is_qobuz = t.source.as_deref() == Some("qobuz") - || (t.source.is_none() && !t.is_local); + // The row decides. This was a hand-written + // `source == "qobuz" || (source.is_none() && !is_local)` + // — the one correct rule for an unlabelled queue row, and + // now the rule `QobuzSource::claim` applies, in ONE place, + // for every source word (`qobuz_download` and `offline` + // are local FILES and are correctly excluded here). + let is_qobuz = registry + .claim(&RawRef::from_queue_track(&t)) + .map(|m| m.source() == SourceId::QOBUZ) + .unwrap_or(false); if is_qobuz { ids.push(t.id.to_string()); } @@ -436,7 +463,7 @@ pub(crate) fn item_action(source_item_id: String, action: String) { return; }; - let tracks = resolve_single_item(&runtime, &item).await; + let tracks = resolve_single_item(&item).await; if tracks.is_empty() { crate::toast_qt::error(qbz_i18n::t("This item resolved to 0 playable tracks")); return; @@ -526,11 +553,10 @@ fn bulk_enqueue_inner(items: Vec, mode: BulkEnqueueMode) } let runtime = crate::app(); crate::spawn(async move { - let Some(client) = qobuz_client(&runtime, "bulk-enqueue").await else { - crate::toast_qt::error(qbz_i18n::t("These items resolved to 0 playable tracks")); - return; - }; - let resolver = ProdItemResolver::new(&client, resolve_local); + // No client gate: a selection of local / Plex items enqueues with no + // Qobuz session at all, and a Qobuz item that cannot resolve is skipped + // per item (below) instead of failing the whole batch. + let resolver = registry_resolver(); let mut tracks: Vec = Vec::new(); for item in &items { diff --git a/crates/qbz-source/src/acceptance.rs b/crates/qbz-source/src/acceptance.rs new file mode 100644 index 000000000..9d7655ad6 --- /dev/null +++ b/crates/qbz-source/src/acceptance.rs @@ -0,0 +1,575 @@ +//! **R1 / R3 — which container accepts which item.** One rule, one predicate, +//! one place. +//! +//! # The rules, verbatim +//! +//! - **R1.** A **Collection** takes albums, singles and EPs, from ANY source — +//! `qobuz` or `local`, and `local` covers folders, Plex and anything added +//! later. A **Mixtape** takes albums, playlists and tracks. +//! - **R2.** An unsupported combination must NEVER be visible. The `Err` a +//! resolver returns stays where it is as the safety net that only reaches the +//! log; the fix belongs in the UI, which must simply not offer the impossible +//! action. That is what this module is for: [`Container::accepts`] is the +//! filter the UI applies BEFORE it renders an action. +//! - **R3.** Ephemeral honours its name. An ephemeral item can only be PLAYED +//! and only be added to the QUEUE — never to a playlist, a mixtape or a +//! collection. It leaves no trace in any library. +//! - **R4.** A row's source comes from the ROW, never from a literal. Note how +//! the rule below reads: the Collection arm never looks at +//! [`ItemFacts::source`] at all (R1 says "any source"), and the one arm that +//! does needs POSITIVE evidence of Qobuz. There is no `"qobuz"` literal for a +//! caller to get wrong — build [`ItemFacts`] from the row with +//! [`ItemFacts::from_raw`] / [`ItemFacts::from_media`] / [`ItemFacts::with_source_word`]. +//! +//! # What this replaces +//! +//! `myqbz_add_qt`'s `restrict_to_mixtape` boolean, which could only say +//! "mixtapes only" vs "everything" — it could not say that a LOCAL album +//! belongs in a Collection while a LOCAL playlist belongs nowhere, and it could +//! not say anything at all about a per-row flyout. The rule is a property of +//! the ITEM, so it lives with the item's other properties. +//! +//! # "album", "single" and "EP" are RELEASE TYPES, not kinds +//! +//! The codebase already spells this out: `myqbz_builder_qt::classify_release` +//! (myqbz_builder_qt.rs:287-327) returns one of `"album" | "ep" | "single" | +//! "live" | "compilation"` for something that is one `ItemType::Album`, and +//! `qbz_models`' `Album.release_type` (types.rs:470) carries the same word off +//! the wire. So R1's "albums, singles, EPs" is ONE [`ItemKind::Album`] slot +//! ([`Accepted::Release`]), not three kinds — see [`RELEASE_TYPES`]. + +use qbz_library::ephemeral::EPHEMERAL_ID_FLOOR; + +use crate::id::{ItemKind, MediaRef, RawRef, SourceId}; +use crate::meta::ItemMeta; + +/// Every release-type word the codebase can produce for an +/// [`ItemKind::Album`], from `classify_release` (myqbz_builder_qt.rs:287-327) +/// and `Album.release_type` (`qbz-models/src/types.rs:470`). +/// +/// R1 names the first three. The predicate does **not** gate on this list: a +/// live record and a compilation are still albums, and an unrecognised or +/// missing word must not make a legitimate album disappear from the picker +/// (that would be R2 in reverse — an invisible refusal). The list is here so +/// the vocabulary has ONE home and so the tests can pin it. +pub const RELEASE_TYPES: [&str; 5] = ["album", "single", "ep", "live", "compilation"]; + +/// Is `w` a release-type word this codebase knows? +pub fn is_release_word(w: &str) -> bool { + RELEASE_TYPES + .iter() + .any(|r| w.trim().eq_ignore_ascii_case(r)) +} + +/// Is this id a session-scoped EPHEMERAL id (R3)? +/// +/// The test is on the ID ALONE, deliberately — never on the row's source word. +/// Several call sites stamp a literal `"source": "qobuz"` onto whatever is +/// playing (`PlayerBar.qml:559`, `NowPlayingBarSmall.qml:508`), so a +/// source-gated test would wave an ephemeral now-playing track straight +/// through. Ephemeral ids are synthetic and high (`>= 2^48`, +/// `qbz_library::ephemeral::EPHEMERAL_ID_FLOOR`); neither a `local_tracks` row +/// id, nor a Plex namespaced id (`2^40`), nor a Qobuz track id ever reaches +/// that floor. Same test as `LocalSource::is_ephemeral` +/// (`sources/local.rs:244`) and `local_ephemeral::is_ephemeral_id`. +pub fn is_ephemeral_id(id: &str) -> bool { + matches!(id.trim().parse::(), Ok(n) if n >= EPHEMERAL_ID_FLOOR) +} + +/// A container the user can put an item INTO. +/// +/// [`Collection`](Container::Collection) and [`Mixtape`](Container::Mixtape) +/// are R1's two. [`Queue`](Container::Queue) and +/// [`Playlist`](Container::Playlist) are named by R3 ("only be added to the +/// QUEUE… never to a playlist"), and are needed for R3 to be expressible at +/// all. +/// +/// The MyQBZ picker's third row kind, an *artist collection* (a built +/// discography), is a [`Collection`](Container::Collection) here: it holds +/// whole albums and accepts exactly what a collection accepts. The only +/// difference is that it cannot be created from the picker, which is a UI +/// property, not an acceptance rule. +#[derive(Clone, Copy, PartialEq, Eq, Debug, Hash)] +pub enum Container { + /// The play queue. The only container R3 lets an ephemeral item reach. + Queue, + /// A MyQBZ collection (and an artist collection). + Collection, + /// A MyQBZ mixtape. + Mixtape, + /// A playlist. + Playlist, +} + +/// One thing a container accepts — R1's vocabulary, as data. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Accepted { + /// A release: an album, a single or an EP. All three are RELEASE TYPES of + /// the one [`ItemKind::Album`] (see [`RELEASE_TYPES`]), so this is one + /// slot, not three. + Release, + /// A playlist — and only one this build can actually resolve. + /// + /// `qbz_mixtape::enqueue::resolve_local_item` answers `(Playlist, Local)` + /// with the hard error `"local playlists not supported in this release"` + /// (enqueue.rs:418-424: the schema has no local-only playlist id to + /// resolve against), and neither [`crate::LocalSource`] nor + /// [`crate::PlexSource`] claims a playlist either (`local.rs:601-606`, + /// `plex.rs:307-310` both answer `Unsupported`). That error is exactly the + /// unsupported combination R2 talks about; it stays where it is as the + /// safety net, and this slot is what makes it unreachable from the UI. + Playlist, + /// A single track. + Track, + /// A filesystem folder. `local_playback::enqueue` routes kind `"folder"` + /// and [`crate::LocalSource`] resolves it (`local.rs:589-591`); it is + /// playable, so it is queueable, and it is storable nowhere. + Folder, +} + +/// Why a container refused an item. +/// +/// **Log-only, by R2.** These strings are never rendered: an unsupported +/// combination must never be visible, so a refusal means the UI should not have +/// offered the action in the first place, and reaching one is a bug report to +/// the log — not a message to a user. There is deliberately no msgid for any of +/// it and there must not be one. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Refusal { + /// R3: ephemeral leaves no trace. + Ephemeral, + /// R1: this container does not take this kind of item at all. + Kind(ItemKind), + /// The container takes playlists, but not one from this source — nothing + /// outside Qobuz resolves to a playlist (see [`Accepted::Playlist`]). + /// `None` = the row did not say where it came from. + UnresolvablePlaylist(Option), +} + +impl std::fmt::Display for Refusal { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Refusal::Ephemeral => f.write_str("ephemeral items are play/queue only (R3)"), + Refusal::Kind(k) => write!(f, "this container does not accept a {} (R1)", k.label()), + Refusal::UnresolvablePlaylist(src) => match src { + Some(s) => write!( + f, + "a {s} playlist cannot be resolved; only qobuz playlists can" + ), + None => f.write_str("a playlist with no source cannot be resolved"), + }, + } + } +} + +/// The facts a row carries about ONE item — everything the rule needs, and +/// nothing else. +/// +/// Every field comes from the ROW (R4). Build it with [`ItemFacts::from_raw`] +/// or [`ItemFacts::from_media`] where a reference is already in hand, or with +/// [`ItemFacts::new`] plus the `with_*` builders where the caller is holding +/// loose row fields. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct ItemFacts<'a> { + /// What the row is. + pub kind: ItemKind, + /// The release-type word for an [`ItemKind::Album`] — one of + /// [`RELEASE_TYPES`], as `classify_release` spells it. Informational: see + /// [`RELEASE_TYPES`] for why the rule does not gate on it. + pub release: Option<&'a str>, + /// Where the row came from, normalised. `None` = the row did not say. + pub source: Option, + /// R3. Normally derived from the id by [`is_ephemeral_id`]. + pub ephemeral: bool, +} + +impl<'a> ItemFacts<'a> { + /// The minimum: a kind, no source word, not ephemeral. + pub fn new(kind: ItemKind) -> Self { + Self { + kind, + release: None, + source: None, + ephemeral: false, + } + } + + /// Stamp the row's source, already typed. + pub fn with_source(mut self, source: SourceId) -> Self { + self.source = Some(source); + self + } + + /// Stamp the row's source WORD, normalised through the one table + /// ([`SourceId::from_word`]). An unknown word stays `None` — it is never + /// guessed into Qobuz. + pub fn with_source_word(mut self, word: &str) -> Self { + self.source = SourceId::from_word(word); + self + } + + /// Stamp the release-type word (`classify_release`'s output, or + /// `Album.release_type` off the wire). + pub fn with_release(mut self, release: &'a str) -> Self { + self.release = Some(release); + self + } + + /// Force the R3 bit, for a caller that knows an item is ephemeral by other + /// means than its id — e.g. an ephemeral ALBUM, whose group key + /// `|` is not numeric and so cannot carry the floor. + pub fn with_ephemeral(mut self, ephemeral: bool) -> Self { + self.ephemeral = ephemeral; + self + } + + /// The facts of a caller-side reference. `None` when the row does not know + /// its own kind — there is nothing to offer for an item nobody can + /// classify, and guessing is what this crate exists to stop. + pub fn from_raw(raw: &RawRef) -> Option> { + Some(ItemFacts { + kind: raw.kind?, + release: None, + source: raw.source, + ephemeral: is_ephemeral_id(&raw.id), + }) + } + + /// The facts of a claimed reference. + pub fn from_media(item: &MediaRef) -> ItemFacts<'static> { + ItemFacts { + kind: item.kind(), + release: None, + source: Some(item.source()), + ephemeral: is_ephemeral_id(item.id()), + } + } + + /// The facts of a claimed reference plus the metadata its source produced — + /// which is where the release-type word comes from + /// ([`ItemMeta::kind_label`], stamped by `QobuzSource::meta` from the V2 + /// wire's `release_type`). + pub fn from_meta(item: &MediaRef, meta: &ItemMeta) -> ItemFacts<'static> { + ItemFacts { + release: Some(meta.kind_label), + ..ItemFacts::from_media(item) + } + } +} + +// ── The rule, as data ─────────────────────────────────────────────────────── +// +// Four lines. R1 is the middle two, verbatim; R3 is the `Queue` line plus the +// ephemeral veto in `refusal` below. + +/// R1: "Collection: albums, singles, EPs." +const COLLECTION: &[Accepted] = &[Accepted::Release]; +/// R1: "Mixtape: albums, playlists, tracks." +const MIXTAPE: &[Accepted] = &[Accepted::Release, Accepted::Playlist, Accepted::Track]; +/// R3: an ephemeral item may be played and queued. Everything else that a +/// source can expand into tracks may be queued too — `Artist` is absent +/// because no source resolves one (`local.rs:601-606`, `plex.rs:307-310`, and +/// `QobuzSource` has no artist arm either). +const QUEUE: &[Accepted] = &[ + Accepted::Release, + Accepted::Playlist, + Accepted::Track, + Accepted::Folder, +]; +/// A playlist holds tracks. R1 does not describe playlists as containers — this +/// line exists so R3's "never to a playlist" is expressible. +const PLAYLIST: &[Accepted] = &[Accepted::Track]; + +impl Accepted { + /// Does this slot match the item? + fn matches(self, item: &ItemFacts<'_>) -> bool { + match self { + // Any release type of the one album kind — see `RELEASE_TYPES`. + Accepted::Release => item.kind == ItemKind::Album, + // POSITIVE evidence of Qobuz, never "not local" (R4, and the + // `_ => Qobuz` guess this crate refuses to reproduce). + Accepted::Playlist => { + item.kind == ItemKind::Playlist && item.source == Some(SourceId::QOBUZ) + } + Accepted::Track => item.kind == ItemKind::Track, + Accepted::Folder => item.kind == ItemKind::Folder, + } + } +} + +impl Container { + /// Every container, for a caller that wants to project the rule onto its + /// own row list in one pass. + pub const ALL: [Container; 4] = [ + Container::Queue, + Container::Collection, + Container::Mixtape, + Container::Playlist, + ]; + + /// What this container accepts — R1/R3 as data, readable and testable + /// without going through the predicate. + pub fn accepted(self) -> &'static [Accepted] { + match self { + Container::Queue => QUEUE, + Container::Collection => COLLECTION, + Container::Mixtape => MIXTAPE, + Container::Playlist => PLAYLIST, + } + } + + /// **THE predicate.** May this item go in this container? + /// + /// This is what the UI asks before it renders an action. A `false` here + /// means the action is NOT OFFERED — never offered and then refused (R2). + pub fn accepts(self, item: &ItemFacts<'_>) -> bool { + self.refusal(item).is_none() + } + + /// The same question, with the reason — for the LOG only (see [`Refusal`]). + pub fn refusal(self, item: &ItemFacts<'_>) -> Option { + // R3 first, and it vetoes every container but the queue. + if item.ephemeral && self != Container::Queue { + return Some(Refusal::Ephemeral); + } + let slots = self.accepted(); + if slots.iter().any(|s| s.matches(item)) { + return None; + } + if item.kind == ItemKind::Playlist && slots.contains(&Accepted::Playlist) { + // The container does take playlists; this one just cannot be + // resolved. Worth its own reason — it is the arm R2 is about. + return Some(Refusal::UnresolvablePlaylist(item.source)); + } + Some(Refusal::Kind(item.kind)) + } + + /// The BATCH form: a container is offered only when it accepts EVERY item. + /// + /// An empty payload accepts nothing — there is nothing to add, so no + /// container should light up. + pub fn accepts_all(self, items: &[ItemFacts<'_>]) -> bool { + !items.is_empty() && items.iter().all(|it| self.accepts(it)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn album(source: SourceId) -> ItemFacts<'static> { + ItemFacts::new(ItemKind::Album).with_source(source) + } + + // ── R1: what a Collection takes ──────────────────────────────────────── + + #[test] + fn a_collection_takes_albums_singles_and_eps_from_any_source() { + for source in [SourceId::QOBUZ, SourceId::LOCAL, SourceId::PLEX] { + for release in ["album", "single", "ep"] { + let it = album(source).with_release(release); + assert!( + Container::Collection.accepts(&it), + "a {release} from {source} belongs in a collection (R1)" + ); + } + } + } + + #[test] + fn a_plex_album_is_accepted_by_a_collection() { + // R1: "from ANY source — qobuz, local (which means folders AND Plex + // AND anything added later)". + assert!(Container::Collection.accepts(&album(SourceId::PLEX))); + assert_eq!(Container::Collection.refusal(&album(SourceId::PLEX)), None); + } + + #[test] + fn a_release_word_never_hides_an_album() { + // "live" and "compilation" are release types of the SAME kind. R1 + // names three of the five; refusing the other two would be an + // invisible refusal of a legitimate album. + for release in RELEASE_TYPES { + assert!(is_release_word(release)); + assert!(Container::Collection.accepts(&album(SourceId::QOBUZ).with_release(release))); + } + // An unclassified album, and one carrying a word nobody knows. + assert!(Container::Collection.accepts(&album(SourceId::LOCAL))); + assert!(Container::Collection.accepts(&album(SourceId::LOCAL).with_release("box set"))); + } + + #[test] + fn a_collection_takes_neither_tracks_nor_playlists() { + let track = ItemFacts::new(ItemKind::Track).with_source(SourceId::QOBUZ); + assert!(!Container::Collection.accepts(&track)); + assert_eq!( + Container::Collection.refusal(&track), + Some(Refusal::Kind(ItemKind::Track)) + ); + + let playlist = ItemFacts::new(ItemKind::Playlist).with_source(SourceId::QOBUZ); + assert!(!Container::Collection.accepts(&playlist)); + assert_eq!( + Container::Collection.refusal(&playlist), + Some(Refusal::Kind(ItemKind::Playlist)), + "a collection does not take playlists at all — not even qobuz ones" + ); + } + + // ── R1: what a Mixtape takes ─────────────────────────────────────────── + + #[test] + fn a_playlist_is_refused_by_a_collection_and_accepted_by_a_mixtape() { + let playlist = ItemFacts::new(ItemKind::Playlist).with_source_word("qobuz"); + assert!(!Container::Collection.accepts(&playlist)); + assert!(Container::Mixtape.accepts(&playlist)); + } + + #[test] + fn a_mixtape_takes_albums_playlists_and_tracks() { + assert!(Container::Mixtape.accepts(&album(SourceId::LOCAL))); + assert!(Container::Mixtape + .accepts(&ItemFacts::new(ItemKind::Track).with_source(SourceId::LOCAL))); + assert!(Container::Mixtape + .accepts(&ItemFacts::new(ItemKind::Playlist).with_source(SourceId::QOBUZ))); + } + + #[test] + fn a_non_qobuz_playlist_belongs_nowhere() { + // enqueue.rs:418-424 — "local playlists not supported in this + // release". R2: the error stays as the safety net; the UI simply never + // offers the action. + for source in [Some(SourceId::LOCAL), Some(SourceId::PLEX), None] { + let mut it = ItemFacts::new(ItemKind::Playlist); + it.source = source; + assert!(!Container::Mixtape.accepts(&it), "{source:?} playlist"); + assert_eq!( + Container::Mixtape.refusal(&it), + Some(Refusal::UnresolvablePlaylist(source)) + ); + assert!(!Container::Collection.accepts(&it)); + assert!(!Container::Queue.accepts(&it)); + } + } + + // ── R3: ephemeral ────────────────────────────────────────────────────── + + #[test] + fn an_ephemeral_item_is_refused_by_both_collection_and_mixtape() { + // qbz-library/src/ephemeral.rs:43 — EPHEMERAL_ID_FLOOR = 2^48. + let raw = RawRef { + kind: Some(ItemKind::Track), + id: "281474976710656".into(), + ..Default::default() + }; + let it = ItemFacts::from_raw(&raw).expect("a track kind"); + assert!(it.ephemeral); + + for c in [ + Container::Collection, + Container::Mixtape, + Container::Playlist, + ] { + assert!(!c.accepts(&it), "{c:?} must refuse an ephemeral item (R3)"); + assert_eq!(c.refusal(&it), Some(Refusal::Ephemeral)); + } + } + + #[test] + fn an_ephemeral_item_can_still_be_queued() { + // R3: "only be played and only be added to the QUEUE". + let it = ItemFacts::new(ItemKind::Track).with_ephemeral(true); + assert!(Container::Queue.accepts(&it)); + assert_eq!(Container::Queue.refusal(&it), None); + } + + #[test] + fn the_ephemeral_test_reads_the_id_not_the_source_word() { + // PlayerBar.qml:559 / NowPlayingBarSmall.qml:508 stamp a literal + // "qobuz" onto whatever is playing; a source-gated test would wave the + // ephemeral row straight through. + let lying = RawRef::new("qobuz", ItemKind::Track, "281474976710657"); + let it = ItemFacts::from_raw(&lying).expect("a track kind"); + assert!(it.ephemeral); + assert_eq!(it.source, Some(SourceId::QOBUZ)); + assert!(!Container::Mixtape.accepts(&it)); + + // …and a normal library row id is NOT ephemeral. + assert!(!is_ephemeral_id("2954")); + assert!(!is_ephemeral_id("139578884")); + assert!(!is_ephemeral_id("HIT ME HARD AND SOFT|Billie Eilish")); + assert!(is_ephemeral_id(&EPHEMERAL_ID_FLOOR.to_string())); + } + + #[test] + fn an_ephemeral_album_needs_the_explicit_bit() { + // An ephemeral ALBUM is grouped by `|`, which carries no + // floor. The caller that knows says so; without it the id test cannot. + let key = RawRef { + kind: Some(ItemKind::Album), + id: "Folder Drop|Some Artist".into(), + ..Default::default() + }; + let derived = ItemFacts::from_raw(&key).expect("an album kind"); + assert!( + !derived.ephemeral, + "no floor in a group key — see the report" + ); + assert!(!Container::Collection.accepts(&derived.with_ephemeral(true))); + } + + // ── Shape ────────────────────────────────────────────────────────────── + + #[test] + fn a_row_with_no_kind_has_no_facts() { + let raw = RawRef { + id: "2954".into(), + ..Default::default() + }; + assert!(ItemFacts::from_raw(&raw).is_none()); + } + + #[test] + fn a_batch_is_offered_only_when_every_item_fits() { + let items = [ + album(SourceId::QOBUZ), + album(SourceId::PLEX), + ItemFacts::new(ItemKind::Track).with_source(SourceId::LOCAL), + ]; + assert!(Container::Mixtape.accepts_all(&items)); + assert!( + !Container::Collection.accepts_all(&items), + "one track in the batch closes the collection" + ); + assert!(Container::Collection.accepts_all(&items[..2])); + // An empty payload lights nothing up. + assert!(!Container::Collection.accepts_all(&[])); + assert!(!Container::Mixtape.accepts_all(&[])); + } + + #[test] + fn the_rule_is_readable_as_data() { + assert_eq!(Container::Collection.accepted(), &[Accepted::Release]); + assert_eq!( + Container::Mixtape.accepted(), + &[Accepted::Release, Accepted::Playlist, Accepted::Track] + ); + assert_eq!(Container::ALL.len(), 4); + } + + #[test] + fn facts_come_from_a_claimed_reference_too() { + let item = MediaRef::new(SourceId::PLEX, ItemKind::Album, "plex:5677211365378243606"); + let facts = ItemFacts::from_media(&item); + assert_eq!(facts.source, Some(SourceId::PLEX)); + assert!(Container::Collection.accepts(&facts)); + + let meta = ItemMeta { + kind_label: "ep", + ..Default::default() + }; + let with_meta = ItemFacts::from_meta(&item, &meta); + assert_eq!(with_meta.release, Some("ep")); + assert!(Container::Collection.accepts(&with_meta)); + } +} diff --git a/crates/qbz-source/src/lib.rs b/crates/qbz-source/src/lib.rs index 256c469de..5dc2d1e9f 100644 --- a/crates/qbz-source/src/lib.rs +++ b/crates/qbz-source/src/lib.rs @@ -38,10 +38,30 @@ //! is on the trait, so a hybrid list makes ONE call per row regardless of //! source and cannot wire fewer pipelines than it has sources. //! +//! # The owner's rules, and where they live +//! +//! [`acceptance`] is the ONE home of **R1** (a Collection takes albums, singles +//! and EPs from any source; a Mixtape takes albums, playlists and tracks) and +//! **R3** (an ephemeral item may only be played and queued — never stored in a +//! playlist, a mixtape or a collection). Ask it with [`Container::accepts`]. +//! +//! **R2** — an unsupported combination must never be VISIBLE — is why the rule +//! belongs here rather than in a view: the resolver errors +//! (`"local playlists not supported in this release"`, [`SourceError`]) stay +//! exactly where they are as the safety net that only reaches the log, and the +//! UI simply does not offer what cannot work. **R4** — a row's source comes +//! from the ROW, never a literal — is enforced by shape: [`ItemFacts`] is built +//! from a [`RawRef`] / [`MediaRef`], and the rule contains no source literal to +//! get wrong. +//! //! # Shape rules //! //! - **Additive.** Nothing in `qbz/src` (Slint) or `qbz-mixtape` changes; this //! crate implements `qbz_mixtape::ItemResolver`, it does not absorb it. +//! - **Nothing long-lived is cached that another crate owns.** The Qobuz client +//! is READ THROUGH a [`ClientLens`] on every call, because `qbz-core` +//! replaces its client wholesale (`core.rs:346`, `:384`) and a cached clone +//! fails silently. Install it once with [`init_registry`]. //! - **The PROTECTED audio backend is not linked.** `qbz-audio` and //! `qbz-player` are not dependencies, so sample rate, resampling and device //! selection are unreachable from here. [`PlaybackTicket`] is data; the @@ -57,10 +77,12 @@ //! `sources/jellyfin.rs` (one impl), one `pub mod` + `pub use` in //! [`sources`], one [`SourceId`] constant, one word in //! [`SourceId::from_word`], one [`SourceBadge`] variant, and one line in -//! [`SourceRegistry::with_defaults`]. **Zero** changes in any view, any +//! [`SourceRegistry`]'s `build`. **Zero** changes in any view, any //! playback path, any artwork path — because all four concerns hang off the one -//! trait. +//! trait. And zero changes in [`acceptance`]: R1 says a Collection takes +//! releases "from ANY source", so the rule has no per-source arm to extend. +pub mod acceptance; pub mod art; pub mod error; pub mod id; @@ -71,14 +93,18 @@ pub mod registry; pub mod source; pub mod sources; +pub use acceptance::{ + is_ephemeral_id, is_release_word, Accepted, Container, ItemFacts, Refusal, RELEASE_TYPES, +}; pub use art::{ArtRef, ArtSize, PLEX_THUMB_PX}; pub use error::SourceError; pub use id::{ItemKind, MediaRef, RawRef, SourceId}; pub use meta::{ItemMeta, QualityHint, SourceBadge}; pub use mixtape_adapter::RegistryResolver; pub use playback::PlaybackTicket; -pub use registry::{registry, SourceRegistry}; +pub use registry::{init_registry, registry, SourceRegistry}; pub use source::Source; pub use sources::{ - local_queue_track, EphemeralTracks, LocalSource, PlexCreds, PlexSource, QobuzSource, + local_queue_track, ClientFuture, ClientLens, EphemeralTracks, LocalSource, PlexCreds, + PlexSource, QobuzSource, }; diff --git a/crates/qbz-source/src/registry.rs b/crates/qbz-source/src/registry.rs index 4c7aadd69..95fadb5aa 100644 --- a/crates/qbz-source/src/registry.rs +++ b/crates/qbz-source/src/registry.rs @@ -11,7 +11,7 @@ use crate::id::{MediaRef, RawRef, SourceId}; use crate::meta::ItemMeta; use crate::playback::PlaybackTicket; use crate::source::Source; -use crate::sources::{LocalSource, PlexSource, QobuzSource}; +use crate::sources::{ClientLens, LocalSource, PlexSource, QobuzSource}; /// Every source in the process. pub struct SourceRegistry { @@ -32,8 +32,23 @@ impl SourceRegistry { /// Order is NOT load-bearing: [`SourceRegistry::claim`] asks everyone (see /// its docs), which removes an invariant a future author could break by /// inserting a source in the wrong slot. + /// + /// The Qobuz source is DETACHED — it has no way to reach a client and every + /// catalog call answers `NotConfigured`. The process wires the live one + /// with [`init_registry`]; see [`crate::ClientLens`] for why the client is + /// read through rather than published into the source. pub fn with_defaults() -> Self { - let qobuz = Arc::new(QobuzSource::new()); + Self::build(QobuzSource::detached()) + } + + /// The registry the process actually runs: Qobuz reads its client through + /// `lens` at CALL time. + pub fn with_client_lens(lens: ClientLens) -> Self { + Self::build(QobuzSource::new(lens)) + } + + fn build(qobuz: QobuzSource) -> Self { + let qobuz = Arc::new(qobuz); let plex = Arc::new(PlexSource::new()); let local = Arc::new(LocalSource::new()); Self { @@ -53,7 +68,12 @@ impl SourceRegistry { self.sources.iter().find(|s| s.id() == id) } - /// The Qobuz source, for `set_client` from the auth path. + /// The Qobuz source. + /// + /// It takes NO configuration from the auth path any more: its client is + /// read through the lens installed at construction ([`init_registry`]), so + /// there is nothing to publish into it and nothing to re-publish when + /// `qbz-core` replaces its client. pub fn qobuz(&self) -> &QobuzSource { &self.qobuz } @@ -219,6 +239,30 @@ pub fn registry() -> &'static SourceRegistry { REGISTRY.get_or_init(SourceRegistry::with_defaults) } +/// Build the process-wide registry with a live Qobuz [`ClientLens`]. +/// +/// Call it ONCE, as early as the process can name its core — before anything +/// else touches [`registry()`], and in particular before +/// `auth_qt::bind_per_user_stores`. It is idempotent and cheap to call again; +/// what it cannot do is re-attach a lens to a registry that was already built +/// without one, so a late call logs an error and leaves Qobuz detached (every +/// catalog call then answers `NotConfigured` — loudly, and never with a stale +/// client). +pub fn init_registry(lens: ClientLens) -> &'static SourceRegistry { + let mut installed = false; + let reg = REGISTRY.get_or_init(|| { + installed = true; + SourceRegistry::with_client_lens(lens) + }); + if !installed { + log::error!( + "[qbz-source] init_registry ran after the registry was already built; \ + the Qobuz source stays detached and every catalog call will answer NotConfigured" + ); + } + reg +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/qbz-source/src/sources/mod.rs b/crates/qbz-source/src/sources/mod.rs index 00b3aa48c..f326c57e0 100644 --- a/crates/qbz-source/src/sources/mod.rs +++ b/crates/qbz-source/src/sources/mod.rs @@ -10,4 +10,4 @@ pub mod qobuz; pub use local::{local_queue_track, EphemeralTracks, LocalSource}; pub use plex::{is_thumb_path, PlexCreds, PlexSource, PLEX_TRACK_ID_FLOOR}; -pub use qobuz::QobuzSource; +pub use qobuz::{ClientFuture, ClientLens, QobuzSource}; diff --git a/crates/qbz-source/src/sources/qobuz.rs b/crates/qbz-source/src/sources/qobuz.rs index 55d11fb55..49d8b0827 100644 --- a/crates/qbz-source/src/sources/qobuz.rs +++ b/crates/qbz-source/src/sources/qobuz.rs @@ -3,9 +3,16 @@ //! Track/meta resolution REUSES `qbz-mixtape`'s existing async free fns //! (`resolve_qobuz_album` / `_track` / `_playlist`, enqueue.rs:218-327) rather //! than re-deriving the mappers; that crate is not modified and stays a leaf. +//! +//! This source holds **no client**. It holds a [`ClientLens`] — a read-through +//! handle it asks again on every call — because `qbz-core` replaces its client +//! wholesale (`core.rs:346` and `:384`, the only two writers) at points that +//! run BEFORE the session is set. See [`ClientLens`] for the full argument. use std::collections::HashMap; -use std::sync::{Mutex, RwLock}; +use std::future::Future; +use std::pin::Pin; +use std::sync::{Arc, Mutex, RwLock}; use qbz_models::QueueTrack; @@ -19,21 +26,74 @@ use crate::source::Source; /// Memo ceiling, mirroring `artwork_qt::MEMO_CAP` (artwork_qt.rs:82). const MEMO_CAP: usize = 8192; +/// The future a [`ClientLens`] hands back: a FRESH clone of whatever client the +/// process holds *right now*. +/// +/// `Pin>` rather than a `futures` alias — this crate +/// depends on neither `tokio` nor `futures` (design §8), and `async-trait` +/// generates exactly this shape anyway. +pub type ClientFuture = Pin> + Send>>; + +/// A READ-THROUGH handle to the process's live Qobuz client. +/// +/// It is supplied ONCE, at construction ([`QobuzSource::new`], normally through +/// [`crate::init_registry`]), and it is read at CALL time — every `tracks` / +/// `meta` invocation asks it again. +/// +/// # Why this is not an `Option` field +/// +/// `qbz-core` REPLACES its whole client, twice over: `QbzCore::init` +/// (`qbz-core/src/core.rs:346`) and `QbzCore::try_init_api` (`:384`) both write +/// a freshly constructed `QobuzClient` into the shared slot, and +/// `try_init_api` runs BEFORE `set_session` on both the login +/// (`auth_qt.rs:83`) and the silent-restore (`:277`) paths. Those are the only +/// two writers in the process — so a source that CACHED a clone would go stale +/// and then fail **silently**, and the "re-inject it after every replacement" +/// setter that used to live here was a discipline every future call site had to +/// remember, not an invariant. Reading through removes the failure mode instead +/// of documenting it. +/// +/// A second property comes free: there is no per-user client held here at all, +/// so the "previous account's authenticated client survives logout in a +/// `'static` registry" leak (design §12.1 defect 4) is unreachable for the +/// client. Only the artwork memo is per-user, and [`Source::teardown`] clears +/// it. +/// +/// # The load-bearing contract for the lens body +/// +/// The lens MUST return an owned clone and MUST NOT hold a lock guard past its +/// own future. That is the property `myqbz_play_qt.rs:117-130` documents: +/// holding `QbzCore::client()`'s `RwLockReadGuard` across the catalog awaits +/// DEADLOCKS against the playback path. The frontend's lens is therefore +/// exactly today's `qobuz_client()` body: +/// +/// ```ignore +/// let lens: ClientLens = Arc::new(|| { +/// Box::pin(async { +/// let lock = runtime().core().client(); // Arc>> +/// let guard = lock.read().await; // guard lives INSIDE this future +/// guard.as_ref().cloned() // …and is dropped when it ends +/// }) +/// }); +/// ``` +pub type ClientLens = Arc ClientFuture + Send + Sync>; + /// The Qobuz source. pub struct QobuzSource { - /// `std::sync::RwLock`, NOT tokio's — this crate has no tokio dep (§8). - /// Its guard is `!Send`, so `#[async_trait]`'s `Send` future bound makes - /// "hold the guard across an await" a COMPILE ERROR rather than a rule. - /// Every accessor clones the client inside a block and drops the guard - /// before the first `.await`: `myqbz_play_qt.rs:117-130` records that - /// holding the guard instead DEADLOCKS against the playback path. - client: RwLock>, - /// The uid the client belongs to. A `bind_user` with a different uid clears - /// the client even if nobody remembered to call `teardown` — the leak class + /// Read-through access to the live client. Immutable after construction — + /// there is deliberately no setter, so there is no ordering rule a caller + /// can break (see [`ClientLens`]). + lens: Option, + /// The uid the artwork memo belongs to. A `bind_user` with a different uid + /// clears it even if nobody remembered to call `teardown` — the leak class /// this port already shipped once (auth_qt.rs:193-197). owner: Mutex>, /// MediaRef id → its CDN cover url, filled by `tracks`/`meta`. /// + /// `std::sync::RwLock`, NOT tokio's — this crate has no tokio dep (§8). + /// Its guard is `!Send`, so `#[async_trait]`'s `Send` future bound makes + /// "hold the guard across an await" a COMPILE ERROR rather than a rule. + /// /// `artwork` is the CHEAP phase and must not hit the network, and a Qobuz /// id carries no cover on its own — so a cold id answers /// `Unavailable` rather than blocking a row on an HTTP round-trip. @@ -41,36 +101,41 @@ pub struct QobuzSource { } impl QobuzSource { - /// A source with no client published yet. - pub fn new() -> Self { + /// A source that reads the live client through `lens` on every call. + pub fn new(lens: ClientLens) -> Self { Self { - client: RwLock::new(None), + lens: Some(lens), owner: Mutex::new(None), art: RwLock::new(HashMap::new()), } } - /// Publish the shared Qobuz client. `None` clears it. + /// A source with NO way to reach a client: every catalog call answers + /// [`SourceError::NotConfigured`]. /// - /// MUST be called AFTER `core.set_session` (auth_qt.rs:145, :284 and the - /// offline entry), never after `try_init_api`: `qbz-core` replaces its own - /// client inside `try_init_api` (`qbz-core/src/core.rs:384`), which - /// `ensure_api_initialized` calls at auth_qt.rs:83 and :277 — i.e. BEFORE - /// `set_session` on both the login and the silent-restore paths. A - /// `QobuzSource` holding a client `qbz-core` has since replaced fails - /// SILENTLY. - pub fn set_client(&self, c: Option) { - if let Ok(mut slot) = self.client.write() { - *slot = c; + /// This is what [`crate::SourceRegistry::with_defaults`] builds, and it is + /// an honest dead end rather than a stale one — the process wires the real + /// lens with [`crate::init_registry`]. + pub fn detached() -> Self { + Self { + lens: None, + owner: Mutex::new(None), + art: RwLock::new(HashMap::new()), } } - /// A CLONE of the client — one clone per call site, replacing the one clone - /// per call the frontend does today. The guard is dropped before the - /// caller's first `.await`. - fn client(&self) -> Result { - let c = { self.client.read().ok().and_then(|g| g.clone()) }; - c.ok_or(SourceError::NotConfigured { + /// A CLONE of the client as it is AT THIS MOMENT, obtained through the + /// lens. No guard of ours is alive when this returns, and nothing is + /// cached between calls. + async fn client(&self) -> Result { + let Some(lens) = self.lens.as_ref() else { + return Err(SourceError::NotConfigured { + by: SourceId::QOBUZ, + why: "no client lens installed (the registry was built before qbz_source::init_registry ran)", + }); + }; + let fut = (**lens)(); + fut.await.ok_or(SourceError::NotConfigured { by: SourceId::QOBUZ, why: "no client", }) @@ -134,7 +199,7 @@ impl QobuzSource { impl Default for QobuzSource { fn default() -> Self { - Self::new() + Self::detached() } } @@ -196,7 +261,7 @@ impl Source for QobuzSource { } async fn tracks(&self, item: &MediaRef) -> Result, SourceError> { - let client = self.client()?; + let client = self.client().await?; // The mappers stay in `qbz-mixtape`; this is the routing only. Their // "album X has 0 tracks" case comes back as an Err STRING, so it lands // in `Backend` rather than `NotFound` — converting it would mean @@ -231,7 +296,7 @@ impl Source for QobuzSource { } async fn meta(&self, item: &MediaRef) -> Result { - let client = self.client()?; + let client = self.client().await?; let meta = match item.kind() { ItemKind::Album => { let a = client @@ -361,10 +426,10 @@ impl Source for QobuzSource { fn bind_user(&self, uid: u64, _dir: &std::path::Path) { let previous = self.owner.lock().ok().and_then(|mut o| o.replace(uid)); if previous.is_some() && previous != Some(uid) { - // A rebind to a DIFFERENT uid clears the client even if nobody - // remembered to tear down; the new one arrives separately, after - // `set_session`. - self.set_client(None); + // A rebind to a DIFFERENT uid drops the previous account's covers + // even if nobody remembered to tear down. There is no client to + // drop: it is read through the lens, so the account swap that + // `qbz-core` performs IS the swap this source sees. if let Ok(mut m) = self.art.write() { m.clear(); } @@ -372,11 +437,10 @@ impl Source for QobuzSource { } fn teardown(&self) { - // MANDATORY override: the trait default is a no-op, and inheriting it - // here would leave the PREVIOUS ACCOUNT's authenticated `QobuzClient` - // alive in a `'static` registry across a logout, serving the next - // account's requests. - self.set_client(None); + // MANDATORY override: the trait default is a no-op, and this source + // holds a per-user artwork memo. The PREVIOUS ACCOUNT's authenticated + // client cannot leak here — nothing caches it (see `ClientLens`) — so + // the memo and the owner are all there is to drop. if let Ok(mut o) = self.owner.lock() { *o = None; } @@ -403,12 +467,36 @@ fn with_art( mod tests { use super::*; + use std::sync::atomic::{AtomicUsize, Ordering}; + fn claimed(raw: &RawRef) -> Result { - QobuzSource::new() + QobuzSource::detached() .claim(raw) .expect("QobuzSource should claim this reference") } + /// Poll a future that is READY on its first poll, with no executor. + /// + /// This crate depends on neither `tokio` nor `futures` (design §8), and the + /// paths exercised below all return before their first suspension point. + fn block_on(fut: F) -> F::Output { + use std::task::{Context, Poll, RawWaker, RawWakerVTable, Waker}; + + unsafe fn nop(_: *const ()) {} + unsafe fn clone_raw(p: *const ()) -> RawWaker { + RawWaker::new(p, &VTABLE) + } + static VTABLE: RawWakerVTable = RawWakerVTable::new(clone_raw, nop, nop, nop); + + let waker = unsafe { Waker::from_raw(RawWaker::new(std::ptr::null(), &VTABLE)) }; + let mut cx = Context::from_waker(&waker); + let mut fut = Box::pin(fut); + match fut.as_mut().poll(&mut cx) { + Poll::Ready(v) => v, + Poll::Pending => panic!("this path must not suspend"), + } + } + #[test] fn owns_opaque_album_ids_verbatim() { // Real ids from `mixtape_collection_items` / `album_play_history` @@ -443,7 +531,7 @@ mod tests { #[test] fn never_claims_by_elimination() { - let q = QobuzSource::new(); + let q = QobuzSource::detached(); // survey IC-4/IC-6: a bare numeric with no source word is NOT Qobuz's. assert!(q .claim(&RawRef { @@ -499,11 +587,48 @@ mod tests { #[test] fn artwork_is_unavailable_not_none_before_a_resolve() { - let q = QobuzSource::new(); + let q = QobuzSource::detached(); let item = MediaRef::new(SourceId::QOBUZ, ItemKind::Album, "0060254702523"); assert!(matches!( Source::artwork(&q, &item, ArtSize::Row), ArtRef::Unavailable(_) )); } + + #[test] + fn a_detached_source_says_not_configured_instead_of_serving_a_stale_client() { + let q = QobuzSource::detached(); + let item = MediaRef::new(SourceId::QOBUZ, ItemKind::Album, "0060254702523"); + let err = block_on(q.tracks(&item)).unwrap_err(); + assert!(matches!( + err, + SourceError::NotConfigured { + by: SourceId::QOBUZ, + .. + } + )); + } + + #[test] + fn the_lens_is_read_on_every_call_never_cached() { + // THE property that replaces `set_client`: `qbz-core` swaps its whole + // client at core.rs:346 / :384, so a clone cached here would go stale + // and fail silently. Two calls ⇒ two reads. + let calls = Arc::new(AtomicUsize::new(0)); + let seen = calls.clone(); + let lens: ClientLens = Arc::new(move || { + let seen = seen.clone(); + Box::pin(async move { + seen.fetch_add(1, Ordering::SeqCst); + None + }) + }); + let q = QobuzSource::new(lens); + let album = MediaRef::new(SourceId::QOBUZ, ItemKind::Album, "0060254702523"); + let track = MediaRef::new(SourceId::QOBUZ, ItemKind::Track, "139578884"); + + assert!(block_on(q.tracks(&album)).is_err()); + assert!(block_on(q.meta(&track)).is_err()); + assert_eq!(calls.load(Ordering::SeqCst), 2); + } }