From 1fb25bdb34f5b629d36dae67c47ff4fbe5622afc Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 19 Aug 2026 23:25:07 +0200 Subject: [PATCH 1/6] feat(api): close the four gaps the fifth audit named MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Carry credential scopes through the OAuth grant, and answer for the three OpenSubsonic fields the catalogue could not. `oauth_authorization` and `session` gain a `scopes_json` column. The consent step records the caller's scopes on the grant, redemption issues the session under them, and `authenticate` reads the limit off the session as it already did off an API token. Rotation carries them too. Both default to the empty list, which is what every existing row meant. `Access::Unrestricted` goes with it. It stood in for the migration — RFC-002 said "until it exists this closes the path without a migration" — and keeping it would have made the new column unreachable, since nothing narrowed could then populate it. Minting asks `Write`: pairing a device is a mutation, and what it mints can no longer be wider than what asked for it. `album.sort_name` and `artist.sort_name` hold the tagged sort forms, read from ALBUMSORT, ALBUMARTISTSORT and ARTISTSORT. A joined credit pairs with its joined sort tag position by position; lengths that disagree yield no sort form at all rather than filing one artist under another's name. Both are emitted with their default, so an untagged album reports sortName="" — supported and unknown, where the field used to be absent and therefore unsupported. The folder level of getMusicDirectory now lists the tracks of that library which belong to no album. They already named it as their `parent`; browsing there found none of them. No attribute changes value. Adding a column to `artist_select!` broke `getArtists` and `search3`: two hand-written projections in services.rs duplicate the macro instead of using it. Both now carry the column. The drift predates this change; the new column is what made it fail loudly. `/api/v2/search` documents the 400 its required `q` already produced. `getLicense` keeps its hard-coded expiry deliberately: the server has no licence, and four clients were validated against what it sends. Signed-off-by: InstaZDLL --- docs/rfcs/RFC-002-waveflow-server-v2.md | 8 +- docs/subsonic-compatibility.md | 11 + ...60819000000_oauth_authorization_scopes.sql | 12 + .../20260819010000_session_scopes.sql | 11 + .../20260819020000_album_artist_sort_name.sql | 12 + src/authentication.rs | 24 +- src/catalog.rs | 57 +++- src/database.rs | 55 ++- src/http.rs | 44 +-- src/scanner.rs | 12 + src/services.rs | 53 ++- src/subsonic.rs | 31 +- tests/v2_foundations.rs | 314 +++++++++++++++++- 13 files changed, 554 insertions(+), 90 deletions(-) create mode 100644 migrations-v2/20260819000000_oauth_authorization_scopes.sql create mode 100644 migrations-v2/20260819010000_session_scopes.sql create mode 100644 migrations-v2/20260819020000_album_artist_sort_name.sql diff --git a/docs/rfcs/RFC-002-waveflow-server-v2.md b/docs/rfcs/RFC-002-waveflow-server-v2.md index 298392f..2ab6982 100644 --- a/docs/rfcs/RFC-002-waveflow-server-v2.md +++ b/docs/rfcs/RFC-002-waveflow-server-v2.md @@ -52,7 +52,7 @@ Both `/rest/` and `/rest/.view` accept GET query parameters and Symfonium 14.1.0 performs a discovery request with the exact unauthenticated tuple `GET ping`, `c=Symfonium`, `u=test`, `p=test` after validating the configured account. WaveFlow returns only the standard successful `ping` envelope for that exact probe and does not create a principal or session. Duplicate identity parameters, alternate clients, POST requests, extra token authentication parameters and every method other than `ping` remain authenticated normally. -The response root freezes `status`, `version=1.16.1`, `type=waveflow`, `serverVersion` and `openSubsonic=true`. XML uses the Subsonic namespace. JSON collection fields are arrays even when they contain one item. Media items freeze the common fields `id`, `parent`, `isDir`, `title`, optional `album`/`artist`/`genre`/`year`/disc-track numbers, seconds-based `duration`, `bitRate`, `size`, `suffix`, `contentType`, `type=music`, optional `coverArt`/`albumId`, and ISO-8601 `created`. Album and artist records include UUID, display name, counts and available artwork/year metadata. Frozen 1.16 metadata is omitted when unknown; the OpenSubsonic additions follow the presence rule instead and are emitted with their default value, see *Deliberate deviations from the v2.0-beta freeze*. Media items carry `mediaType`, `isVideo`, `samplingRate`, `channelCount`, `bitDepth`, `playCount`, `played`, `displayArtist`, `artists[]`, `genres[]`, `musicBrainzId`, `bpm`, `sortName`, `comment`, `isrc[]` and `replayGain`; albums add `isCompilation`, `playCount`, `played` and `displayArtist`. `artists[]` is the credited list in tag order from `track_artist`, of which `artist`/`artistId` remain the display string and the primary credit; `genres[]` comes from `track_genre` ordered by name. `musicBrainzId` on a media item is the MusicBrainz **recording** identifier, the performance; the release and artist identifiers are never exposed at track level, where they would name a different entity. They are exposed on the entities they belong to instead: `album.musicBrainzId` is the release and `artist.musicBrainzId` is the artist, both under the presence rule, and `getAlbumInfo`/`getAlbumInfo2` carry the release id as their `musicBrainzId` element while remaining otherwise empty. Neither is a tag read from one file. Tracks of one album routinely disagree — a library assembled over years holds files tagged against different releases of the same record — so an album takes the identifier most of its available tracks agree on, recomputed at the end of every scan, with ties falling to the earliest disc and track so two scans of unchanged files answer the same thing; an artist takes it from the tracks it is the first credit of, because the tag is one value on a file that may credit several artists. Browsing entries are the exception: `getMusicDirectory` renders artists and albums as `child` elements, where the specification defines `musicBrainzId` as the recording id, so the field is dropped there rather than carrying a different identifier under that name. `replayGain` is the one addition whose *members* are omitted when unknown, on the specification's own instruction, while the container itself is always present because that is what reports the server reads gain tags at all. `moods` is multi-valued and split like the other joined tags; `explicitStatus` is normalised to the two words the specification defines, `explicit` and `clean`, rather than to the per-format spelling the tag used, and a tag saying "no advisory" maps to no value because it is not a claim that the work is clean. Media items additionally carry `albumArtists[]` and `displayAlbumArtist`, which are the album's credit rather than the track's: a guest appearance names the guest, while the album still belongs under the album artist. Albums carry `artists[]` and `genres[]`, derived from their available tracks rather than stored, because an album has no credit or genre of its own in the schema — only the union of its files'; genres are grouped on the canonical name, so an album spelling "Hip-Hop" on some tracks and "Hip Hop" on others reports one genre. What remains unimplemented is what the catalogue cannot answer: `contributors[]` and `displayComposer` need a composer the scanner does not read, and `AlbumID3`'s `sortName`, `moods[]`, `explicitStatus`, `originalReleaseDate`, `releaseDate`, `releaseTypes[]`, `recordLabels[]` and `discTitles[]`, like `ArtistID3`'s `sortName` and `roles[]`, need album and artist columns that do not exist. Those are absent rather than empty, which under the presence rule is the accurate statement that they are not supported. +The response root freezes `status`, `version=1.16.1`, `type=waveflow`, `serverVersion` and `openSubsonic=true`. XML uses the Subsonic namespace. JSON collection fields are arrays even when they contain one item. Media items freeze the common fields `id`, `parent`, `isDir`, `title`, optional `album`/`artist`/`genre`/`year`/disc-track numbers, seconds-based `duration`, `bitRate`, `size`, `suffix`, `contentType`, `type=music`, optional `coverArt`/`albumId`, and ISO-8601 `created`. Album and artist records include UUID, display name, counts and available artwork/year metadata. Frozen 1.16 metadata is omitted when unknown; the OpenSubsonic additions follow the presence rule instead and are emitted with their default value, see *Deliberate deviations from the v2.0-beta freeze*. Media items carry `mediaType`, `isVideo`, `samplingRate`, `channelCount`, `bitDepth`, `playCount`, `played`, `displayArtist`, `artists[]`, `genres[]`, `musicBrainzId`, `bpm`, `sortName`, `comment`, `isrc[]` and `replayGain`; albums add `isCompilation`, `playCount`, `played` and `displayArtist`. `artists[]` is the credited list in tag order from `track_artist`, of which `artist`/`artistId` remain the display string and the primary credit; `genres[]` comes from `track_genre` ordered by name. `musicBrainzId` on a media item is the MusicBrainz **recording** identifier, the performance; the release and artist identifiers are never exposed at track level, where they would name a different entity. They are exposed on the entities they belong to instead: `album.musicBrainzId` is the release and `artist.musicBrainzId` is the artist, both under the presence rule, and `getAlbumInfo`/`getAlbumInfo2` carry the release id as their `musicBrainzId` element while remaining otherwise empty. Neither is a tag read from one file. Tracks of one album routinely disagree — a library assembled over years holds files tagged against different releases of the same record — so an album takes the identifier most of its available tracks agree on, recomputed at the end of every scan, with ties falling to the earliest disc and track so two scans of unchanged files answer the same thing; an artist takes it from the tracks it is the first credit of, because the tag is one value on a file that may credit several artists. Browsing entries are the exception: `getMusicDirectory` renders artists and albums as `child` elements, where the specification defines `musicBrainzId` as the recording id, so the field is dropped there rather than carrying a different identifier under that name. `replayGain` is the one addition whose *members* are omitted when unknown, on the specification's own instruction, while the container itself is always present because that is what reports the server reads gain tags at all. `moods` is multi-valued and split like the other joined tags; `explicitStatus` is normalised to the two words the specification defines, `explicit` and `clean`, rather than to the per-format spelling the tag used, and a tag saying "no advisory" maps to no value because it is not a claim that the work is clean. Media items additionally carry `albumArtists[]` and `displayAlbumArtist`, which are the album's credit rather than the track's: a guest appearance names the guest, while the album still belongs under the album artist. Albums carry `artists[]` and `genres[]`, derived from their available tracks rather than stored, because an album has no credit or genre of its own in the schema — only the union of its files'; genres are grouped on the canonical name, so an album spelling "Hip-Hop" on some tracks and "Hip Hop" on others reports one genre. What remains unimplemented is what the catalogue cannot answer: `contributors[]` and `displayComposer` need a composer the scanner does not read, and `AlbumID3`'s `moods[]`, `explicitStatus`, `originalReleaseDate`, `releaseDate`, `releaseTypes[]`, `recordLabels[]` and `discTitles[]`, like `ArtistID3`'s `roles[]`, need album and artist columns that do not exist. `sortName` on both is no longer among them: `album.sort_name` and `artist.sort_name` hold the tagged sort forms, read from `ALBUMSORT`, `ALBUMARTISTSORT` and `ARTISTSORT`. A joined credit pairs with its joined sort tag position by position, and a pair whose lengths disagree contributes no sort form at all rather than filing one artist under another's name. Both are emitted with their default value like every other supported addition, so an untagged album reports `sortName=""` — supported and unknown — where before the field was absent. Those are absent rather than empty, which under the presence rule is the accurate statement that they are not supported. The columns behind the tag fields are added empty and filled by the next scan, so an instance that never rescans stays correct rather than wrong: it reports the fields supported and unset, which is what the presence rule means. Storing MusicBrainz identifiers is the dedicated data contract RFC-004 requires before its MBID branch can begin, and nothing more: a match on one remains a candidate the user confirms, never an automatic link. @@ -62,7 +62,7 @@ For full-catalogue pagination, `search3` treats the literal query `""` as match- `startScan` and `getScanStatus` close the last operational asymmetry with the native API. Subsonic has no library parameter, so `startScan` fans out over every library the account may scan and answers with the `scanStatus` element. Both surfaces queue through `DomainServices::start_library_scan`, so the question of who may scan what has one implementation and cannot drift. Membership alone is not the answer to it: a scan walks the owner's files and takes the process-wide writer gate, so it is reserved to the `owner` and `manager` roles, enforced in the `scan_job` insert itself rather than in a handler. The per-library native route answers `404`, indistinguishably from a library that does not exist. `startScan` names no library, so it skips the ones the account may only listen to instead of failing on them — refusing the whole call because one library is read-only would put the scannable ones out of reach from Subsonic entirely — and an account that may scan nothing queues nothing and succeeds, like one that reaches no library at all. The fan-out is best effort: a library that cannot be queued does not cancel the others, and the error surfaces only when nothing could be queued at all. Re-queuing a library that is already scanning is allowed, as it is natively: the scanner serialises jobs per library and a scan converges on file content, so a redundant pass costs time and changes nothing. `scanStatus` reports `scanning` across those libraries and a `count` of the available tracks the account can reach, never a total spanning another tenant. -Browsing resolves through targeted queries rather than through a snapshot of the tenant's whole catalogue. `getIndexes`, `getArtists` and the folder and artist levels of `getMusicDirectory` read `DomainServices::catalog_overview`, which stops before the tracks; `getArtist`, `getAlbum` and the album level of `getMusicDirectory` use the single-entity queries; `getRandomSongs`, `getSongsByGenre` and `getStarred`/`getStarred2` resolve in SQL. The track read is the expensive third of a snapshot and, since the OpenSubsonic fields, carries two relation loads of its own, so `getRandomSongs?size=10` used to read every track of the tenant twice over to answer with ten. +Browsing resolves through targeted queries rather than through a snapshot of the tenant's whole catalogue. `getIndexes`, `getArtists` and the folder and artist levels of `getMusicDirectory` read `DomainServices::catalog_overview`, which stops before the tracks; the folder level adds one query of its own, `DomainServices::songs_without_album`, because a track with no album names its library as its `parent` for want of an album id, and a parent that would not list the track is a dead end rather than an identifier — it is listed after that library's artists; `getArtist`, `getAlbum` and the album level of `getMusicDirectory` use the single-entity queries; `getRandomSongs`, `getSongsByGenre` and `getStarred`/`getStarred2` resolve in SQL. The track read is the expensive third of a snapshot and, since the OpenSubsonic fields, carries two relation loads of its own, so `getRandomSongs?size=10` used to read every track of the tenant twice over to answer with ten. Genre matching is one rule across every method. `getGenres` groups by `genre.canonical_name` and `getAlbumList2?type=byGenre` filters on it; `getSongsByGenre` and `getRandomSongs` compared the display string with an ASCII case fold, which folds case but not punctuation or spacing. A single `getGenres` row covering "Hip-Hop" and "Hip Hop" therefore answered with only the tracks spelled the way the caller happened to send, so a client displayed a genre it had just been handed and found it empty. All four now match on the canonical name. @@ -70,9 +70,9 @@ Genre matching is one rule across every method. `getGenres` groups by `genre.can `getArtistInfo` and `getArtistInfo2` resolve the requested artist through tenant-scoped catalogue access and return their standard empty containers until artist biography enrichment is implemented. This preserves compatibility with clients such as DSub without fabricating biography or similar-artist metadata. `getAlbumInfo` and `getAlbumInfo2` behave the same way for albums, for the same reason: Feishin and Symfonium call them as soon as an album page opens, and an unimplemented-method error there reads to the client as a broken album rather than as absent enrichment. They carry one real value, the album's release identifier, as their `musicBrainzId` element; notes and biography images stay absent because WaveFlow queries no remote source. `AlbumInfo` predates the presence rule and its members are elements rather than attributes, so an album with no release id omits the element instead of sending it empty. -API tokens are restricted by their scopes on every route, not only on the administrative ones. `Access` names what a route needs — `Read`, `Write`, `Admin` or `Unrestricted` — and is chosen at the single point where the caller is resolved, so a route cannot exist without answering the question. An empty scope list is unrestricted, which is what sessions, OAuth grants and tokens issued without scopes carry; a non-empty list grants only what it names, and a name the server does not know grants nothing, so no vocabulary has to be enumerated. `admin` implies `write`, because a credential trusted to create accounts is not usefully barred from creating a playlist. Issuing a token stays administrative on both surfaces: a token carries the authority of the account it belongs to, so who may mint one is a question about the instance. +API tokens are restricted by their scopes on every route, not only on the administrative ones. `Access` names what a route needs — `Read`, `Write` or `Admin` — and is chosen at the single point where the caller is resolved, so a route cannot exist without answering the question. An empty scope list is unrestricted, which is what a password login, a grant made by an unscoped credential and a token issued without scopes carry; a non-empty list grants only what it names, and a name the server does not know grants nothing, so no vocabulary has to be enumerated. `admin` implies `write`, because a credential trusted to create accounts is not usefully barred from creating a playlist. Issuing a token stays administrative on both surfaces: a token carries the authority of the account it belongs to, so who may mint one is a question about the instance. -Minting a credential is `Unrestricted`, which is a restriction and not a role: only a credential carrying no scope list at all may do it. The authorization code flow returns a session with the account's whole authority, and neither the authorization row nor the session records what asked for it, so a `write` token could reach `POST /api/v2/oauth/authorize` and come back holding a session that answered to `Admin` — an escalation in two requests, from a token deliberately issued without `admin`. The category error was treating "mutate user data" and "issue a reference to this account" as one authority. The durable fix is to carry the scopes through the grant, so that a session is never broader than what issued it; that needs a column on `oauth_authorization` and on `session`, and until it exists this closes the path without a migration. An ordinary account still pairs its own devices from its own session, which is what the flow is for. +Scopes travel through the authorization code flow, so a session is never broader than the credential that issued it. `oauth_authorization` and `session` each carry a `scopes_json` column: the consent step records the caller's scopes on the grant, redeeming it issues the session under those same scopes, and `authenticate` reads the limit off the session exactly as it already read it off an API token. Rotation carries them too — refreshing a narrowed session returns a narrowed one. Both columns default to the empty list, which is what every row written before them meant. The flaw this replaces was an escalation in two requests: the flow returned a session with the account's whole authority and neither the authorization row nor the session recorded what asked for it, so a `write` token could reach `POST /api/v2/oauth/authorize` and come back answering to `Admin`. The category error was treating "mutate user data" and "issue a reference to this account" as one authority; the answer is not to forbid the second but to make it inherit. Minting therefore asks `Write` — pairing a device is a mutation on the account — rather than the blanket restriction that stood in while the columns did not exist. An ordinary account still pairs its own devices from its own session, which is what the flow is for. `playlist.owner` and `share.username` carry the authenticated username. Both collections are read scoped to their owner, so no other name is reachable; the empty string previously emitted made Feishin treat every playlist as another account's and refuse to edit it. diff --git a/docs/subsonic-compatibility.md b/docs/subsonic-compatibility.md index 4ff00e7..22177e5 100644 --- a/docs/subsonic-compatibility.md +++ b/docs/subsonic-compatibility.md @@ -81,4 +81,15 @@ the request; media items gained `albumArtists[]` and `displayAlbumArtist`, and albums gained `artists[]` and `genres[]`; and `getAlbum` returns an album in sleeve order rather than alphabetically. +Two wire changes land after that replay, both additive. `album` and `artist` +gained `sortName`, emitted with its default like every other supported +OpenSubsonic addition, so an album whose files carry no `ALBUMSORT` reports it +empty rather than omitting it — the difference between unknown and +unsupported. And the folder level of `getMusicDirectory` now lists the tracks +of that library which belong to no album, alongside its artists: those tracks +already named the library as their `parent`, and browsing there used to find +none of them. No existing attribute changed value, so a client validated +against the 2026-08-19 replay sees the same catalogue with two more fields on +it and one directory that is no longer a dead end. + Browser-hosted clients need their exact origins in the comma-separated `WAVEFLOW_ALLOWED_ORIGINS` setting. The server permits GET, form POST and OPTIONS from those origins and exposes the byte-range response headers used by web audio players. Wildcard origins are deliberately unsupported. diff --git a/migrations-v2/20260819000000_oauth_authorization_scopes.sql b/migrations-v2/20260819000000_oauth_authorization_scopes.sql new file mode 100644 index 0000000..96aa1ab --- /dev/null +++ b/migrations-v2/20260819000000_oauth_authorization_scopes.sql @@ -0,0 +1,12 @@ +-- Carry the issuing credential's scopes onto the grant. +-- +-- Until now nothing on an authorization recorded what asked for it, so the +-- session redeemed from it carried the account's whole authority whatever the +-- credential that minted it. `Access::Unrestricted` closed that path at the +-- one route that mints, which is a local property; this makes it structural. +-- +-- The default is the empty list, which is exactly what every grant written +-- before this migration meant: unrestricted, because sessions were the only +-- thing that could reach the authorize route and a session is unscoped. +ALTER TABLE oauth_authorization + ADD COLUMN scopes_json TEXT NOT NULL DEFAULT '[]' CHECK (json_valid(scopes_json)); diff --git a/migrations-v2/20260819010000_session_scopes.sql b/migrations-v2/20260819010000_session_scopes.sql new file mode 100644 index 0000000..73acb5a --- /dev/null +++ b/migrations-v2/20260819010000_session_scopes.sql @@ -0,0 +1,11 @@ +-- The other half of carrying scopes through the grant. +-- +-- A session is no longer unscoped by construction: one redeemed from a +-- narrowed authorization inherits that narrowing, so `authenticate` reads the +-- limit off the session the same way it already reads it off an API token. +-- +-- Existing rows default to the empty list — a session issued before this +-- migration came from a password login or from a grant that could only have +-- been made by an unscoped credential, so unrestricted is what they were. +ALTER TABLE session + ADD COLUMN scopes_json TEXT NOT NULL DEFAULT '[]' CHECK (json_valid(scopes_json)); diff --git a/migrations-v2/20260819020000_album_artist_sort_name.sql b/migrations-v2/20260819020000_album_artist_sort_name.sql new file mode 100644 index 0000000..f995409 --- /dev/null +++ b/migrations-v2/20260819020000_album_artist_sort_name.sql @@ -0,0 +1,12 @@ +-- `sortName` for AlbumID3 and ArtistID3. +-- +-- The scanner already reads the track's own sort title; the album's and the +-- artist's had nowhere to go, so both fields were absent — which under the +-- presence rule said "not supported" rather than "unknown". Now they can say +-- the true thing. +-- +-- Added empty and filled by the next scan, like every other tag column: an +-- instance that never rescans reports the field supported and unset rather +-- than reporting something wrong. +ALTER TABLE album ADD COLUMN sort_name TEXT; +ALTER TABLE artist ADD COLUMN sort_name TEXT; diff --git a/src/authentication.rs b/src/authentication.rs index aeb1a40..3363507 100644 --- a/src/authentication.rs +++ b/src/authentication.rs @@ -32,11 +32,11 @@ pub struct AuthUser { pub id: Uuid, pub username: String, pub role: AccountRole, - /// The scopes limiting this request, from the API token it arrived on. + /// The scopes limiting this request, from the credential it arrived on — + /// an API token, or a session issued under a grant that carried some. /// - /// Empty means unrestricted: that is a session, an OAuth grant, or a - /// token issued without any, all of which carry the account's full - /// authority. A non-empty list is a restriction, and a route that needs + /// Empty means unrestricted: a password login, or a token issued without + /// any, both of which carry the account's full authority. A non-empty list is a restriction, and a route that needs /// more than the list grants must refuse rather than trust the account /// behind it. A token is handed to a script, and the point of writing /// scopes on it is that they hold. @@ -119,6 +119,8 @@ impl AuthService { account.role, device_id, now_ms, + // A password login is the account itself: nothing narrows it. + &[], ) .await } @@ -132,6 +134,7 @@ impl AuthService { &self, user_id: Uuid, device_name: &str, + scopes: &[String], ) -> Result { let device_name = device_name.trim(); if device_name.is_empty() || device_name.len() > 120 { @@ -158,6 +161,7 @@ impl AuthService { account.role, device_id, now_ms, + scopes, ) .await } @@ -204,8 +208,9 @@ impl AuthService { id: session.user_id, username: session.username, role: session.role, - // A session carries the account's full authority. - scopes: Vec::new(), + // Rotation must not widen: the refreshed session answers to + // exactly the scopes the original was issued under. + scopes: session.scopes, }, device_id: session.device_id, }) @@ -254,6 +259,7 @@ impl AuthService { role: AccountRole, device_id: Uuid, now_ms: i64, + scopes: &[String], ) -> Result { let access_token = security::generate_token("wfa_"); let refresh_token = security::generate_token("wfr_"); @@ -271,6 +277,7 @@ impl AuthService { access_expires_at, refresh_expires_at, now_ms, + scopes, }) .await .map_err(db_unavailable)?; @@ -284,8 +291,9 @@ impl AuthService { id: user_id, username, role, - // A session carries the account's full authority. - scopes: Vec::new(), + // Whatever the credential that issued it carried — empty for a + // password login, narrowed for a grant made by a scoped token. + scopes: scopes.to_vec(), }, device_id, }) diff --git a/src/catalog.rs b/src/catalog.rs index 37821bf..1f3caf0 100644 --- a/src/catalog.rs +++ b/src/catalog.rs @@ -96,6 +96,11 @@ pub struct CatalogTrackInput { pub replay_gain_album_peak: Option, pub bpm: Option, pub sort_title: Option, + /// Sort forms of the album title and of the two artist credits, as + /// tagged. Multi-valued credits carry them joined by the same separator. + pub sort_album: Option, + pub sort_album_artist: Option, + pub sort_artist: Option, pub comment: Option, /// Multi-valued, split like `artist` and `genre`. pub isrc: Option, @@ -432,7 +437,10 @@ impl Database { let names = split_values(Some(credit.as_str())); let mut first = None; for name in &names { - let id = upsert_artist(&mut tx, library_id, name, now).await?; + // The repair pass works from stored credits, not from tags: + // it has no sort form to offer. `COALESCE` on the stored side + // means passing none leaves whatever a scan already wrote. + let id = upsert_artist(&mut tx, library_id, name, None, now).await?; first.get_or_insert(id); } let Some(first) = first else { continue }; @@ -675,9 +683,10 @@ impl Database { let track_id = existing_id.unwrap_or_else(Uuid::new_v4); let artwork_hash = upsert_artwork(tx, input.artwork.as_ref(), now).await?; let artist_names = split_values(input.artist.as_deref()); + let artist_sorts = sort_names_for(&artist_names, input.sort_artist.as_deref()); let mut artist_ids = Vec::with_capacity(artist_names.len()); - for artist in &artist_names { - artist_ids.push(upsert_artist(tx, library_id, artist, now).await?); + for (artist, sort) in artist_names.iter().zip(&artist_sorts) { + artist_ids.push(upsert_artist(tx, library_id, artist, sort.as_deref(), now).await?); } let album_artist = input .album_artist @@ -695,9 +704,11 @@ impl Database { // The album keeps the joined string as its display name, and hangs // off the first credited artist. let album_artist_names = split_values(album_artist.as_deref()); + let album_artist_sorts = + sort_names_for(&album_artist_names, input.sort_album_artist.as_deref()); let mut album_artist_id = None; - for name in &album_artist_names { - let id = upsert_artist(tx, library_id, name, now).await?; + for (name, sort) in album_artist_names.iter().zip(&album_artist_sorts) { + let id = upsert_artist(tx, library_id, name, sort.as_deref(), now).await?; album_artist_id.get_or_insert(id); } let album_id = upsert_album( @@ -1008,17 +1019,38 @@ async fn upsert_artist( tx: &mut Transaction<'_, Sqlite>, library: Uuid, name: &str, + sort_name: Option<&str>, now: i64, ) -> Result { let id = Uuid::new_v4(); let canonical = waveflow_core::scanner::canonical_name(name); - let value: String = sqlx::query_scalar("INSERT INTO artist (id, library_id, name, canonical_name, created_at, updated_at) \ - VALUES (?, ?, ?, ?, ?, ?) ON CONFLICT (library_id, canonical_name) DO UPDATE SET name=excluded.name, updated_at=excluded.updated_at RETURNING id") - .bind(id.to_string()).bind(library.to_string()).bind(name.trim()).bind(canonical).bind(now).bind(now) + // COALESCE on the stored side: a file that carries no sort tag must not + // erase the one a sibling file supplied, and the artist row is written + // once per track of every album it appears on. + let value: String = sqlx::query_scalar("INSERT INTO artist (id, library_id, name, canonical_name, sort_name, created_at, updated_at) \ + VALUES (?, ?, ?, ?, ?, ?, ?) ON CONFLICT (library_id, canonical_name) DO UPDATE SET name=excluded.name, \ + sort_name=COALESCE(excluded.sort_name, artist.sort_name), updated_at=excluded.updated_at RETURNING id") + .bind(id.to_string()).bind(library.to_string()).bind(name.trim()).bind(canonical).bind(sort_name).bind(now).bind(now) .fetch_one(&mut **tx).await?; parse_uuid(value) } +/// Pairs a split credit with its split sort form. +/// +/// `ARTISTSORT` carries the same separator as `ARTIST` when a credit is +/// joined, so the two lists line up position by position — that is the +/// convention taggers write. When they do not line up, the tag is describing +/// something this cannot map, and guessing would file an artist under another +/// artist's name: every credit then gets no sort form rather than a wrong one. +fn sort_names_for(names: &[String], raw_sort: Option<&str>) -> Vec> { + let sorts = split_values(raw_sort); + if sorts.len() == names.len() { + sorts.into_iter().map(Some).collect() + } else { + vec![None; names.len()] + } +} + async fn upsert_genre( tx: &mut Transaction<'_, Sqlite>, library: Uuid, @@ -1050,12 +1082,13 @@ async fn upsert_album( let canonical = waveflow_core::scanner::canonical_name(title); let identity = album_identity(&canonical, album_artist_id, album_artist); let id = Uuid::new_v4(); - let value: String = sqlx::query_scalar("INSERT INTO album (id, library_id, title, canonical_title, identity_key, album_artist_id, album_artist_name, is_compilation, year, artwork_hash, created_at, updated_at) \ - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT (library_id, identity_key) DO UPDATE SET title=excluded.title, album_artist_name=excluded.album_artist_name, \ - is_compilation=excluded.is_compilation, year=COALESCE(excluded.year, album.year), artwork_hash=COALESCE(excluded.artwork_hash, album.artwork_hash), updated_at=excluded.updated_at RETURNING id") + let value: String = sqlx::query_scalar("INSERT INTO album (id, library_id, title, canonical_title, identity_key, album_artist_id, album_artist_name, is_compilation, year, artwork_hash, sort_name, created_at, updated_at) \ + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT (library_id, identity_key) DO UPDATE SET title=excluded.title, album_artist_name=excluded.album_artist_name, \ + is_compilation=excluded.is_compilation, year=COALESCE(excluded.year, album.year), artwork_hash=COALESCE(excluded.artwork_hash, album.artwork_hash), \ + sort_name=COALESCE(excluded.sort_name, album.sort_name), updated_at=excluded.updated_at RETURNING id") .bind(id.to_string()).bind(library.to_string()).bind(title.trim()).bind(canonical).bind(identity) .bind(album_artist_id.map(|id| id.to_string())).bind(album_artist).bind(i64::from(input.is_compilation)) - .bind(input.year).bind(artwork).bind(now).bind(now).fetch_one(&mut **tx).await?; + .bind(input.year).bind(artwork).bind(input.sort_album.as_deref()).bind(now).bind(now).fetch_one(&mut **tx).await?; parse_uuid(value).map(Some) } diff --git a/src/database.rs b/src/database.rs index 273db38..4857f10 100644 --- a/src/database.rs +++ b/src/database.rs @@ -148,6 +148,10 @@ pub struct SessionRecord { pub username: String, pub role: AccountRole, pub refresh_expires_at: i64, + /// The scopes this session was issued under. Empty for a password login + /// and for a grant made by an unscoped credential; a narrowed token that + /// authorized a device leaves its narrowing here. + pub scopes: Vec, } #[derive(Debug, Clone)] @@ -171,6 +175,9 @@ pub struct NewSession<'a> { pub access_expires_at: i64, pub refresh_expires_at: i64, pub now_ms: i64, + /// Carried from the credential that issued this session, so it can never + /// be broader than what asked for it. + pub scopes: &'a [String], } #[derive(Debug, Clone)] @@ -183,6 +190,8 @@ pub struct NewAuthorization<'a> { pub device_name: &'a str, pub now_ms: i64, pub expires_at: i64, + /// The scopes of the credential that authorized this grant. + pub scopes: &'a [String], } #[derive(Debug, Clone)] @@ -192,6 +201,8 @@ pub struct AuthorizationRecord { pub redirect_uri: String, pub code_challenge: String, pub device_name: String, + /// Handed to the session this grant is redeemed for. + pub scopes: Vec, } #[derive(Debug)] @@ -555,8 +566,8 @@ impl Database { sqlx::query( "INSERT INTO oauth_authorization \ (code_hash, user_id, client_id, redirect_uri, code_challenge, device_name, \ - created_at, expires_at) \ - VALUES (?, ?, ?, ?, ?, ?, ?, ?)", + created_at, expires_at, scopes_json) \ + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)", ) .bind(grant.code_hash.as_slice()) .bind(grant.user_id.to_string()) @@ -566,6 +577,7 @@ impl Database { .bind(grant.device_name) .bind(grant.now_ms) .bind(grant.expires_at) + .bind(encode_scopes(grant.scopes)?) .execute(&mut *tx) .await?; // Delegating access to another application is exactly the kind of event @@ -602,7 +614,8 @@ impl Database { let row = sqlx::query( "UPDATE oauth_authorization SET redeemed_at = ? \ WHERE code_hash = ? AND redeemed_at IS NULL AND expires_at > ? \ - RETURNING user_id, client_id, redirect_uri, code_challenge, device_name", + RETURNING user_id, client_id, redirect_uri, code_challenge, device_name, \ + scopes_json", ) .bind(now_ms) .bind(code_hash) @@ -616,6 +629,7 @@ impl Database { redirect_uri: row.try_get("redirect_uri")?, code_challenge: row.try_get("code_challenge")?, device_name: row.try_get("device_name")?, + scopes: decode_scopes(row.try_get("scopes_json")?)?, }) }) .transpose() @@ -682,8 +696,8 @@ impl Database { sqlx::query( "INSERT INTO session \ (id, user_id, device_id, access_token_hash, refresh_token_hash, access_expires_at, \ - refresh_expires_at, created_at, last_used_at) \ - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)", + refresh_expires_at, created_at, last_used_at, scopes_json) \ + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", ) .bind(session.id.to_string()) .bind(session.user_id.to_string()) @@ -694,6 +708,7 @@ impl Database { .bind(session.refresh_expires_at) .bind(session.now_ms) .bind(session.now_ms) + .bind(encode_scopes(session.scopes)?) .execute(&self.pool) .await?; Ok(()) @@ -705,7 +720,8 @@ impl Database { now_ms: i64, ) -> Result, sqlx::Error> { let row = sqlx::query( - "SELECT s.id, s.user_id, s.device_id, a.username, a.role, s.refresh_expires_at \ + "SELECT s.id, s.user_id, s.device_id, a.username, a.role, s.refresh_expires_at, \ + s.scopes_json \ FROM session s JOIN account a ON a.id = s.user_id \ WHERE s.refresh_token_hash = ? AND s.revoked_at IS NULL \ AND s.refresh_expires_at > ? AND a.disabled = 0", @@ -723,7 +739,7 @@ impl Database { now_ms: i64, ) -> Result, sqlx::Error> { let row = sqlx::query( - "SELECT a.id, a.username, a.role FROM session s \ + "SELECT a.id, a.username, a.role, s.scopes_json FROM session s \ JOIN account a ON a.id = s.user_id \ JOIN device d ON d.id = s.device_id \ WHERE s.access_token_hash = ? AND s.revoked_at IS NULL \ @@ -738,7 +754,7 @@ impl Database { user_id: parse_uuid(row.try_get("id")?)?, username: row.try_get("username")?, role: parse_role(row.try_get("role")?)?, - scopes: Vec::new(), + scopes: decode_scopes(row.try_get("scopes_json")?)?, }) }) .transpose() @@ -767,8 +783,7 @@ impl Database { user_id: parse_uuid(row.try_get("id")?)?, username: row.try_get("username")?, role: parse_role(row.try_get("role")?)?, - scopes: serde_json::from_str(row.try_get::<&str, _>("scopes_json")?) - .map_err(|error| sqlx::Error::Decode(error.into()))?, + scopes: decode_scopes(row.try_get("scopes_json")?)?, }; if last_used_at.is_none_or(|last_used| last_used <= now_ms.saturating_sub(60_000)) { let _writer = self.writer_guard().await; @@ -874,7 +889,7 @@ impl Database { .bind(user_id.to_string()) .bind(name.trim()) .bind(token_hash) - .bind(serde_json::to_string(scopes).expect("string list serializes")) + .bind(encode_scopes(scopes)?) .bind(now_ms) .fetch_one(&self.pool) .await?; @@ -1144,9 +1159,24 @@ fn session_from_row(row: sqlx::sqlite::SqliteRow) -> Result Result { + serde_json::to_string(scopes).map_err(|error| sqlx::Error::Encode(error.into())) +} + +/// Counterpart of [`encode_scopes`]. An unreadable list is a decode error +/// rather than an empty one: silently reading a corrupt limit as "no limit" +/// would widen a credential exactly where it must not. +fn decode_scopes(value: &str) -> Result, sqlx::Error> { + serde_json::from_str(value).map_err(|error| sqlx::Error::Decode(error.into())) +} + fn parse_uuid(value: String) -> Result { Uuid::parse_str(&value).map_err(|error| sqlx::Error::Decode(Box::new(error))) } @@ -1159,8 +1189,7 @@ fn api_token_from_row(row: sqlx::sqlite::SqliteRow) -> Result("scopes_json")?) - .map_err(|error| sqlx::Error::Decode(error.into()))?, + scopes: decode_scopes(row.try_get("scopes_json")?)?, expires_at: row.try_get("expires_at")?, created_at: row.try_get("created_at")?, last_used_at: row.try_get("last_used_at")?, diff --git a/src/http.rs b/src/http.rs index 8de950c..6fb36b1 100644 --- a/src/http.rs +++ b/src/http.rs @@ -1026,7 +1026,7 @@ pub async fn get_artist( .map_err(service_error) } -#[utoipa::path(get, path = "/api/v2/search", tag = "catalog", params(("q" = String, Query), ("offset" = Option, Query), ("limit" = Option, Query)), responses((status = 200, body = crate::services::SearchResult), (status = 401, body = ErrorResponse), (status = 422, body = ErrorResponse)))] +#[utoipa::path(get, path = "/api/v2/search", tag = "catalog", params(("q" = String, Query), ("offset" = Option, Query), ("limit" = Option, Query)), responses((status = 200, body = crate::services::SearchResult), (status = 400, description = "q is required"), (status = 401, body = ErrorResponse), (status = 422, body = ErrorResponse)))] pub async fn search_catalog( State(state): State, Query(query): Query, @@ -1110,10 +1110,12 @@ pub async fn oauth_authorize( // The browser session is the proof of identity; the consent screen is a // route of the embedded client, so this is a JSON call rather than a form. // - // Unrestricted rather than Write: this mints a credential, and the one it - // mints carries the account's whole authority. A `write` token reaching - // here came back holding a session that answered to `Admin`. - let user = authenticated(&state, &headers, Access::Unrestricted).await?; + // A write, because pairing a device is a mutation on the account and + // nothing more. What used to make this Unrestricted — that the session it + // minted carried the account's whole authority whatever asked for it — is + // gone: the caller's scopes are recorded on the grant just below and the + // redeemed session is issued under them, so this cannot widen a credential. + let user = authenticated(&state, &headers, Access::Write).await?; let redirect_to = state .services .authorize_native_client( @@ -1125,6 +1127,7 @@ pub async fn oauth_authorize( code_challenge_method: &request.code_challenge_method, device_name: &request.device_name, state: request.state.as_deref(), + scopes: &user.scopes, }, ) .await @@ -1155,7 +1158,7 @@ pub async fn oauth_token( } state .auth - .issue_session_for_account(grant.user_id, &grant.device_name) + .issue_session_for_account(grant.user_id, &grant.device_name, &grant.scopes) .await .map(Json) .map_err(|error| match error { @@ -2246,25 +2249,6 @@ pub(crate) enum Access { Write, /// Acts on the instance: accounts, libraries, memberships, credentials. Admin, - /// Mints a new credential from this one. - /// - /// Only an unrestricted credential may, because the credential minted is - /// itself unrestricted: a session issued through the authorization code - /// flow carries the account's whole authority, and nothing on the grant - /// records what asked for it. Letting a narrowed credential mint one is - /// how a token widens itself — `write` was enough to reach the code flow, - /// and the session that came back answered to `Admin`. - /// - /// This is a restriction, not a role: an ordinary account pairs its own - /// devices from its own session, which is what the flow is for. What it - /// refuses is doing so on behalf of a credential that was deliberately - /// narrowed. - /// - /// The durable fix is to carry the scopes through the grant, so a session - /// is never broader than what issued it. That needs a column on - /// `oauth_authorization` and on `session`; until then, this closes the - /// path without a migration. - Unrestricted, } /// The scope that admits the administrative routes. @@ -2293,9 +2277,6 @@ impl Access { Self::Read => true, Self::Write => holds(WRITE_SCOPE) || holds(ADMIN_SCOPE), Self::Admin => holds(ADMIN_SCOPE), - // Unreachable: the early return above already answered for every - // empty list, and a non-empty one is by definition restricted. - Self::Unrestricted => false, } } } @@ -2308,8 +2289,11 @@ impl Access { /// promote an ordinary account, and an administrator's token is not widened by /// whose account it belongs to. /// -/// It could widen itself, once. Minting a credential is [`Access::Unrestricted`] -/// for that reason, and not merely a write. +/// It could widen itself, once: minting a session through the authorization +/// code flow returned one carrying the account's whole authority, whatever the +/// credential that asked. That is closed where it belongs now — the grant +/// records the caller's scopes and the session inherits them — rather than by +/// a rule this function has to know about. pub(crate) async fn authenticated( state: &AppState, headers: &HeaderMap, diff --git a/src/scanner.rs b/src/scanner.rs index 3b25680..9e000df 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -455,6 +455,9 @@ fn extract_file(path: &Path, artwork_dir: &Path) -> Result, bpm: Option, sort_title: Option, + sort_album: Option, + sort_album_artist: Option, + sort_artist: Option, comment: Option, isrc: Option, moods: Option, @@ -631,6 +640,9 @@ fn extended_tags(tag: Option<&lofty::tag::Tag>) -> ExtendedTags { }) .filter(|value| *value > 0), sort_title: text(ItemKey::TrackTitleSortOrder), + sort_album: text(ItemKey::AlbumTitleSortOrder), + sort_album_artist: text(ItemKey::AlbumArtistSortOrder), + sort_artist: text(ItemKey::TrackArtistSortOrder), comment: text(ItemKey::Comment), isrc: text(ItemKey::Isrc), moods: text(ItemKey::Mood), diff --git a/src/services.rs b/src/services.rs index 0187ec8..a7d1379 100644 --- a/src/services.rs +++ b/src/services.rs @@ -77,7 +77,7 @@ macro_rules! song_year_clause { macro_rules! album_select { () => { "SELECT al.id, al.library_id, al.title, al.album_artist_name, al.album_artist_id, \ - al.artwork_hash, al.year, al.is_compilation, al.musicbrainz_id, \ + al.artwork_hash, al.year, al.is_compilation, al.musicbrainz_id, al.sort_name, \ al.created_at, us.starred_at, \ ur.rating AS user_rating, \ (SELECT COUNT(*) FROM play_event pe JOIN track pt ON pt.id=pe.track_id \ @@ -114,7 +114,7 @@ macro_rules! album_scope { macro_rules! artist_select { () => { "SELECT ar.id, ar.library_id, ar.name, ar.artwork_hash, ar.musicbrainz_id, \ - us.starred_at, \ + ar.sort_name, us.starred_at, \ ur.rating AS user_rating, \ (SELECT COUNT(*) FROM album al WHERE al.album_artist_id=ar.id) AS album_count \ FROM artist ar JOIN library_member m ON m.library_id=ar.library_id \ @@ -142,6 +142,10 @@ pub struct ArtistItem { pub name: String, pub artwork_hash: Option, pub musicbrainz_id: Option, + /// The tagged sort form of the name, `None` when no file supplied one. + /// The Subsonic node emits it empty in that case rather than omitting it: + /// the field is supported, and this artist is untagged. + pub sort_name: Option, pub starred_at: Option, pub user_rating: Option, } @@ -157,6 +161,8 @@ pub struct AlbumItem { pub year: Option, pub is_compilation: bool, pub musicbrainz_id: Option, + /// The tagged sort form of the title, on the same terms as the artist's. + pub sort_name: Option, /// Every artist credited on the album's available tracks, and every /// genre they carry. Derived rather than stored: an album has no credit /// or genre of its own in the schema, only the union of its files'. @@ -429,6 +435,10 @@ pub struct AuthorizationRequest<'a> { pub code_challenge_method: &'a str, pub device_name: &'a str, pub state: Option<&'a str>, + /// The scopes of the credential authorizing this grant. Recorded on the + /// grant so the session redeemed from it inherits them, which is what + /// keeps a session from ever being broader than what asked for it. + pub scopes: &'a [String], } #[derive(Debug, Clone, Serialize, ToSchema)] @@ -791,7 +801,7 @@ impl DomainServices { let folders = self.music_folders(user_id, folder_ids).await?; let artists = sqlx::query( "SELECT ar.id, ar.library_id, ar.name, ar.artwork_hash, ar.musicbrainz_id, \ - us.starred_at, ur.rating AS user_rating FROM artist ar \ + ar.sort_name, us.starred_at, ur.rating AS user_rating FROM artist ar \ JOIN library_member m ON m.library_id=ar.library_id \ LEFT JOIN user_star us ON us.user_id=m.user_id AND us.entity_type='artist' AND us.entity_id=ar.id \ LEFT JOIN user_rating ur ON ur.user_id=m.user_id AND ur.entity_type='artist' AND ur.entity_id=ar.id \ @@ -925,7 +935,7 @@ impl DomainServices { let artists = sqlx::query( "SELECT ar.id, ar.library_id, ar.name, ar.artwork_hash, ar.musicbrainz_id, \ - us.starred_at, ur.rating AS user_rating FROM artist ar \ + ar.sort_name, us.starred_at, ur.rating AS user_rating FROM artist ar \ JOIN library_member m ON m.library_id=ar.library_id \ LEFT JOIN user_star us ON us.user_id=m.user_id AND us.entity_type='artist' AND us.entity_id=ar.id \ LEFT JOIN user_rating ur ON ur.user_id=m.user_id AND ur.entity_type='artist' AND ur.entity_id=ar.id \ @@ -1142,6 +1152,38 @@ impl DomainServices { Ok(songs) } + /// The available tracks of one library that belong to no album. + /// + /// A track without an album has no album id to be the `parent` of its + /// Subsonic `child`, so it names its library instead. That was a + /// dead end until now: browsing to that identifier listed the library's + /// artists and nothing else, so a track reachable by search was reachable + /// by no amount of browsing. Answering here is what makes the `parent` + /// it already advertised true. + /// + /// Scoped to one library rather than paged: it is the tail of a folder + /// listing that already answers with every artist of that library. + pub async fn songs_without_album( + &self, + user_id: Uuid, + library_id: Uuid, + ) -> Result, ServiceError> { + let mut songs = sqlx::query(concat!( + song_select!(), + " AND t.library_id=? AND t.album_id IS NULL \ + ORDER BY t.title COLLATE NOCASE, t.id" + )) + .bind(user_id.to_string()) + .bind(library_id.to_string()) + .fetch_all(self.db.pool()) + .await? + .into_iter() + .map(song_from_row) + .collect::, _>>()?; + attach_song_relations(&mut *self.db.pool().acquire().await?, user_id, &mut songs).await?; + Ok(songs) + } + /// A random selection, drawn in SQL rather than by shuffling the catalogue. /// /// The facade used to read every visible track, filter in Rust and shuffle @@ -1553,6 +1595,7 @@ impl DomainServices { device_name, now_ms: now, expires_at: now + crate::oauth::AUTHORIZATION_CODE_TTL_MS, + scopes: request.scopes, }) .await?; Ok(crate::oauth::redirect_with_code( @@ -3905,6 +3948,7 @@ fn artist_from_row(row: sqlx::sqlite::SqliteRow) -> Result Result("is_compilation")? != 0, + sort_name: row.try_get("sort_name")?, musicbrainz_id: row.try_get("musicbrainz_id")?, // Loaded in a batch by `attach_album_relations`, never row by row. artists: Vec::new(), diff --git a/src/subsonic.rs b/src/subsonic.rs index 7444025..23e7e07 100644 --- a/src/subsonic.rs +++ b/src/subsonic.rs @@ -852,13 +852,25 @@ async fn music_directory( let overview = overview(state, principal, params).await?; let mut directory = Node::new("directory").attr("id", id.to_string()); if let Some(folder) = overview.folders.iter().find(|item| item.id == id) { - directory = directory.attr("name", folder.name.clone()).children( - overview - .artists - .iter() - .filter(|artist| artist.library_id == id) - .map(|artist| directory_child(artist_node(artist, 0))), - ); + // The artists of the library, then the tracks that belong to no album. + // Those name this folder as their `parent` for want of an album id, so + // this is the level that has to answer for them — otherwise a track + // advertises a parent that does not contain it. + let orphans = state + .services + .songs_without_album(principal.id, id) + .await + .map_err(service_protocol)?; + directory = directory + .attr("name", folder.name.clone()) + .children( + overview + .artists + .iter() + .filter(|artist| artist.library_id == id) + .map(|artist| directory_child(artist_node(artist, 0))), + ) + .children(orphans.iter().map(|song| song_node(song).renamed("child"))); } else if let Some(artist) = overview.artists.iter().find(|item| item.id == id) { directory = directory.attr("name", artist.name.clone()).children( overview @@ -1654,6 +1666,10 @@ fn artist_node(artist: &ArtistItem, album_count: usize) -> Node { "musicBrainzId", artist.musicbrainz_id.clone().unwrap_or_default(), ) + // Now that the column exists the field is supported, so it is emitted + // with its default rather than omitted: absent would go on saying the + // server cannot answer, which stopped being true. + .attr("sortName", artist.sort_name.clone().unwrap_or_default()) } /// `songCount` and `duration` come from the album projection rather than from @@ -1677,6 +1693,7 @@ fn album_node(album: &AlbumItem) -> Node { .attr("isCompilation", album.is_compilation) .attr("playCount", album.play_count) .attr("displayArtist", album.artist.clone().unwrap_or_default()) + .attr("sortName", album.sort_name.clone().unwrap_or_default()) .maybe_attr("played", album.last_played_at.map(iso_time)) .children(album.artists.iter().map(|artist| { Node::new("artists") diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index b3f48f8..2ede622 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -3299,6 +3299,9 @@ fn catalog_input(index: usize, artist: &str) -> CatalogTrackInput { replay_gain_album_peak: None, bpm: None, sort_title: None, + sort_album: None, + sort_album_artist: None, + sort_artist: None, comment: None, isrc: None, moods: None, @@ -3440,6 +3443,18 @@ fn generate_audio_fixture(path: &std::path::Path, codec: &str, extension: &str) ); } +/// Pulls the authorization code out of the redirect the consent step returns. +fn code_from(redirect_to: &str) -> String { + redirect_to + .split("code=") + .nth(1) + .expect("the redirect carries a code") + .split('&') + .next() + .expect("the code is delimited") + .to_owned() +} + fn json_request(uri: &str, body: serde_json::Value) -> Request { Request::post(uri) .header("content-type", "application/json") @@ -6362,6 +6377,9 @@ fn browse_input( replay_gain_album_peak: None, bpm: None, sort_title: None, + sort_album: None, + sort_album_artist: None, + sort_artist: None, comment: None, isrc: None, moods: None, @@ -8398,11 +8416,13 @@ async fn pkce_authorization_grants_a_native_session_exactly_once() { StatusCode::UNAUTHORIZED ); - // A narrowed credential cannot mint an unrestricted one. The session the - // code flow returns carries the account's whole authority and records - // nothing about what asked for it, so a `write` token reaching this route - // came back holding a session that answered to `Admin`: two requests from - // a token deliberately issued without `admin`. + // A narrowed credential may mint one, and what it mints is narrowed the + // same way: the grant records the caller's scopes and the redeemed session + // is issued under them. `write` in, `write` out. + // + // Before the scopes travelled, the session came back carrying the + // account's whole authority whatever asked for it, so a token deliberately + // issued without `admin` reached `Admin` in two requests. let scoped = json_body( router .clone() @@ -8420,21 +8440,70 @@ async fn pkce_authorization_grants_a_native_session_exactly_once() { ) .await; let scoped = scoped["secret"].as_str().expect("the secret").to_owned(); + let mut narrowed = grant.clone(); + narrowed["device_name"] = "Scoped Agent".into(); + let granted_narrow = authorize(narrowed, Some(scoped.clone())).await; + assert_eq!(granted_narrow.status(), StatusCode::OK); + let narrow_code = code_from( + json_body(granted_narrow).await["redirect_to"] + .as_str() + .unwrap(), + ); + let narrow_session = json_body( + exchange(serde_json::json!({ + "code": narrow_code, + "code_verifier": verifier, + "client_id": "com.waveflow.desktop", + "redirect_uri": redirect_uri + })) + .await, + ) + .await; + let narrow_access = narrow_session["access_token"].as_str().unwrap().to_owned(); + let narrow_refresh = narrow_session["refresh_token"].as_str().unwrap().to_owned(); + // The client is told what it holds, rather than having to discover the + // limit by being refused. assert_eq!( - authorize(grant.clone(), Some(scoped.clone())) - .await - .status(), - StatusCode::FORBIDDEN + narrow_session["user"]["scopes"], + serde_json::json!(["write"]) ); - // It is the narrowing that refuses, not the account: the same account's - // session still grants, and the token still writes what it was given. + + // `pkce-user` is an administrator — it minted the token above — so the + // only thing that can refuse an admin route to this session is the scope + // list it inherited from the token that authorized it. Drop the carrying + // and this answers 200. + let admin_route = |bearer: String| { + let router = router.clone(); + async move { + router + .oneshot( + Request::get("/api/v2/admin/users") + .header("authorization", format!("Bearer {bearer}")) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap() + .status() + } + }; + assert_eq!( + admin_route(narrow_access.clone()).await, + StatusCode::FORBIDDEN, + "a session redeemed from a `write` grant must not answer to `admin`" + ); + // It is the narrowing that refuses and not the account: the same account's + // password session still reaches the same route. + assert_eq!(admin_route(token.clone()).await, StatusCode::OK); + + // And it is a working session, not a broken one: what `write` names, it does. let writes = router .clone() .oneshot( Request::builder() .method(Method::PUT) .uri(format!("/api/v2/ratings/track/{track}")) - .header("authorization", format!("Bearer {scoped}")) + .header("authorization", format!("Bearer {narrow_access}")) .header("content-type", "application/json") .body(Body::from(serde_json::json!({"rating": 3}).to_string())) .unwrap(), @@ -8449,6 +8518,27 @@ async fn pkce_authorization_grants_a_native_session_exactly_once() { assert_eq!(ratings[0].entity_id, track); assert_eq!(ratings[0].rating, 3); + // Rotation must not widen either: the refreshed session answers to exactly + // the scopes the original was issued under. + let rotated = json_body( + router + .clone() + .oneshot(json_request( + "/api/v2/auth/refresh", + serde_json::json!({"refresh_token": narrow_refresh}), + )) + .await + .unwrap(), + ) + .await; + assert_eq!(rotated["user"]["scopes"], serde_json::json!(["write"])); + let rotated = rotated["access_token"].as_str().unwrap().to_owned(); + assert_eq!( + admin_route(rotated).await, + StatusCode::FORBIDDEN, + "refreshing a narrowed session must not hand back a wide one" + ); + // A redirect that could carry the code off the machine is refused. let mut remote = grant.clone(); remote["redirect_uri"] = "http://evil.example.com/cb".into(); @@ -8699,3 +8789,203 @@ async fn subsonic_search_matches_through_the_fts_index() { .await; assert_eq!(titles(&all).len(), 3); } + +/// The three OpenSubsonic gaps the fifth audit named, pinned together because +/// they are one statement: what the catalogue can answer, it now says. +/// +/// `sortName` moves from absent to present-and-possibly-empty — the presence +/// rule's difference between "not supported" and "not tagged". `song.parent` +/// stops naming a directory that would not list the song. And the native +/// search documents the 400 its required parameter already produced. +#[tokio::test] +async fn the_catalogue_answers_for_sort_names_and_for_songs_without_an_album() { + let (_temp, config, state) = test_app().await; + let subsonic_password = "subsonic-secret-123"; + let api_key = "wfsk_sortname-key"; + let admin = state + .db + .create_account( + "sort-admin", + &security::hash_password("correct horse battery staple").unwrap(), + AccountRole::Admin, + now_ms(), + ) + .await + .unwrap(); + let encrypted = state + .secret_box + .encrypt(subsonic_password.as_bytes()) + .unwrap(); + state + .db + .set_subsonic_credential( + admin, + admin, + &encrypted, + &security::token_hash(api_key), + now_ms(), + ) + .await + .unwrap(); + let music = config.data_dir.join("sort-music"); + std::fs::create_dir_all(&music).unwrap(); + let root = std::fs::canonicalize(&music).unwrap(); + let library = state + .db + .create_library(admin, "Sorted", &root, LibraryVisibility::Private, now_ms()) + .await + .unwrap(); + let scan = state + .db + .create_scan_job(library, Some(admin), "manual") + .await + .unwrap(); + state.db.start_scan_job(scan, 3).await.unwrap(); + + // A tagged album, whose sort forms differ from the display forms — the + // only case where the field carries information. + let mut tagged = catalog_input(0, "The Nocturnes"); + tagged.title = "Opening".into(); + tagged.album = Some("The Night Sessions".into()); + tagged.album_artist = Some("The Nocturnes".into()); + tagged.is_compilation = false; + tagged.sort_album = Some("Night Sessions, The".into()); + tagged.sort_album_artist = Some("Nocturnes, The".into()); + tagged.sort_artist = Some("Nocturnes, The".into()); + state + .db + .apply_catalog_track(library, scan, &tagged, None, false) + .await + .unwrap(); + + // An untagged album by another artist: supported and unknown, which is not + // the same statement as unsupported. + let mut untagged = catalog_input(1, "Plain Ensemble"); + untagged.title = "Untitled".into(); + untagged.album = Some("Plain Record".into()); + untagged.album_artist = Some("Plain Ensemble".into()); + untagged.is_compilation = false; + state + .db + .apply_catalog_track(library, scan, &untagged, None, false) + .await + .unwrap(); + + // And a track belonging to no album at all: the one that names its library + // as its parent for want of an album id. + let mut orphan = catalog_input(2, "Lone Voice"); + orphan.title = "Single Only".into(); + orphan.album = None; + orphan.album_artist = None; + orphan.is_compilation = false; + state + .db + .apply_catalog_track(library, scan, &orphan, None, false) + .await + .unwrap(); + state.db.finish_scan_job(scan, 0).await.unwrap(); + let router = waveflow_server::app(&config, state.clone()); + + // --- sortName on AlbumID3 ------------------------------------------- + let albums = subsonic_json( + &router, + "getAlbumList2", + api_key, + "&type=alphabeticalByName", + ) + .await; + let albums = albums["subsonic-response"]["albumList2"]["album"] + .as_array() + .expect("the album list") + .clone(); + let sort_of = |name: &str| -> String { + albums + .iter() + .find(|album| album["name"] == name) + .unwrap_or_else(|| panic!("{name} is listed"))["sortName"] + .as_str() + .expect("sortName is emitted for every album") + .to_owned() + }; + assert_eq!(sort_of("The Night Sessions"), "Night Sessions, The"); + // Emitted empty rather than omitted: the difference between a server that + // cannot answer and an album no file supplied a sort tag for. + assert_eq!(sort_of("Plain Record"), ""); + + // --- sortName on ArtistID3 ------------------------------------------ + let artists = subsonic_json(&router, "getArtists", api_key, "").await; + let mut seen = std::collections::BTreeMap::new(); + for index in artists["subsonic-response"]["artists"]["index"] + .as_array() + .expect("the artist index") + { + for artist in index["artist"].as_array().expect("an index holds artists") { + seen.insert( + artist["name"].as_str().unwrap().to_owned(), + artist["sortName"] + .as_str() + .expect("sortName is emitted for every artist") + .to_owned(), + ); + } + } + assert_eq!( + seen.get("The Nocturnes").map(String::as_str), + Some("Nocturnes, The") + ); + assert_eq!(seen.get("Plain Ensemble").map(String::as_str), Some("")); + + // --- the parent of a song without an album -------------------------- + let directory = subsonic_json( + &router, + "getMusicDirectory", + api_key, + &format!("&id={library}"), + ) + .await; + let children = directory["subsonic-response"]["directory"]["child"] + .as_array() + .expect("the folder lists children") + .clone(); + let orphan_child = children + .iter() + .find(|child| child["title"] == "Single Only") + .expect("a song with no album is reachable by browsing its library"); + // The claim is coherence, not a new identifier: the song already said this + // was its parent, and browsing there now finds it. + assert_eq!( + orphan_child["parent"].as_str(), + Some(library.to_string()).as_deref() + ); + assert_eq!(orphan_child["isDir"], serde_json::json!(false)); + // The artists of the library are still listed alongside it. + assert!( + children + .iter() + .any(|child| child["title"] == "The Nocturnes" || child["name"] == "The Nocturnes"), + "the folder still lists its artists: {children:?}" + ); + // An album's own track is not duplicated into the folder level. + assert!( + !children.iter().any(|child| child["title"] == "Opening"), + "only album-less tracks belong at the folder level" + ); + + // --- the native search's required parameter ------------------------- + let token = login_token(&router, "sort-admin", "correct horse battery staple").await; + let missing_q = router + .clone() + .oneshot( + Request::get("/api/v2/search") + .header("authorization", format!("Bearer {token}")) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!( + missing_q.status(), + StatusCode::BAD_REQUEST, + "the OpenAPI document now says 400, so the route must mean it" + ); +} From a00b7f48e17d37f8e35b4d4a224c2f8cca084185 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Thu, 20 Aug 2026 10:52:20 +0200 Subject: [PATCH 2/6] fix(catalog): derive sort names from the tags a scan still finds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found that `sort_name` written under a COALESCE outlived the tag: remove ALBUMSORT, rescan, and the old value stayed. `consolidate_musicbrainz_ids` had already answered this for the identifiers — "a tag removed from the files has to disappear from the catalogue too" — and it can only do that because the identifiers live on the track. The sort tags now do the same. `track.sort_album` and `track_artist.sort_name` hold them, the latter because a joined ARTISTSORT is paired with its joined ARTIST where the names are split rather than in SQL. A credit the track tag says nothing about falls back to the album artist tag when the two name the same artist, matched on the name so this cannot file one artist under another's sort form. `consolidate_sort_names` re-derives both at the end of every scan, beside the identifier vote: majority, deterministic tie-break, and NULL when no available track carries a tag. Neither upsert writes a sort name any more, so there is no preservation left to outlive anything. The regression test removes the tags from an already-indexed file and rescans; it fails with the old preservation, reporting the stale sort name. Also pins the three sort keys in the scanner's MP4 tag test, which covered every other key `extended_tags` reads. Signed-off-by: InstaZDLL --- docs/rfcs/RFC-002-waveflow-server-v2.md | 2 +- .../20260820000000_track_sort_tags.sql | 17 +++ src/catalog.rs | 126 ++++++++++++++---- src/scanner.rs | 20 +++ tests/v2_foundations.rs | 78 +++++++++++ 5 files changed, 213 insertions(+), 30 deletions(-) create mode 100644 migrations-v2/20260820000000_track_sort_tags.sql diff --git a/docs/rfcs/RFC-002-waveflow-server-v2.md b/docs/rfcs/RFC-002-waveflow-server-v2.md index 2ab6982..fa1e1fa 100644 --- a/docs/rfcs/RFC-002-waveflow-server-v2.md +++ b/docs/rfcs/RFC-002-waveflow-server-v2.md @@ -52,7 +52,7 @@ Both `/rest/` and `/rest/.view` accept GET query parameters and Symfonium 14.1.0 performs a discovery request with the exact unauthenticated tuple `GET ping`, `c=Symfonium`, `u=test`, `p=test` after validating the configured account. WaveFlow returns only the standard successful `ping` envelope for that exact probe and does not create a principal or session. Duplicate identity parameters, alternate clients, POST requests, extra token authentication parameters and every method other than `ping` remain authenticated normally. -The response root freezes `status`, `version=1.16.1`, `type=waveflow`, `serverVersion` and `openSubsonic=true`. XML uses the Subsonic namespace. JSON collection fields are arrays even when they contain one item. Media items freeze the common fields `id`, `parent`, `isDir`, `title`, optional `album`/`artist`/`genre`/`year`/disc-track numbers, seconds-based `duration`, `bitRate`, `size`, `suffix`, `contentType`, `type=music`, optional `coverArt`/`albumId`, and ISO-8601 `created`. Album and artist records include UUID, display name, counts and available artwork/year metadata. Frozen 1.16 metadata is omitted when unknown; the OpenSubsonic additions follow the presence rule instead and are emitted with their default value, see *Deliberate deviations from the v2.0-beta freeze*. Media items carry `mediaType`, `isVideo`, `samplingRate`, `channelCount`, `bitDepth`, `playCount`, `played`, `displayArtist`, `artists[]`, `genres[]`, `musicBrainzId`, `bpm`, `sortName`, `comment`, `isrc[]` and `replayGain`; albums add `isCompilation`, `playCount`, `played` and `displayArtist`. `artists[]` is the credited list in tag order from `track_artist`, of which `artist`/`artistId` remain the display string and the primary credit; `genres[]` comes from `track_genre` ordered by name. `musicBrainzId` on a media item is the MusicBrainz **recording** identifier, the performance; the release and artist identifiers are never exposed at track level, where they would name a different entity. They are exposed on the entities they belong to instead: `album.musicBrainzId` is the release and `artist.musicBrainzId` is the artist, both under the presence rule, and `getAlbumInfo`/`getAlbumInfo2` carry the release id as their `musicBrainzId` element while remaining otherwise empty. Neither is a tag read from one file. Tracks of one album routinely disagree — a library assembled over years holds files tagged against different releases of the same record — so an album takes the identifier most of its available tracks agree on, recomputed at the end of every scan, with ties falling to the earliest disc and track so two scans of unchanged files answer the same thing; an artist takes it from the tracks it is the first credit of, because the tag is one value on a file that may credit several artists. Browsing entries are the exception: `getMusicDirectory` renders artists and albums as `child` elements, where the specification defines `musicBrainzId` as the recording id, so the field is dropped there rather than carrying a different identifier under that name. `replayGain` is the one addition whose *members* are omitted when unknown, on the specification's own instruction, while the container itself is always present because that is what reports the server reads gain tags at all. `moods` is multi-valued and split like the other joined tags; `explicitStatus` is normalised to the two words the specification defines, `explicit` and `clean`, rather than to the per-format spelling the tag used, and a tag saying "no advisory" maps to no value because it is not a claim that the work is clean. Media items additionally carry `albumArtists[]` and `displayAlbumArtist`, which are the album's credit rather than the track's: a guest appearance names the guest, while the album still belongs under the album artist. Albums carry `artists[]` and `genres[]`, derived from their available tracks rather than stored, because an album has no credit or genre of its own in the schema — only the union of its files'; genres are grouped on the canonical name, so an album spelling "Hip-Hop" on some tracks and "Hip Hop" on others reports one genre. What remains unimplemented is what the catalogue cannot answer: `contributors[]` and `displayComposer` need a composer the scanner does not read, and `AlbumID3`'s `moods[]`, `explicitStatus`, `originalReleaseDate`, `releaseDate`, `releaseTypes[]`, `recordLabels[]` and `discTitles[]`, like `ArtistID3`'s `roles[]`, need album and artist columns that do not exist. `sortName` on both is no longer among them: `album.sort_name` and `artist.sort_name` hold the tagged sort forms, read from `ALBUMSORT`, `ALBUMARTISTSORT` and `ARTISTSORT`. A joined credit pairs with its joined sort tag position by position, and a pair whose lengths disagree contributes no sort form at all rather than filing one artist under another's name. Both are emitted with their default value like every other supported addition, so an untagged album reports `sortName=""` — supported and unknown — where before the field was absent. Those are absent rather than empty, which under the presence rule is the accurate statement that they are not supported. +The response root freezes `status`, `version=1.16.1`, `type=waveflow`, `serverVersion` and `openSubsonic=true`. XML uses the Subsonic namespace. JSON collection fields are arrays even when they contain one item. Media items freeze the common fields `id`, `parent`, `isDir`, `title`, optional `album`/`artist`/`genre`/`year`/disc-track numbers, seconds-based `duration`, `bitRate`, `size`, `suffix`, `contentType`, `type=music`, optional `coverArt`/`albumId`, and ISO-8601 `created`. Album and artist records include UUID, display name, counts and available artwork/year metadata. Frozen 1.16 metadata is omitted when unknown; the OpenSubsonic additions follow the presence rule instead and are emitted with their default value, see *Deliberate deviations from the v2.0-beta freeze*. Media items carry `mediaType`, `isVideo`, `samplingRate`, `channelCount`, `bitDepth`, `playCount`, `played`, `displayArtist`, `artists[]`, `genres[]`, `musicBrainzId`, `bpm`, `sortName`, `comment`, `isrc[]` and `replayGain`; albums add `isCompilation`, `playCount`, `played` and `displayArtist`. `artists[]` is the credited list in tag order from `track_artist`, of which `artist`/`artistId` remain the display string and the primary credit; `genres[]` comes from `track_genre` ordered by name. `musicBrainzId` on a media item is the MusicBrainz **recording** identifier, the performance; the release and artist identifiers are never exposed at track level, where they would name a different entity. They are exposed on the entities they belong to instead: `album.musicBrainzId` is the release and `artist.musicBrainzId` is the artist, both under the presence rule, and `getAlbumInfo`/`getAlbumInfo2` carry the release id as their `musicBrainzId` element while remaining otherwise empty. Neither is a tag read from one file. Tracks of one album routinely disagree — a library assembled over years holds files tagged against different releases of the same record — so an album takes the identifier most of its available tracks agree on, recomputed at the end of every scan, with ties falling to the earliest disc and track so two scans of unchanged files answer the same thing; an artist takes it from the tracks it is the first credit of, because the tag is one value on a file that may credit several artists. Browsing entries are the exception: `getMusicDirectory` renders artists and albums as `child` elements, where the specification defines `musicBrainzId` as the recording id, so the field is dropped there rather than carrying a different identifier under that name. `replayGain` is the one addition whose *members* are omitted when unknown, on the specification's own instruction, while the container itself is always present because that is what reports the server reads gain tags at all. `moods` is multi-valued and split like the other joined tags; `explicitStatus` is normalised to the two words the specification defines, `explicit` and `clean`, rather than to the per-format spelling the tag used, and a tag saying "no advisory" maps to no value because it is not a claim that the work is clean. Media items additionally carry `albumArtists[]` and `displayAlbumArtist`, which are the album's credit rather than the track's: a guest appearance names the guest, while the album still belongs under the album artist. Albums carry `artists[]` and `genres[]`, derived from their available tracks rather than stored, because an album has no credit or genre of its own in the schema — only the union of its files'; genres are grouped on the canonical name, so an album spelling "Hip-Hop" on some tracks and "Hip Hop" on others reports one genre. What remains unimplemented is what the catalogue cannot answer: `contributors[]` and `displayComposer` need a composer the scanner does not read, and `AlbumID3`'s `moods[]`, `explicitStatus`, `originalReleaseDate`, `releaseDate`, `releaseTypes[]`, `recordLabels[]` and `discTitles[]`, like `ArtistID3`'s `roles[]`, need album and artist columns that do not exist. `sortName` on both is no longer among them: `album.sort_name` and `artist.sort_name` hold the tagged sort forms, read from `ALBUMSORT`, `ALBUMARTISTSORT` and `ARTISTSORT`. A joined credit pairs with its joined sort tag position by position, and a pair whose lengths disagree contributes no sort form at all rather than filing one artist under another's name; the pairing happens where the names are split, so the per-credit result is stored on `track_artist` and the album's on `track`. Neither is written onto the album or artist row by the per-track upsert. Both are re-derived at the end of every scan by `consolidate_sort_names`, a majority vote with a deterministic tie-break, beside the identifier vote and for the same reason: a value derived from tags must follow the tags, including when they are removed. Writing it during the upsert could not do that — the row is rewritten once per track of every album the artist appears on, so a file with no tag had to be stopped from erasing what a sibling supplied, and that preservation outlived the tag. An album artist credited on none of the album's tracks therefore carries no sort form of its own, because no tag in the catalogue is about it alone. Both are emitted with their default value like every other supported addition, so an untagged album reports `sortName=""` — supported and unknown — where before the field was absent. Those are absent rather than empty, which under the presence rule is the accurate statement that they are not supported. The columns behind the tag fields are added empty and filled by the next scan, so an instance that never rescans stays correct rather than wrong: it reports the fields supported and unset, which is what the presence rule means. Storing MusicBrainz identifiers is the dedicated data contract RFC-004 requires before its MBID branch can begin, and nothing more: a match on one remains a candidate the user confirms, never an automatic link. diff --git a/migrations-v2/20260820000000_track_sort_tags.sql b/migrations-v2/20260820000000_track_sort_tags.sql new file mode 100644 index 0000000..7871c68 --- /dev/null +++ b/migrations-v2/20260820000000_track_sort_tags.sql @@ -0,0 +1,17 @@ +-- Where the sort tags live so a scan can re-derive from them. +-- +-- `album.sort_name` and `artist.sort_name` were written straight from whichever +-- track happened to be applied, under a COALESCE so a sibling file carrying no +-- tag would not erase what another supplied. That preserved a value the files +-- no longer carry: remove ALBUMSORT and rescan, and the old sort name stayed. +-- +-- `consolidate_musicbrainz_ids` already answered this question the other way +-- for the identifiers — "a tag removed from the files has to disappear from the +-- catalogue too" — and it can only do that because the identifiers live on the +-- track. The sort tags now do the same. +-- +-- The credit's sort form goes on `track_artist` rather than on `track`: a +-- joined ARTISTSORT is paired with its joined ARTIST position by position, and +-- that pairing is done where the names are split, not in SQL. +ALTER TABLE track ADD COLUMN sort_album TEXT; +ALTER TABLE track_artist ADD COLUMN sort_name TEXT; diff --git a/src/catalog.rs b/src/catalog.rs index 1f3caf0..87cd156 100644 --- a/src/catalog.rs +++ b/src/catalog.rs @@ -386,6 +386,58 @@ impl Database { /// Albums and artists whose tracks carry no identifier are set back to /// `NULL` rather than left alone: this runs after every scan, so a tag /// removed from the files has to disappear from the catalogue too. + /// Re-derives `album.sort_name` and `artist.sort_name` from the tags the + /// library's available tracks still carry. + /// + /// Sibling of [`Self::consolidate_musicbrainz_ids`] and run beside it, for + /// the same reason and by the same rule: a value derived from tags has to + /// follow the tags, including when they go away. Writing the sort name + /// during the per-track upsert could not do that — the row is rewritten + /// once per track of every album the artist appears on, so a file carrying + /// no tag had to be prevented from erasing what a sibling supplied, and + /// that preservation outlived the tag itself. + /// + /// The majority wins, ties broken deterministically, so two scans of + /// unchanged files answer the same thing. An album takes the sort title + /// its tracks agree on; an artist takes the sort form its credits agree + /// on, from `track_artist` where the joined tag was already paired with + /// the joined credit. An album artist credited on none of the tracks + /// therefore has no sort form of its own — the catalogue holds no tag + /// that is about it alone. + pub async fn consolidate_sort_names(&self, library_id: Uuid) -> Result<(), sqlx::Error> { + let _writer = self.writer_guard().await; + let mut tx = self.pool().begin().await?; + sqlx::query( + "UPDATE album SET sort_name = ( \ + SELECT t.sort_album FROM track t \ + WHERE t.album_id = album.id AND t.is_available = 1 \ + AND t.sort_album IS NOT NULL \ + GROUP BY t.sort_album \ + ORDER BY COUNT(*) DESC, t.sort_album \ + LIMIT 1) \ + WHERE library_id = ?", + ) + .bind(library_id.to_string()) + .execute(&mut *tx) + .await?; + sqlx::query( + "UPDATE artist SET sort_name = ( \ + SELECT ta.sort_name FROM track_artist ta \ + JOIN track t ON t.id = ta.track_id \ + WHERE ta.artist_id = artist.id AND t.is_available = 1 \ + AND ta.sort_name IS NOT NULL \ + GROUP BY ta.sort_name \ + ORDER BY COUNT(*) DESC, ta.sort_name \ + LIMIT 1) \ + WHERE library_id = ?", + ) + .bind(library_id.to_string()) + .execute(&mut *tx) + .await?; + tx.commit().await?; + Ok(()) + } + pub async fn consolidate_musicbrainz_ids(&self, library_id: Uuid) -> Result<(), sqlx::Error> { let _writer = self.writer_guard().await; let mut tx = self.pool().begin().await?; @@ -437,10 +489,7 @@ impl Database { let names = split_values(Some(credit.as_str())); let mut first = None; for name in &names { - // The repair pass works from stored credits, not from tags: - // it has no sort form to offer. `COALESCE` on the stored side - // means passing none leaves whatever a scan already wrote. - let id = upsert_artist(&mut tx, library_id, name, None, now).await?; + let id = upsert_artist(&mut tx, library_id, name, now).await?; first.get_or_insert(id); } let Some(first) = first else { continue }; @@ -683,10 +732,9 @@ impl Database { let track_id = existing_id.unwrap_or_else(Uuid::new_v4); let artwork_hash = upsert_artwork(tx, input.artwork.as_ref(), now).await?; let artist_names = split_values(input.artist.as_deref()); - let artist_sorts = sort_names_for(&artist_names, input.sort_artist.as_deref()); let mut artist_ids = Vec::with_capacity(artist_names.len()); - for (artist, sort) in artist_names.iter().zip(&artist_sorts) { - artist_ids.push(upsert_artist(tx, library_id, artist, sort.as_deref(), now).await?); + for artist in &artist_names { + artist_ids.push(upsert_artist(tx, library_id, artist, now).await?); } let album_artist = input .album_artist @@ -704,13 +752,35 @@ impl Database { // The album keeps the joined string as its display name, and hangs // off the first credited artist. let album_artist_names = split_values(album_artist.as_deref()); - let album_artist_sorts = - sort_names_for(&album_artist_names, input.sort_album_artist.as_deref()); let mut album_artist_id = None; - for (name, sort) in album_artist_names.iter().zip(&album_artist_sorts) { - let id = upsert_artist(tx, library_id, name, sort.as_deref(), now).await?; + for name in &album_artist_names { + let id = upsert_artist(tx, library_id, name, now).await?; album_artist_id.get_or_insert(id); } + // The sort form of each credit *on this track*, which is where a + // joined ARTISTSORT can still be paired with its joined ARTIST. A + // credit the track tag says nothing about falls back to the album + // artist tag when the two name the same artist — ALBUMARTISTSORT is + // the more commonly written of the two, and matching on the name + // rather than on position means this cannot file one artist under + // another's sort form. + let credit_sorts = { + let track_sorts = sort_names_for(&artist_names, input.sort_artist.as_deref()); + let album_sorts = + sort_names_for(&album_artist_names, input.sort_album_artist.as_deref()); + artist_names + .iter() + .zip(track_sorts) + .map(|(name, sort)| { + sort.or_else(|| { + album_artist_names + .iter() + .position(|candidate| candidate == name) + .and_then(|at| album_sorts.get(at).cloned().flatten()) + }) + }) + .collect::>() + }; let album_id = upsert_album( tx, library_id, @@ -733,11 +803,12 @@ impl Database { year, track_number, disc_number, duration_ms, bitrate, sample_rate, channels, bit_depth, \ codec, musical_key, tag_rating, musicbrainz_recording_id, musicbrainz_release_id, \ musicbrainz_artist_id, replay_gain_track_gain, replay_gain_track_peak, \ - replay_gain_album_gain, replay_gain_album_peak, bpm, sort_title, comment, isrc, \ + replay_gain_album_gain, replay_gain_album_peak, bpm, sort_title, sort_album, \ + comment, isrc, \ moods, explicit_status, \ lyrics_hash, is_available, last_seen_scan_id, created_at, updated_at) \ VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, \ - ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, \ + ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, \ 1, ?, ?, ?) \ ON CONFLICT (id) DO UPDATE SET album_id=excluded.album_id, artwork_hash=excluded.artwork_hash, \ relative_path=excluded.relative_path, file_size=excluded.file_size, \ @@ -755,7 +826,8 @@ impl Database { replay_gain_track_peak=excluded.replay_gain_track_peak, \ replay_gain_album_gain=excluded.replay_gain_album_gain, \ replay_gain_album_peak=excluded.replay_gain_album_peak, \ - bpm=excluded.bpm, sort_title=excluded.sort_title, comment=excluded.comment, \ + bpm=excluded.bpm, sort_title=excluded.sort_title, sort_album=excluded.sort_album, \ + comment=excluded.comment, \ isrc=excluded.isrc, moods=excluded.moods, \ explicit_status=excluded.explicit_status, \ lyrics_hash=excluded.lyrics_hash, is_available=1, last_seen_scan_id=excluded.last_seen_scan_id, \ @@ -774,7 +846,8 @@ impl Database { .bind(input.musicbrainz_artist_id.as_deref()) .bind(input.replay_gain_track_gain).bind(input.replay_gain_track_peak) .bind(input.replay_gain_album_gain).bind(input.replay_gain_album_peak) - .bind(input.bpm).bind(input.sort_title.as_deref()).bind(input.comment.as_deref()) + .bind(input.bpm).bind(input.sort_title.as_deref()).bind(input.sort_album.as_deref()) + .bind(input.comment.as_deref()) .bind(input.isrc.as_deref()) .bind(input.moods.as_deref()).bind(input.explicit_status.as_deref()) .bind(&input.lyrics_hash) @@ -807,9 +880,9 @@ impl Database { .execute(&mut **tx) .await?; for (position, artist_id) in artist_ids.iter().enumerate() { - sqlx::query("INSERT INTO track_artist (track_id, artist_id, library_id, position) VALUES (?, ?, ?, ?)") + sqlx::query("INSERT INTO track_artist (track_id, artist_id, library_id, position, sort_name) VALUES (?, ?, ?, ?, ?)") .bind(track_id.to_string()).bind(artist_id.to_string()).bind(library_id.to_string()) - .bind(position as i64).execute(&mut **tx).await?; + .bind(position as i64).bind(credit_sorts.get(position).and_then(Option::as_deref)).execute(&mut **tx).await?; } sqlx::query("DELETE FROM track_genre WHERE track_id = ?") .bind(track_id.to_string()) @@ -1019,18 +1092,13 @@ async fn upsert_artist( tx: &mut Transaction<'_, Sqlite>, library: Uuid, name: &str, - sort_name: Option<&str>, now: i64, ) -> Result { let id = Uuid::new_v4(); let canonical = waveflow_core::scanner::canonical_name(name); - // COALESCE on the stored side: a file that carries no sort tag must not - // erase the one a sibling file supplied, and the artist row is written - // once per track of every album it appears on. - let value: String = sqlx::query_scalar("INSERT INTO artist (id, library_id, name, canonical_name, sort_name, created_at, updated_at) \ - VALUES (?, ?, ?, ?, ?, ?, ?) ON CONFLICT (library_id, canonical_name) DO UPDATE SET name=excluded.name, \ - sort_name=COALESCE(excluded.sort_name, artist.sort_name), updated_at=excluded.updated_at RETURNING id") - .bind(id.to_string()).bind(library.to_string()).bind(name.trim()).bind(canonical).bind(sort_name).bind(now).bind(now) + let value: String = sqlx::query_scalar("INSERT INTO artist (id, library_id, name, canonical_name, created_at, updated_at) \ + VALUES (?, ?, ?, ?, ?, ?) ON CONFLICT (library_id, canonical_name) DO UPDATE SET name=excluded.name, updated_at=excluded.updated_at RETURNING id") + .bind(id.to_string()).bind(library.to_string()).bind(name.trim()).bind(canonical).bind(now).bind(now) .fetch_one(&mut **tx).await?; parse_uuid(value) } @@ -1082,13 +1150,13 @@ async fn upsert_album( let canonical = waveflow_core::scanner::canonical_name(title); let identity = album_identity(&canonical, album_artist_id, album_artist); let id = Uuid::new_v4(); - let value: String = sqlx::query_scalar("INSERT INTO album (id, library_id, title, canonical_title, identity_key, album_artist_id, album_artist_name, is_compilation, year, artwork_hash, sort_name, created_at, updated_at) \ - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT (library_id, identity_key) DO UPDATE SET title=excluded.title, album_artist_name=excluded.album_artist_name, \ + let value: String = sqlx::query_scalar("INSERT INTO album (id, library_id, title, canonical_title, identity_key, album_artist_id, album_artist_name, is_compilation, year, artwork_hash, created_at, updated_at) \ + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) ON CONFLICT (library_id, identity_key) DO UPDATE SET title=excluded.title, album_artist_name=excluded.album_artist_name, \ is_compilation=excluded.is_compilation, year=COALESCE(excluded.year, album.year), artwork_hash=COALESCE(excluded.artwork_hash, album.artwork_hash), \ - sort_name=COALESCE(excluded.sort_name, album.sort_name), updated_at=excluded.updated_at RETURNING id") + updated_at=excluded.updated_at RETURNING id") .bind(id.to_string()).bind(library.to_string()).bind(title.trim()).bind(canonical).bind(identity) .bind(album_artist_id.map(|id| id.to_string())).bind(album_artist).bind(i64::from(input.is_compilation)) - .bind(input.year).bind(artwork).bind(input.sort_album.as_deref()).bind(now).bind(now).fetch_one(&mut **tx).await?; + .bind(input.year).bind(artwork).bind(now).bind(now).fetch_one(&mut **tx).await?; parse_uuid(value).map(Some) } diff --git a/src/scanner.rs b/src/scanner.rs index 9e000df..ea1268b 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -290,6 +290,7 @@ impl ScanManager { // identifiers are a majority vote over the tracks that are still there, // so a file that vanished must stop voting first. self.db.consolidate_musicbrainz_ids(library.id).await?; + self.db.consolidate_sort_names(library.id).await?; self.db .finish_scan_job(scan_id, progress.unavailable) .await?; @@ -771,6 +772,16 @@ mod tests { assert!(tag.insert_text(ItemKey::Isrc, "FRZ039800212".into())); assert!(tag.insert_text(ItemKey::Mood, "Melancholic; Warm".into())); assert!(tag.insert_text(ItemKey::ParentalAdvisory, "2".into())); + // The three sort keys, which the catalogue derives album and artist + // sort names from. A joined credit carries a joined sort tag, and the + // pairing happens where the names are split — what this pins is that + // all three arrive at all, under the tag type that spells them. + assert!(tag.insert_text(ItemKey::AlbumTitleSortOrder, "Night Sessions, The".into())); + assert!(tag.insert_text(ItemKey::AlbumArtistSortOrder, "Nocturnes, The".into())); + assert!(tag.insert_text( + ItemKey::TrackArtistSortOrder, + "Nocturnes, The; Reed, Ada".into() + )); let extended = extended_tags(Some(&tag)); assert_eq!(extended.replay_gain_track_gain, Some(-7.32)); assert_eq!(extended.replay_gain_album_gain, Some(1.5)); @@ -787,6 +798,15 @@ mod tests { // The per-format spelling is normalised to the two words the // specification defines; a client compares against nothing else. assert_eq!(extended.explicit_status.as_deref(), Some("clean")); + assert_eq!(extended.sort_album.as_deref(), Some("Night Sessions, The")); + assert_eq!( + extended.sort_album_artist.as_deref(), + Some("Nocturnes, The") + ); + assert_eq!( + extended.sort_artist.as_deref(), + Some("Nocturnes, The; Reed, Ada") + ); // Unparseable measurements are dropped rather than stored: a NaN gain // would reach a player as a volume adjustment. diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index 2ede622..c32b1e7 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -8883,6 +8883,10 @@ async fn the_catalogue_answers_for_sort_names_and_for_songs_without_an_album() { .apply_catalog_track(library, scan, &orphan, None, false) .await .unwrap(); + // Sort names are derived at the end of a scan, like the identifiers: the + // scanner runs both passes here, so a test driving the catalogue directly + // runs them too. + state.db.consolidate_sort_names(library).await.unwrap(); state.db.finish_scan_job(scan, 0).await.unwrap(); let router = waveflow_server::app(&config, state.clone()); @@ -8988,4 +8992,78 @@ async fn the_catalogue_answers_for_sort_names_and_for_songs_without_an_album() { StatusCode::BAD_REQUEST, "the OpenAPI document now says 400, so the route must mean it" ); + + // --- a sort tag removed from the files leaves the catalogue --------- + // Writing the value during the per-track upsert could not do this: the + // artist row is rewritten once per track, so a file with no tag had to be + // stopped from erasing what a sibling supplied — and that preservation + // outlived the tag. Deriving at the end of the scan is what makes removal + // mean removal, exactly as it already does for the MusicBrainz ids. + let rescan = state + .db + .create_scan_job(library, Some(admin), "manual") + .await + .unwrap(); + state.db.start_scan_job(rescan, 1).await.unwrap(); + let mut untagged_now = tagged.clone(); + untagged_now.sort_album = None; + untagged_now.sort_album_artist = None; + untagged_now.sort_artist = None; + // The same file, retagged: re-applied onto its own row rather than added + // beside it, which is what a rescan of an edited file does. + let existing = state + .services + .catalog_snapshot(admin, &[]) + .await + .unwrap() + .songs + .into_iter() + .find(|song| song.title == "Opening") + .expect("the tagged track is in the catalogue") + .id; + state + .db + .apply_catalog_track(library, rescan, &untagged_now, Some(existing), false) + .await + .unwrap(); + state.db.consolidate_sort_names(library).await.unwrap(); + state.db.finish_scan_job(rescan, 0).await.unwrap(); + + let after = subsonic_json( + &router, + "getAlbumList2", + api_key, + "&type=alphabeticalByName", + ) + .await; + let after_album = after["subsonic-response"]["albumList2"]["album"] + .as_array() + .expect("the album list") + .iter() + .find(|album| album["name"] == "The Night Sessions") + .expect("the album is still listed") + .clone(); + assert_eq!( + after_album["sortName"].as_str(), + Some(""), + "a sort tag removed from the files must leave the catalogue with it" + ); + + let after_artists = subsonic_json(&router, "getArtists", api_key, "").await; + let mut after_seen = std::collections::BTreeMap::new(); + for index in after_artists["subsonic-response"]["artists"]["index"] + .as_array() + .expect("the artist index") + { + for artist in index["artist"].as_array().expect("an index holds artists") { + after_seen.insert( + artist["name"].as_str().unwrap().to_owned(), + artist["sortName"].as_str().unwrap().to_owned(), + ); + } + } + assert_eq!( + after_seen.get("The Nocturnes").map(String::as_str), + Some("") + ); } From c6177fd8a0fc393c57f11b9cf7cce1b53843939c Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Thu, 20 Aug 2026 10:58:49 +0200 Subject: [PATCH 3/6] docs(subsonic): say what an artist reference carries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review read the new sortName sentence as covering the entries of artists[] and albumArtists[], which it does not. Those entries, and the artist and album entries getMusicDirectory renders as child, are references rather than records: id and name, and nothing else — no musicBrainzId, no albumCount, no starred, no sortName. The RFC never said so, which is what left the reading open. No wire change. Signed-off-by: InstaZDLL --- docs/rfcs/RFC-002-waveflow-server-v2.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/rfcs/RFC-002-waveflow-server-v2.md b/docs/rfcs/RFC-002-waveflow-server-v2.md index fa1e1fa..f9d16c3 100644 --- a/docs/rfcs/RFC-002-waveflow-server-v2.md +++ b/docs/rfcs/RFC-002-waveflow-server-v2.md @@ -52,7 +52,7 @@ Both `/rest/` and `/rest/.view` accept GET query parameters and Symfonium 14.1.0 performs a discovery request with the exact unauthenticated tuple `GET ping`, `c=Symfonium`, `u=test`, `p=test` after validating the configured account. WaveFlow returns only the standard successful `ping` envelope for that exact probe and does not create a principal or session. Duplicate identity parameters, alternate clients, POST requests, extra token authentication parameters and every method other than `ping` remain authenticated normally. -The response root freezes `status`, `version=1.16.1`, `type=waveflow`, `serverVersion` and `openSubsonic=true`. XML uses the Subsonic namespace. JSON collection fields are arrays even when they contain one item. Media items freeze the common fields `id`, `parent`, `isDir`, `title`, optional `album`/`artist`/`genre`/`year`/disc-track numbers, seconds-based `duration`, `bitRate`, `size`, `suffix`, `contentType`, `type=music`, optional `coverArt`/`albumId`, and ISO-8601 `created`. Album and artist records include UUID, display name, counts and available artwork/year metadata. Frozen 1.16 metadata is omitted when unknown; the OpenSubsonic additions follow the presence rule instead and are emitted with their default value, see *Deliberate deviations from the v2.0-beta freeze*. Media items carry `mediaType`, `isVideo`, `samplingRate`, `channelCount`, `bitDepth`, `playCount`, `played`, `displayArtist`, `artists[]`, `genres[]`, `musicBrainzId`, `bpm`, `sortName`, `comment`, `isrc[]` and `replayGain`; albums add `isCompilation`, `playCount`, `played` and `displayArtist`. `artists[]` is the credited list in tag order from `track_artist`, of which `artist`/`artistId` remain the display string and the primary credit; `genres[]` comes from `track_genre` ordered by name. `musicBrainzId` on a media item is the MusicBrainz **recording** identifier, the performance; the release and artist identifiers are never exposed at track level, where they would name a different entity. They are exposed on the entities they belong to instead: `album.musicBrainzId` is the release and `artist.musicBrainzId` is the artist, both under the presence rule, and `getAlbumInfo`/`getAlbumInfo2` carry the release id as their `musicBrainzId` element while remaining otherwise empty. Neither is a tag read from one file. Tracks of one album routinely disagree — a library assembled over years holds files tagged against different releases of the same record — so an album takes the identifier most of its available tracks agree on, recomputed at the end of every scan, with ties falling to the earliest disc and track so two scans of unchanged files answer the same thing; an artist takes it from the tracks it is the first credit of, because the tag is one value on a file that may credit several artists. Browsing entries are the exception: `getMusicDirectory` renders artists and albums as `child` elements, where the specification defines `musicBrainzId` as the recording id, so the field is dropped there rather than carrying a different identifier under that name. `replayGain` is the one addition whose *members* are omitted when unknown, on the specification's own instruction, while the container itself is always present because that is what reports the server reads gain tags at all. `moods` is multi-valued and split like the other joined tags; `explicitStatus` is normalised to the two words the specification defines, `explicit` and `clean`, rather than to the per-format spelling the tag used, and a tag saying "no advisory" maps to no value because it is not a claim that the work is clean. Media items additionally carry `albumArtists[]` and `displayAlbumArtist`, which are the album's credit rather than the track's: a guest appearance names the guest, while the album still belongs under the album artist. Albums carry `artists[]` and `genres[]`, derived from their available tracks rather than stored, because an album has no credit or genre of its own in the schema — only the union of its files'; genres are grouped on the canonical name, so an album spelling "Hip-Hop" on some tracks and "Hip Hop" on others reports one genre. What remains unimplemented is what the catalogue cannot answer: `contributors[]` and `displayComposer` need a composer the scanner does not read, and `AlbumID3`'s `moods[]`, `explicitStatus`, `originalReleaseDate`, `releaseDate`, `releaseTypes[]`, `recordLabels[]` and `discTitles[]`, like `ArtistID3`'s `roles[]`, need album and artist columns that do not exist. `sortName` on both is no longer among them: `album.sort_name` and `artist.sort_name` hold the tagged sort forms, read from `ALBUMSORT`, `ALBUMARTISTSORT` and `ARTISTSORT`. A joined credit pairs with its joined sort tag position by position, and a pair whose lengths disagree contributes no sort form at all rather than filing one artist under another's name; the pairing happens where the names are split, so the per-credit result is stored on `track_artist` and the album's on `track`. Neither is written onto the album or artist row by the per-track upsert. Both are re-derived at the end of every scan by `consolidate_sort_names`, a majority vote with a deterministic tie-break, beside the identifier vote and for the same reason: a value derived from tags must follow the tags, including when they are removed. Writing it during the upsert could not do that — the row is rewritten once per track of every album the artist appears on, so a file with no tag had to be stopped from erasing what a sibling supplied, and that preservation outlived the tag. An album artist credited on none of the album's tracks therefore carries no sort form of its own, because no tag in the catalogue is about it alone. Both are emitted with their default value like every other supported addition, so an untagged album reports `sortName=""` — supported and unknown — where before the field was absent. Those are absent rather than empty, which under the presence rule is the accurate statement that they are not supported. +The response root freezes `status`, `version=1.16.1`, `type=waveflow`, `serverVersion` and `openSubsonic=true`. XML uses the Subsonic namespace. JSON collection fields are arrays even when they contain one item. Media items freeze the common fields `id`, `parent`, `isDir`, `title`, optional `album`/`artist`/`genre`/`year`/disc-track numbers, seconds-based `duration`, `bitRate`, `size`, `suffix`, `contentType`, `type=music`, optional `coverArt`/`albumId`, and ISO-8601 `created`. Album and artist records include UUID, display name, counts and available artwork/year metadata. Frozen 1.16 metadata is omitted when unknown; the OpenSubsonic additions follow the presence rule instead and are emitted with their default value, see *Deliberate deviations from the v2.0-beta freeze*. Media items carry `mediaType`, `isVideo`, `samplingRate`, `channelCount`, `bitDepth`, `playCount`, `played`, `displayArtist`, `artists[]`, `genres[]`, `musicBrainzId`, `bpm`, `sortName`, `comment`, `isrc[]` and `replayGain`; albums add `isCompilation`, `playCount`, `played` and `displayArtist`. `artists[]` is the credited list in tag order from `track_artist`, of which `artist`/`artistId` remain the display string and the primary credit; `genres[]` comes from `track_genre` ordered by name. Its entries — like those of `albumArtists[]`, and like the artist and album entries `getMusicDirectory` renders as `child` — are references rather than records: `id` and `name`, and nothing else. They carry no `musicBrainzId`, no `albumCount`, no `starred` and no `sortName`. A client that wants an artist's own fields asks for the artist; that is what the identifier in the reference is for. `musicBrainzId` on a media item is the MusicBrainz **recording** identifier, the performance; the release and artist identifiers are never exposed at track level, where they would name a different entity. They are exposed on the entities they belong to instead: `album.musicBrainzId` is the release and `artist.musicBrainzId` is the artist, both under the presence rule, and `getAlbumInfo`/`getAlbumInfo2` carry the release id as their `musicBrainzId` element while remaining otherwise empty. Neither is a tag read from one file. Tracks of one album routinely disagree — a library assembled over years holds files tagged against different releases of the same record — so an album takes the identifier most of its available tracks agree on, recomputed at the end of every scan, with ties falling to the earliest disc and track so two scans of unchanged files answer the same thing; an artist takes it from the tracks it is the first credit of, because the tag is one value on a file that may credit several artists. Browsing entries are the exception: `getMusicDirectory` renders artists and albums as `child` elements, where the specification defines `musicBrainzId` as the recording id, so the field is dropped there rather than carrying a different identifier under that name. `replayGain` is the one addition whose *members* are omitted when unknown, on the specification's own instruction, while the container itself is always present because that is what reports the server reads gain tags at all. `moods` is multi-valued and split like the other joined tags; `explicitStatus` is normalised to the two words the specification defines, `explicit` and `clean`, rather than to the per-format spelling the tag used, and a tag saying "no advisory" maps to no value because it is not a claim that the work is clean. Media items additionally carry `albumArtists[]` and `displayAlbumArtist`, which are the album's credit rather than the track's: a guest appearance names the guest, while the album still belongs under the album artist. Albums carry `artists[]` and `genres[]`, derived from their available tracks rather than stored, because an album has no credit or genre of its own in the schema — only the union of its files'; genres are grouped on the canonical name, so an album spelling "Hip-Hop" on some tracks and "Hip Hop" on others reports one genre. What remains unimplemented is what the catalogue cannot answer: `contributors[]` and `displayComposer` need a composer the scanner does not read, and `AlbumID3`'s `moods[]`, `explicitStatus`, `originalReleaseDate`, `releaseDate`, `releaseTypes[]`, `recordLabels[]` and `discTitles[]`, like `ArtistID3`'s `roles[]`, need album and artist columns that do not exist. `sortName` on both is no longer among them: `album.sort_name` and `artist.sort_name` hold the tagged sort forms, read from `ALBUMSORT`, `ALBUMARTISTSORT` and `ARTISTSORT`. A joined credit pairs with its joined sort tag position by position, and a pair whose lengths disagree contributes no sort form at all rather than filing one artist under another's name; the pairing happens where the names are split, so the per-credit result is stored on `track_artist` and the album's on `track`. Neither is written onto the album or artist row by the per-track upsert. Both are re-derived at the end of every scan by `consolidate_sort_names`, a majority vote with a deterministic tie-break, beside the identifier vote and for the same reason: a value derived from tags must follow the tags, including when they are removed. Writing it during the upsert could not do that — the row is rewritten once per track of every album the artist appears on, so a file with no tag had to be stopped from erasing what a sibling supplied, and that preservation outlived the tag. An album artist credited on none of the album's tracks therefore carries no sort form of its own, because no tag in the catalogue is about it alone. Both are emitted with their default value like every other supported addition, so an untagged album reports `sortName=""` — supported and unknown — where before the field was absent. Those are absent rather than empty, which under the presence rule is the accurate statement that they are not supported. The columns behind the tag fields are added empty and filled by the next scan, so an instance that never rescans stays correct rather than wrong: it reports the fields supported and unset, which is what the presence rule means. Storing MusicBrainz identifiers is the dedicated data contract RFC-004 requires before its MBID branch can begin, and nothing more: a match on one remains a candidate the user confirms, never an automatic link. From 3154214e94ea379a3629093ca1deb3367e1a94a2 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Thu, 20 Aug 2026 11:11:02 +0200 Subject: [PATCH 4/6] docs(subsonic): state the artist-reference deviation, and pin it Review offered a choice: carry sortName in the artists[] and albumArtists[] references, or document the deviation and pin it with fixtures. Taking the second, because the first inlines a second copy of every artist record into every track and album payload, on a contract frozen for v2.0-beta and revalidated on four clients the day before. It also corrects the previous commit, which lumped the getMusicDirectory children in with the references and said they carry neither. They do: directory_child removes musicBrainzId and nothing else, so a browsing child carries sortName, albumCount and the rest. Measured, not read. an_artist_reference_is_not_an_artist_record pins both projections against each other in JSON and XML, so a field added to the artist node cannot leak into the reference. The vacuity guard earned its place immediately: the folder listing holds no reference at all when the library has no album-less track, so the references are read from the album document. Signed-off-by: InstaZDLL --- docs/rfcs/RFC-002-waveflow-server-v2.md | 2 +- docs/subsonic-compatibility.md | 11 ++ tests/v2_foundations.rs | 145 ++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 1 deletion(-) diff --git a/docs/rfcs/RFC-002-waveflow-server-v2.md b/docs/rfcs/RFC-002-waveflow-server-v2.md index f9d16c3..98a67c8 100644 --- a/docs/rfcs/RFC-002-waveflow-server-v2.md +++ b/docs/rfcs/RFC-002-waveflow-server-v2.md @@ -52,7 +52,7 @@ Both `/rest/` and `/rest/.view` accept GET query parameters and Symfonium 14.1.0 performs a discovery request with the exact unauthenticated tuple `GET ping`, `c=Symfonium`, `u=test`, `p=test` after validating the configured account. WaveFlow returns only the standard successful `ping` envelope for that exact probe and does not create a principal or session. Duplicate identity parameters, alternate clients, POST requests, extra token authentication parameters and every method other than `ping` remain authenticated normally. -The response root freezes `status`, `version=1.16.1`, `type=waveflow`, `serverVersion` and `openSubsonic=true`. XML uses the Subsonic namespace. JSON collection fields are arrays even when they contain one item. Media items freeze the common fields `id`, `parent`, `isDir`, `title`, optional `album`/`artist`/`genre`/`year`/disc-track numbers, seconds-based `duration`, `bitRate`, `size`, `suffix`, `contentType`, `type=music`, optional `coverArt`/`albumId`, and ISO-8601 `created`. Album and artist records include UUID, display name, counts and available artwork/year metadata. Frozen 1.16 metadata is omitted when unknown; the OpenSubsonic additions follow the presence rule instead and are emitted with their default value, see *Deliberate deviations from the v2.0-beta freeze*. Media items carry `mediaType`, `isVideo`, `samplingRate`, `channelCount`, `bitDepth`, `playCount`, `played`, `displayArtist`, `artists[]`, `genres[]`, `musicBrainzId`, `bpm`, `sortName`, `comment`, `isrc[]` and `replayGain`; albums add `isCompilation`, `playCount`, `played` and `displayArtist`. `artists[]` is the credited list in tag order from `track_artist`, of which `artist`/`artistId` remain the display string and the primary credit; `genres[]` comes from `track_genre` ordered by name. Its entries — like those of `albumArtists[]`, and like the artist and album entries `getMusicDirectory` renders as `child` — are references rather than records: `id` and `name`, and nothing else. They carry no `musicBrainzId`, no `albumCount`, no `starred` and no `sortName`. A client that wants an artist's own fields asks for the artist; that is what the identifier in the reference is for. `musicBrainzId` on a media item is the MusicBrainz **recording** identifier, the performance; the release and artist identifiers are never exposed at track level, where they would name a different entity. They are exposed on the entities they belong to instead: `album.musicBrainzId` is the release and `artist.musicBrainzId` is the artist, both under the presence rule, and `getAlbumInfo`/`getAlbumInfo2` carry the release id as their `musicBrainzId` element while remaining otherwise empty. Neither is a tag read from one file. Tracks of one album routinely disagree — a library assembled over years holds files tagged against different releases of the same record — so an album takes the identifier most of its available tracks agree on, recomputed at the end of every scan, with ties falling to the earliest disc and track so two scans of unchanged files answer the same thing; an artist takes it from the tracks it is the first credit of, because the tag is one value on a file that may credit several artists. Browsing entries are the exception: `getMusicDirectory` renders artists and albums as `child` elements, where the specification defines `musicBrainzId` as the recording id, so the field is dropped there rather than carrying a different identifier under that name. `replayGain` is the one addition whose *members* are omitted when unknown, on the specification's own instruction, while the container itself is always present because that is what reports the server reads gain tags at all. `moods` is multi-valued and split like the other joined tags; `explicitStatus` is normalised to the two words the specification defines, `explicit` and `clean`, rather than to the per-format spelling the tag used, and a tag saying "no advisory" maps to no value because it is not a claim that the work is clean. Media items additionally carry `albumArtists[]` and `displayAlbumArtist`, which are the album's credit rather than the track's: a guest appearance names the guest, while the album still belongs under the album artist. Albums carry `artists[]` and `genres[]`, derived from their available tracks rather than stored, because an album has no credit or genre of its own in the schema — only the union of its files'; genres are grouped on the canonical name, so an album spelling "Hip-Hop" on some tracks and "Hip Hop" on others reports one genre. What remains unimplemented is what the catalogue cannot answer: `contributors[]` and `displayComposer` need a composer the scanner does not read, and `AlbumID3`'s `moods[]`, `explicitStatus`, `originalReleaseDate`, `releaseDate`, `releaseTypes[]`, `recordLabels[]` and `discTitles[]`, like `ArtistID3`'s `roles[]`, need album and artist columns that do not exist. `sortName` on both is no longer among them: `album.sort_name` and `artist.sort_name` hold the tagged sort forms, read from `ALBUMSORT`, `ALBUMARTISTSORT` and `ARTISTSORT`. A joined credit pairs with its joined sort tag position by position, and a pair whose lengths disagree contributes no sort form at all rather than filing one artist under another's name; the pairing happens where the names are split, so the per-credit result is stored on `track_artist` and the album's on `track`. Neither is written onto the album or artist row by the per-track upsert. Both are re-derived at the end of every scan by `consolidate_sort_names`, a majority vote with a deterministic tie-break, beside the identifier vote and for the same reason: a value derived from tags must follow the tags, including when they are removed. Writing it during the upsert could not do that — the row is rewritten once per track of every album the artist appears on, so a file with no tag had to be stopped from erasing what a sibling supplied, and that preservation outlived the tag. An album artist credited on none of the album's tracks therefore carries no sort form of its own, because no tag in the catalogue is about it alone. Both are emitted with their default value like every other supported addition, so an untagged album reports `sortName=""` — supported and unknown — where before the field was absent. Those are absent rather than empty, which under the presence rule is the accurate statement that they are not supported. +The response root freezes `status`, `version=1.16.1`, `type=waveflow`, `serverVersion` and `openSubsonic=true`. XML uses the Subsonic namespace. JSON collection fields are arrays even when they contain one item. Media items freeze the common fields `id`, `parent`, `isDir`, `title`, optional `album`/`artist`/`genre`/`year`/disc-track numbers, seconds-based `duration`, `bitRate`, `size`, `suffix`, `contentType`, `type=music`, optional `coverArt`/`albumId`, and ISO-8601 `created`. Album and artist records include UUID, display name, counts and available artwork/year metadata. Frozen 1.16 metadata is omitted when unknown; the OpenSubsonic additions follow the presence rule instead and are emitted with their default value, see *Deliberate deviations from the v2.0-beta freeze*. Media items carry `mediaType`, `isVideo`, `samplingRate`, `channelCount`, `bitDepth`, `playCount`, `played`, `displayArtist`, `artists[]`, `genres[]`, `musicBrainzId`, `bpm`, `sortName`, `comment`, `isrc[]` and `replayGain`; albums add `isCompilation`, `playCount`, `played` and `displayArtist`. `artists[]` is the credited list in tag order from `track_artist`, of which `artist`/`artistId` remain the display string and the primary credit; `genres[]` comes from `track_genre` ordered by name. Its entries, like those of `albumArtists[]`, are references rather than records: `id` and `name`, and nothing else — no `musicBrainzId`, no `albumCount`, no `starred`, no `sortName`. This is a deliberate deviation from `ArtistID3`, which those arrays are typed as. A client that wants an artist's own fields asks for the artist, which is what the identifier in the reference is for; inlining a second copy of every record into every track and album payload is what the reference exists to avoid. The entries `getMusicDirectory` renders as `child` are a different projection and are not affected: they are the artist and album nodes themselves with `musicBrainzId` removed, so they do carry `sortName` and the rest. The two are pinned against each other in `an_artist_reference_is_not_an_artist_record`, in both encodings, so a field added to the node cannot leak into the reference. `musicBrainzId` on a media item is the MusicBrainz **recording** identifier, the performance; the release and artist identifiers are never exposed at track level, where they would name a different entity. They are exposed on the entities they belong to instead: `album.musicBrainzId` is the release and `artist.musicBrainzId` is the artist, both under the presence rule, and `getAlbumInfo`/`getAlbumInfo2` carry the release id as their `musicBrainzId` element while remaining otherwise empty. Neither is a tag read from one file. Tracks of one album routinely disagree — a library assembled over years holds files tagged against different releases of the same record — so an album takes the identifier most of its available tracks agree on, recomputed at the end of every scan, with ties falling to the earliest disc and track so two scans of unchanged files answer the same thing; an artist takes it from the tracks it is the first credit of, because the tag is one value on a file that may credit several artists. Browsing entries are the exception: `getMusicDirectory` renders artists and albums as `child` elements, where the specification defines `musicBrainzId` as the recording id, so the field is dropped there rather than carrying a different identifier under that name. `replayGain` is the one addition whose *members* are omitted when unknown, on the specification's own instruction, while the container itself is always present because that is what reports the server reads gain tags at all. `moods` is multi-valued and split like the other joined tags; `explicitStatus` is normalised to the two words the specification defines, `explicit` and `clean`, rather than to the per-format spelling the tag used, and a tag saying "no advisory" maps to no value because it is not a claim that the work is clean. Media items additionally carry `albumArtists[]` and `displayAlbumArtist`, which are the album's credit rather than the track's: a guest appearance names the guest, while the album still belongs under the album artist. Albums carry `artists[]` and `genres[]`, derived from their available tracks rather than stored, because an album has no credit or genre of its own in the schema — only the union of its files'; genres are grouped on the canonical name, so an album spelling "Hip-Hop" on some tracks and "Hip Hop" on others reports one genre. What remains unimplemented is what the catalogue cannot answer: `contributors[]` and `displayComposer` need a composer the scanner does not read, and `AlbumID3`'s `moods[]`, `explicitStatus`, `originalReleaseDate`, `releaseDate`, `releaseTypes[]`, `recordLabels[]` and `discTitles[]`, like `ArtistID3`'s `roles[]`, need album and artist columns that do not exist. `sortName` on both is no longer among them: `album.sort_name` and `artist.sort_name` hold the tagged sort forms, read from `ALBUMSORT`, `ALBUMARTISTSORT` and `ARTISTSORT`. A joined credit pairs with its joined sort tag position by position, and a pair whose lengths disagree contributes no sort form at all rather than filing one artist under another's name; the pairing happens where the names are split, so the per-credit result is stored on `track_artist` and the album's on `track`. Neither is written onto the album or artist row by the per-track upsert. Both are re-derived at the end of every scan by `consolidate_sort_names`, a majority vote with a deterministic tie-break, beside the identifier vote and for the same reason: a value derived from tags must follow the tags, including when they are removed. Writing it during the upsert could not do that — the row is rewritten once per track of every album the artist appears on, so a file with no tag had to be stopped from erasing what a sibling supplied, and that preservation outlived the tag. An album artist credited on none of the album's tracks therefore carries no sort form of its own, because no tag in the catalogue is about it alone. Both are emitted with their default value like every other supported addition, so an untagged album reports `sortName=""` — supported and unknown — where before the field was absent. Those are absent rather than empty, which under the presence rule is the accurate statement that they are not supported. The columns behind the tag fields are added empty and filled by the next scan, so an instance that never rescans stays correct rather than wrong: it reports the fields supported and unset, which is what the presence rule means. Storing MusicBrainz identifiers is the dedicated data contract RFC-004 requires before its MBID branch can begin, and nothing more: a match on one remains a candidate the user confirms, never an automatic link. diff --git a/docs/subsonic-compatibility.md b/docs/subsonic-compatibility.md index 22177e5..2479ab2 100644 --- a/docs/subsonic-compatibility.md +++ b/docs/subsonic-compatibility.md @@ -92,4 +92,15 @@ none of them. No existing attribute changed value, so a client validated against the 2026-08-19 replay sees the same catalogue with two more fields on it and one directory that is no longer a dead end. +One deviation is worth stating rather than leaving to be discovered. The +`artists[]` and `albumArtists[]` arrays on media items and albums are typed as +`ArtistID3` but carry only `id` and `name`. They are references: no +`musicBrainzId`, no `albumCount`, no `starred`, and — since sort names landed — +no `sortName` either. A client reading an artist's own fields out of one of +those entries finds them missing and should fetch the artist by the identifier +the entry carries. The artist and album entries `getMusicDirectory` returns as +`child` are a different shape and do carry those fields, `musicBrainzId` +excepted; the two projections are pinned against each other in the test suite, +in JSON and in XML. + Browser-hosted clients need their exact origins in the comma-separated `WAVEFLOW_ALLOWED_ORIGINS` setting. The server permits GET, form POST and OPTIONS from those origins and exposes the byte-range response headers used by web audio players. Wildcard origins are deliberately unsupported. diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index c32b1e7..4375558 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -9067,3 +9067,148 @@ async fn the_catalogue_answers_for_sort_names_and_for_songs_without_an_album() { Some("") ); } + +/// Two artist projections that are deliberately not the same shape, pinned in +/// both encodings so neither drifts into the other. +/// +/// `artists[]` and `albumArtists[]` are *references*: an identifier and a +/// display name, and nothing else. The entries `getMusicDirectory` renders as +/// `child` are the artist and album nodes themselves, minus `musicBrainzId`, +/// so they do carry `sortName` and the rest. A field added to the node reaches +/// the second and must not leak into the first. +#[tokio::test] +async fn an_artist_reference_is_not_an_artist_record() { + let (_temp, config, state) = test_app().await; + let api_key = "wfsk_reference-key"; + let admin = state + .db + .create_account( + "reference-admin", + &security::hash_password("correct horse battery staple").unwrap(), + AccountRole::Admin, + now_ms(), + ) + .await + .unwrap(); + let encrypted = state.secret_box.encrypt(b"subsonic-secret-123").unwrap(); + state + .db + .set_subsonic_credential( + admin, + admin, + &encrypted, + &security::token_hash(api_key), + now_ms(), + ) + .await + .unwrap(); + let music = config.data_dir.join("reference-music"); + std::fs::create_dir_all(&music).unwrap(); + let root = std::fs::canonicalize(&music).unwrap(); + let library = state + .db + .create_library(admin, "Refs", &root, LibraryVisibility::Private, now_ms()) + .await + .unwrap(); + let scan = state + .db + .create_scan_job(library, Some(admin), "manual") + .await + .unwrap(); + state.db.start_scan_job(scan, 1).await.unwrap(); + let mut input = catalog_input(0, "The Nocturnes"); + input.title = "Opening".into(); + input.album = Some("The Night Sessions".into()); + input.album_artist = Some("The Nocturnes".into()); + input.is_compilation = false; + input.sort_album = Some("Night Sessions, The".into()); + input.sort_album_artist = Some("Nocturnes, The".into()); + input.sort_artist = Some("Nocturnes, The".into()); + state + .db + .apply_catalog_track(library, scan, &input, None, false) + .await + .unwrap(); + state.db.consolidate_sort_names(library).await.unwrap(); + state.db.finish_scan_job(scan, 0).await.unwrap(); + let router = waveflow_server::app(&config, state.clone()); + + // JSON: the album carries its own sortName; its `artists[]` entry carries + // an identifier and a name, and no third key. + let albums = subsonic_json( + &router, + "getAlbumList2", + api_key, + "&type=alphabeticalByName", + ) + .await; + let album = albums["subsonic-response"]["albumList2"]["album"][0].clone(); + assert_eq!(album["sortName"], serde_json::json!("Night Sessions, The")); + let reference = album["artists"][0] + .as_object() + .expect("an album lists its credited artists"); + let mut keys: Vec<&str> = reference.keys().map(String::as_str).collect(); + keys.sort_unstable(); + assert_eq!( + keys, + vec!["id", "name"], + "an artists[] entry is a reference, not an ArtistID3: {reference:?}" + ); + + // XML: the same statement, in the encoding where an absent attribute is + // absent rather than a missing key. + let directory = router + .clone() + .oneshot( + Request::get(format!( + "/rest/getMusicDirectory.view?apiKey={api_key}&v=1.16.1&c=fixtures&id={library}" + )) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(directory.status(), StatusCode::OK); + let directory = body_text(directory).await; + // The browsing child is the artist node minus musicBrainzId, so the sort + // name reaches it, carrying the tagged value. + assert!( + directory.contains(r#"sortName="Nocturnes, The""#), + "a getMusicDirectory child carries the artist's sortName: {directory}" + ); + // ...while the references carry neither that field nor any other one + // belonging to the record. They live on the album and on its songs, so + // this reads the album document rather than the folder listing — where a + // library holding no album-less track has no song child at all. + let album_xml = router + .clone() + .oneshot( + Request::get(format!( + "/rest/getAlbum.view?apiKey={api_key}&v=1.16.1&c=fixtures&id={}", + album["id"].as_str().expect("the album id") + )) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(album_xml.status(), StatusCode::OK); + let album_xml = body_text(album_xml).await; + let mut references = 0; + for element in album_xml.split("").next().unwrap_or_default(); + references += 1; + assert!( + !element.contains("sortName"), + "an artists[] reference must not grow a sortName: {element}" + ); + assert!( + !element.contains("albumCount"), + "nor any other ArtistID3 field: {element}" + ); + } + assert!( + references > 0, + "the fixture must exercise a reference: {album_xml}" + ); +} From 9a5f65ceb3a292c6b661c195675293f92c14096e Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Thu, 20 Aug 2026 13:11:39 +0200 Subject: [PATCH 5/6] test(subsonic): pin albumArtists[] as a reference, and name the absent fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review points, both valid. The RFC's closing sentence said "Those are absent rather than empty" of a list that had drifted several sentences away once the sortName paragraph was inserted in front of it, so its nearest antecedent had become sortName itself — which is emitted empty. It now names contributors[], displayComposer and the AlbumID3/ArtistID3 fields it means, and states the distinction outright: empty means the server reads the tag and this record has none, absent means it does not read it. albumArtists[] was documented as a reference and pinned by nothing. It is emitted on media items rather than on the album — the album's own credit is artists[] — so it is pinned on a song of the album document, in both encodings. Both collections now assert the whole attribute set equals {id, name} rather than rejecting a list of known fields, so a field added to the artist node fails here even if nobody thought to name it. Verified by adding sortName to the reference: the assertion reports it. The XML side splits attributes on the quote rather than on whitespace; the first attempt read name="The Nocturnes" as two attributes. Signed-off-by: InstaZDLL --- docs/rfcs/RFC-002-waveflow-server-v2.md | 2 +- tests/v2_foundations.rs | 77 +++++++++++++++++-------- 2 files changed, 54 insertions(+), 25 deletions(-) diff --git a/docs/rfcs/RFC-002-waveflow-server-v2.md b/docs/rfcs/RFC-002-waveflow-server-v2.md index 98a67c8..17932e1 100644 --- a/docs/rfcs/RFC-002-waveflow-server-v2.md +++ b/docs/rfcs/RFC-002-waveflow-server-v2.md @@ -52,7 +52,7 @@ Both `/rest/` and `/rest/.view` accept GET query parameters and Symfonium 14.1.0 performs a discovery request with the exact unauthenticated tuple `GET ping`, `c=Symfonium`, `u=test`, `p=test` after validating the configured account. WaveFlow returns only the standard successful `ping` envelope for that exact probe and does not create a principal or session. Duplicate identity parameters, alternate clients, POST requests, extra token authentication parameters and every method other than `ping` remain authenticated normally. -The response root freezes `status`, `version=1.16.1`, `type=waveflow`, `serverVersion` and `openSubsonic=true`. XML uses the Subsonic namespace. JSON collection fields are arrays even when they contain one item. Media items freeze the common fields `id`, `parent`, `isDir`, `title`, optional `album`/`artist`/`genre`/`year`/disc-track numbers, seconds-based `duration`, `bitRate`, `size`, `suffix`, `contentType`, `type=music`, optional `coverArt`/`albumId`, and ISO-8601 `created`. Album and artist records include UUID, display name, counts and available artwork/year metadata. Frozen 1.16 metadata is omitted when unknown; the OpenSubsonic additions follow the presence rule instead and are emitted with their default value, see *Deliberate deviations from the v2.0-beta freeze*. Media items carry `mediaType`, `isVideo`, `samplingRate`, `channelCount`, `bitDepth`, `playCount`, `played`, `displayArtist`, `artists[]`, `genres[]`, `musicBrainzId`, `bpm`, `sortName`, `comment`, `isrc[]` and `replayGain`; albums add `isCompilation`, `playCount`, `played` and `displayArtist`. `artists[]` is the credited list in tag order from `track_artist`, of which `artist`/`artistId` remain the display string and the primary credit; `genres[]` comes from `track_genre` ordered by name. Its entries, like those of `albumArtists[]`, are references rather than records: `id` and `name`, and nothing else — no `musicBrainzId`, no `albumCount`, no `starred`, no `sortName`. This is a deliberate deviation from `ArtistID3`, which those arrays are typed as. A client that wants an artist's own fields asks for the artist, which is what the identifier in the reference is for; inlining a second copy of every record into every track and album payload is what the reference exists to avoid. The entries `getMusicDirectory` renders as `child` are a different projection and are not affected: they are the artist and album nodes themselves with `musicBrainzId` removed, so they do carry `sortName` and the rest. The two are pinned against each other in `an_artist_reference_is_not_an_artist_record`, in both encodings, so a field added to the node cannot leak into the reference. `musicBrainzId` on a media item is the MusicBrainz **recording** identifier, the performance; the release and artist identifiers are never exposed at track level, where they would name a different entity. They are exposed on the entities they belong to instead: `album.musicBrainzId` is the release and `artist.musicBrainzId` is the artist, both under the presence rule, and `getAlbumInfo`/`getAlbumInfo2` carry the release id as their `musicBrainzId` element while remaining otherwise empty. Neither is a tag read from one file. Tracks of one album routinely disagree — a library assembled over years holds files tagged against different releases of the same record — so an album takes the identifier most of its available tracks agree on, recomputed at the end of every scan, with ties falling to the earliest disc and track so two scans of unchanged files answer the same thing; an artist takes it from the tracks it is the first credit of, because the tag is one value on a file that may credit several artists. Browsing entries are the exception: `getMusicDirectory` renders artists and albums as `child` elements, where the specification defines `musicBrainzId` as the recording id, so the field is dropped there rather than carrying a different identifier under that name. `replayGain` is the one addition whose *members* are omitted when unknown, on the specification's own instruction, while the container itself is always present because that is what reports the server reads gain tags at all. `moods` is multi-valued and split like the other joined tags; `explicitStatus` is normalised to the two words the specification defines, `explicit` and `clean`, rather than to the per-format spelling the tag used, and a tag saying "no advisory" maps to no value because it is not a claim that the work is clean. Media items additionally carry `albumArtists[]` and `displayAlbumArtist`, which are the album's credit rather than the track's: a guest appearance names the guest, while the album still belongs under the album artist. Albums carry `artists[]` and `genres[]`, derived from their available tracks rather than stored, because an album has no credit or genre of its own in the schema — only the union of its files'; genres are grouped on the canonical name, so an album spelling "Hip-Hop" on some tracks and "Hip Hop" on others reports one genre. What remains unimplemented is what the catalogue cannot answer: `contributors[]` and `displayComposer` need a composer the scanner does not read, and `AlbumID3`'s `moods[]`, `explicitStatus`, `originalReleaseDate`, `releaseDate`, `releaseTypes[]`, `recordLabels[]` and `discTitles[]`, like `ArtistID3`'s `roles[]`, need album and artist columns that do not exist. `sortName` on both is no longer among them: `album.sort_name` and `artist.sort_name` hold the tagged sort forms, read from `ALBUMSORT`, `ALBUMARTISTSORT` and `ARTISTSORT`. A joined credit pairs with its joined sort tag position by position, and a pair whose lengths disagree contributes no sort form at all rather than filing one artist under another's name; the pairing happens where the names are split, so the per-credit result is stored on `track_artist` and the album's on `track`. Neither is written onto the album or artist row by the per-track upsert. Both are re-derived at the end of every scan by `consolidate_sort_names`, a majority vote with a deterministic tie-break, beside the identifier vote and for the same reason: a value derived from tags must follow the tags, including when they are removed. Writing it during the upsert could not do that — the row is rewritten once per track of every album the artist appears on, so a file with no tag had to be stopped from erasing what a sibling supplied, and that preservation outlived the tag. An album artist credited on none of the album's tracks therefore carries no sort form of its own, because no tag in the catalogue is about it alone. Both are emitted with their default value like every other supported addition, so an untagged album reports `sortName=""` — supported and unknown — where before the field was absent. Those are absent rather than empty, which under the presence rule is the accurate statement that they are not supported. +The response root freezes `status`, `version=1.16.1`, `type=waveflow`, `serverVersion` and `openSubsonic=true`. XML uses the Subsonic namespace. JSON collection fields are arrays even when they contain one item. Media items freeze the common fields `id`, `parent`, `isDir`, `title`, optional `album`/`artist`/`genre`/`year`/disc-track numbers, seconds-based `duration`, `bitRate`, `size`, `suffix`, `contentType`, `type=music`, optional `coverArt`/`albumId`, and ISO-8601 `created`. Album and artist records include UUID, display name, counts and available artwork/year metadata. Frozen 1.16 metadata is omitted when unknown; the OpenSubsonic additions follow the presence rule instead and are emitted with their default value, see *Deliberate deviations from the v2.0-beta freeze*. Media items carry `mediaType`, `isVideo`, `samplingRate`, `channelCount`, `bitDepth`, `playCount`, `played`, `displayArtist`, `artists[]`, `genres[]`, `musicBrainzId`, `bpm`, `sortName`, `comment`, `isrc[]` and `replayGain`; albums add `isCompilation`, `playCount`, `played` and `displayArtist`. `artists[]` is the credited list in tag order from `track_artist`, of which `artist`/`artistId` remain the display string and the primary credit; `genres[]` comes from `track_genre` ordered by name. Its entries, like those of `albumArtists[]`, are references rather than records: `id` and `name`, and nothing else — no `musicBrainzId`, no `albumCount`, no `starred`, no `sortName`. This is a deliberate deviation from `ArtistID3`, which those arrays are typed as. A client that wants an artist's own fields asks for the artist, which is what the identifier in the reference is for; inlining a second copy of every record into every track and album payload is what the reference exists to avoid. The entries `getMusicDirectory` renders as `child` are a different projection and are not affected: they are the artist and album nodes themselves with `musicBrainzId` removed, so they do carry `sortName` and the rest. The two are pinned against each other in `an_artist_reference_is_not_an_artist_record`, in both encodings, so a field added to the node cannot leak into the reference. `musicBrainzId` on a media item is the MusicBrainz **recording** identifier, the performance; the release and artist identifiers are never exposed at track level, where they would name a different entity. They are exposed on the entities they belong to instead: `album.musicBrainzId` is the release and `artist.musicBrainzId` is the artist, both under the presence rule, and `getAlbumInfo`/`getAlbumInfo2` carry the release id as their `musicBrainzId` element while remaining otherwise empty. Neither is a tag read from one file. Tracks of one album routinely disagree — a library assembled over years holds files tagged against different releases of the same record — so an album takes the identifier most of its available tracks agree on, recomputed at the end of every scan, with ties falling to the earliest disc and track so two scans of unchanged files answer the same thing; an artist takes it from the tracks it is the first credit of, because the tag is one value on a file that may credit several artists. Browsing entries are the exception: `getMusicDirectory` renders artists and albums as `child` elements, where the specification defines `musicBrainzId` as the recording id, so the field is dropped there rather than carrying a different identifier under that name. `replayGain` is the one addition whose *members* are omitted when unknown, on the specification's own instruction, while the container itself is always present because that is what reports the server reads gain tags at all. `moods` is multi-valued and split like the other joined tags; `explicitStatus` is normalised to the two words the specification defines, `explicit` and `clean`, rather than to the per-format spelling the tag used, and a tag saying "no advisory" maps to no value because it is not a claim that the work is clean. Media items additionally carry `albumArtists[]` and `displayAlbumArtist`, which are the album's credit rather than the track's: a guest appearance names the guest, while the album still belongs under the album artist. Albums carry `artists[]` and `genres[]`, derived from their available tracks rather than stored, because an album has no credit or genre of its own in the schema — only the union of its files'; genres are grouped on the canonical name, so an album spelling "Hip-Hop" on some tracks and "Hip Hop" on others reports one genre. What remains unimplemented is what the catalogue cannot answer: `contributors[]` and `displayComposer` need a composer the scanner does not read, and `AlbumID3`'s `moods[]`, `explicitStatus`, `originalReleaseDate`, `releaseDate`, `releaseTypes[]`, `recordLabels[]` and `discTitles[]`, like `ArtistID3`'s `roles[]`, need album and artist columns that do not exist. `sortName` on both is no longer among them: `album.sort_name` and `artist.sort_name` hold the tagged sort forms, read from `ALBUMSORT`, `ALBUMARTISTSORT` and `ARTISTSORT`. A joined credit pairs with its joined sort tag position by position, and a pair whose lengths disagree contributes no sort form at all rather than filing one artist under another's name; the pairing happens where the names are split, so the per-credit result is stored on `track_artist` and the album's on `track`. Neither is written onto the album or artist row by the per-track upsert. Both are re-derived at the end of every scan by `consolidate_sort_names`, a majority vote with a deterministic tie-break, beside the identifier vote and for the same reason: a value derived from tags must follow the tags, including when they are removed. Writing it during the upsert could not do that — the row is rewritten once per track of every album the artist appears on, so a file with no tag had to be stopped from erasing what a sibling supplied, and that preservation outlived the tag. An album artist credited on none of the album's tracks therefore carries no sort form of its own, because no tag in the catalogue is about it alone. Both are emitted with their default value like every other supported addition, so an untagged album reports `sortName=""` — supported and unknown — where before the field was absent. The fields still unimplemented behave the other way round: `contributors[]`, `displayComposer`, `AlbumID3`'s `moods[]`, `explicitStatus`, `originalReleaseDate`, `releaseDate`, `releaseTypes[]`, `recordLabels[]` and `discTitles[]`, and `ArtistID3`'s `roles[]` are absent rather than empty, which under the presence rule is the accurate statement that they are not supported. Empty means the server reads the tag and this record has none; absent means the server does not read it. The columns behind the tag fields are added empty and filled by the next scan, so an instance that never rescans stays correct rather than wrong: it reports the fields supported and unset, which is what the presence rule means. Storing MusicBrainz identifiers is the dedicated data contract RFC-004 requires before its MBID branch can begin, and nothing more: a match on one remains a candidate the user confirms, never an automatic link. diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index 4375558..287454f 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -9144,16 +9144,34 @@ async fn an_artist_reference_is_not_an_artist_record() { .await; let album = albums["subsonic-response"]["albumList2"]["album"][0].clone(); assert_eq!(album["sortName"], serde_json::json!("Night Sessions, The")); - let reference = album["artists"][0] - .as_object() - .expect("an album lists its credited artists"); - let mut keys: Vec<&str> = reference.keys().map(String::as_str).collect(); - keys.sort_unstable(); - assert_eq!( - keys, - vec!["id", "name"], - "an artists[] entry is a reference, not an ArtistID3: {reference:?}" - ); + let reference_keys = |value: &serde_json::Value, what: &str| { + let entry = value + .as_object() + .unwrap_or_else(|| panic!("{what} is an object: {value}")) + .clone(); + let mut keys: Vec = entry.keys().cloned().collect(); + keys.sort_unstable(); + assert_eq!( + keys, + vec!["id".to_owned(), "name".to_owned()], + "{what} is a reference, not an ArtistID3: {entry:?}" + ); + }; + reference_keys(&album["artists"][0], "an album's artists[] entry"); + + // `albumArtists[]` is a reference too, and it is emitted on media items + // rather than on the album — the album's own credit is `artists[]`. So it + // is pinned on a song of the album. + let album_json = subsonic_json( + &router, + "getAlbum", + api_key, + &format!("&id={}", album["id"].as_str().expect("the album id")), + ) + .await; + let song = album_json["subsonic-response"]["album"]["song"][0].clone(); + reference_keys(&song["artists"][0], "a song's artists[] entry"); + reference_keys(&song["albumArtists"][0], "a song's albumArtists[] entry"); // XML: the same statement, in the encoding where an absent attribute is // absent rather than a missing key. @@ -9194,21 +9212,32 @@ async fn an_artist_reference_is_not_an_artist_record() { .unwrap(); assert_eq!(album_xml.status(), StatusCode::OK); let album_xml = body_text(album_xml).await; - let mut references = 0; - for element in album_xml.split("").next().unwrap_or_default(); - references += 1; - assert!( - !element.contains("sortName"), - "an artists[] reference must not grow a sortName: {element}" - ); + for name in ["").next().unwrap_or_default(); + references += 1; + // The whitelist, rather than a list of fields to reject: a field + // added to the artist node has to fail here even if nobody thought + // to name it. + // Split on the quote rather than on whitespace: an attribute + // value holds spaces, and `name="The Nocturnes"` would otherwise + // read as two attributes. + let attributes: Vec = element + .split('"') + .step_by(2) + .map(|key| key.trim().trim_end_matches('=').trim().to_owned()) + .filter(|key| !key.is_empty()) + .collect(); + assert_eq!( + attributes, + vec!["id".to_owned(), "name".to_owned()], + "{name} is a reference, not an ArtistID3: {element}" + ); + } assert!( - !element.contains("albumCount"), - "nor any other ArtistID3 field: {element}" + references > 0, + "the fixture must exercise {name}: {album_xml}" ); } - assert!( - references > 0, - "the fixture must exercise a reference: {album_xml}" - ); } From e849e7576486ffebdd90f2427f92c02214639319 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Thu, 20 Aug 2026 13:19:59 +0200 Subject: [PATCH 6/6] test(subsonic): sort XML attributes before comparing the reference shape The JSON side already sorted its keys; the XML side compared them in document order, so it pinned the order the node builder happens to write id and name in. XML attribute order carries no meaning, and a reordering that changes nothing observable would have failed the test. Verified both ways: emitting name before id now passes, and adding albumCount alongside them still fails. Which attributes are present is the contract; the order is not. Signed-off-by: InstaZDLL --- tests/v2_foundations.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/v2_foundations.rs b/tests/v2_foundations.rs index 287454f..0352d61 100644 --- a/tests/v2_foundations.rs +++ b/tests/v2_foundations.rs @@ -9223,12 +9223,17 @@ async fn an_artist_reference_is_not_an_artist_record() { // Split on the quote rather than on whitespace: an attribute // value holds spaces, and `name="The Nocturnes"` would otherwise // read as two attributes. - let attributes: Vec = element + let mut attributes: Vec = element .split('"') .step_by(2) .map(|key| key.trim().trim_end_matches('=').trim().to_owned()) .filter(|key| !key.is_empty()) .collect(); + // Sorted before comparing, like the JSON side: which attributes + // are present is the contract, the order they are written in is + // not, and pinning it would fail a reordering that changes + // nothing observable. + attributes.sort_unstable(); assert_eq!( attributes, vec!["id".to_owned(), "name".to_owned()],