From ba2c5c2d02f3b25307fbd6b91b71cd11d0373793 Mon Sep 17 00:00:00 2001 From: Austin Schaefer Date: Sun, 20 Sep 2026 10:57:14 +0200 Subject: [PATCH] Make release sources polymorphic via a ReleaseSource trait Replaces the Endpoints enum and checker's per-host dispatch: checker.rs now holds only the ReleaseSource trait (latest release + API root), each host implements it in its own module (github.rs, forgejo.rs), and source.rs is a factory returning a Box per package. Adding a host no longer touches existing ones, and the pipeline only sees the trait. Co-Authored-By: Claude Sonnet 5 --- docs/ARCHITECTURE.md | 9 +- docs/SPEC.md | 19 ++-- src/checker.rs | 250 ++++--------------------------------------- src/fetcher.rs | 2 +- src/forgejo.rs | 112 +++++++++++++++++++ src/github.rs | 137 ++++++++++++++++++++++++ src/main.rs | 1 + src/pipeline.rs | 48 +++++---- src/source.rs | 74 ++++--------- 9 files changed, 336 insertions(+), 316 deletions(-) create mode 100644 src/forgejo.rs diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index c42f9ad..4440080 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -61,8 +61,9 @@ practice rather than asserted from habit — see Further reading. Same testability goal as #3, applied to the specific ways this program reaches outside itself. A consistent shape beats ad hoc mocking invented per call site. - **Already in force**: `source::Endpoints` (checker), the plain API root - it hands to fetcher/verifier, `repo_add_bin` and the pacman.conf path (publisher), `PKGWATCH_REPO_DIR` + **Already in force**: `GithubEndpoints`/`ForgejoEndpoints` (the + `ReleaseSource` implementations, whose API root is what fetcher/verifier + take), `repo_add_bin` and the pacman.conf path (publisher), `PKGWATCH_REPO_DIR` (main, for manual dry runs against a scratch repo instead of the real one). A new external call follows the same shape: production code calls a thin wrapper with the real default; tests call the parameterized @@ -106,8 +107,8 @@ practice rather than asserted from habit — see Further reading. directory, but the same information — why this way and not the obvious alternative — needs to live somewhere a future reader will actually see it: the doc comment on the thing itself. - **Example already here**: `checker.rs`'s doc comment on - `latest_github_release` explains why the newest Atom-feed entry isn't + **Example already here**: `github.rs`'s doc comment on + `GithubEndpoints::latest_release` explains why the newest Atom-feed entry isn't trusted outright (scaleway-cli's `-dbg1` tag has no real Release behind it) — the reasoning lives right next to the code it justifies, not in a commit message or a separate design doc no one will find later. diff --git a/docs/SPEC.md b/docs/SPEC.md index 220d05c..b3262bd 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -287,7 +287,7 @@ implements `repo`, `asset_pattern`, and `verification.method` matches the line by filename instead of assuming a single-hash file. - Separately, scaleway-cli's Atom feed lists a `vX.Y.Z-dbg1` tag newest, with no real Release object behind it (`releases/tags/` 404s) — - `checker::latest_github_release` now confirms each feed candidate + the GitHub source's `latest_release` now confirms each feed candidate against the releases API in feed order rather than trusting the first entry outright. @@ -373,10 +373,15 @@ Open questions on the schema: likely reuses `nvchecker`'s logic/sources conceptually for non-GitHub sources eventually. For GitHub sources, prefers the `github-atom` feed (see Scaling > Check method) over unconditional REST polling. - *(Implemented for GitHub and Forgejo — `src/checker.rs`. GitHub - regex-matches the first `releases/tag/` link in the feed rather than - doing a full XML parse; fine while the feed's newest-entry-first shape - holds, revisit if that ever changes. Forgejo is one call to + *(Implemented for GitHub and Forgejo. `src/checker.rs` defines the + `ReleaseSource` trait (latest release + API root); each host implements + it in its own module (`github.rs`, `forgejo.rs`), and `source.rs` picks + one per package, so adding a host doesn't touch existing ones. A trait + rather than an enum match because there are now two real hosts with + genuinely different logic. GitHub regex-matches the first + `releases/tag/` link in the feed rather than doing a full XML + parse; fine while the feed's newest-entry-first shape holds, revisit if + that ever changes. Forgejo is one call to `/api/v1/repos//releases/latest`, which already returns only the newest non-draft, non-prerelease release, so it needs none of GitHub's confirm-each-tag step. `check_interval`/per-package cadence not @@ -519,8 +524,8 @@ Open questions on the schema: confirmed correct — no build-provenance attestations upstream. Required adding `{version}`-placeholder support to `asset_pattern`/ `checksum_asset_pattern`, filename-matched parsing of combined - multi-asset checksum files, and having `latest_github_release` - confirm each Atom-feed candidate against the releases API (this + multi-asset checksum files, and having the GitHub source's + `latest_release` confirm each Atom-feed candidate against the releases API (this repo's newest feed entry, a `-dbg1` tag, has no real Release behind it). Still just flags for human review, same as any tier 4-6 pass — not auto-installed; see the unchecked build/publish item below. diff --git a/src/checker.rs b/src/checker.rs index 4f2057e..33aa612 100644 --- a/src/checker.rs +++ b/src/checker.rs @@ -1,89 +1,24 @@ -use crate::github::GithubEndpoints; -use crate::source::Endpoints; -use anyhow::{Context, Result, bail}; -use regex::Regex; -use serde::Deserialize; +//! The check stage's contract: a `ReleaseSource` says what a package's +//! latest release is and where its releases API lives, and +//! `version_from_tag` turns the resulting tag into a version string. Each +//! hosting service implements the trait in its own module (`github`, +//! `forgejo`) so per-host logic doesn't accumulate here; +//! `source::for_package` picks the implementation for a package. -/// Resolves the latest release tag for `repo` on whichever service -/// `endpoints` points at. -pub fn latest_release( - client: &reqwest::blocking::Client, - endpoints: &Endpoints, - repo: &str, -) -> Result { - match endpoints { - Endpoints::Github(github) => latest_github_release(client, github, repo), - Endpoints::Forgejo { api } => latest_forgejo_release(client, api, repo), - } -} +use anyhow::Result; -/// Resolves the latest release tag for `repo` on a Forgejo/Gitea instance. -/// -/// One call, unlike GitHub's feed-then-confirm dance below: Forgejo's -/// `releases/latest` already returns only the newest non-draft, -/// non-prerelease *release object*, so a stray tag with no release behind -/// it (GitHub's scaleway-cli `-dbg1` problem) can't be returned. -pub fn latest_forgejo_release( - client: &reqwest::blocking::Client, - api: &str, - repo: &str, -) -> Result { - #[derive(Deserialize)] - struct Latest { - tag_name: String, - } +/// A hosting service a package's releases are published on. `Debug` so a +/// `Box` can sit in a `Result` that tests unwrap. +pub trait ReleaseSource: std::fmt::Debug { + /// The latest release tag for `repo`. + fn latest_release(&self, client: &reqwest::blocking::Client, repo: &str) -> Result; - let url = format!("{api}/repos/{repo}/releases/latest"); - let response = client.get(&url).send()?; - // Forgejo answers 404 both for an unknown repo and for one with no - // releases yet — the common state for a project's very first release. - if response.status() == reqwest::StatusCode::NOT_FOUND { - bail!("no published release found at {url} (repo missing, or nothing released yet)"); - } - let latest: Latest = response - .error_for_status() - .with_context(|| format!("fetching latest release from {url}"))? - .json()?; - Ok(latest.tag_name) -} - -/// Resolves the latest release tag for `repo` via its public Atom feed. -/// -/// Deliberately not a full XML parse: the feed lists entries newest-first, -/// and each `"/>` is matched -/// in document order. Revisit with a real XML parser if GitHub's feed -/// shape ever changes. -/// -/// The feed can list a tag newer than any tag with a real Release object -/// behind it — observed on scaleway/scaleway-cli, which pushes a -/// `vX.Y.Z-dbg1` tag (no corresponding Release; `releases/tags/` -/// 404s) right after each real release, and that tag sorts newest in the -/// feed. So each candidate is confirmed against the releases API in feed -/// order, returning the first that actually resolves. -pub fn latest_github_release( - client: &reqwest::blocking::Client, - endpoints: &GithubEndpoints, - repo: &str, -) -> Result { - let url = format!("{}/{repo}/releases.atom", endpoints.web); - let body = client.get(&url).send()?.error_for_status()?.text()?; - - let re = Regex::new(r#"releases/tag/([^"]+)""#)?; - let mut candidates = re - .captures_iter(&body) - .map(|caps| caps[1].to_string()) - .peekable(); - if candidates.peek().is_none() { - bail!("no release tag found in {url}"); - } - - for tag in candidates { - let release_url = format!("{}/repos/{repo}/releases/tags/{tag}", endpoints.api); - if client.get(&release_url).send()?.status().is_success() { - return Ok(tag); - } - } - bail!("no release tag in {url} resolved to a real release via the API") + /// The releases API root, which `fetcher` and `verifier` build their + /// own paths under. GitHub and Forgejo both serve + /// `/repos/{owner}/{repo}/releases/tags/{tag}` there with the same + /// `assets[].{name, browser_download_url}` shape, which is why those + /// stages need only this and not the source itself. + fn api(&self) -> &str; } /// Strips a leading `v` from a release tag, e.g. `v2.62.0` -> `2.62.0`. @@ -110,153 +45,4 @@ mod tests { fn version_from_tag_leaves_bare_version_unchanged() { assert_eq!(version_from_tag("0.12.15"), "0.12.15"); } - - fn atom_feed(tags: &[&str]) -> String { - let entries: String = tags - .iter() - .map(|t| { - format!(r#""#) - }) - .collect(); - format!("{entries}") - } - - #[test] - fn latest_github_release_skips_tags_with_no_real_release() { - let mut server = mockito::Server::new(); - let endpoints = GithubEndpoints { - web: server.url(), - api: server.url(), - }; - - // Mirrors the real scaleway-cli case: newest feed entry (a -dbg1 - // tag) has no Release object behind it and 404s. - let _feed = server - .mock("GET", "/o/r/releases.atom") - .with_status(200) - .with_body(atom_feed(&["v2.62.0-dbg1", "v2.62.0"])) - .create(); - let _missing = server - .mock("GET", "/repos/o/r/releases/tags/v2.62.0-dbg1") - .with_status(404) - .create(); - let _real = server - .mock("GET", "/repos/o/r/releases/tags/v2.62.0") - .with_status(200) - .with_body("{}") - .create(); - - let client = reqwest::blocking::Client::new(); - let tag = latest_github_release(&client, &endpoints, "o/r").unwrap(); - assert_eq!(tag, "v2.62.0"); - } - - #[test] - fn latest_github_release_errors_when_feed_has_no_tags() { - let mut server = mockito::Server::new(); - let endpoints = GithubEndpoints { - web: server.url(), - api: server.url(), - }; - let _feed = server - .mock("GET", "/o/r/releases.atom") - .with_status(200) - .with_body("") - .create(); - - let client = reqwest::blocking::Client::new(); - let err = latest_github_release(&client, &endpoints, "o/r").unwrap_err(); - assert!(err.to_string().contains("no release tag found")); - } - - #[test] - fn latest_github_release_errors_when_no_candidate_resolves() { - let mut server = mockito::Server::new(); - let endpoints = GithubEndpoints { - web: server.url(), - api: server.url(), - }; - let _feed = server - .mock("GET", "/o/r/releases.atom") - .with_status(200) - .with_body(atom_feed(&["v1.0.0-dbg1"])) - .create(); - let _missing = server - .mock("GET", "/repos/o/r/releases/tags/v1.0.0-dbg1") - .with_status(404) - .create(); - - let client = reqwest::blocking::Client::new(); - let err = latest_github_release(&client, &endpoints, "o/r").unwrap_err(); - assert!(err.to_string().contains("resolved to a real release")); - } - - #[test] - fn latest_forgejo_release_returns_tag_name() { - let mut server = mockito::Server::new(); - let _latest = server - .mock("GET", "/repos/o/r/releases/latest") - .with_status(200) - .with_body(r#"{"tag_name": "v0.1.0", "assets": []}"#) - .create(); - - let client = reqwest::blocking::Client::new(); - let tag = latest_forgejo_release(&client, &server.url(), "o/r").unwrap(); - assert_eq!(tag, "v0.1.0"); - } - - #[test] - fn latest_forgejo_release_names_the_no_releases_case() { - let mut server = mockito::Server::new(); - let _latest = server - .mock("GET", "/repos/o/r/releases/latest") - .with_status(404) - .create(); - - let client = reqwest::blocking::Client::new(); - let err = latest_forgejo_release(&client, &server.url(), "o/r").unwrap_err(); - assert!(err.to_string().contains("no published release")); - } - - #[test] - fn latest_forgejo_release_surfaces_server_errors() { - let mut server = mockito::Server::new(); - let _latest = server - .mock("GET", "/repos/o/r/releases/latest") - .with_status(500) - .create(); - - let client = reqwest::blocking::Client::new(); - let err = latest_forgejo_release(&client, &server.url(), "o/r").unwrap_err(); - assert!(err.to_string().contains("fetching latest release")); - } - - #[test] - fn latest_release_dispatches_on_endpoint_kind() { - let mut server = mockito::Server::new(); - let _forgejo = server - .mock("GET", "/repos/o/r/releases/latest") - .with_status(200) - .with_body(r#"{"tag_name": "v2.0.0"}"#) - .create(); - let _feed = server - .mock("GET", "/o/r/releases.atom") - .with_body(atom_feed(&["v1.0.0"])) - .create(); - let _release = server - .mock("GET", "/repos/o/r/releases/tags/v1.0.0") - .with_status(200) - .with_body("{}") - .create(); - - let client = reqwest::blocking::Client::new(); - let forgejo = Endpoints::Forgejo { api: server.url() }; - assert_eq!(latest_release(&client, &forgejo, "o/r").unwrap(), "v2.0.0"); - - let github = Endpoints::Github(GithubEndpoints { - web: server.url(), - api: server.url(), - }); - assert_eq!(latest_release(&client, &github, "o/r").unwrap(), "v1.0.0"); - } } diff --git a/src/fetcher.rs b/src/fetcher.rs index 4770ac8..3fbafaa 100644 --- a/src/fetcher.rs +++ b/src/fetcher.rs @@ -29,7 +29,7 @@ pub struct DownloadedAsset { /// Downloads the release asset named exactly `asset_name` for `repo`@`tag` /// into `dest_dir`, returning the local path and its origin URL. `api` is -/// the releases API root (see `source::Endpoints::api`); GitHub and Forgejo +/// the releases API root (see `ReleaseSource::api`); GitHub and Forgejo /// serve the same endpoint and JSON shape under it. pub fn download_asset( client: &reqwest::blocking::Client, diff --git a/src/forgejo.rs b/src/forgejo.rs new file mode 100644 index 0000000..af1ba00 --- /dev/null +++ b/src/forgejo.rs @@ -0,0 +1,112 @@ +//! A Forgejo (or Gitea) instance as a release source: its API root, and +//! how to find a repo's latest release there. + +use crate::checker::ReleaseSource; +use anyhow::{Context, Result, bail}; +use serde::Deserialize; + +#[derive(Debug, Clone)] +pub struct ForgejoEndpoints { + /// The instance's API root, i.e. `/api/v1`. + pub api: String, +} + +impl ForgejoEndpoints { + /// `base_url` is the instance's web root, e.g. + /// `https://code.austinschaefer.com`; a trailing slash is tolerated. + pub fn from_base_url(base_url: &str) -> Self { + Self { + api: format!("{}/api/v1", base_url.trim_end_matches('/')), + } + } +} + +impl ReleaseSource for ForgejoEndpoints { + /// One call, unlike GitHub's feed-then-confirm dance: Forgejo's + /// `releases/latest` already returns only the newest non-draft, + /// non-prerelease *release object*, so a stray tag with no release + /// behind it (GitHub's scaleway-cli `-dbg1` problem) can't be returned. + fn latest_release(&self, client: &reqwest::blocking::Client, repo: &str) -> Result { + #[derive(Deserialize)] + struct Latest { + tag_name: String, + } + + let url = format!("{}/repos/{repo}/releases/latest", self.api); + let response = client.get(&url).send()?; + // Forgejo answers 404 both for an unknown repo and for one with no + // releases yet — the common state for a project's very first + // release. + if response.status() == reqwest::StatusCode::NOT_FOUND { + bail!("no published release found at {url} (repo missing, or nothing released yet)"); + } + let latest: Latest = response + .error_for_status() + .with_context(|| format!("fetching latest release from {url}"))? + .json()?; + Ok(latest.tag_name) + } + + fn api(&self) -> &str { + &self.api + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn from_base_url_appends_api_v1() { + let endpoints = ForgejoEndpoints::from_base_url("https://code.example.com"); + assert_eq!(endpoints.api(), "https://code.example.com/api/v1"); + } + + #[test] + fn from_base_url_tolerates_trailing_slash() { + let endpoints = ForgejoEndpoints::from_base_url("https://code.example.com/"); + assert_eq!(endpoints.api(), "https://code.example.com/api/v1"); + } + + #[test] + fn latest_release_returns_tag_name() { + let mut server = mockito::Server::new(); + let _latest = server + .mock("GET", "/repos/o/r/releases/latest") + .with_status(200) + .with_body(r#"{"tag_name": "v0.1.0", "assets": []}"#) + .create(); + + let client = reqwest::blocking::Client::new(); + let endpoints = ForgejoEndpoints { api: server.url() }; + assert_eq!(endpoints.latest_release(&client, "o/r").unwrap(), "v0.1.0"); + } + + #[test] + fn latest_release_names_the_no_releases_case() { + let mut server = mockito::Server::new(); + let _latest = server + .mock("GET", "/repos/o/r/releases/latest") + .with_status(404) + .create(); + + let client = reqwest::blocking::Client::new(); + let endpoints = ForgejoEndpoints { api: server.url() }; + let err = endpoints.latest_release(&client, "o/r").unwrap_err(); + assert!(err.to_string().contains("no published release")); + } + + #[test] + fn latest_release_surfaces_server_errors() { + let mut server = mockito::Server::new(); + let _latest = server + .mock("GET", "/repos/o/r/releases/latest") + .with_status(500) + .create(); + + let client = reqwest::blocking::Client::new(); + let endpoints = ForgejoEndpoints { api: server.url() }; + let err = endpoints.latest_release(&client, "o/r").unwrap_err(); + assert!(err.to_string().contains("fetching latest release")); + } +} diff --git a/src/github.rs b/src/github.rs index a511f16..124349c 100644 --- a/src/github.rs +++ b/src/github.rs @@ -1,3 +1,10 @@ +//! GitHub as a release source: its endpoints, and how to find a repo's +//! latest release there. + +use crate::checker::ReleaseSource; +use anyhow::{Result, bail}; +use regex::Regex; + /// Base URLs for GitHub's public web host (Atom feeds, release pages) and /// its REST API, factored out so tests can point both at a local mock /// server instead of the real github.com/api.github.com. @@ -16,6 +23,47 @@ impl Default for GithubEndpoints { } } +impl ReleaseSource for GithubEndpoints { + /// Resolves the latest release tag for `repo` via its public Atom feed. + /// + /// Deliberately not a full XML parse: the feed lists entries + /// newest-first, and each `"/>` is matched in document order. Revisit + /// with a real XML parser if GitHub's feed shape ever changes. + /// + /// The feed can list a tag newer than any tag with a real Release + /// object behind it — observed on scaleway/scaleway-cli, which pushes a + /// `vX.Y.Z-dbg1` tag (no corresponding Release; `releases/tags/` + /// 404s) right after each real release, and that tag sorts newest in + /// the feed. So each candidate is confirmed against the releases API in + /// feed order, returning the first that actually resolves. + fn latest_release(&self, client: &reqwest::blocking::Client, repo: &str) -> Result { + let url = format!("{}/{repo}/releases.atom", self.web); + let body = client.get(&url).send()?.error_for_status()?.text()?; + + let re = Regex::new(r#"releases/tag/([^"]+)""#)?; + let mut candidates = re + .captures_iter(&body) + .map(|caps| caps[1].to_string()) + .peekable(); + if candidates.peek().is_none() { + bail!("no release tag found in {url}"); + } + + for tag in candidates { + let release_url = format!("{}/repos/{repo}/releases/tags/{tag}", self.api); + if client.get(&release_url).send()?.status().is_success() { + return Ok(tag); + } + } + bail!("no release tag in {url} resolved to a real release via the API") + } + + fn api(&self) -> &str { + &self.api + } +} + #[cfg(test)] mod tests { use super::*; @@ -26,4 +74,93 @@ mod tests { assert_eq!(endpoints.web, "https://github.com"); assert_eq!(endpoints.api, "https://api.github.com"); } + + fn atom_feed(tags: &[&str]) -> String { + let entries: String = tags + .iter() + .map(|t| { + format!(r#""#) + }) + .collect(); + format!("{entries}") + } + + #[test] + fn latest_release_skips_tags_with_no_real_release() { + let mut server = mockito::Server::new(); + let endpoints = GithubEndpoints { + web: server.url(), + api: server.url(), + }; + + // Mirrors the real scaleway-cli case: newest feed entry (a -dbg1 + // tag) has no Release object behind it and 404s. + let _feed = server + .mock("GET", "/o/r/releases.atom") + .with_status(200) + .with_body(atom_feed(&["v2.62.0-dbg1", "v2.62.0"])) + .create(); + let _missing = server + .mock("GET", "/repos/o/r/releases/tags/v2.62.0-dbg1") + .with_status(404) + .create(); + let _real = server + .mock("GET", "/repos/o/r/releases/tags/v2.62.0") + .with_status(200) + .with_body("{}") + .create(); + + let client = reqwest::blocking::Client::new(); + let tag = endpoints.latest_release(&client, "o/r").unwrap(); + assert_eq!(tag, "v2.62.0"); + } + + #[test] + fn latest_release_errors_when_feed_has_no_tags() { + let mut server = mockito::Server::new(); + let endpoints = GithubEndpoints { + web: server.url(), + api: server.url(), + }; + let _feed = server + .mock("GET", "/o/r/releases.atom") + .with_status(200) + .with_body("") + .create(); + + let client = reqwest::blocking::Client::new(); + let err = endpoints.latest_release(&client, "o/r").unwrap_err(); + assert!(err.to_string().contains("no release tag found")); + } + + #[test] + fn latest_release_errors_when_no_candidate_resolves() { + let mut server = mockito::Server::new(); + let endpoints = GithubEndpoints { + web: server.url(), + api: server.url(), + }; + let _feed = server + .mock("GET", "/o/r/releases.atom") + .with_status(200) + .with_body(atom_feed(&["v1.0.0-dbg1"])) + .create(); + let _missing = server + .mock("GET", "/repos/o/r/releases/tags/v1.0.0-dbg1") + .with_status(404) + .create(); + + let client = reqwest::blocking::Client::new(); + let err = endpoints.latest_release(&client, "o/r").unwrap_err(); + assert!(err.to_string().contains("resolved to a real release")); + } + + #[test] + fn api_is_the_configured_api_root() { + let endpoints = GithubEndpoints { + web: "http://w".into(), + api: "http://a".into(), + }; + assert_eq!(endpoints.api(), "http://a"); + } } diff --git a/src/main.rs b/src/main.rs index 9b9255b..3cc70d1 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,6 +6,7 @@ mod builder; mod checker; mod config; mod fetcher; +mod forgejo; mod github; mod hash; mod notifier; diff --git a/src/pipeline.rs b/src/pipeline.rs index 88fa9e7..5bcb61f 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -6,14 +6,14 @@ //! `main.rs`. use crate::builder; -use crate::checker; +use crate::checker::{self, ReleaseSource}; use crate::config::{self, Package}; use crate::fetcher::{self, DownloadedAsset}; use crate::notifier::{self, Event}; use crate::paths::Paths; use crate::publisher; use crate::sanity; -use crate::source::Endpoints; +use crate::source; use crate::state; use crate::verifier::{self, VerificationResult}; use anyhow::{Context, Result, bail}; @@ -72,8 +72,15 @@ pub fn run_check() -> Result<()> { let mut any_failed = false; for (name, pkg) in &packages { println!("== {name} ({}) ==", pkg.repo); - let result = Endpoints::for_package(pkg).and_then(|endpoints| { - process_package(&client, &endpoints, &state_dir, &work_dir, name, pkg) + let result = source::for_package(pkg).and_then(|release_source| { + process_package( + &client, + release_source.as_ref(), + &state_dir, + &work_dir, + name, + pkg, + ) }); if let Err(err) = result { eprintln!(" error: {err:#}"); @@ -120,13 +127,13 @@ fn decide_tier_action(tier: u8, passed: bool, already_pending_this_version: bool fn process_package( client: &reqwest::blocking::Client, - endpoints: &Endpoints, + release_source: &dyn ReleaseSource, state_dir: &Path, work_dir: &Path, name: &str, pkg: &Package, ) -> Result<()> { - let latest = checker::latest_release(client, endpoints, &pkg.repo)?; + let latest = release_source.latest_release(client, &pkg.repo)?; let last_seen = state::load_last_version(state_dir, name); if last_seen.as_deref() == Some(latest.as_str()) { println!(" up to date at {latest}"); @@ -134,7 +141,7 @@ fn process_package( } println!(" new version detected: {latest} (previously: {last_seen:?})"); - let fetched = fetch_and_verify(client, endpoints, work_dir, name, pkg, &latest)?; + let fetched = fetch_and_verify(client, release_source, work_dir, name, pkg, &latest)?; println!(" fetched {}", fetched.asset.path.display()); println!( " verification (tier {}): {} — {}", @@ -207,7 +214,7 @@ struct FetchVerifyResult { /// trusting a possibly-stale flag from an earlier run). fn fetch_and_verify( client: &reqwest::blocking::Client, - endpoints: &Endpoints, + release_source: &dyn ReleaseSource, work_dir: &Path, name: &str, pkg: &Package, @@ -217,7 +224,7 @@ fn fetch_and_verify( let asset_name = pkg.asset_pattern.replace("{version}", &version); let dest_dir = work_dir.join(name).join(tag); - let api = endpoints.api(); + let api = release_source.api(); let asset = fetcher::download_asset(client, api, &pkg.repo, tag, &asset_name, &dest_dir)?; let verification = verifier::verify( client, @@ -320,11 +327,11 @@ pub fn run_review(args: &[String]) -> Result<()> { fn approve(state_dir: &Path, work_dir: &Path, name: &str, pkg: &Package, tag: &str) -> Result<()> { let client = build_client()?; - let endpoints = Endpoints::for_package(pkg)?; + let release_source = source::for_package(pkg)?; // Re-verify rather than trusting the earlier flag: the artifact at // this tag could in principle have changed since it was queued. - let fetched = fetch_and_verify(&client, &endpoints, work_dir, name, pkg, tag)?; + let fetched = fetch_and_verify(&client, release_source.as_ref(), work_dir, name, pkg, tag)?; if !fetched.verification.passed { bail!( "re-verification failed on approve: {}", @@ -346,6 +353,7 @@ fn approve(state_dir: &Path, work_dir: &Path, name: &str, pkg: &Package, tag: &s #[cfg(test)] mod tests { use super::*; + use crate::forgejo::ForgejoEndpoints; use crate::github::GithubEndpoints; use crate::test_support::same_origin_package; @@ -381,7 +389,7 @@ mod tests { /// Mocks the release-tag, asset, and checksum endpoints for `o/r@v1.0.0` /// with a checksum that doesn't match the asset, so verification fails. - /// These are identical on GitHub and Forgejo (see `Endpoints::api`); only + /// These are identical on GitHub and Forgejo (see `ReleaseSource::api`); only /// how the latest tag is found differs per test. Returned mocks must /// stay alive for the test's duration. fn mock_release_with_bad_checksum(server: &mut mockito::ServerGuard) -> Vec { @@ -418,14 +426,14 @@ mod tests { /// Runs `process_package` for a package whose checksum is wrong and /// asserts it errors with "verification failed" while leaving no trace /// in state. - fn assert_verification_failure_is_an_error(endpoints: &Endpoints, pkg: &Package) { + fn assert_verification_failure_is_an_error(release_source: &dyn ReleaseSource, pkg: &Package) { let client = reqwest::blocking::Client::new(); let state_dir = tempfile::tempdir().unwrap(); let work_dir = tempfile::tempdir().unwrap(); let err = process_package( &client, - endpoints, + release_source, state_dir.path(), work_dir.path(), "thing", @@ -450,10 +458,10 @@ mod tests { #[test] fn process_package_returns_err_on_verification_failure() { let mut server = mockito::Server::new(); - let endpoints = Endpoints::Github(GithubEndpoints { + let github = GithubEndpoints { web: server.url(), api: server.url(), - }); + }; let feed = format!( r#""#, server.url() @@ -465,7 +473,7 @@ mod tests { .create(); let _release_mocks = mock_release_with_bad_checksum(&mut server); - assert_verification_failure_is_an_error(&endpoints, &same_origin_package("")); + assert_verification_failure_is_an_error(&github, &same_origin_package("")); } /// Same as above but through a Forgejo source, proving the whole @@ -474,7 +482,7 @@ mod tests { #[test] fn process_package_returns_err_on_verification_failure_via_forgejo() { let mut server = mockito::Server::new(); - let endpoints = Endpoints::Forgejo { api: server.url() }; + let forgejo = ForgejoEndpoints { api: server.url() }; let _latest = server .mock("GET", "/repos/o/r/releases/latest") .with_status(200) @@ -482,8 +490,8 @@ mod tests { .create(); let _release_mocks = mock_release_with_bad_checksum(&mut server); - // The package's own `source` is irrelevant here: `endpoints` is + // The package's own `source` is irrelevant here: `forgejo` is // hand-built to point at the mock server, bypassing `for_package`. - assert_verification_failure_is_an_error(&endpoints, &same_origin_package("")); + assert_verification_failure_is_an_error(&forgejo, &same_origin_package("")); } } diff --git a/src/source.rs b/src/source.rs index c014dba..2d42a6d 100644 --- a/src/source.rs +++ b/src/source.rs @@ -1,46 +1,24 @@ -//! Maps a package's configured `source` to the endpoints the pipeline -//! stages talk to. `checker` and `fetcher` take what it hands them and -//! build their own paths under it. +//! Picks the `ReleaseSource` implementation for a package from its +//! configured `source`. The one place that knows which hosts exist; the +//! pipeline and the stages it calls only ever see the trait. +use crate::checker::ReleaseSource; use crate::config::{Package, Source}; +use crate::forgejo::ForgejoEndpoints; use crate::github::GithubEndpoints; use anyhow::{Context, Result}; -#[derive(Debug, Clone)] -pub enum Endpoints { - Github(GithubEndpoints), - /// A Forgejo/Gitea instance's API root, i.e. `/api/v1`. - Forgejo { - api: String, - }, -} - -impl Endpoints { - /// Defensive: errors only if a `forgejo-release` package has no - /// `base_url`, which `config::load_packages_dir` already guarantees. - pub fn for_package(pkg: &Package) -> Result { - match pkg.source { - Source::GithubRelease => Ok(Endpoints::Github(GithubEndpoints::default())), - Source::ForgejoRelease => { - let base_url = pkg - .base_url - .as_deref() - .context("source = \"forgejo-release\" needs a base_url")?; - Ok(Endpoints::Forgejo { - api: format!("{}/api/v1", base_url.trim_end_matches('/')), - }) - } - } - } - - /// The releases API root. GitHub and Forgejo both serve - /// `/repos/{owner}/{repo}/releases/tags/{tag}` under it with the same - /// `assets[].{name, browser_download_url}` shape, which is why - /// `fetcher` and `verifier` need only this and not the enum. - pub fn api(&self) -> &str { - match self { - Endpoints::Github(github) => &github.api, - Endpoints::Forgejo { api } => api, +/// Defensive: errors only if a `forgejo-release` package has no +/// `base_url`, which `config::load_packages_dir` already guarantees. +pub fn for_package(pkg: &Package) -> Result> { + match pkg.source { + Source::GithubRelease => Ok(Box::new(GithubEndpoints::default())), + Source::ForgejoRelease => { + let base_url = pkg + .base_url + .as_deref() + .context("source = \"forgejo-release\" needs a base_url")?; + Ok(Box::new(ForgejoEndpoints::from_base_url(base_url))) } } } @@ -52,28 +30,20 @@ mod tests { #[test] fn github_source_uses_real_github() { - let endpoints = Endpoints::for_package(&package("")).unwrap(); - assert_eq!(endpoints.api(), "https://api.github.com"); - assert!(matches!(endpoints, Endpoints::Github(_))); + let source = for_package(&package("")).unwrap(); + assert_eq!(source.api(), "https://api.github.com"); } #[test] - fn forgejo_source_appends_api_v1() { + fn forgejo_source_uses_the_instances_api() { let pkg = package("source = \"forgejo-release\"\nbase_url = \"https://code.example.com\""); - let endpoints = Endpoints::for_package(&pkg).unwrap(); - assert_eq!(endpoints.api(), "https://code.example.com/api/v1"); - } - - #[test] - fn forgejo_source_tolerates_trailing_slash() { - let pkg = package("source = \"forgejo-release\"\nbase_url = \"https://code.example.com/\""); - let endpoints = Endpoints::for_package(&pkg).unwrap(); - assert_eq!(endpoints.api(), "https://code.example.com/api/v1"); + let source = for_package(&pkg).unwrap(); + assert_eq!(source.api(), "https://code.example.com/api/v1"); } #[test] fn forgejo_source_without_base_url_errors() { - let err = Endpoints::for_package(&package("source = \"forgejo-release\"")).unwrap_err(); + let err = for_package(&package("source = \"forgejo-release\"")).unwrap_err(); assert!(err.to_string().contains("needs a base_url")); } }