From 2624ff7b8a3e2460ad41f527a2089585f8a31ac0 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Thu, 10 Sep 2026 11:04:20 +0200 Subject: [PATCH 1/2] fix(api): name the genre route for what it does MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #179. `GET /api/v2/songs` required `genre` and answered 400 without it, while being wired to a handler called `list_songs_by_genre`. The path said "songs", the handler said "songs by genre", and the contract said no. The behaviour is the deliberate half. A test already pinned that 400 with its reason — the genre is what the request is about, so its absence is a malformed request and not an unfiltered catalogue — and the general listing exists elsewhere, as `/libraries/{id}/tracks`, which pages and searches. Relaxing `genre` to an option would have undone a decision somebody made on purpose and left two ways to list songs. So the path moves rather than the parameter: `/api/v2/songs/by-genre`, beside `/songs/random`. `/api/v2/songs` now belongs to nobody, and a test says it answers 404 — it was its readability that sent people looking for a bug, not its status code, and a path that means nothing is better than one that means something it will not do. The last reference was found by the suite rather than by me: the search that swept the rename looked for `"/api/v2/songs"` closed, and missed the one carrying a query string. Claude-Session: https://claude.ai/code/session_01TyKunaKXS16hyFDBHwc5KK Signed-off-by: InstaZDLL --- docs/api-v2-guide.md | 17 +++++++++++++++-- docs/opensubsonic-gap-analysis.md | 2 +- src/api/catalog.rs | 7 ++++++- src/api/mod.rs | 2 +- tests/catalog.rs | 14 +++++++++++--- webapp/src/api.ts | 2 +- 6 files changed, 35 insertions(+), 9 deletions(-) diff --git a/docs/api-v2-guide.md b/docs/api-v2-guide.md index 0b006920..fff57e29 100644 --- a/docs/api-v2-guide.md +++ b/docs/api-v2-guide.md @@ -196,7 +196,7 @@ curl https://music.example.com/api/v2/artists/ARTIST_UUID \ -H "Authorization: Bearer ACCESS_TOKEN" ``` -`GET /api/v2/songs` takes a required `genre` and pages through it; `GET +`GET /api/v2/songs/by-genre` takes a required `genre` and pages through it; `GET /api/v2/songs/random` draws a selection in SQL, with optional `genre`, `from_year` and `to_year`. Both match the genre on its canonical name like every other genre filter, and both are the native form of a Subsonic method @@ -425,10 +425,23 @@ a client's position in its own user journal. } ], "next_cursor": 41, - "has_more": false + "has_more": false, + "purged_through": 0 } ``` +`purged_through` is the highest cursor retention has already cut away. A cursor +strictly below it is refused with **409**, and the only way back from there is +the snapshot — so subtract it from the cursor you hold to know how much slack +you have, and spend that slack deliberately. Resyncing on a network you like, +while the application is idle, is a different experience from resyncing at the +moment the server refuses you. + +It is what was *purged*, not what survives: a feed whose oldest surviving row +sits at a high cursor has lost nothing, it merely started late. Measure against +the cursor **you** hold, not against `next_cursor` — that one is the end of the +page just served, so it tells you the margin you will have after applying it. + A track `upsert` carries the file's `full_hash`, and it is the only place on the wire that does. **A file retagged outside the API keeps its track id while its bytes move** — the scan's skip test compares hashes, so different bytes make it diff --git a/docs/opensubsonic-gap-analysis.md b/docs/opensubsonic-gap-analysis.md index e253c3b5..f2552f1e 100644 --- a/docs/opensubsonic-gap-analysis.md +++ b/docs/opensubsonic-gap-analysis.md @@ -161,7 +161,7 @@ des dettes nommées plutôt que des défauts. un `getMusicDirectory` sur cet identifiant ne renverra pas la piste. 4. **Deux inexactitudes de surface** : `getLicense` expire en dur au 2099-12-31, et `search_catalog` n'annote pas son 400 sur `q` manquant alors - que `/api/v2/songs` vient de le faire. + que `/api/v2/songs/by-genre` vient de le faire. Un point de cadrage plutôt qu'un défaut : le prochain tag reposera sur **quatre** clients rejoués, pas cinq. Le document le dit lui-même et n'essaie pas de faire diff --git a/src/api/catalog.rs b/src/api/catalog.rs index 0ef9136c..aeb43bf9 100644 --- a/src/api/catalog.rs +++ b/src/api/catalog.rs @@ -211,7 +211,12 @@ pub async fn list_random_songs( /// The native form of `getSongsByGenre`. `genre` is required: answering an /// unfiltered catalogue would drop the filter in silence. -#[utoipa::path(get, path = "/api/v2/songs", tag = "catalog", params(("genre" = String, Query), ("library_id" = Option, Query), ("offset" = Option, Query), ("limit" = Option, Query)), responses((status = 200, body = [crate::services::SongItem]), (status = 400, description = "genre is required"), (status = 401, body = ErrorResponse), (status = 422, body = ErrorResponse)))] +/// +/// Served at `/songs/by-genre` and not `/songs`, so the path says what the +/// handler does. Under `/songs` it read as the general listing and answered +/// 400 to anyone who took it for one; the general listing is +/// `/libraries/{id}/tracks`, which pages and searches. +#[utoipa::path(get, path = "/api/v2/songs/by-genre", tag = "catalog", params(("genre" = String, Query), ("library_id" = Option, Query), ("offset" = Option, Query), ("limit" = Option, Query)), responses((status = 200, body = [crate::services::SongItem]), (status = 400, description = "genre is required"), (status = 401, body = ErrorResponse), (status = 422, body = ErrorResponse)))] pub async fn list_songs_by_genre( State(state): State, Query(query): Query, diff --git a/src/api/mod.rs b/src/api/mod.rs index 2e230c2c..771b7e71 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -133,7 +133,7 @@ pub fn router(state: AppState) -> Router { .route("/api/v2/artists", get(list_artists)) .route("/api/v2/artists/{artist_id}", get(get_artist)) .route("/api/v2/search", get(search_catalog)) - .route("/api/v2/songs", get(list_songs_by_genre)) + .route("/api/v2/songs/by-genre", get(list_songs_by_genre)) .route("/api/v2/songs/random", get(list_random_songs)) .route( "/api/v2/playlists", diff --git a/tests/catalog.rs b/tests/catalog.rs index 0a373e4c..805283f7 100644 --- a/tests/catalog.rs +++ b/tests/catalog.rs @@ -804,7 +804,8 @@ async fn genre_matching_is_canonical_on_every_surface() { } }; // Any spelling reaches the same three tracks, natively too. - let by_genre = json_body(native("/api/v2/songs?genre=hip%20hop&limit=50".into()).await).await; + let by_genre = + json_body(native("/api/v2/songs/by-genre?genre=hip%20hop&limit=50".into()).await).await; assert_eq!(by_genre.as_array().expect("a list").len(), 3); let random = json_body(native("/api/v2/songs/random?genre=HIP%20%20HOP&limit=50".into()).await).await; @@ -817,14 +818,21 @@ async fn genre_matching_is_canonical_on_every_surface() { ) .await; assert!(narrowed.as_array().expect("a list").is_empty()); - let unused = json_body(native("/api/v2/songs?genre=Polka".into()).await).await; + let unused = json_body(native("/api/v2/songs/by-genre?genre=Polka".into()).await).await; assert!(unused.as_array().expect("a list").is_empty()); // The genre is what the request is about, so its absence is a malformed // request and not an unfiltered catalogue. assert_eq!( - native("/api/v2/songs".into()).await.status(), + native("/api/v2/songs/by-genre".into()).await.status(), StatusCode::BAD_REQUEST ); + // And `/songs` is nobody's route: it read as the general listing while + // answering 400, which is what sent a reader looking for a bug. The + // general listing is `/libraries/{id}/tracks`. + assert_eq!( + native("/api/v2/songs".into()).await.status(), + StatusCode::NOT_FOUND + ); // Search pages each kind on its own offset. let paged = json_body(native("/api/v2/search?q=Boom&limit=10&song_offset=5".into()).await).await; diff --git a/webapp/src/api.ts b/webapp/src/api.ts index 6aab6f2e..5cad6917 100644 --- a/webapp/src/api.ts +++ b/webapp/src/api.ts @@ -708,7 +708,7 @@ export const listGenres = (libraryId?: string) => /** List songs for a genre in the active library. */ export const listGenreSongs = (genre: string, libraryId?: string) => - collect("/api/v2/songs", scoped(libraryId, { genre })); + collect("/api/v2/songs/by-genre", scoped(libraryId, { genre })); /** * `GET /history` answers plays, not songs — `track_id`, `submission` and From 04e1f041400f2e1ec82676b17728b29319302d9e Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Thu, 10 Sep 2026 11:04:29 +0200 Subject: [PATCH 2/2] feat(api): report where the event stream has been cut MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #178. A client following a library's events keeps a cursor, and the server refuses it once retention has cut past it. Nothing said how close that was, so the first sign of trouble was the refusal — the worst moment to pay for a full resync, since it has to happen right then whatever else is going on. The watermark was not missing, only unreported. `library.events_purged_through` already exists, is already read in the same transaction that builds the page, and is already what decides the refusal. It is now a field on that page, at no extra query and describing the same instant as the rows it travels with, which a second read could not promise. The test found an error in the sentence that documented it. The margin is the client's **own** cursor minus the watermark, not `next_cursor` minus the watermark: `next_cursor` is the end of the page just served, so it says what the margin becomes once the page is applied, not what it is. The assertion failed on exactly that gap; the field's documentation and the API guide now draw the distinction rather than glossing it. Claude-Session: https://claude.ai/code/session_01TyKunaKXS16hyFDBHwc5KK Signed-off-by: InstaZDLL --- src/services/library_events.rs | 4 ++++ src/services/mod.rs | 19 +++++++++++++++++++ tests/sync.rs | 20 +++++++++++++++++++- 3 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/services/library_events.rs b/src/services/library_events.rs index 9a479225..c93beb6b 100644 --- a/src/services/library_events.rs +++ b/src/services/library_events.rs @@ -406,6 +406,10 @@ impl DomainServices { events, next_cursor, has_more, + // Read in the same snapshot as the rows above, so the margin a + // client computes from it cannot describe a different moment than + // the page it came with. + purged_through: watermark, }) } } diff --git a/src/services/mod.rs b/src/services/mod.rs index 16814d51..367df3ab 100644 --- a/src/services/mod.rs +++ b/src/services/mod.rs @@ -514,6 +514,25 @@ pub struct LibraryEventPage { pub events: Vec, pub next_cursor: i64, pub has_more: bool, + /// The highest cursor retention has already cut away. Everything at or + /// below it is gone; a cursor strictly below it is refused, and the only + /// way back is the snapshot. + /// + /// Reported so a client can see the edge before it reaches it. Without it + /// the first sign of trouble was the refusal itself, which is the worst + /// moment to pay for a resync. + /// + /// The margin is the client's **own** cursor minus this, not + /// `next_cursor` minus this: `next_cursor` is the end of the page just + /// served, so it describes the margin the client will have once it has + /// processed the page rather than the one it holds now. Either way it can + /// spend that slack deliberately — on a network it likes, while nobody is + /// watching — instead of at whatever moment the server says no. + /// + /// It is what was *purged*, not what survives. A feed whose oldest row sits + /// at a high cursor has lost nothing, it merely started late, and a floor + /// derived from surviving rows cannot tell those two apart. + pub purged_through: i64, } /// Everything one account has starred, across the three entity kinds. diff --git a/tests/sync.rs b/tests/sync.rs index 5c283657..817210a0 100644 --- a/tests/sync.rs +++ b/tests/sync.rs @@ -1095,6 +1095,11 @@ async fn the_library_feed_reports_what_a_scan_changed() { let (status, page) = feed(owner_token.clone(), 0, 500).await; assert_eq!(status, StatusCode::OK); + // Nothing has been cut, so the client's margin is the whole feed. Reported + // rather than left to be discovered: without it the first sign of trouble + // is the refusal itself, and by then the resync has to happen right now + // instead of at a moment the client would have chosen. + assert_eq!(page["purged_through"], 0); let events = page["events"].as_array().unwrap(); assert_eq!(events.len(), 2, "one upsert per track the scan applied"); for event in events { @@ -1212,6 +1217,19 @@ async fn the_library_feed_reports_what_a_scan_changed() { let (status, _) = feed(owner_token.clone(), 2, 500).await; assert_eq!(status, StatusCode::CONFLICT); // Standing exactly at the cut is not standing before it. - let (status, _) = feed(owner_token, 3, 500).await; + let (status, page) = feed(owner_token, 3, 500).await; assert_eq!(status, StatusCode::OK); + // And the page says where the cut is, while it is still being served + // rather than after it is refused. This client asked from 3 and the cut is + // at 3, so it holds no slack at all — one more purge and it is sent to the + // snapshot — and it can read that off the page it just received. + assert_eq!(page["purged_through"], 3); + // The margin is measured against the cursor the client held, not against + // `next_cursor`: that one is the end of the page just served, so it says + // what the margin becomes once this page is processed. + assert_eq!(3 - page["purged_through"].as_i64().unwrap(), 0); + assert!( + page["next_cursor"].as_i64().unwrap() > 3, + "the page advanced it" + ); }