diff --git a/docs/api-v2-guide.md b/docs/api-v2-guide.md index 0b00692..fff57e2 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 e253c3b..f2552f1 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 0ef9136..aeb43bf 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 2e230c2..771b7e7 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/src/services/library_events.rs b/src/services/library_events.rs index 9a47922..c93beb6 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 16814d5..367df3a 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/catalog.rs b/tests/catalog.rs index 0a373e4..805283f 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/tests/sync.rs b/tests/sync.rs index 5c28365..817210a 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" + ); } diff --git a/webapp/src/api.ts b/webapp/src/api.ts index 6aab6f2..5cad691 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