From 065b11a99d9458bff1d701f869f518bb35bb65d0 Mon Sep 17 00:00:00 2001 From: atlas Date: Thu, 24 Sep 2026 11:59:50 +0200 Subject: [PATCH] swarm-controller: page repo_search past forgejo's first page list_repos_with_open_issues only read one repo_search page (forgejo's 30-row default), so a repo past position 30 in the default alpha sort silently dropped out of the issue-report/repo-dropdown data source, with no truncation signal. The comment claiming RepoSearchQuery has no page field was wrong -- Request::page()/page_size() are generic builder methods independent of the query struct. Adds page_search_results, a small paging loop over a fetch closure for search-shaped (data: Option>, no header) responses that can't use the existing .all() helper (that needs a (Headers, Vec) response shape). Pages until a short page or a 40-page bound, erroring on the bound rather than truncating again. Closes #4675 --- swarm-controller/src/forge.rs | 130 +++++++++++++++++++++++++++++----- 1 file changed, 113 insertions(+), 17 deletions(-) diff --git a/swarm-controller/src/forge.rs b/swarm-controller/src/forge.rs index 23bbdf22..cdfb0540 100644 --- a/swarm-controller/src/forge.rs +++ b/swarm-controller/src/forge.rs @@ -601,30 +601,68 @@ impl Client { Ok(out) } + /// Pages a search-shaped result (`data: Option>`, no total-count + /// header) to exhaustion via `fetch`, which must issue one request per + /// `page` (1-based) at `page_size` and return its `data`. `repo_search`'s + /// `SearchResults` is this shape, not the `(Headers, Vec)` shape the + /// `.all()` helper on `Request` requires, so it can't use that helper — + /// this is its manual equivalent. Stops at the first page shorter than + /// `page_size` (forgejo's own last-page signal); a page exactly + /// `page_size` long always triggers one more fetch to confirm there + /// isn't a further page. Errors instead of returning a partial result if + /// `max_pages` is reached without a short page, so a misbehaving or + /// unexpectedly huge instance fails loudly rather than looping forever + /// or silently truncating again. + async fn page_search_results( + page_size: u32, + max_pages: u32, + mut fetch: F, + ) -> Result> + where + F: FnMut(u32) -> Fut, + Fut: std::future::Future>>>, + { + let mut out = Vec::new(); + for page in 1..=max_pages { + let rows = fetch(page).await?.unwrap_or_default(); + let got = rows.len(); + out.extend(rows); + if got < page_size as usize { + return Ok(out); + } + } + anyhow::bail!( + "search paging hit the {max_pages}-page bound (page size {page_size}) without a \ + short page — the forge may hold more results than this scan can safely enumerate" + ) + } + /// Repos with at least one open issue, as `owner/name` full names — /// the data source for swarm-ui's repo-filter dropdown, which lists /// only repos that currently have open issues rather than every repo /// the forge hosts. Filtered on `Repository::open_issues_count` from /// the search result itself rather than a follow-up /// `issue_list_issues` call per repo — forgejo's own repo summary - /// already carries that count, so this stays one request regardless - /// of how many repos exist. - /// - /// One search page (forgejo's own default page size) — this binding's - /// `RepoSearchQuery` has no `page`/`limit` field to page through, unlike - /// the `(Headers, Vec)`-shaped list endpoints elsewhere in this file - /// that `.all()` fully drains. Fine for admin-facing tooling at today's - /// repo count; revisit if an instance's repo count ever outgrows one - /// page. + /// already carries that count, so this stays one request per page + /// regardless of how many repos exist. pub async fn list_repos_with_open_issues(&self) -> Result> { - let results = self - .api - .repo_search(RepoSearchQuery::default()) - .await - .context("search repos")?; - Ok(results - .data - .unwrap_or_default() + const PAGE_SIZE: u32 = 50; + // 40 pages at 50/page is 2000 repos — comfortably past any repo + // count this instance has today; see `page_search_results` for why + // hitting it is an error rather than a truncated result. + const MAX_PAGES: u32 = 40; + let repos = Self::page_search_results(PAGE_SIZE, MAX_PAGES, |page| async move { + Ok(self + .api + .repo_search(RepoSearchQuery::default()) + .page(page) + .page_size(PAGE_SIZE) + .await + .context("search repos")? + .data) + }) + .await?; + Ok(repos .into_iter() .filter(|r| r.open_issues_count.unwrap_or(0) > 0) .filter_map(|r| r.full_name) @@ -1171,6 +1209,64 @@ mod tests { } } + #[tokio::test] + async fn page_search_results_pages_past_a_full_first_page() { + #[derive(Clone)] + struct FakeRepo { + full_name: String, + open_issues: u32, + } + + // Page 1 is exactly PAGE_SIZE long, which must not be mistaken for + // the last page — a repo living on page 2 still has to turn up. + const PAGE_SIZE: u32 = 2; + let pages = vec![ + vec![ + FakeRepo { + full_name: "org/a".to_owned(), + open_issues: 0, + }, + FakeRepo { + full_name: "org/b".to_owned(), + open_issues: 0, + }, + ], + vec![FakeRepo { + full_name: "org/c".to_owned(), + open_issues: 3, + }], + ]; + let repos = Client::page_search_results(PAGE_SIZE, 10, |page| { + let pages = pages.clone(); + async move { Ok(pages.get(page as usize - 1).cloned()) } + }) + .await + .unwrap(); + + let with_open_issues: Vec<_> = repos + .into_iter() + .filter(|r| r.open_issues > 0) + .map(|r| r.full_name) + .collect(); + assert_eq!(with_open_issues, vec!["org/c".to_owned()]); + } + + #[tokio::test] + async fn page_search_results_errors_instead_of_looping_forever() { + // A server that never returns a short page (misbehaving, or an + // instance that genuinely outgrew `max_pages`) must fail loudly + // rather than paging forever or handing back a partial result. + const PAGE_SIZE: u32 = 2; + let result = + Client::page_search_results( + PAGE_SIZE, + 3, + |_page| async move { Ok(Some(vec![1u32, 2])) }, + ) + .await; + assert!(result.is_err()); + } + #[test] fn transitive_reach_walks_a_chain_not_just_the_direct_edge() { // 3 depends on 2 depends on 1: reverse_adj is "who depends on me",