From 850e4342606cba40e0eb4a2f191e30d1244d37f9 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 29 Oct 2025 15:04:13 +0100 Subject: [PATCH] trustpub: Add cursor-based pagination to GitHub configs list endpoint Add cursor-based pagination to the `/api/v1/trusted_publishing/github_configs` endpoint using the `id` field as the cursor. Pagination is always enabled with a default of 10 items per page (following the codebase standard). Key changes: - Add `ListResponseMeta` with `total` and `next_page` fields - Implement seek-based pagination ordered by `id` ascending - Support `?per_page=N` (max 100) and `?seek=` query parameters - Optimize to avoid COUNT query when first page fits all results - Add comprehensive pagination test covering multiple pages The implementation follows existing pagination patterns from the versions endpoint, using the `seek!()` macro for type-safe cursor serialization. --- .../trustpub/github_configs/json.rs | 14 +++ .../trustpub/github_configs/list/mod.rs | 108 ++++++++++++++++-- .../routes/trustpub/github_configs/list.rs | 64 +++++++++++ ...onfigs__list__crate_with_no_configs-2.snap | 6 +- ...b__github_configs__list__happy_path-2.snap | 6 +- ...b__github_configs__list__happy_path-4.snap | 6 +- ...ub_configs__list__legacy_token_auth-2.snap | 6 +- ...b__github_configs__list__pagination-2.snap | 62 ++++++++++ ...b__github_configs__list__pagination-4.snap | 62 ++++++++++ ...b__github_configs__list__pagination-6.snap | 62 ++++++++++ ...b__github_configs__list__pagination-8.snap | 11 ++ ..._auth_with_trusted_publishing_scope-2.snap | 6 +- ...oken_auth_with_wildcard_crate_scope-2.snap | 6 +- ...egration__openapi__openapi_snapshot-2.snap | 56 ++++++++- 14 files changed, 461 insertions(+), 14 deletions(-) create mode 100644 src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-2.snap create mode 100644 src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-4.snap create mode 100644 src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-6.snap create mode 100644 src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-8.snap diff --git a/src/controllers/trustpub/github_configs/json.rs b/src/controllers/trustpub/github_configs/json.rs index 40902eba98a..9b52009a7dd 100644 --- a/src/controllers/trustpub/github_configs/json.rs +++ b/src/controllers/trustpub/github_configs/json.rs @@ -18,4 +18,18 @@ pub struct CreateResponse { #[derive(Debug, Serialize, utoipa::ToSchema)] pub struct ListResponse { pub github_configs: Vec, + + #[schema(inline)] + pub meta: ListResponseMeta, +} + +#[derive(Debug, Serialize, utoipa::ToSchema)] +pub struct ListResponseMeta { + /// The total number of GitHub configs belonging to the crate. + #[schema(example = 42)] + pub total: i64, + + /// Query string to the next page of results, if any. + #[schema(example = "?seek=abc123")] + pub next_page: Option, } diff --git a/src/controllers/trustpub/github_configs/list/mod.rs b/src/controllers/trustpub/github_configs/list/mod.rs index 8ae8db4f5ce..f6fdb8db5e2 100644 --- a/src/controllers/trustpub/github_configs/list/mod.rs +++ b/src/controllers/trustpub/github_configs/list/mod.rs @@ -1,7 +1,11 @@ use crate::app::AppState; use crate::auth::AuthCheck; +use crate::controllers::helpers::pagination::{ + Page, PaginationOptions, PaginationQueryParams, encode_seek, +}; use crate::controllers::krate::load_crate; -use crate::controllers::trustpub::github_configs::json::{self, ListResponse}; +use crate::controllers::trustpub::github_configs::json::{self, ListResponse, ListResponseMeta}; +use crate::util::RequestUtils; use crate::util::errors::{AppResult, bad_request}; use axum::Json; use axum::extract::{FromRequestParts, Query}; @@ -13,6 +17,7 @@ use diesel::dsl::{exists, select}; use diesel::prelude::*; use diesel_async::RunQueryDsl; use http::request::Parts; +use indexmap::IndexMap; use serde::Deserialize; #[derive(Debug, Deserialize, FromRequestParts, utoipa::IntoParams)] @@ -28,7 +33,7 @@ pub struct ListQueryParams { #[utoipa::path( get, path = "/api/v1/trusted_publishing/github_configs", - params(ListQueryParams), + params(ListQueryParams, PaginationQueryParams), security(("cookie" = []), ("api_token" = [])), tag = "trusted_publishing", responses((status = 200, description = "Successful Response", body = inline(ListResponse))), @@ -64,10 +69,13 @@ pub async fn list_trustpub_github_configs( return Err(bad_request("You are not an owner of this crate")); } - let configs = GitHubConfig::query() - .filter(trustpub_configs_github::crate_id.eq(krate.id)) - .load(&mut conn) - .await?; + let pagination = PaginationOptions::builder() + .enable_seek(true) + .enable_pages(false) + .gather(&parts)?; + + let (configs, total, next_page) = + list_configs(&mut conn, krate.id, &pagination, &parts).await?; let github_configs = configs .into_iter() @@ -83,5 +91,91 @@ pub async fn list_trustpub_github_configs( }) .collect(); - Ok(Json(ListResponse { github_configs })) + Ok(Json(ListResponse { + github_configs, + meta: ListResponseMeta { total, next_page }, + })) +} + +async fn list_configs( + conn: &mut diesel_async::AsyncPgConnection, + crate_id: i32, + options: &PaginationOptions, + req: &Parts, +) -> AppResult<(Vec, i64, Option)> { + use seek::*; + + let seek = Seek::Id; + + assert!( + !matches!(&options.page, Page::Numeric(_)), + "?page= is not supported" + ); + + let make_base_query = || { + GitHubConfig::query() + .filter(trustpub_configs_github::crate_id.eq(crate_id)) + .into_boxed() + }; + + let mut query = make_base_query(); + query = query.limit(options.per_page); + query = query.order(trustpub_configs_github::id.asc()); + + if let Some(SeekPayload::Id(Id { id })) = seek.after(&options.page)? { + query = query.filter(trustpub_configs_github::id.gt(id)); + } + + let data: Vec = query.load(conn).await?; + + let next_page = next_seek_params(&data, options, |last| seek.to_payload(last))? + .map(|p| req.query_with_params(p)); + + // Avoid the count query if we're on the first page and got fewer results than requested + let total = + if matches!(options.page, Page::Unspecified) && data.len() < options.per_page as usize { + data.len() as i64 + } else { + make_base_query().count().get_result(conn).await? + }; + + Ok((data, total, next_page)) +} + +fn next_seek_params( + records: &[T], + options: &PaginationOptions, + f: F, +) -> AppResult>> +where + F: Fn(&T) -> S, + S: serde::Serialize, +{ + if records.len() < options.per_page as usize { + return Ok(None); + } + + let seek = f(records.last().unwrap()); + let mut opts = IndexMap::new(); + opts.insert("seek".into(), encode_seek(seek)?); + Ok(Some(opts)) +} + +mod seek { + use crate::controllers::helpers::pagination::seek; + use crates_io_database::models::trustpub::GitHubConfig; + + seek!( + pub enum Seek { + Id { id: i32 }, + } + ); + + impl Seek { + pub(crate) fn to_payload(&self, record: &GitHubConfig) -> SeekPayload { + match *self { + Seek::Id => SeekPayload::Id(Id { id: record.id }), + } + } + } } diff --git a/src/tests/routes/trustpub/github_configs/list.rs b/src/tests/routes/trustpub/github_configs/list.rs index 1f0bd667a89..4b36cd61fc7 100644 --- a/src/tests/routes/trustpub/github_configs/list.rs +++ b/src/tests/routes/trustpub/github_configs/list.rs @@ -257,3 +257,67 @@ async fn test_token_auth_with_wildcard_crate_scope() -> anyhow::Result<()> { Ok(()) } + +#[tokio::test(flavor = "multi_thread")] +async fn test_pagination() -> anyhow::Result<()> { + let (app, _, cookie_client) = TestApp::full().with_user().await; + let mut conn = app.db_conn().await; + + let owner_id = cookie_client.as_model().id; + let krate = CrateBuilder::new("foo", owner_id).build(&mut conn).await?; + + // Create 15 configs + for i in 0..15 { + create_config(&mut conn, krate.id, &format!("repo-{i}")).await?; + } + + // Request first page with per_page=5 + let response = cookie_client + .get_with_query::<()>(URL, "crate=foo&per_page=5") + .await; + assert_snapshot!(response.status(), @"200 OK"); + let json = response.json(); + assert_json_snapshot!(json, { + ".github_configs[].created_at" => "[datetime]", + }); + + // Extract the next_page URL and make a second request + let next_page = json["meta"]["next_page"] + .as_str() + .expect("next_page should be present"); + let next_url = format!("{}{}", URL, next_page); + let response = cookie_client.get::<()>(&next_url).await; + assert_snapshot!(response.status(), @"200 OK"); + let json = response.json(); + assert_json_snapshot!(json, { + ".github_configs[].created_at" => "[datetime]", + }); + + // Third page (last page with data) + let next_page = json["meta"]["next_page"] + .as_str() + .expect("next_page should be present"); + let next_url = format!("{}{}", URL, next_page); + let response = cookie_client.get::<()>(&next_url).await; + assert_snapshot!(response.status(), @"200 OK"); + let json = response.json(); + assert_json_snapshot!(json, { + ".github_configs[].created_at" => "[datetime]", + }); + + // The third page has exactly 5 items, so next_page will be present + // (cursor-based pagination is conservative about indicating more pages) + // Following it should give us an empty fourth page + let next_page = json["meta"]["next_page"] + .as_str() + .expect("next_page should be present on third page"); + let next_url = format!("{}{}", URL, next_page); + let response = cookie_client.get::<()>(&next_url).await; + assert_snapshot!(response.status(), @"200 OK"); + let json = response.json(); + assert_json_snapshot!(json, { + ".github_configs[].created_at" => "[datetime]", + }); + + Ok(()) +} diff --git a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__crate_with_no_configs-2.snap b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__crate_with_no_configs-2.snap index 1cc9b449789..55fec0837ab 100644 --- a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__crate_with_no_configs-2.snap +++ b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__crate_with_no_configs-2.snap @@ -3,5 +3,9 @@ source: src/tests/routes/trustpub/github_configs/list.rs expression: response.json() --- { - "github_configs": [] + "github_configs": [], + "meta": { + "next_page": null, + "total": 0 + } } diff --git a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__happy_path-2.snap b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__happy_path-2.snap index 244aafc8b46..71df96ec96a 100644 --- a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__happy_path-2.snap +++ b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__happy_path-2.snap @@ -24,5 +24,9 @@ expression: response.json() "repository_owner_id": 42, "workflow_filename": "publish.yml" } - ] + ], + "meta": { + "next_page": null, + "total": 2 + } } diff --git a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__happy_path-4.snap b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__happy_path-4.snap index 1183786b30b..1961c89eae6 100644 --- a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__happy_path-4.snap +++ b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__happy_path-4.snap @@ -14,5 +14,9 @@ expression: response.json() "repository_owner_id": 42, "workflow_filename": "publish.yml" } - ] + ], + "meta": { + "next_page": null, + "total": 1 + } } diff --git a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__legacy_token_auth-2.snap b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__legacy_token_auth-2.snap index 9daedb86795..91485a5c920 100644 --- a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__legacy_token_auth-2.snap +++ b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__legacy_token_auth-2.snap @@ -14,5 +14,9 @@ expression: response.json() "repository_owner_id": 42, "workflow_filename": "publish.yml" } - ] + ], + "meta": { + "next_page": null, + "total": 1 + } } diff --git a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-2.snap b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-2.snap new file mode 100644 index 00000000000..f3d7bb1e868 --- /dev/null +++ b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-2.snap @@ -0,0 +1,62 @@ +--- +source: src/tests/routes/trustpub/github_configs/list.rs +expression: json +--- +{ + "github_configs": [ + { + "crate": "foo", + "created_at": "[datetime]", + "environment": null, + "id": 1, + "repository_name": "repo-0", + "repository_owner": "rust-lang", + "repository_owner_id": 42, + "workflow_filename": "publish.yml" + }, + { + "crate": "foo", + "created_at": "[datetime]", + "environment": null, + "id": 2, + "repository_name": "repo-1", + "repository_owner": "rust-lang", + "repository_owner_id": 42, + "workflow_filename": "publish.yml" + }, + { + "crate": "foo", + "created_at": "[datetime]", + "environment": null, + "id": 3, + "repository_name": "repo-2", + "repository_owner": "rust-lang", + "repository_owner_id": 42, + "workflow_filename": "publish.yml" + }, + { + "crate": "foo", + "created_at": "[datetime]", + "environment": null, + "id": 4, + "repository_name": "repo-3", + "repository_owner": "rust-lang", + "repository_owner_id": 42, + "workflow_filename": "publish.yml" + }, + { + "crate": "foo", + "created_at": "[datetime]", + "environment": null, + "id": 5, + "repository_name": "repo-4", + "repository_owner": "rust-lang", + "repository_owner_id": 42, + "workflow_filename": "publish.yml" + } + ], + "meta": { + "next_page": "?crate=foo&per_page=5&seek=NQ", + "total": 15 + } +} diff --git a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-4.snap b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-4.snap new file mode 100644 index 00000000000..d114b81bd38 --- /dev/null +++ b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-4.snap @@ -0,0 +1,62 @@ +--- +source: src/tests/routes/trustpub/github_configs/list.rs +expression: json +--- +{ + "github_configs": [ + { + "crate": "foo", + "created_at": "[datetime]", + "environment": null, + "id": 6, + "repository_name": "repo-5", + "repository_owner": "rust-lang", + "repository_owner_id": 42, + "workflow_filename": "publish.yml" + }, + { + "crate": "foo", + "created_at": "[datetime]", + "environment": null, + "id": 7, + "repository_name": "repo-6", + "repository_owner": "rust-lang", + "repository_owner_id": 42, + "workflow_filename": "publish.yml" + }, + { + "crate": "foo", + "created_at": "[datetime]", + "environment": null, + "id": 8, + "repository_name": "repo-7", + "repository_owner": "rust-lang", + "repository_owner_id": 42, + "workflow_filename": "publish.yml" + }, + { + "crate": "foo", + "created_at": "[datetime]", + "environment": null, + "id": 9, + "repository_name": "repo-8", + "repository_owner": "rust-lang", + "repository_owner_id": 42, + "workflow_filename": "publish.yml" + }, + { + "crate": "foo", + "created_at": "[datetime]", + "environment": null, + "id": 10, + "repository_name": "repo-9", + "repository_owner": "rust-lang", + "repository_owner_id": 42, + "workflow_filename": "publish.yml" + } + ], + "meta": { + "next_page": "?crate=foo&per_page=5&seek=MTA", + "total": 15 + } +} diff --git a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-6.snap b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-6.snap new file mode 100644 index 00000000000..c8583daf4bf --- /dev/null +++ b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-6.snap @@ -0,0 +1,62 @@ +--- +source: src/tests/routes/trustpub/github_configs/list.rs +expression: json +--- +{ + "github_configs": [ + { + "crate": "foo", + "created_at": "[datetime]", + "environment": null, + "id": 11, + "repository_name": "repo-10", + "repository_owner": "rust-lang", + "repository_owner_id": 42, + "workflow_filename": "publish.yml" + }, + { + "crate": "foo", + "created_at": "[datetime]", + "environment": null, + "id": 12, + "repository_name": "repo-11", + "repository_owner": "rust-lang", + "repository_owner_id": 42, + "workflow_filename": "publish.yml" + }, + { + "crate": "foo", + "created_at": "[datetime]", + "environment": null, + "id": 13, + "repository_name": "repo-12", + "repository_owner": "rust-lang", + "repository_owner_id": 42, + "workflow_filename": "publish.yml" + }, + { + "crate": "foo", + "created_at": "[datetime]", + "environment": null, + "id": 14, + "repository_name": "repo-13", + "repository_owner": "rust-lang", + "repository_owner_id": 42, + "workflow_filename": "publish.yml" + }, + { + "crate": "foo", + "created_at": "[datetime]", + "environment": null, + "id": 15, + "repository_name": "repo-14", + "repository_owner": "rust-lang", + "repository_owner_id": 42, + "workflow_filename": "publish.yml" + } + ], + "meta": { + "next_page": "?crate=foo&per_page=5&seek=MTU", + "total": 15 + } +} diff --git a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-8.snap b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-8.snap new file mode 100644 index 00000000000..e5444aa1d21 --- /dev/null +++ b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__pagination-8.snap @@ -0,0 +1,11 @@ +--- +source: src/tests/routes/trustpub/github_configs/list.rs +expression: json +--- +{ + "github_configs": [], + "meta": { + "next_page": null, + "total": 15 + } +} diff --git a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__token_auth_with_trusted_publishing_scope-2.snap b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__token_auth_with_trusted_publishing_scope-2.snap index 9daedb86795..91485a5c920 100644 --- a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__token_auth_with_trusted_publishing_scope-2.snap +++ b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__token_auth_with_trusted_publishing_scope-2.snap @@ -14,5 +14,9 @@ expression: response.json() "repository_owner_id": 42, "workflow_filename": "publish.yml" } - ] + ], + "meta": { + "next_page": null, + "total": 1 + } } diff --git a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__token_auth_with_wildcard_crate_scope-2.snap b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__token_auth_with_wildcard_crate_scope-2.snap index 9daedb86795..91485a5c920 100644 --- a/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__token_auth_with_wildcard_crate_scope-2.snap +++ b/src/tests/routes/trustpub/github_configs/snapshots/integration__routes__trustpub__github_configs__list__token_auth_with_wildcard_crate_scope-2.snap @@ -14,5 +14,9 @@ expression: response.json() "repository_owner_id": 42, "workflow_filename": "publish.yml" } - ] + ], + "meta": { + "next_page": null, + "total": 1 + } } diff --git a/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap b/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap index 1fe04ce2381..4848238b292 100644 --- a/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap +++ b/src/tests/snapshots/integration__openapi__openapi_snapshot-2.snap @@ -4212,6 +4212,37 @@ expression: response.json() "schema": { "type": "string" } + }, + { + "description": "The page number to request.\n\nThis parameter is mutually exclusive with `seek` and not supported for\nall requests.", + "in": "query", + "name": "page", + "required": false, + "schema": { + "format": "int32", + "minimum": 1, + "type": "integer" + } + }, + { + "description": "The number of items to request per page.", + "in": "query", + "name": "per_page", + "required": false, + "schema": { + "format": "int32", + "minimum": 1, + "type": "integer" + } + }, + { + "description": "The seek key to request.\n\nThis parameter is mutually exclusive with `page` and not supported for\nall requests.\n\nThe seek key can usually be found in the `meta.next_page` field of\npaginated responses.", + "in": "query", + "name": "seek", + "required": false, + "schema": { + "type": "string" + } } ], "responses": { @@ -4225,10 +4256,33 @@ expression: response.json() "$ref": "#/components/schemas/GitHubConfig" }, "type": "array" + }, + "meta": { + "properties": { + "next_page": { + "description": "Query string to the next page of results, if any.", + "example": "?seek=abc123", + "type": [ + "string", + "null" + ] + }, + "total": { + "description": "The total number of GitHub configs belonging to the crate.", + "example": 42, + "format": "int64", + "type": "integer" + } + }, + "required": [ + "total" + ], + "type": "object" } }, "required": [ - "github_configs" + "github_configs", + "meta" ], "type": "object" }