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<Vec<T>>, no header) responses that can't use the existing .all() helper (that needs a (Headers, Vec<T>) response shape). Pages until a short page or a 40-page bound, erroring on the bound rather than truncating again. Closes #4675
This commit is contained in:
parent
a5259146dc
commit
065b11a99d
1 changed files with 113 additions and 17 deletions
|
|
@ -601,30 +601,68 @@ impl Client {
|
||||||
Ok(out)
|
Ok(out)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Pages a search-shaped result (`data: Option<Vec<T>>`, 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<T>)` 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<T, F, Fut>(
|
||||||
|
page_size: u32,
|
||||||
|
max_pages: u32,
|
||||||
|
mut fetch: F,
|
||||||
|
) -> Result<Vec<T>>
|
||||||
|
where
|
||||||
|
F: FnMut(u32) -> Fut,
|
||||||
|
Fut: std::future::Future<Output = Result<Option<Vec<T>>>>,
|
||||||
|
{
|
||||||
|
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 —
|
/// Repos with at least one open issue, as `owner/name` full names —
|
||||||
/// the data source for swarm-ui's repo-filter dropdown, which lists
|
/// the data source for swarm-ui's repo-filter dropdown, which lists
|
||||||
/// only repos that currently have open issues rather than every repo
|
/// only repos that currently have open issues rather than every repo
|
||||||
/// the forge hosts. Filtered on `Repository::open_issues_count` from
|
/// the forge hosts. Filtered on `Repository::open_issues_count` from
|
||||||
/// the search result itself rather than a follow-up
|
/// the search result itself rather than a follow-up
|
||||||
/// `issue_list_issues` call per repo — forgejo's own repo summary
|
/// `issue_list_issues` call per repo — forgejo's own repo summary
|
||||||
/// already carries that count, so this stays one request regardless
|
/// already carries that count, so this stays one request per page
|
||||||
/// of how many repos exist.
|
/// 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<T>)`-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.
|
|
||||||
pub async fn list_repos_with_open_issues(&self) -> Result<Vec<String>> {
|
pub async fn list_repos_with_open_issues(&self) -> Result<Vec<String>> {
|
||||||
let results = self
|
const PAGE_SIZE: u32 = 50;
|
||||||
.api
|
// 40 pages at 50/page is 2000 repos — comfortably past any repo
|
||||||
.repo_search(RepoSearchQuery::default())
|
// count this instance has today; see `page_search_results` for why
|
||||||
.await
|
// hitting it is an error rather than a truncated result.
|
||||||
.context("search repos")?;
|
const MAX_PAGES: u32 = 40;
|
||||||
Ok(results
|
let repos = Self::page_search_results(PAGE_SIZE, MAX_PAGES, |page| async move {
|
||||||
.data
|
Ok(self
|
||||||
.unwrap_or_default()
|
.api
|
||||||
|
.repo_search(RepoSearchQuery::default())
|
||||||
|
.page(page)
|
||||||
|
.page_size(PAGE_SIZE)
|
||||||
|
.await
|
||||||
|
.context("search repos")?
|
||||||
|
.data)
|
||||||
|
})
|
||||||
|
.await?;
|
||||||
|
Ok(repos
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.filter(|r| r.open_issues_count.unwrap_or(0) > 0)
|
.filter(|r| r.open_issues_count.unwrap_or(0) > 0)
|
||||||
.filter_map(|r| r.full_name)
|
.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]
|
#[test]
|
||||||
fn transitive_reach_walks_a_chain_not_just_the_direct_edge() {
|
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",
|
// 3 depends on 2 depends on 1: reverse_adj is "who depends on me",
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue