Skip to content

feat(albums,artists): browse endpoints for per-library album + artist surface (phase 4.d.0.4) - #36

Merged
InstaZDLL merged 2 commits into
mainfrom
feat/4-d-0-4-album-artist-endpoints
Jun 8, 2026
Merged

feat(albums,artists): browse endpoints for per-library album + artist surface (phase 4.d.0.4)#36
InstaZDLL merged 2 commits into
mainfrom
feat/4-d-0-4-album-artist-endpoints

Conversation

@InstaZDLL

@InstaZDLL InstaZDLL commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Summary

Closes the loop on phase 4.d.0's chain: the album / artist / track_artist schema landed in 4.d.0.1 (#34), the apply pipeline started populating it in 4.d.0.2 (#35), the desktop started emitting rows for it in WaveFlow#206 (4.d.0.3) — this PR adds the read endpoints the web client needs to browse what got materialised.

Four new endpoints, all read-only, all tenant-scoped behind the existing JWT middleware:

GET /api/v1/profiles/{p}/libraries/{l}/albums list albums under the library, ordered `(updated_at DESC, id ASC)`
GET /api/v1/profiles/{p}/libraries/{l}/albums/{id}/tracks drill-down to the album's tracks in sleeve order `(disc_number, track_number)`
GET /api/v1/profiles/{p}/libraries/{l}/artists list artists under the library, same ordering
GET /api/v1/profiles/{p}/libraries/{l}/artists/{id}/tracks drill-down to every track contributed by the artist (joins through `track_artist` — multi-artist tracks surface under each contributor)

Key design choices

  • 2-query ownership-check + fetch (lifted from `db::playlist_track::fetch_for_owner`): the ownership SELECT distinguishes 404 ("library / album / artist missing or foreign-owned") from `200 []` ("owned but empty"). The race window between the two queries is benign — every parent carries `ON DELETE CASCADE` so a row vanishing mid-request collapses to the empty-list answer.
  • Reuse the indexes the schema migration planted: `album_library_updated_idx`, `artist_library_updated_idx`, `track_album_idx`, `track_artist_artist_idx`. `id ASC` is the deterministic tiebreaker on equal `updated_at` — guards against the apply pipeline batch-stamping several rows at the same epoch millisecond shuffling between requests.
  • Album list joins `artist` once to surface `album_artist_name` so the web album-grid doesn't fan out N lookups. Compilation rows project `album_artist_id = null` + `album_artist_name = null` + `is_compilation = true` (UI renders "Various Artists" client-side).
  • Writes deliberately not exposed. Album / artist rows materialise from `apply::track` in 4.d.0.2 — they're derived from the desktop's tag metadata, not user-curated on the server.
  • `TrackResponse.album_id` added so the artist drill-down can deep-link contributed tracks to their album page. The legacy `/tracks` collection still NULL-projects the column (its SELECT lives in waveflow-core); follow-up bumps core.
  • db::album + db::artist modules rather than going through `waveflow-core` — read-only addition, no need to bump the core rev. Consistent with the `db::playlist_track::fetch_for_owner` precedent from phase 1.j.c.

Test plan

  • `cargo check --all-targets` clean
  • `cargo fmt --all -- --check` clean
  • `cargo clippy --all-targets --all-features -- -D warnings` clean
  • `cargo test --no-run` builds every binary
  • CI runs `cargo test` against real Postgres — verifies the SQL + the integration tests
  • OpenAPI snapshot extended (4 new `paths.contains_key(...)` asserts) so dropping `#[utoipa::path]` from any new handler fails the snapshot

Coverage

  • Empty library → `200 []`
  • Foreign library / foreign user / wrong-library-id pivot → `404` (full no-existence-leak blur)
  • Ordering by `updated_at DESC`
  • Tied `updated_at` → `id ASC` tiebreak (guards the apply-pipeline batch invariant — without this test a refactor that drops `, id ASC` would ship green)
  • Compilation projects `album_artist_*` as null + `is_compilation = true`
  • Album drill-down orders by `(disc_number, track_number)` + `album_id` surfaces on every row
  • Artist drill-down surfaces every contributor link (multi-artist track appears under both)
  • Artist drill-down surfaces `album_id` (linked + orphan tracks)

Summary by CodeRabbit

  • New Features

    • Navigation en lecture des bibliothèques par albums et artistes, avec listes ordonnées et drill-down pistes.
    • Endpoints pour consulter les pistes d’un album ou d’un artiste; les réponses incluent désormais l’album_id quand pertinent.
  • Documentation

    • Documentation ajoutée décrivant la surface de consultation (Phase 4.d.0.4) et les conventions de réponse 200/404.
  • Tests

    • Tests end-to-end ajoutés pour valider tri, ordonnancement, comportements vides/404 et projection des champs.

…t surface (phase 4.d.0.4)

Adds 4 read-only endpoints that close the loop on the album / artist
surface materialised by `apply::track` (phase 4.d.0.2) + the schema
shipped in `20260608120000_album_artist.sql` (phase 4.d.0.1):

  GET /api/v1/profiles/{p}/libraries/{l}/albums
  GET /api/v1/profiles/{p}/libraries/{l}/albums/{id}/tracks
  GET /api/v1/profiles/{p}/libraries/{l}/artists
  GET /api/v1/profiles/{p}/libraries/{l}/artists/{id}/tracks

Pattern lifts the 2-query ownership-check + fetch idiom from
`db::playlist_track::fetch_for_owner` so 404 (library / album /
artist missing or foreign-owned) stays distinct from 200 [] (owned
but empty). The race window is benign — every parent cascades on
DELETE so a row vanishing between the two queries collapses to the
empty-list answer.

Album list joins `artist` once to surface `album_artist_name` (avoids
N artist lookups client-side). Compilation rows project null +
is_compilation=true. Drill-down ORDER BY rides the indexes the
schema migration planted: album → (disc_number, track_number, id),
artist → (disc_number, track_number, id) via track_artist join.

Writes deliberately not exposed — album / artist rows come from the
apply pipeline only.

TrackResponse gains `album_id` so the artist drill-down can deep-link
contributed tracks to their album page without N extra round-trips.
Existing `/tracks` collection still NULL-projects the column (its
SELECT lives in waveflow-core); follow-up PR bumps core.

Test coverage: empty → 200 [], ordering by updated_at DESC, tied
updated_at → id ASC tiebreak (guards the apply-pipeline-batch
invariant), compilation NULL projection, multi-artist tracks surface
under every contributor, full cross-tenant battery (foreign user,
foreign profile, wrong library_id pivot). OpenAPI snapshot guards
the 4 new paths.

Doc bullet added to server CLAUDE.md.

Signed-off-by: InstaZDLL <github.105mh@8shield.net>
@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0badddb8-e59c-4e06-9898-01d4646b9241

📥 Commits

Reviewing files that changed from the base of the PR and between 566e6ed and 063ad37.

📒 Files selected for processing (1)
  • src/api/mod.rs

📝 Walkthrough

Walkthrough

Introduction de la Phase 4.d.0.4 : quatre endpoints read-only pour parcourir albums et artistes au sein d'une bibliothèque tenant-scoped, incluant drill-down vers les pistes associées, avec TrackResponse enrichi et validation des scopes composite (profile/library/user).

Changes

Phase 4.d.0.4 Browse Surface: Albums et Artistes Read-Only

Layer / File(s) Summary
Helpers DB pour albums et artistes
src/db.rs
Deux nouveaux modules publics album et artist avec structures AlbumRow/ArtistRow et fonctions list_for_library / list_tracks_for_album / list_tracks_for_artist validant la possession (library→profile→user) et retournant Ok(None) ou Ok(Some([])) selon l'autorisation/contenu.
Contrats de réponse API
src/api/tracks.rs, src/api/albums.rs, src/api/artists.rs
Ajout de album_id optionnel à TrackResponse ; création de AlbumResponse (artiste, titre, année, cover_hash, is_compilation, timestamps) et ArtistResponse (id, nom, picture_hash optionnel, timestamps) avec conversions Fromdb::*Row.
Endpoints albums (liste et drill-down tracks)
src/api/albums.rs
Nouveau module avec router et deux handlers : list_albums (liste albums ordonnés par updated_at DESC, isolation tenant, 404 si non autorisé) et list_album_tracks (pistes de l'album ordonnées par disc_number, track_number, isolation scope composite).
Endpoints artists (liste et drill-down tracks)
src/api/artists.rs
Nouveau module avec router et deux handlers : list_artists (liste artistes ordonnés par updated_at DESC, isolation tenant) et list_artist_tracks (pistes via track_artist avec DISTINCT, album_id exposé, isolation scope).
Intégration routeurs dans api/mod.rs
src/api/mod.rs
Déclaration des modules albums/artists, création de sous-routers JWT-layered via authenticate, merge dans l'OpenApiRouter global et mise à jour de la documentation API.
Tests end-to-end albums
tests/albums.rs
Helpers (mint_profile, mint_library, insert_*) et tests validant list_albums (vide/tri/tiebreaker/compilations/isolation) et list_album_tracks (tri disc/track, vide/missing/isolation scope, album_id exposé).
Tests end-to-end artists
tests/artists.rs
Helpers d'insertion SQLx (artist/track/album/track_artist) et tests validant list_artists (vide/tri/tiebreaker/isolation) et list_artist_tracks (drill-down, album_id projection, isolation scope).
Validation OpenAPI
tests/openapi.rs
Assertions supplémentaires vérifiant que les chemins albums/artists (collections et drill-down, incluant /tracks) figurent dans la spec OpenAPI.
Documentation
CLAUDE.md
Section Phase 4.d.0.4 décrivant les quatre endpoints read-only, l'ordonnancement déterministe, le drill-down via track_artist, et le pattern d'accès ownership-check + fetch.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant ListAlbumsHandler as list_albums<br/>Handler
  participant db_album as db::album
  participant Database
  Client->>ListAlbumsHandler: GET /profiles/{profile_id}/libraries/{library_id}/albums
  ListAlbumsHandler->>db_album: list_for_library(library_id, profile_id, user_id)
  db_album->>Database: SELECT library WHERE id=? AND<br/>profile_id=? AND profile.user_id=?
  Database-->>db_album: library_row ou null
  db_album->>Database: SELECT album FROM album<br/>WHERE library_id=?<br/>ORDER BY updated_at DESC, id ASC
  Database-->>db_album: Vec[AlbumRow]
  db_album-->>ListAlbumsHandler: Ok(Some(albums))
  ListAlbumsHandler-->>Client: 200 JSON[AlbumResponse]
Loading
sequenceDiagram
  participant Client
  participant ListArtistTracksHandler as list_artist_tracks<br/>Handler
  participant db_artist as db::artist
  participant Database
  Client->>ListArtistTracksHandler: GET /profiles/{profile_id}/libraries/{library_id}/artists/{id}/tracks
  ListArtistTracksHandler->>db_artist: list_tracks_for_artist(artist_id, library_id, profile_id, user_id)
  db_artist->>Database: SELECT artist WHERE id=? AND<br/>library_id=? AND library.profile_id=?<br/>AND profile.user_id=?
  Database-->>db_artist: artist_row ou null
  db_artist->>Database: SELECT DISTINCT track FROM track_artist<br/>JOIN track WHERE artist_id=?<br/>AND track.library_id=?<br/>ORDER BY disc_number, track_number
  Database-->>db_artist: Vec[TrackRow]
  db_artist-->>ListArtistTracksHandler: Ok(Some(tracks))
  ListArtistTracksHandler-->>Client: 200 JSON[TrackResponse+album_id]
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • InstaZDLL/waveflow-server#34: Les handlers et helpers DB db::album/db::artist dépendent du schéma track_artist et de la colonne track.album_id introduits par ce PR.
  • InstaZDLL/waveflow-server#17: Harmonisation du montage des routes derrière le middleware authenticate (JWT-only) affecte l'intégration des nouveaux sous-routers.
  • InstaZDLL/waveflow-server#9: Changements connexes à la représentation TrackResponse (album_id) et à la logique tracks tenant-scoped.

Poem

🎶 Quatre routes s'ouvrent sur disques et visages,
Parcours tenant-scoped, ordres sûrs sans nuages,
Track_artist tisse, album_id éclaire,
Tests veillent l'isolement et l'ordre clair. 🎧

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Le titre décrit précisément les changements apportés : ajout d'endpoints de navigation pour albums et artistes par bibliothèque, avec la référence de phase explicite.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/4-d-0-4-album-artist-endpoints

Comment @coderabbitai help to get the list of available commands and usage tips.

@InstaZDLL InstaZDLL self-assigned this Jun 8, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/api/mod.rs`:
- Around line 11-12: Mettre à jour le commentaire de module en haut de
src/api/mod.rs pour ne plus qualifier `sync` et `stream` de « Future » :
modifier la phrase "//! Future modules will cover `sync`, `stream` (per RFC-001
§6 / §7)." pour refléter queles modules api::sync et api::stream existent déjà
(ou reformuler en "`sync` and `stream` (per RFC-001 §6 / §7)"); localisez le
commentaire dans le module racine (où api::sync et api::stream sont déclarés) et
remplacez simplement le mot "Future" ou reformulez la phrase sans changer les
noms des modules ni les routes déjà câblées (voir déclarations de modules `sync`
et `stream` et le routeur).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 84e25030-1fcd-49cb-bf22-488fc28677d7

📥 Commits

Reviewing files that changed from the base of the PR and between 8bc697e and 566e6ed.

📒 Files selected for processing (9)
  • CLAUDE.md
  • src/api/albums.rs
  • src/api/artists.rs
  • src/api/mod.rs
  • src/api/tracks.rs
  • src/db.rs
  • tests/albums.rs
  • tests/artists.rs
  • tests/openapi.rs

Comment thread src/api/mod.rs Outdated
CR catch on PR #36: the header still said "Future modules will cover
sync, stream" but those modules have been declared + wired into the
router since 1.f and 1.e respectively. Refresh the list so it matches
what `mod.rs` actually mounts (sync, stream, artwork, share alongside
the CRUD modules).

@coderabbitai resolved

Signed-off-by: InstaZDLL <github.105mh@8shield.net>
@InstaZDLL

Copy link
Copy Markdown
Owner Author

@coderabbitai resolved in 063ad37 — refreshed the api/mod.rs header to list sync/stream/artwork/share as the modules they actually are now, plus the per-module phase reference, rather than the stale "future modules" wording.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@InstaZDLL Noted, the header now accurately reflects the current module landscape. Thanks for the update!


🧠 Learnings used
Learnt from: CR
Repo: InstaZDLL/waveflow-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-06-07T23:59:10.640Z
Learning: Applies to src/api/**/*.rs : Use camelCase for payload field names in Rust serde structures that map to the desktop wire format (e.g. snapshot_title, album_artist_name, track_ids)

Learnt from: CR
Repo: InstaZDLL/waveflow-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-06-07T23:59:10.640Z
Learning: Applies to src/api/**/*.rs : Use `entity_id = file_path` for track sync wire shape with `UNIQUE (library_id, file_path)`, and carry BLAKE3 hash as a payload field (not entity_id) to avoid upsert misses on tag-editor re-emits

Learnt from: CR
Repo: InstaZDLL/waveflow-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-06-07T23:59:10.640Z
Learning: Applies to src/api/**/*.rs : Verify streaming token ownership and sign short-lived (≤ 60s) URLs via `stream_token::mint` in the streaming endpoint; canonicalize paths and refuse anything resolving outside `WAVEFLOW_MUSIC_ROOT` via std::fs::canonicalize + prefix check

Learnt from: CR
Repo: InstaZDLL/waveflow-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-06-07T23:59:10.640Z
Learning: Applies to src/api/**/*.rs : Serve artwork variants with Cache-Control: public, max-age=31536000, immutable and ETag = hash; fall back to metadata_artwork_variant for direct variant hash lookups

Learnt from: InstaZDLL
Repo: InstaZDLL/waveflow-server PR: 0
File: :0-0
Timestamp: 2026-06-07T22:50:38.169Z
Learning: Dans waveflow-server (migrations/20260608120000_album_artist.sql + tests/album_artist.rs), la contrainte cross-library est enforced au niveau schéma via des FK composites : chaque lien entité-à-entité porte `library_id` dans LES DEUX colonnes de la FK, avec `UNIQUE (id, library_id)` sur la table parente comme cible. Détails :
- `track`, `artist`, `album` ont tous `UNIQUE (id, library_id)`.
- `album.album_artist_id` → `FOREIGN KEY (album_artist_id, library_id) REFERENCES artist (id, library_id) ON DELETE SET NULL (album_artist_id)` (PG15+ column-level SET NULL).
- `track.album_id` → `FOREIGN KEY (album_id, library_id) REFERENCES album (id, library_id) ON DELETE SET NULL (album_id)`.
- `track_artist` porte une colonne dénormalisée `library_id BIGINT NOT NULL` + deux FK composites vers `track(id, library_id)` et `artist(id, library_id)` ON DELETE CASCADE — la même colonne `library_id` partagée rend impossible l'insertion d'une paire inter-bibliothèque.
- Cible PG17 (même gate que `UNIQUE NULLS NOT DISTINCT`). Le apply pipeline (4.d.0.2) dérive `track_artist.library_id` de `track.library_id` à l'upsert.

Learnt from: CR
Repo: InstaZDLL/waveflow-server PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-06-07T23:59:10.640Z
Learning: Applies to src/api/**/*.rs : Reject empty-string album_title and album_artist_name at the apply boundary to prevent CHECK constraint violations; pack full track metadata + album/artist plumbing in INSERT payloads

@InstaZDLL
InstaZDLL merged commit d71d490 into main Jun 8, 2026
8 checks passed
@InstaZDLL
InstaZDLL deleted the feat/4-d-0-4-album-artist-endpoints branch June 8, 2026 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant