diff --git a/docs/rfcs/RFC-002-waveflow-server-v2.md b/docs/rfcs/RFC-002-waveflow-server-v2.md index 44b768d..6910bb5 100644 --- a/docs/rfcs/RFC-002-waveflow-server-v2.md +++ b/docs/rfcs/RFC-002-waveflow-server-v2.md @@ -58,7 +58,7 @@ The columns behind the tag fields are added empty and filled by the next scan, s Pagination is capped at 500 items per page. `offset` is zero-based; `search3` applies its independent `artistOffset`, `albumOffset` and `songOffset` after tenant filtering. Repeated `songId`, `songIdToAdd`, `songIndexToRemove`, scrobble `id`/`time`, queue `id`, star IDs, share IDs and `musicFolderId` values retain request order. Catalogue endpoints accept the union of repeated authorized `musicFolderId` values and return no foreign data for inaccessible IDs. `createUser` grants every current library when the parameter is absent or exactly the repeated selection when present; `updateUser` replaces Subsonic-managed listener memberships only, preserving owner/manager roles. `getUser` and `getUsers` expose the effective UUID list as `folder[]`. Default catalogue order is case-insensitive display name through the SQLite `NOCASE` collation, which folds ASCII; indexes group by uppercase ASCII initial with `#` fallback. Album lists implement `random`, `newest`, `highest`, `frequent`, `recent`, `starred`, both alphabetical modes, `byYear` (including reversed ranges) and `byGenre`; non-random ties use stable title/UUID ordering. All ten resolve in `DomainServices::list_albums` and are ordered, filtered and paged in SQL. `byGenre` matches on the canonical genre name — case, punctuation and spacing folded — rather than on a raw display string, so one genre is never split in two; it requires `genre` and is rejected with error code 10 without it, rather than answering with an unfiltered catalogue. A `size` of zero is an empty container, not an error. Random-song year and genre filters are applied after repository authorization. Playlist positions are contiguous and removals are applied from highest index downward before additions. The legacy `getAlbumList` method remains an alias for older clients and uses the `albumList` response container; `getAlbumList2` uses `albumList2`. `star` and `unstar` accept the generic `id` form for tracks, albums and artists in addition to the typed album/artist parameters; resolution remains tenant-scoped and rejects ambiguous or invisible entities. -For full-catalogue pagination, `search3` treats the literal query `""` as match-all, as used by Symfonium. There is nothing to match and FTS5 has no expression meaning "everything", so that request is three ordinary listings wearing the search response, each ordered and paged in SQL through `DomainServices::browse_all`. It previously read the whole visible catalogue and sliced it in Rust once per page, which made a client's initial synchronization quadratic in the library. `getBookmarks`, `createBookmark` and `deleteBookmark` are backed by a real `bookmark` row per account and track: a bookmark answers "where did I stop in this file", so setting one twice moves it rather than adding a second, and omitting the comment clears it. Deleting one that is not there succeeds — the caller asked for the track to carry no bookmark, and it does not — which also avoids answering a question about another account's catalogue. Bookmarks are a domain mutation like every other piece of user data, so they claim a sync operation, appear in the journal under the `bookmark` entity type and ship in the bootstrap snapshot; the journal's `entity_type` vocabulary is CHECK-constrained, so admitting them was a deliberate contract change rather than a side effect. The bookmarked entry is rendered through the shared media projection and carries `bookmarkPosition`, which no other media item does because a track only has a position inside the bookmark holding it. `getSimilarSongs` and `getSimilarSongs2` answer the same way, for the same reason: WaveFlow computes no recommendations, and an empty standard container is the honest answer where a not-implemented error would read to a client as a broken server on a page it opens by default. `getInternetRadioStations` used to be in that company and no longer is. It reads a `radio_station` row per account, and `createInternetRadioStation`, `updateInternetRadioStation` and `deleteInternetRadioStation` write them, each answering the bare envelope the specification documents. **The server never fetches a station's address**: it validates it as absolute `http` or `https`, stores it as typed and hands it to whichever client asked. That is the whole of the security question the feature raises — a station is a URL a user supplies, so a server that dereferenced one would be a request forger aimed at whatever it can reach — and it is also why the feature needs no allowlist, no timeout and no egress policy. Two deviations from the reference server are deliberate. The list is **per account**, where the reference server keeps one global list only an administrator may change: WaveFlow has no global library setting of any kind, and a per-account list needs no new privilege to be safe, so a station belonging to somebody else is 404 like everything else here. And `homePageUrl` is spelled with a capital `P` on the way out while the parameter that sets it is `homepageUrl` — the specification's own asymmetry, copied rather than corrected, because a client following the document reads the field it asked for. Filling the container distinguished it from an object in JSON, exactly as `getTopSongs` had been: `internetRadioStations.internetRadioStation` is an array whatever its length. Stations are account configuration rather than playback state, so they sit beside the API tokens and the scrobbling links and **are not in the sync journal** — admitting a new `entity_type` is a contract change the desktop client has to be ready for, and nothing has asked for it. `getTopSongs` no longer answers that way. It returns the artist's most played tracks as counted from the calling account's own `play_event` rows, which is a narrower question than the reference server's — it asks a remote service what is popular, and this reports what was played here — and the only one the catalogue can answer. `artist` is a name, so it is matched on `artist.canonical_name`, the folding rule every other catalogue match uses, and a name that resolves in two libraries contributes both; an unknown name is an empty container rather than error 70, because a name is not an identifier and there is no existence to blur. A missing `artist` stays error 10. `count` defaults to the specification's fifty, is capped at five hundred, and `count=0` is an empty container as it is for `getAlbumList`. A track the account has never played is absent rather than ranked last, so the list is a history and not a discography in disguise. Filling the container is also what first distinguished it from an object in JSON: `topSongs.song` is now an array whatever its length, like every other collection field. `getAvatar` answers error code 70 rather than code 0, because no avatars are stored: the data is missing, not the method. `search2` and `getStarred` are container renames of `search3` and `getStarred2`, as `getAlbumList` is of `getAlbumList2`. +For full-catalogue pagination, `search3` treats the literal query `""` as match-all, as used by Symfonium. There is nothing to match and FTS5 has no expression meaning "everything", so that request is three ordinary listings wearing the search response, each ordered and paged in SQL through `DomainServices::browse_all`. It previously read the whole visible catalogue and sliced it in Rust once per page, which made a client's initial synchronization quadratic in the library. `getBookmarks`, `createBookmark` and `deleteBookmark` are backed by a real `bookmark` row per account and track: a bookmark answers "where did I stop in this file", so setting one twice moves it rather than adding a second, and omitting the comment clears it. Deleting one that is not there succeeds — the caller asked for the track to carry no bookmark, and it does not — which also avoids answering a question about another account's catalogue. Bookmarks are a domain mutation like every other piece of user data, so they claim a sync operation, appear in the journal under the `bookmark` entity type and ship in the bootstrap snapshot; the journal's `entity_type` vocabulary is CHECK-constrained, so admitting them was a deliberate contract change rather than a side effect. The bookmarked entry is rendered through the shared media projection and carries `bookmarkPosition`, which no other media item does because a track only has a position inside the bookmark holding it. `getSimilarSongs` and `getSimilarSongs2` used to answer the same way and no longer do once an operator configures a metadata provider ([RFC-011](RFC-011-metadata-enrichment.md)): they resolve the artist, album or song id first and for its refusal, an album standing for its album artist and a song for its track artist, and answer a random draw of at most `count` (default 50) from that artist's tracks and their neighbours' in the libraries the caller belongs to. With no provider configured, which is the default, they answer the standard empty container they always did, for any id the caller can reach. `getInternetRadioStations` used to be in that company and no longer is. It reads a `radio_station` row per account, and `createInternetRadioStation`, `updateInternetRadioStation` and `deleteInternetRadioStation` write them, each answering the bare envelope the specification documents. **The server never fetches a station's address**: it validates it as absolute `http` or `https`, stores it as typed and hands it to whichever client asked. That is the whole of the security question the feature raises — a station is a URL a user supplies, so a server that dereferenced one would be a request forger aimed at whatever it can reach — and it is also why the feature needs no allowlist, no timeout and no egress policy. Two deviations from the reference server are deliberate. The list is **per account**, where the reference server keeps one global list only an administrator may change: WaveFlow has no global library setting of any kind, and a per-account list needs no new privilege to be safe, so a station belonging to somebody else is 404 like everything else here. And `homePageUrl` is spelled with a capital `P` on the way out while the parameter that sets it is `homepageUrl` — the specification's own asymmetry, copied rather than corrected, because a client following the document reads the field it asked for. Filling the container distinguished it from an object in JSON, exactly as `getTopSongs` had been: `internetRadioStations.internetRadioStation` is an array whatever its length. Stations are account configuration rather than playback state, so they sit beside the API tokens and the scrobbling links and **are not in the sync journal** — admitting a new `entity_type` is a contract change the desktop client has to be ready for, and nothing has asked for it. `getTopSongs` no longer answers that way. It returns the artist's most played tracks as counted from the calling account's own `play_event` rows, which is a narrower question than the reference server's — it asks a remote service what is popular, and this reports what was played here — and the only one the catalogue can answer. `artist` is a name, so it is matched on `artist.canonical_name`, the folding rule every other catalogue match uses, and a name that resolves in two libraries contributes both; an unknown name is an empty container rather than error 70, because a name is not an identifier and there is no existence to blur. A missing `artist` stays error 10. `count` defaults to the specification's fifty, is capped at five hundred, and `count=0` is an empty container as it is for `getAlbumList`. A track the account has never played is absent rather than ranked last, so the list is a history and not a discography in disguise. Filling the container is also what first distinguished it from an object in JSON: `topSongs.song` is now an array whatever its length, like every other collection field. `getAvatar` answers error code 70 rather than code 0, because no avatars are stored: the data is missing, not the method. `search2` and `getStarred` are container renames of `search3` and `getStarred2`, as `getAlbumList` is of `getAlbumList2`. `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. diff --git a/docs/rfcs/RFC-011-metadata-enrichment.md b/docs/rfcs/RFC-011-metadata-enrichment.md index 1118dd9..3d18a94 100644 --- a/docs/rfcs/RFC-011-metadata-enrichment.md +++ b/docs/rfcs/RFC-011-metadata-enrichment.md @@ -5,9 +5,9 @@ [the implementation plan](../metadata-enrichment-plan.md): InstaZDLL/WaveFlow#855 (the core's clients accept a client, an address and a key), #270 (the pin moved onto them), #271 (the cache and the engine that - fills it), and the pull request closing #266 (`getArtistInfo`, - `getArtistInfo2` and the native artist page read it). Similar songs and - portraits are still to come. The *Status* field above never flips in this + fills it), #272 (`getArtistInfo`, `getArtistInfo2` and the native artist + page read it), and the pull request closing #267 (`getSimilarSongs` and + `getSimilarSongs2` draw from it). Portraits are still to come. The *Status* field above never flips in this project; this line is the one that says what is running. - **Date**: 2026-10-14 - **Revised**: 2026-10-15, while implementing the cache (#265). Decisions 4, 6 diff --git a/docs/subsonic-api-guide.md b/docs/subsonic-api-guide.md index c5a7415..f4cf2e5 100644 --- a/docs/subsonic-api-guide.md +++ b/docs/subsonic-api-guide.md @@ -241,7 +241,7 @@ with Subsonic error code `0`, like every other protocol failure. | Library maintenance | `startScan`, `getScanStatus` | Rescans every library the account owns or manages and reports progress. | | Bookmarks | `getBookmarks`, `createBookmark`, `deleteBookmark` | One position per account and track; setting it again moves it. | | Internet radio | `getInternetRadioStations`, `createInternetRadioStation`, `updateInternetRadioStation`, `deleteInternetRadioStation` | One list **per account**, where the reference server keeps a global one. The three writes answer the bare envelope. `homePageUrl` is returned with a capital `P` and set with `homepageUrl`, as the specification spells them. An address must be absolute `http` or `https`; the server stores it and never connects to it. `homepageUrl=` with no value clears the page rather than failing. | -| Compatibility | `getTopSongs`, `getSimilarSongs`, `getSimilarSongs2` | `getTopSongs` answers the artist's most played tracks from this account's history. The other two return the standard empty container: WaveFlow computes no recommendations. | +| Compatibility | `getTopSongs`, `getSimilarSongs`, `getSimilarSongs2` | `getTopSongs` answers the artist's most played tracks from this account's history. The other two take an artist, album or song id, resolved first (error 70 when the caller cannot reach it), and answer a random draw of at most `count` (default 50) from the artist's tracks and their visible neighbours', read from the metadata cache. The standard empty container when no provider is configured, which is the default. | | Missing data | `getAvatar` | No avatars are stored, so it answers error code 70. | ### Genres diff --git a/docs/subsonic-compatibility.md b/docs/subsonic-compatibility.md index 71538c0..f034283 100644 --- a/docs/subsonic-compatibility.md +++ b/docs/subsonic-compatibility.md @@ -31,7 +31,9 @@ Automated protocol coverage is enforced by the `subsonic_contract`, `subsonic_br > Feishin row below records it as empty, which it was, and #215 has since filled > it from local play history — so that observation has stopped being true the > same way `getNowPlaying`'s child name did. `getArtistInfo`, -> `getSimilarSongs` and `getSimilarSongs2` are unchanged and still empty. +> `getSimilarSongs` and `getSimilarSongs2` stay empty by default, and fill +> from the metadata cache once an operator configures a provider (RFC-011, +> #272 and #267) — no client in this table has seen them do so yet. > **Two things to re-check in the next campaign**, neither of which these runs > can answer: whether the clients that call `getTopSongs` render what they are > now given, and that `topSongs.song` decodes as a list — it had never carried diff --git a/src/services/artist_enrichment.rs b/src/services/artist_enrichment.rs index 34d39ac..aa31a02 100644 --- a/src/services/artist_enrichment.rs +++ b/src/services/artist_enrichment.rs @@ -66,19 +66,9 @@ impl DomainServices { } let rows = engine.artist(name).await?; let canonical = waveflow_core::scanner::canonical_name(name); - let mut similar = None; - for row in rows.iter().filter(|row| row.outcome == Outcome::Hit) { - let artists = self - .visible_similar(user_id, row.provider, &canonical, similar_limit) - .await?; - if !artists.is_empty() { - similar = Some(EnrichedSimilar { - artists, - source: source_of(row), - }); - break; - } - } + let similar = self + .neighbours(user_id, &rows, &canonical, similar_limit) + .await?; let bio = merged_bio(&rows); // Only a fresh row is an answer. An expired one has just had its // refresh scheduled, and an expired miss is not "nobody knows" but @@ -98,6 +88,147 @@ impl DomainServices { }) } + /// The neighbours from the first provider, in the operator's order, whose + /// list names an artist the caller can see. + async fn neighbours( + &self, + user_id: Uuid, + rows: &[CachedArtist], + canonical: &str, + limit: usize, + ) -> Result, ServiceError> { + for row in rows.iter().filter(|row| row.outcome == Outcome::Hit) { + let artists = self + .visible_similar(user_id, row.provider, canonical, limit) + .await?; + if !artists.is_empty() { + return Ok(Some(EnrichedSimilar { + artists, + source: source_of(row), + })); + } + } + Ok(None) + } + + /// `getSimilarSongs` and `getSimilarSongs2`: a random draw of at most + /// `limit` tracks from the artist behind `id` and from their neighbours. + /// + /// `id` names an artist, an album or a track; an album stands for its + /// album artist and a track for its track artist. It is resolved first and + /// for its refusal, so an id the caller cannot reach answers as one naming + /// nothing, before the cache is read. An album or a track credited to no + /// artist has no one to be similar to, and draws nothing. + /// + /// **The artist is in the draw**, not the neighbours alone: the + /// specification's "songs from the given artist and similar artists". + /// Every name draws through [`artist_credit_clause!`] by canonical name, + /// so an artist the caller sees in two libraries contributes both, and + /// through `song_select!`, whose membership bind is what keeps a track in + /// a library the caller is not in out of the draw. + /// + /// With enrichment disabled — the default — the answer is the empty + /// container these methods always gave, rather than a draw from the + /// artist alone: no operator switched anything on, so nothing on the wire + /// moves. Enabled but not yet answered, the artist alone is drawn. + pub async fn similar_songs( + &self, + user_id: Uuid, + id: Uuid, + limit: i64, + ) -> Result, ServiceError> { + if !(0..=MAX_BROWSE_LIMIT).contains(&limit) { + return Err(ServiceError::Invalid); + } + let Some(artist_id) = self.artist_behind(user_id, id).await? else { + return Ok(Vec::new()); + }; + let artist = self.artist_summary(user_id, artist_id).await?; + let engine = self + .enrichment + .read() + .unwrap_or_else(|poison| poison.into_inner()) + .clone(); + if limit == 0 || !engine.is_enabled() { + return Ok(Vec::new()); + } + let canonical = waveflow_core::scanner::canonical_name(&artist.artist.name); + let rows = engine.artist(&artist.artist.name).await?; + let mut names = vec![canonical.clone()]; + if let Some(similar) = self + .neighbours(user_id, &rows, &canonical, usize::MAX) + .await? + { + names.extend( + similar.artists.iter().map(|neighbour| { + waveflow_core::scanner::canonical_name(&neighbour.artist.name) + }), + ); + } + let names = serde_json::to_string(&names).map_err(|_| ServiceError::Invalid)?; + // No library narrowing: every library the caller reaches. + let folders = folder_filter(&[]); + let mut connection = self.db.pool().acquire().await?; + let mut songs = sqlx::query(concat!( + song_select!(), + song_folder_clause!(), + artist_credit_clause!( + "SELECT a.id FROM artist a \ + WHERE a.canonical_name IN (SELECT value FROM json_each(?))" + ), + " ORDER BY RANDOM() LIMIT ?" + )) + .bind(user_id.to_string()) + .bind(folders.as_deref()) + .bind(folders.as_deref()) + .bind(&names) + .bind(&names) + .bind(limit) + .fetch_all(&mut *connection) + .await? + .into_iter() + .map(song_from_row) + .collect::, _>>()?; + attach_song_relations(&mut connection, user_id, &mut songs).await?; + Ok(songs) + } + + /// The artist an artist, album or track id stands for, if the caller can + /// reach it: `NotFound` when nothing reachable has that id, `None` when an + /// album or a track has it but credits no artist. + async fn artist_behind(&self, user_id: Uuid, id: Uuid) -> Result, ServiceError> { + match self.artist_summary(user_id, id).await { + Err(ServiceError::NotFound) => {} + found => return found.map(|_| Some(id)), + } + for query in [ + "SELECT al.album_artist_id FROM album al \ + JOIN library_member m ON m.library_id=al.library_id \ + WHERE m.user_id=? AND al.id=?", + "SELECT (SELECT tp.artist_id FROM track_participant tp \ + WHERE tp.track_id=t.id AND tp.role='artist' \ + ORDER BY tp.position LIMIT 1) \ + FROM track t JOIN library_member m ON m.library_id=t.library_id \ + WHERE m.user_id=? AND t.id=?", + ] { + let found = sqlx::query_scalar::<_, Option>(query) + .bind(user_id.to_string()) + .bind(id.to_string()) + .fetch_optional(self.db.pool()) + .await?; + if let Some(artist) = found { + return artist + .map(|artist| { + Uuid::parse_str(&artist).map_err(|error| { + ServiceError::Database(sqlx::Error::Decode(error.into())) + }) + }) + .transpose(); + } + } + Err(ServiceError::NotFound) + } + /// One provider's cached neighbours for `canonical`, resolved to artists /// `user_id` can see, best first, at most `limit`. /// diff --git a/src/services/artists.rs b/src/services/artists.rs index 7a6ef58..859a5a8 100644 --- a/src/services/artists.rs +++ b/src/services/artists.rs @@ -158,15 +158,9 @@ impl DomainServices { .await } - /// What both forms of the question share, so the definition of "this - /// artist's tracks" exists once. - /// - /// Two credits count, and they are the two an artist page shows: a track - /// that names the artist in its own `artist` role, and a track whose album - /// is credited to the artist as album artist. The second is read from - /// `album_participant`, the same relation [`Self::artist`] lists the albums - /// from — a classical record whose files credit performers per track would - /// otherwise have a page full of albums and a most-played list of nothing. + /// What both forms of the question share. Which tracks are "this + /// artist's" is [`artist_credit_clause!`], shared with the similar-songs + /// draw so the definition exists once. async fn top_songs_on( &self, connection: &mut SqliteConnection, @@ -197,12 +191,7 @@ impl DomainServices { "SELECT * FROM (", song_select!(), song_folder_clause!(), - " AND (EXISTS (SELECT 1 FROM track_participant tp \ - WHERE tp.track_id=t.id AND tp.role='artist' \ - AND tp.artist_id IN (SELECT value FROM json_each(?))) \ - OR EXISTS (SELECT 1 FROM album_participant ap \ - WHERE ap.album_id=t.album_id AND ap.role='albumartist' \ - AND ap.artist_id IN (SELECT value FROM json_each(?))))", + artist_credit_clause!("SELECT value FROM json_each(?)"), ") AS s WHERE play_count > 0 \ ORDER BY play_count DESC, title COLLATE NOCASE, id LIMIT ?" )) diff --git a/src/services/mod.rs b/src/services/mod.rs index 2a3631c..01d5bd3 100644 --- a/src/services/mod.rs +++ b/src/services/mod.rs @@ -83,6 +83,32 @@ macro_rules! song_year_clause { }; } +/// Restricts [`song_select!`] to an artist's tracks, given a subquery naming +/// the artist ids; binds whatever that subquery binds, twice. +/// +/// Two credits count, and they are the two an artist page shows: a track that +/// names the artist in its own `artist` role, and a track whose album is +/// credited to the artist as album artist. The second is read from +/// `album_participant`, the same relation `DomainServices::artist` lists the +/// albums from — a classical record whose files credit performers per track +/// would otherwise have a page full of albums and no tracks. +macro_rules! artist_credit_clause { + ($artist_ids:literal) => { + concat!( + " AND (EXISTS (SELECT 1 FROM track_participant tp \ + WHERE tp.track_id=t.id AND tp.role='artist' \ + AND tp.artist_id IN (", + $artist_ids, + ")) \ + OR EXISTS (SELECT 1 FROM album_participant ap \ + WHERE ap.album_id=t.album_id AND ap.role='albumartist' \ + AND ap.artist_id IN (", + $artist_ids, + ")))" + ) + }; +} + /// The song projection, wrapped so an ordering can name its own output. /// /// Mirrors [`album_scope!`], and for the same reason: `song_select!` computes diff --git a/src/subsonic/browse.rs b/src/subsonic/browse.rs index 4f3486d..8f87475 100644 --- a/src/subsonic/browse.rs +++ b/src/subsonic/browse.rs @@ -413,6 +413,33 @@ pub(super) async fn top_songs( Ok(Node::new("topSongs").children(songs.iter().map(song_node))) } +/// `getSimilarSongs` and `getSimilarSongs2`: one implementation, two entry +/// points — OpenSubsonic says the two answer alike for an artist id. +/// +/// `getSimilarSongs` documents "the artist, album or song ID" and +/// `getSimilarSongs2` an artist id; both accept all three, since refusing an +/// album id the service can resolve would only make the second method worse. +/// The id is resolved first and for its refusal, so one the caller cannot +/// reach is error 70, like one naming nothing. +/// +/// A random draw from the artist's own tracks and their visible neighbours', +/// at most `count` (default 50, as specified). With enrichment disabled, the +/// empty container these methods always answered. +pub(super) async fn similar_songs( + state: &AppState, + principal: &Principal, + params: &Params, + container: &'static str, +) -> Result { + let count = params.usize_or("count", 50, 500)?; + let songs = state + .services + .similar_songs(principal.id, params.uuid("id")?, count as i64) + .await + .map_err(service_protocol)?; + Ok(Node::new(container).children(songs.iter().map(song_node))) +} + pub(super) async fn search( state: &AppState, principal: &Principal, diff --git a/src/subsonic/mod.rs b/src/subsonic/mod.rs index dea165a..3d3184f 100644 --- a/src/subsonic/mod.rs +++ b/src/subsonic/mod.rs @@ -416,12 +416,11 @@ async fn dispatch( // listening. Not a recommendation: the reference server asks a remote // service what is popular, and this answers what was played here. "getTopSongs" => top_songs(state, principal, params).await, - // The recommendation surfaces WaveFlow still does not compute. The - // standard empty container is the honest answer and, unlike the - // not-implemented error, does not read to a client as a broken - // server on a page it opens by default. - "getSimilarSongs" => Ok(Node::new("similarSongs")), - "getSimilarSongs2" => Ok(Node::new("similarSongs2")), + // A draw from the artist and their neighbours, read from the + // enrichment cache (RFC-011). With no provider configured — the + // default — still the standard empty container. + "getSimilarSongs" => similar_songs(state, principal, params, "similarSongs").await, + "getSimilarSongs2" => similar_songs(state, principal, params, "similarSongs2").await, // The last empty container to be filled. It is per account, which the // reference server's list is not — see `src/subsonic/radio.rs` for that // deviation and the one about `homePageUrl`'s spelling. diff --git a/src/subsonic/protocol.rs b/src/subsonic/protocol.rs index 13d86fb..70f5f27 100644 --- a/src/subsonic/protocol.rs +++ b/src/subsonic/protocol.rs @@ -208,6 +208,8 @@ pub(super) fn json_array_field(parent: &str, name: &str) -> bool { // Added with the container's first real answer: it held nothing, // so nothing had ever told it apart from an object. | ("topSongs", "song") + // And again with RFC-011: these held nothing until #267. + | ("similarSongs" | "similarSongs2", "song") // The same case one mission later: `internetRadioStations` held // nothing until #217, so nothing had ever told it apart from an // object either. diff --git a/tests/enrichment.rs b/tests/enrichment.rs index 811843e..144ed3c 100644 --- a/tests/enrichment.rs +++ b/tests/enrichment.rs @@ -756,19 +756,7 @@ async fn an_expired_miss_being_refreshed_is_pending() { #[tokio::test] async fn get_artist_info_2_carries_the_biography_and_the_visible_neighbours() { let (_temp, config, state, owner, _outsider) = two_members().await; - let api_key = format!("wfsk_{}", run_key()); - let password = run_key(); - state - .db - .set_subsonic_credential( - owner, - owner, - &state.secret_box.encrypt(password.as_bytes()).unwrap(), - &waveflow_server::security::token_hash(&api_key), - waveflow_server::authentication::now_ms(), - ) - .await - .unwrap(); + let api_key = subsonic_key(&state, owner).await; let listener = fake_lastfm(Mode::Hit, Duration::ZERO).await; let enrichment = engine( &state, @@ -799,3 +787,193 @@ async fn get_artist_info_2_carries_the_biography_and_the_visible_neighbours() { assert_eq!(similar[0]["name"], "Sigur Rós"); assert!(similar[0]["roles"].is_array(), "{next}"); } + +/// A Subsonic API key for `user`, minted at run time, with the encrypted +/// password beside it the credential row requires. +async fn subsonic_key(state: &waveflow_server::AppState, user: uuid::Uuid) -> String { + let api_key = format!("wfsk_{}", run_key()); + let password = run_key(); + state + .db + .set_subsonic_credential( + user, + user, + &state.secret_box.encrypt(password.as_bytes()).unwrap(), + &waveflow_server::security::token_hash(&api_key), + waveflow_server::authentication::now_ms(), + ) + .await + .unwrap(); + api_key +} + +// ── The draw: getSimilarSongs and getSimilarSongs2 ── + +/// The track artists of a draw, sorted, so a random order compares. +async fn drawn(state: &waveflow_server::AppState, user: uuid::Uuid, id: uuid::Uuid) -> Vec { + let mut names: Vec<_> = state + .services + .similar_songs(user, id, 50) + .await + .unwrap() + .into_iter() + .map(|song| song.artist.unwrap_or_default()) + .collect(); + names.sort(); + names +} + +/// `getSimilarSongs` takes "the artist, album or song ID": an album stands for +/// its album artist and a track for its track artist, and both then draw what +/// the artist id draws. +#[tokio::test] +async fn an_album_id_and_a_song_id_reach_the_same_artist_as_the_artist_id() { + let (_temp, _config, state, owner, _outsider) = two_members().await; + let listener = fake_lastfm(Mode::Hit, Duration::ZERO).await; + let enrichment = engine( + &state, + &settings(vec![Provider::LastFm], &run_key()), + &listener, + ); + state.services.use_enrichment(enrichment.clone()); + let bjork = artist_id(&state, owner, "Björk").await; + let album = state.services.artist(owner, bjork).await.unwrap().albums[0].id; + let song = state.services.album(owner, album).await.unwrap().songs[0].id; + drawn(&state, owner, bjork).await; + enrichment.idle().await; + + for id in [bjork, album, song] { + assert_eq!(drawn(&state, owner, id).await, ["Björk", "Sigur Rós"]); + } +} + +/// "Songs from the given artist *and* similar artists": the artist is in the +/// draw, before the neighbours are known and after. +#[tokio::test] +async fn the_artist_s_own_tracks_are_in_the_draw() { + let (_temp, _config, state, owner, _outsider) = two_members().await; + let listener = fake_lastfm(Mode::Hit, Duration::ZERO).await; + let enrichment = engine( + &state, + &settings(vec![Provider::LastFm], &run_key()), + &listener, + ); + state.services.use_enrichment(enrichment.clone()); + let bjork = artist_id(&state, owner, "Björk").await; + + assert_eq!(drawn(&state, owner, bjork).await, ["Björk"]); + enrichment.idle().await; + assert_eq!(drawn(&state, owner, bjork).await, ["Björk", "Sigur Rós"]); +} + +/// A neighbour drawn by name draws only from the caller's libraries: Sigur +/// Rós is also in a library of the outsider's, and that track stays out. And +/// an id of the outsider's answers as a missing one, before any provider is +/// asked about it. +#[tokio::test] +async fn no_track_from_a_library_the_caller_is_not_in() { + let (_temp, config, state, owner, outsider) = two_members().await; + library_of(&state, &config, outsider, "elsewhere", &["Sigur Rós"]).await; + let listener = fake_lastfm(Mode::Hit, Duration::ZERO).await; + let enrichment = engine( + &state, + &settings(vec![Provider::LastFm], &run_key()), + &listener, + ); + state.services.use_enrichment(enrichment.clone()); + let bjork = artist_id(&state, owner, "Björk").await; + let mum = artist_id(&state, outsider, "Múm").await; + let foreign_album = state.services.artist(outsider, mum).await.unwrap().albums[0].id; + + for id in [mum, foreign_album, uuid::Uuid::new_v4()] { + assert!(matches!( + state.services.similar_songs(owner, id, 50).await, + Err(waveflow_server::services::ServiceError::NotFound) + )); + } + assert_eq!(listener.fake.info.load(Ordering::SeqCst), 0); + drawn(&state, owner, bjork).await; + enrichment.idle().await; + + let songs = state + .services + .similar_songs(owner, bjork, 50) + .await + .unwrap(); + let mine = state + .services + .list_artists( + owner, + None, + waveflow_server::services::BrowsePage::new(None, None).unwrap(), + ) + .await + .unwrap() + .into_iter() + .map(|artist| artist.artist.library_id) + .collect::>(); + assert_eq!(songs.len(), 2); + assert!(songs.iter().all(|song| mine.contains(&song.library_id))); +} + +/// The façade end to end: disabled, both methods are the empty container they +/// always were; enabled, `count` absent draws the specification's fifty, the +/// track list is an array, and a foreign id is error 70. +#[tokio::test] +async fn count_absent_yields_at_most_fifty() { + let (_temp, config, state) = support::test_app().await; + let owner = account(&state, "similar-owner").await; + let outsider = account(&state, "similar-outsider").await; + library_of(&state, &config, owner, "many", &["Björk"; 60]).await; + library_of(&state, &config, outsider, "foreign", &["Múm"]).await; + let api_key = subsonic_key(&state, owner).await; + let bjork = artist_id(&state, owner, "Björk").await; + let mum = artist_id(&state, outsider, "Múm").await; + let router = waveflow_server::app(&config, state.clone()); + let id = format!("&id={bjork}"); + + for (method, container) in [ + ("getSimilarSongs", "similarSongs"), + ("getSimilarSongs2", "similarSongs2"), + ] { + let off = support::subsonic_json(&router, method, &api_key, &id).await; + assert_eq!( + off["subsonic-response"][container], + serde_json::json!({}), + "{off}" + ); + } + + let listener = fake_lastfm(Mode::Miss, Duration::ZERO).await; + let enrichment = engine( + &state, + &settings(vec![Provider::LastFm], &run_key()), + &listener, + ); + state.services.use_enrichment(enrichment.clone()); + for (method, container) in [ + ("getSimilarSongs", "similarSongs"), + ("getSimilarSongs2", "similarSongs2"), + ] { + let on = support::subsonic_json(&router, method, &api_key, &id).await; + let songs = on["subsonic-response"][container]["song"] + .as_array() + .unwrap_or_else(|| panic!("an array: {on}")); + assert_eq!(songs.len(), 50, "{method}"); + let one = support::subsonic_json(&router, method, &api_key, &format!("{id}&count=1")).await; + assert_eq!( + one["subsonic-response"][container]["song"] + .as_array() + .map(Vec::len), + Some(1), + "{one}" + ); + let refused = + support::subsonic_json(&router, method, &api_key, &format!("&id={mum}")).await; + assert_eq!( + refused["subsonic-response"]["error"]["code"], 70, + "{refused}" + ); + } + enrichment.idle().await; +} diff --git a/tests/subsonic_methods.rs b/tests/subsonic_methods.rs index d75aa82..20c5788 100644 --- a/tests/subsonic_methods.rs +++ b/tests/subsonic_methods.rs @@ -283,20 +283,21 @@ async fn facade_controls_scans_and_answers_its_remaining_methods() { serde_json::json!({}) ); - // Surfaces WaveFlow does not compute answer the standard empty container - // rather than a not-implemented error. - // `getInternetRadioStations` left this list with #217: the container now - // holds whatever the account put in it, and its own test below asserts - // both the empty case and the filled one. - for (method, container) in [ - ("getSimilarSongs", "similarSongs"), - ("getSimilarSongs2", "similarSongs2"), - ] { - let response = subsonic_json(&router, method, api_key, "&id=whatever&count=5").await; - assert_eq!(response["subsonic-response"]["status"], "ok", "{method}"); + // The two similar-songs methods used to answer the empty container for + // any id at all. Since #267 the id is resolved first and for its refusal + // (RFC-011 decision 7), so one naming nothing is error 70; what they + // answer for a real one, enrichment off and on, is in `tests/enrichment.rs`. + // `getInternetRadioStations` left the empty-container list with #217. + for method in ["getSimilarSongs", "getSimilarSongs2"] { + let response = subsonic_json( + &router, + method, + api_key, + &format!("&id={}&count=5", uuid::Uuid::new_v4()), + ) + .await; assert_eq!( - response["subsonic-response"][container], - serde_json::json!({}), + response["subsonic-response"]["error"]["code"], 70, "{method}" ); }