diff --git a/Cargo.lock b/Cargo.lock index 27ebec10e..0c97ddfe3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2665,11 +2665,11 @@ dependencies = [ "jsonwebtoken", "serde", "serde_json", + "strum 0.28.0", "thiserror 2.0.18", "tokio", "tracing", "tracing-subscriber", - "url", ] [[package]] diff --git a/lib/components/fabro-github/Cargo.toml b/lib/components/fabro-github/Cargo.toml index d77b75922..60d003620 100644 --- a/lib/components/fabro-github/Cargo.toml +++ b/lib/components/fabro-github/Cargo.toml @@ -26,7 +26,7 @@ tracing.workspace = true tokio = { workspace = true } base64.workspace = true thiserror.workspace = true -url.workspace = true +strum.workspace = true [dev-dependencies] fabro-macros = { path = "../../foundation/fabro-macros" } diff --git a/lib/components/fabro-github/src/lib.rs b/lib/components/fabro-github/src/lib.rs index 3e31e9c55..007d06c66 100644 --- a/lib/components/fabro-github/src/lib.rs +++ b/lib/components/fabro-github/src/lib.rs @@ -11,7 +11,7 @@ use tokio::process::Command; mod repository_reader; -pub use repository_reader::{GitHubRepositoryReader, RepositoryReadError}; +pub use repository_reader::{GitHubRepositoryReader, Operation, RepositoryReadError}; pub const GITHUB_API_BASE_URL: &str = "https://api.github.com"; @@ -1011,7 +1011,7 @@ pub async fn branch_head_sha( .context("Failed to read remote branch head")?; match reader.resolve_commit(&format!("heads/{branch}")).await { Ok(sha) => Ok(Some(sha)), - Err(RepositoryReadError::RevisionNotFound) => Ok(None), + Err(RepositoryReadError::NotFound { .. }) => Ok(None), Err(error) => Err(anyhow::Error::new(error).context("Failed to read remote branch head")), } } diff --git a/lib/components/fabro-github/src/repository_reader.rs b/lib/components/fabro-github/src/repository_reader.rs index 2f6d82ffc..c214c6038 100644 --- a/lib/components/fabro-github/src/repository_reader.rs +++ b/lib/components/fabro-github/src/repository_reader.rs @@ -1,10 +1,6 @@ -#![expect( - clippy::disallowed_types, - reason = "Validated URL values are used only for request construction and are never logged or included in errors." -)] - use fabro_http::header::{ACCEPT, CONTENT_TYPE, RETRY_AFTER, USER_AGENT}; -use fabro_http::{HeaderMap, HttpClient, Response, StatusCode, Url}; +use fabro_http::{HeaderMap, HttpClient, Response, StatusCode}; +use fabro_redact::{DisplaySafeUrl, DisplaySafeUrlError}; use fabro_types::{GitHubRepositorySlug, repository}; use crate::GitHubContext; @@ -13,14 +9,24 @@ const SHA_MEDIA_TYPE: &str = "application/vnd.github.sha"; const RAW_CONTENT_MEDIA_TYPE: &str = "application/vnd.github.raw+json"; const MAX_SHA_RESPONSE_BYTES: usize = 128; +/// Which repository read a failure came from. Carried on the errors that can +/// arise from either, so one status classification serves both. +#[derive(Clone, Copy, Debug, PartialEq, Eq, strum::Display)] +#[strum(serialize_all = "lowercase")] +pub enum Operation { + Revision, + Content, +} + /// Failures while opening or using a repository-scoped GitHub reader. #[derive(Debug, thiserror::Error)] +#[non_exhaustive] pub enum RepositoryReadError { #[error("invalid GitHub API base URL ({reason})")] InvalidApiBaseUrl { reason: &'static str, #[source] - source: Option, + source: Option, }, #[error("invalid GitHub ref selector")] InvalidRefSelector, @@ -44,28 +50,26 @@ pub enum RepositoryReadError { PermissionDenied, #[error("GitHub rate limit reached (status {status})")] RateLimited { status: u16 }, - /// GitHub returned 404 while resolving a revision. GitHub may also use - /// 404 to hide a private resource that these credentials cannot access. - #[error("GitHub revision was not observable")] - RevisionNotFound, - /// GitHub returned 404 while reading content. GitHub may also use 404 to - /// hide a private resource that these credentials cannot access. - #[error("GitHub repository content was not observable")] - ContentNotFound, + /// GitHub returned 404. GitHub may also use 404 to hide a private resource + /// that these credentials cannot access, so this means "not observable" + /// rather than "does not exist". + #[error("GitHub {operation} was not observable")] + NotFound { operation: Operation }, #[error("GitHub repository content is not a file")] ContentNotFile, - #[error("GitHub revision is unavailable (status {status})")] - RevisionUnavailable { status: u16 }, - #[error("GitHub repository content is unavailable (status {status})")] - ContentUnavailable { status: u16 }, + #[error("GitHub {operation} is unavailable (status {status})")] + Unavailable { + operation: Operation, + status: u16, + }, #[error("GitHub {operation} service is unavailable (status {status})")] UpstreamUnavailable { - operation: &'static str, + operation: Operation, status: u16, }, #[error("unexpected GitHub {operation} status {status}")] UnexpectedStatus { - operation: &'static str, + operation: Operation, status: u16, }, #[error("GitHub response exceeded the {max_bytes}-byte limit")] @@ -80,26 +84,13 @@ pub enum RepositoryReadError { } /// An authenticated read session scoped to one GitHub repository. +/// +/// Opening resolves credentials once; every read reuses that token. pub struct GitHubRepositoryReader { - client: HttpClient, - api_base: Url, - repository: GitHubRepositorySlug, - bearer_token: String, -} - -#[derive(Clone, Copy)] -enum Operation { - Revision, - Content, -} - -impl Operation { - const fn name(self) -> &'static str { - match self { - Self::Revision => "revision lookup", - Self::Content => "content read", - } - } + client: HttpClient, + /// `{api_base}/repos/{owner}/{repo}`, validated and built once at open. + repository_base: DisplaySafeUrl, + bearer_token: String, } impl GitHubRepositoryReader { @@ -111,14 +102,14 @@ impl GitHubRepositoryReader { let client = ctx .http_client() .map_err(|source| RepositoryReadError::RequestTransport { source })?; - let (api_base, normalized_base) = parse_api_base(ctx.base_url)?; + let api_base = parse_api_base(ctx.base_url)?; let bearer_token = ctx .creds .resolve_bearer_token( &client, repository.owner(), repository.repo(), - &normalized_base, + api_base.as_str().trim_end_matches('/'), serde_json::json!({ "contents": "read" }), ) .await @@ -126,8 +117,7 @@ impl GitHubRepositoryReader { Ok(Self { client, - api_base, - repository: repository.clone(), + repository_base: repository_base(&api_base, repository), bearer_token, }) } @@ -138,19 +128,9 @@ impl GitHubRepositoryReader { return Err(RepositoryReadError::InvalidRefSelector); } - let url = commit_url(&self.api_base, &self.repository, selector)?; let response = self - .client - .get(url) - .bearer_auth(&self.bearer_token) - .header(USER_AGENT, "fabro") - .header(ACCEPT, SHA_MEDIA_TYPE) - .send() - .await - .map_err(|source| RepositoryReadError::RequestTransport { - source: anyhow::Error::new(source.without_url()), - })?; - + .send(&self.commit_url(selector), SHA_MEDIA_TYPE) + .await?; classify_status(response.status(), response.headers(), Operation::Revision)?; let bytes = collect_bounded(response, MAX_SHA_RESPONSE_BYTES).await?; parse_resolved_commit_sha(bytes) @@ -168,24 +148,8 @@ impl GitHubRepositoryReader { } validate_repository_path(canonical_repo_path)?; - let url = content_url( - &self.api_base, - &self.repository, - commit_sha, - canonical_repo_path, - )?; - let response = self - .client - .get(url) - .bearer_auth(&self.bearer_token) - .header(USER_AGENT, "fabro") - .header(ACCEPT, RAW_CONTENT_MEDIA_TYPE) - .send() - .await - .map_err(|source| RepositoryReadError::RequestTransport { - source: anyhow::Error::new(source.without_url()), - })?; - + let url = self.content_url(commit_sha, canonical_repo_path); + let response = self.send(&url, RAW_CONTENT_MEDIA_TYPE).await?; classify_status(response.status(), response.headers(), Operation::Content)?; if !has_raw_content_media_type(response.headers()) { return Err(RepositoryReadError::ContentNotFile); @@ -195,14 +159,61 @@ impl GitHubRepositoryReader { source: source.utf8_error(), }) } + + /// `{repository_base}/commits/{selector}`, with the selector's slashes + /// escaped so a `heads/a/b` ref stays one path segment. + fn commit_url(&self, selector: &str) -> DisplaySafeUrl { + let mut url = self.repository_base.clone(); + url.path_segments_mut() + .expect("repository base is a hierarchical URL") + .push("commits") + .push(selector); + url + } + + /// `{repository_base}/contents/{path}?ref={commit_sha}`, with each path + /// component escaped individually so separators survive as separators. + fn content_url(&self, commit_sha: &str, path: &str) -> DisplaySafeUrl { + let mut url = self.repository_base.clone(); + { + let mut segments = url + .path_segments_mut() + .expect("repository base is a hierarchical URL"); + segments.push("contents"); + for component in path.split('/') { + segments.push(component); + } + } + url.query_pairs_mut().append_pair("ref", commit_sha); + url + } + + /// Issues one authenticated GET asking for exactly `media_type`. + async fn send( + &self, + url: &DisplaySafeUrl, + media_type: &str, + ) -> Result { + self.client + .get(url.raw_string()) + .bearer_auth(&self.bearer_token) + .header(USER_AGENT, "fabro") + .header(ACCEPT, media_type) + .send() + .await + .map_err(|source| RepositoryReadError::RequestTransport { + source: anyhow::Error::new(source.without_url()), + }) + } } -fn parse_api_base(base_url: &str) -> Result<(Url, String), RepositoryReadError> { - let mut url = - Url::parse(base_url).map_err(|source| RepositoryReadError::InvalidApiBaseUrl { +fn parse_api_base(base_url: &str) -> Result { + let mut url = DisplaySafeUrl::parse(base_url).map_err(|source| { + RepositoryReadError::InvalidApiBaseUrl { reason: "parse", source: Some(source), - })?; + } + })?; if url.cannot_be_a_base() { return Err(invalid_base("cannot be a base")); } @@ -222,14 +233,10 @@ fn parse_api_base(base_url: &str) -> Result<(Url, String), RepositoryReadError> return Err(invalid_base("fragment")); } - let mut segments = url - .path_segments_mut() - .map_err(|()| invalid_base("cannot be a base"))?; - segments.pop_if_empty(); - drop(segments); - - let normalized = url.as_str().trim_end_matches('/').to_string(); - Ok((url, normalized)) + url.path_segments_mut() + .map_err(|()| invalid_base("cannot be a base"))? + .pop_if_empty(); + Ok(url) } const fn invalid_base(reason: &'static str) -> RepositoryReadError { @@ -239,62 +246,30 @@ const fn invalid_base(reason: &'static str) -> RepositoryReadError { } } -fn commit_url( - base: &Url, - repository: &GitHubRepositorySlug, - selector: &str, -) -> Result { - let mut url = base.clone(); - let mut segments = url - .path_segments_mut() - .map_err(|()| invalid_base("cannot be a base"))?; - segments +/// `{api_base}/repos/{owner}/{repo}`, the prefix every read extends. +/// +/// Infallible: `parse_api_base` has already rejected cannot-be-a-base URLs. +fn repository_base(api_base: &DisplaySafeUrl, repository: &GitHubRepositorySlug) -> DisplaySafeUrl { + let mut url = api_base.clone(); + url.path_segments_mut() + .expect("api base is a hierarchical URL") .pop_if_empty() .push("repos") .push(repository.owner()) - .push(repository.repo()) - .push("commits") - .push(selector); - drop(segments); - Ok(url) -} - -fn content_url( - base: &Url, - repository: &GitHubRepositorySlug, - commit_sha: &str, - path: &str, -) -> Result { - let mut url = base.clone(); - let mut segments = url - .path_segments_mut() - .map_err(|()| invalid_base("cannot be a base"))?; - segments - .pop_if_empty() - .push("repos") - .push(repository.owner()) - .push(repository.repo()) - .push("contents"); - for component in path.split('/') { - segments.push(component); - } - drop(segments); - url.query_pairs_mut().append_pair("ref", commit_sha); - Ok(url) + .push(repository.repo()); + url } fn is_exact_commit_sha(bytes: &[u8]) -> bool { bytes.len() == 40 && bytes.iter().all(u8::is_ascii_hexdigit) } -fn parse_resolved_commit_sha(bytes: Vec) -> Result { +fn parse_resolved_commit_sha(mut bytes: Vec) -> Result { if !is_exact_commit_sha(&bytes) { return Err(RepositoryReadError::MalformedCommitSha); } - Ok(bytes - .into_iter() - .map(|byte| char::from(byte.to_ascii_lowercase())) - .collect()) + bytes.make_ascii_lowercase(); + String::from_utf8(bytes).map_err(|_| RepositoryReadError::MalformedCommitSha) } fn validate_repository_path(path: &str) -> Result<(), RepositoryReadError> { @@ -343,31 +318,32 @@ fn classify_status( return Ok(()); } - let code = status.as_u16(); - match status { - StatusCode::UNAUTHORIZED => Err(RepositoryReadError::AuthenticationRejected), - StatusCode::FORBIDDEN if is_rate_limited(headers) => { - Err(RepositoryReadError::RateLimited { status: code }) + let status_code = status.as_u16(); + Err(match status { + StatusCode::UNAUTHORIZED => RepositoryReadError::AuthenticationRejected, + StatusCode::FORBIDDEN if is_rate_limited(headers) => RepositoryReadError::RateLimited { + status: status_code, + }, + StatusCode::FORBIDDEN => RepositoryReadError::PermissionDenied, + StatusCode::TOO_MANY_REQUESTS => RepositoryReadError::RateLimited { + status: status_code, + }, + StatusCode::NOT_FOUND => RepositoryReadError::NotFound { operation }, + StatusCode::CONFLICT | StatusCode::UNPROCESSABLE_ENTITY => { + RepositoryReadError::Unavailable { + operation, + status: status_code, + } } - StatusCode::FORBIDDEN => Err(RepositoryReadError::PermissionDenied), - StatusCode::TOO_MANY_REQUESTS => Err(RepositoryReadError::RateLimited { status: code }), - StatusCode::NOT_FOUND => match operation { - Operation::Revision => Err(RepositoryReadError::RevisionNotFound), - Operation::Content => Err(RepositoryReadError::ContentNotFound), + status if status.is_server_error() => RepositoryReadError::UpstreamUnavailable { + operation, + status: status_code, }, - StatusCode::CONFLICT | StatusCode::UNPROCESSABLE_ENTITY => match operation { - Operation::Revision => Err(RepositoryReadError::RevisionUnavailable { status: code }), - Operation::Content => Err(RepositoryReadError::ContentUnavailable { status: code }), + _ => RepositoryReadError::UnexpectedStatus { + operation, + status: status_code, }, - status if status.is_server_error() => Err(RepositoryReadError::UpstreamUnavailable { - operation: operation.name(), - status: code, - }), - _ => Err(RepositoryReadError::UnexpectedStatus { - operation: operation.name(), - status: code, - }), - } + }) } fn is_rate_limited(headers: &HeaderMap) -> bool { @@ -400,14 +376,18 @@ async fn collect_bounded( mut response: Response, max_bytes: usize, ) -> Result, RepositoryReadError> { - if response - .content_length() - .is_some_and(|length| length > max_bytes as u64) - { - return Err(RepositoryReadError::BodyTooLarge { max_bytes }); - } + // A declared length over the cap fails before any body is read; otherwise it + // sizes the buffer exactly. Chunked responses start empty and grow under the + // same bound, enforced per chunk below. + let capacity = match response.content_length() { + Some(length) => usize::try_from(length) + .ok() + .filter(|length| *length <= max_bytes) + .ok_or(RepositoryReadError::BodyTooLarge { max_bytes })?, + None => 0, + }; - let mut bytes = Vec::new(); + let mut bytes = Vec::with_capacity(capacity); while let Some(chunk) = response .chunk() @@ -436,13 +416,19 @@ mod tests { #[test] fn api_base_preserves_prefix_and_normalizes_trailing_slash() { - let (base, normalized) = parse_api_base("https://ghe.example/api/v3/").unwrap(); + let base = parse_api_base("https://ghe.example/api/v3/").unwrap(); assert_eq!(base.as_str(), "https://ghe.example/api/v3"); - assert_eq!(normalized, "https://ghe.example/api/v3"); + assert_eq!( + base.as_str().trim_end_matches('/'), + "https://ghe.example/api/v3" + ); - let (root, normalized) = parse_api_base("https://api.github.com/").unwrap(); + let root = parse_api_base("https://api.github.com/").unwrap(); assert_eq!(root.as_str(), "https://api.github.com/"); - assert_eq!(normalized, "https://api.github.com"); + assert_eq!( + root.as_str().trim_end_matches('/'), + "https://api.github.com" + ); } #[test] @@ -480,16 +466,26 @@ mod tests { assert!(error.source().is_some()); } + /// Builds the URL a read would request without issuing it. + fn reader_at(base_url: &str) -> GitHubRepositoryReader { + let api_base = parse_api_base(base_url).unwrap(); + GitHubRepositoryReader { + client: fabro_http::test_http_client().unwrap(), + repository_base: repository_base(&api_base, &repository()), + bearer_token: "sentinel-token".to_string(), + } + } + #[test] fn url_builders_encode_selectors_and_paths_once() { - let (base, _) = parse_api_base("https://ghe.example/api/v3").unwrap(); - let commit = commit_url(&base, &repository(), "heads/fabro/run/123").unwrap(); + let reader = reader_at("https://ghe.example/api/v3"); + let commit = reader.commit_url("heads/fabro/run/123"); assert_eq!( commit.as_str(), "https://ghe.example/api/v3/repos/owner/repo/commits/heads%2Ffabro%2Frun%2F123" ); - let content = content_url(&base, &repository(), SHA, "dir/a b/%2F/#?é.toml").unwrap(); + let content = reader.content_url(SHA, "dir/a b/%2F/#?é.toml"); assert_eq!( content.as_str(), "https://ghe.example/api/v3/repos/owner/repo/contents/dir/a%20b/%252F/%23%3F%C3%A9.toml?ref=0123456789abcdef0123456789abcdef01234567" @@ -498,12 +494,7 @@ mod tests { #[tokio::test] async fn method_inputs_are_rejected_before_network_access() { - let reader = GitHubRepositoryReader { - client: fabro_http::test_http_client().unwrap(), - api_base: Url::parse("http://127.0.0.1:1").unwrap(), - repository: repository(), - bearer_token: "sentinel-token".to_string(), - }; + let reader = reader_at("http://127.0.0.1:1"); for invalid in ["", " main", "heads//main", "tags/v1.lock"] { assert!(matches!( @@ -519,10 +510,15 @@ mod tests { #[test] fn canonical_ref_selectors_reach_url_construction() { - let (base, _) = parse_api_base("https://api.github.com").unwrap(); + let reader = reader_at("https://api.github.com"); for selector in ["main", "heads/fabro/run/123", "tags/v1.0.0", SHA] { assert!(repository::is_valid_github_ref_selector(selector)); - assert!(commit_url(&base, &repository(), selector).is_ok()); + assert!( + reader + .commit_url(selector) + .as_str() + .starts_with("https://api.github.com/repos/owner/repo/commits/") + ); } } @@ -610,11 +606,15 @@ mod tests { )); assert!(matches!( classify_status(StatusCode::NOT_FOUND, &empty, Operation::Revision), - Err(RepositoryReadError::RevisionNotFound) + Err(RepositoryReadError::NotFound { + operation: Operation::Revision, + }) )); assert!(matches!( classify_status(StatusCode::NOT_FOUND, &empty, Operation::Content), - Err(RepositoryReadError::ContentNotFound) + Err(RepositoryReadError::NotFound { + operation: Operation::Content, + }) )); assert!(matches!( classify_status(StatusCode::FORBIDDEN, &empty, Operation::Content), @@ -638,44 +638,31 @@ mod tests { classify_status(StatusCode::TOO_MANY_REQUESTS, &empty, Operation::Content), Err(RepositoryReadError::RateLimited { status: 429 }) )); - for operation in [Operation::Revision, Operation::Content] { - let conflict = classify_status(StatusCode::CONFLICT, &empty, operation); - let unprocessable = - classify_status(StatusCode::UNPROCESSABLE_ENTITY, &empty, operation); - match operation { - Operation::Revision => { - assert!(matches!( - conflict, - Err(RepositoryReadError::RevisionUnavailable { status: 409 }) - )); - assert!(matches!( - unprocessable, - Err(RepositoryReadError::RevisionUnavailable { status: 422 }) - )); - } - Operation::Content => { - assert!(matches!( - conflict, - Err(RepositoryReadError::ContentUnavailable { status: 409 }) - )); - assert!(matches!( - unprocessable, - Err(RepositoryReadError::ContentUnavailable { status: 422 }) - )); - } - } - } + assert!(matches!( + classify_status(StatusCode::CONFLICT, &empty, Operation::Revision), + Err(RepositoryReadError::Unavailable { + operation: Operation::Revision, + status: 409, + }) + )); + assert!(matches!( + classify_status(StatusCode::UNPROCESSABLE_ENTITY, &empty, Operation::Content), + Err(RepositoryReadError::Unavailable { + operation: Operation::Content, + status: 422, + }) + )); assert!(matches!( classify_status(StatusCode::BAD_GATEWAY, &empty, Operation::Content), Err(RepositoryReadError::UpstreamUnavailable { - operation: "content read", + operation: Operation::Content, status: 502, }) )); assert!(matches!( classify_status(StatusCode::IM_A_TEAPOT, &empty, Operation::Revision), Err(RepositoryReadError::UnexpectedStatus { - operation: "revision lookup", + operation: Operation::Revision, status: 418, }) )); diff --git a/lib/components/fabro-github/tests/integration.rs b/lib/components/fabro-github/tests/integration.rs index f98d88f1d..c99131393 100644 --- a/lib/components/fabro-github/tests/integration.rs +++ b/lib/components/fabro-github/tests/integration.rs @@ -45,36 +45,19 @@ fn standard_app_state() -> GitHubAppState { state.set_repository_ref("acme", "widgets", "tags/release", TAG_SHA); state.set_repository_ref("acme", "widgets", "heads/fabro/run/123", HEAD_SHA); state.set_repository_ref("acme", "widgets", "heads/uppercase", UPPER_SHA); - state.add_repository_file( - "acme", - "widgets", - HEAD_SHA, - ".fabro/workflows/build/workflow.toml", - b"name = \"build\"\n".to_vec(), - ); - state.add_repository_file( - "acme", - "widgets", - HEAD_SHA, - "dir/hello world#.txt", - b"hello\n".to_vec(), - ); - state.add_repository_file("acme", "widgets", HEAD_SHA, "invalid.txt", vec![0xff, 0xfe]); - state.add_repository_file("acme", "widgets", HEAD_SHA, "empty.txt", Vec::new()); - state.add_repository_file( - "acme", - "widgets", - HEAD_SHA, - "five-bytes.txt", - b"12345".to_vec(), - ); - state.add_repository_file( - "acme", - "widgets", - HEAD_SHA, - "nested/a b/%/é.toml", - "unicode: 🦀\n".as_bytes().to_vec(), - ); + for (path, contents) in [ + ( + ".fabro/workflows/build/workflow.toml", + b"name = \"build\"\n".to_vec(), + ), + ("dir/hello world#.txt", b"hello\n".to_vec()), + ("invalid.txt", vec![0xff, 0xfe]), + ("empty.txt", Vec::new()), + ("five-bytes.txt", b"12345".to_vec()), + ("nested/a b/%/é.toml", "unicode: 🦀\n".as_bytes().to_vec()), + ] { + state.add_repository_file("acme", "widgets", HEAD_SHA, path, contents); + } state } @@ -82,6 +65,18 @@ fn repository() -> GitHubRepositorySlug { GitHubRepositorySlug::try_new("acme/widgets").expect("test repository slug should be valid") } +/// Open a reader for the standard repository against `base_url`. +async fn open_reader( + base_url: &str, + credentials: &GitHubCredentials, +) -> Result { + GitHubRepositoryReader::open( + &GitHubContext::with_http_client(credentials, base_url, fabro_test::test_http_client()), + &repository(), + ) + .await +} + #[fabro_macros::e2e_test(twin)] async fn create_and_get_pull_request() { let twin = TwinGitHub::start(standard_app_state()).await; @@ -279,12 +274,7 @@ async fn repository_reader_reuses_one_app_token_for_ref_and_file_reads() { assert_eq!(twin.active_token_count().await, 0); let creds = github_credentials(); let base_url = format!("{}/", twin.base_url); - let reader = GitHubRepositoryReader::open( - &GitHubContext::with_http_client(&creds, &base_url, fabro_test::test_http_client()), - &repository(), - ) - .await - .unwrap(); + let reader = open_reader(&base_url, &creds).await.unwrap(); assert_eq!(twin.active_token_count().await, 1); assert_eq!(reader.resolve_commit("heads/main").await.unwrap(), HEAD_SHA); @@ -325,12 +315,7 @@ async fn repository_reader_reuses_one_app_token_for_ref_and_file_reads() { async fn repository_reader_preserves_exact_ref_namespaces() { let twin = TwinGitHub::start(standard_app_state()).await; let creds = github_credentials(); - let reader = GitHubRepositoryReader::open( - &GitHubContext::with_http_client(&creds, &twin.base_url, fabro_test::test_http_client()), - &repository(), - ) - .await - .unwrap(); + let reader = open_reader(&twin.base_url, &creds).await.unwrap(); assert_eq!( reader.resolve_commit("heads/release").await.unwrap(), @@ -351,7 +336,9 @@ async fn repository_reader_preserves_exact_ref_namespaces() { ); assert!(matches!( reader.resolve_commit("heads/missing").await, - Err(RepositoryReadError::RevisionNotFound) + Err(RepositoryReadError::NotFound { + operation: fabro_github::Operation::Revision, + }) )); twin.shutdown().await; @@ -361,12 +348,7 @@ async fn repository_reader_preserves_exact_ref_namespaces() { async fn repository_reader_classifies_file_failures_without_retaining_bytes() { let twin = TwinGitHub::start(standard_app_state()).await; let creds = github_credentials(); - let reader = GitHubRepositoryReader::open( - &GitHubContext::with_http_client(&creds, &twin.base_url, fabro_test::test_http_client()), - &repository(), - ) - .await - .unwrap(); + let reader = open_reader(&twin.base_url, &creds).await.unwrap(); assert!(matches!( reader @@ -399,7 +381,9 @@ async fn repository_reader_classifies_file_failures_without_retaining_bytes() { reader .read_utf8_file_at(HEAD_SHA, "missing.txt", 1024) .await, - Err(RepositoryReadError::ContentNotFound) + Err(RepositoryReadError::NotFound { + operation: fabro_github::Operation::Content, + }) )); assert_eq!( reader @@ -444,16 +428,7 @@ async fn static_repository_credentials_do_not_mint_tokens() { expires_at: chrono::Utc::now() + chrono::Duration::hours(1), }), ] { - let reader = GitHubRepositoryReader::open( - &GitHubContext::with_http_client( - &credentials, - &twin.base_url, - fabro_test::test_http_client(), - ), - &repository(), - ) - .await - .unwrap(); + let reader = open_reader(&twin.base_url, &credentials).await.unwrap(); assert_eq!(reader.resolve_commit("heads/main").await.unwrap(), HEAD_SHA); assert_eq!( reader @@ -476,17 +451,10 @@ async fn expired_installation_token_keeps_credential_error_source() { expires_at: chrono::Utc::now() - chrono::Duration::minutes(1), }); - let error = GitHubRepositoryReader::open( - &GitHubContext::with_http_client( - &credentials, - &twin.base_url, - fabro_test::test_http_client(), - ), - &repository(), - ) - .await - .err() - .expect("expired installation token should fail"); + let error = open_reader(&twin.base_url, &credentials) + .await + .err() + .expect("expired installation token should fail"); let RepositoryReadError::CredentialResolution { source } = error else { panic!("expected credential resolution error"); }; @@ -530,48 +498,25 @@ async fn repository_reader_keeps_auth_and_malformed_sha_failures_distinct() { let twin = TwinGitHub::start(state).await; let invalid_credentials = GitHubCredentials::Pat("invalid-token".to_string()); - let invalid_reader = GitHubRepositoryReader::open( - &GitHubContext::with_http_client( - &invalid_credentials, - &twin.base_url, - fabro_test::test_http_client(), - ), - &repository(), - ) - .await - .unwrap(); + let invalid_reader = open_reader(&twin.base_url, &invalid_credentials) + .await + .unwrap(); assert!(matches!( invalid_reader.resolve_commit("heads/main").await, Err(RepositoryReadError::AuthenticationRejected) )); let denied_credentials = GitHubCredentials::Pat(denied_token); - let denied_reader = GitHubRepositoryReader::open( - &GitHubContext::with_http_client( - &denied_credentials, - &twin.base_url, - fabro_test::test_http_client(), - ), - &repository(), - ) - .await - .unwrap(); + let denied_reader = open_reader(&twin.base_url, &denied_credentials) + .await + .unwrap(); assert!(matches!( denied_reader.resolve_commit("heads/main").await, Err(RepositoryReadError::PermissionDenied) )); let app_credentials = github_credentials(); - let app_reader = GitHubRepositoryReader::open( - &GitHubContext::with_http_client( - &app_credentials, - &twin.base_url, - fabro_test::test_http_client(), - ), - &repository(), - ) - .await - .unwrap(); + let app_reader = open_reader(&twin.base_url, &app_credentials).await.unwrap(); assert!(matches!( app_reader.resolve_commit("heads/malformed").await, Err(RepositoryReadError::MalformedCommitSha) diff --git a/lib/components/fabro-workflow/src/pipeline/pull_request.rs b/lib/components/fabro-workflow/src/pipeline/pull_request.rs index 03b78b309..50ec787e8 100644 --- a/lib/components/fabro-workflow/src/pipeline/pull_request.rs +++ b/lib/components/fabro-workflow/src/pipeline/pull_request.rs @@ -3,14 +3,16 @@ use std::sync::{Arc, LazyLock}; use std::time::Duration; use fabro_auth::CredentialSource; -use fabro_github::{self as github_app, ssh_url_to_https}; +use fabro_github::{ + self as github_app, GitHubRepositoryReader, RepositoryReadError, ssh_url_to_https, +}; use fabro_graphviz::parser; use fabro_llm::client::Client; use fabro_llm::generate::{GenerateParams, generate_object}; use fabro_model::{Catalog, ProviderId}; use fabro_store::RunProjection; -use fabro_types::PullRequestLink; use fabro_types::settings::run::MergeStrategy; +use fabro_types::{GitHubRepositorySlug, PullRequestLink}; use fabro_util::text::strip_goal_decoration; use tokio::time::sleep; use tracing::{debug, info, warn}; @@ -548,15 +550,29 @@ const BRANCH_HEAD_RETRY_DELAY: Duration = Duration::from_millis(500); /// Confirm the remote branch points at the run's final commit. /// /// Publish failures are terminal, so a replica that has not caught up yet must -/// not be mistaken for a genuinely stale branch. +/// not be mistaken for a genuinely stale branch. Credentials are resolved once +/// and reused across attempts; only the ref lookup is retried. async fn verify_remote_head( req: &OpenPullRequestRequest<'_>, owner: &str, repo: &str, ) -> Result<(), String> { + let repository = + GitHubRepositorySlug::try_new(&format!("{owner}/{repo}")).ok_or_else(|| { + format!("failed to verify remote branch head: invalid repository {owner}/{repo}") + })?; + let reader = GitHubRepositoryReader::open(&req.github, &repository) + .await + .map_err(|err| format!("failed to verify remote branch head: {err:#}"))?; + let selector = format!("heads/{}", req.head_branch); + let mut last_seen = Ok(None); for attempt in 1..=BRANCH_HEAD_ATTEMPTS { - last_seen = github_app::branch_head_sha(&req.github, owner, repo, req.head_branch).await; + last_seen = match reader.resolve_commit(&selector).await { + Ok(sha) => Ok(Some(sha)), + Err(RepositoryReadError::NotFound { .. }) => Ok(None), + Err(err) => Err(err), + }; match &last_seen { Ok(Some(head)) if head == req.expected_head_sha => return Ok(()), Ok(head) => debug!( @@ -702,11 +718,11 @@ mod tests { use tokio::sync::RwLock as AsyncRwLock; use super::*; + use crate::event::{Event, append_event}; + use crate::records::StageSummary; const FINAL_SHA: &str = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; const STALE_SHA: &str = "bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"; - use crate::event::{Event, append_event}; - use crate::records::StageSummary; struct MockProvider { name: String, diff --git a/test/twin/github/src/fixtures.rs b/test/twin/github/src/fixtures.rs index abc9b2160..12bd7d0aa 100644 --- a/test/twin/github/src/fixtures.rs +++ b/test/twin/github/src/fixtures.rs @@ -213,28 +213,16 @@ impl FixtureState { state.repositories = self .repositories .into_iter() - .map(|repository| { - let refs = repository - .branches - .into_iter() - .map(|branch| { - ( - format!("heads/{branch}"), - crate::state::DEFAULT_REPOSITORY_SHA.to_string(), - ) - }) - .collect(); - Repository { - owner: repository.owner, - name: repository.name, - refs, - files: HashMap::new(), - default_branch: repository - .default_branch - .unwrap_or_else(|| "main".to_string()), - private: repository.private, - git_dir: None, - } + .map(|repository| Repository { + owner: repository.owner, + name: repository.name, + refs: crate::state::head_refs(repository.branches), + files: HashMap::new(), + default_branch: repository + .default_branch + .unwrap_or_else(|| "main".to_string()), + private: repository.private, + git_dir: None, }) .collect(); diff --git a/test/twin/github/src/handlers/branches.rs b/test/twin/github/src/handlers/branches.rs index 36d4e1c48..51978f531 100644 --- a/test/twin/github/src/handlers/branches.rs +++ b/test/twin/github/src/handlers/branches.rs @@ -58,7 +58,7 @@ pub async fn get_branch( // Find repository and check branch. for repo_data in &state.repositories { if repo_data.owner == owner && repo_data.name == repo { - if let Some(sha) = repo_data.refs.get(&format!("heads/{branch}")) { + if let Some(sha) = repo_data.refs.get(&crate::state::heads_selector(&branch)) { return ( StatusCode::OK, Json(serde_json::json!({ diff --git a/test/twin/github/src/handlers/commits.rs b/test/twin/github/src/handlers/commits.rs index 2d092c920..96f08036b 100644 --- a/test/twin/github/src/handlers/commits.rs +++ b/test/twin/github/src/handlers/commits.rs @@ -1,15 +1,11 @@ -use axum::Json; use axum::extract::{Path, State}; -use axum::http::header::{ACCEPT, CONTENT_TYPE}; +use axum::http::header::CONTENT_TYPE; use axum::http::{HeaderMap, StatusCode}; use axum::response::{IntoResponse, Response}; -use crate::auth::{ - BearerTokenError, InstallationTokenAccessError, authorize_installation_token, - ensure_repo_permission, -}; +use crate::handlers::support::{accepts, authorize_repo_access, is_exact_commit_sha, message}; use crate::server::SharedState; -use crate::state::{AppState, PermissionLevel, TokenPermission}; +use crate::state::{PermissionLevel, TokenPermission}; const SHA_MEDIA_TYPE: &str = "application/vnd.github.sha"; @@ -23,25 +19,25 @@ pub async fn get_commit( if !accepts(&headers, SHA_MEDIA_TYPE) { return message(StatusCode::NOT_ACCEPTABLE, "Not Acceptable"); } - if let Err(error) = authorize(&headers, &state, &repo) { - return authorization_error(error); + if let Err(response) = authorize_repo_access( + &headers, + &state, + &repo, + TokenPermission::Contents, + PermissionLevel::Read, + ) { + return *response; } - let Some(repository) = state - .repositories - .iter() - .find(|repository| repository.owner == owner && repository.name == repo) - else { + let Some(repository) = state.find_repository(&owner, &repo) else { return message(StatusCode::NOT_FOUND, "Not Found"); }; + // A selector is either a known ref, or an exact SHA this repository can + // already serve; an unknown SHA must not resolve to itself. let sha = repository.refs.get(&selector).cloned().or_else(|| { - is_exact_commit_sha(&selector) + (is_exact_commit_sha(&selector) && repository.knows_commit(&selector)) .then(|| selector.clone()) - .filter(|sha| { - repository.refs.values().any(|known| known == sha) - || repository.files.keys().any(|(known, _)| known == sha) - }) }); let Some(sha) = sha else { return message(StatusCode::NOT_FOUND, "Not Found"); @@ -50,65 +46,10 @@ pub async fn get_commit( (StatusCode::OK, [(CONTENT_TYPE, SHA_MEDIA_TYPE)], sha).into_response() } -#[derive(Clone, Copy)] -enum AuthorizationError { - MissingCredentials, - InvalidCredentials, - RepoNotAccessible, - PermissionDenied, -} - -fn authorize(headers: &HeaderMap, state: &AppState, repo: &str) -> Result<(), AuthorizationError> { - let token = authorize_installation_token(headers, state).map_err(|error| match error { - BearerTokenError::Missing => AuthorizationError::MissingCredentials, - BearerTokenError::Invalid => AuthorizationError::InvalidCredentials, - })?; - ensure_repo_permission( - &token, - repo, - TokenPermission::Contents, - PermissionLevel::Read, - ) - .map_err(|error| match error { - InstallationTokenAccessError::RepoNotAccessible => AuthorizationError::RepoNotAccessible, - InstallationTokenAccessError::PermissionDenied => AuthorizationError::PermissionDenied, - }) -} - -fn authorization_error(error: AuthorizationError) -> Response { - match error { - AuthorizationError::MissingCredentials => message(StatusCode::UNAUTHORIZED, "Unauthorized"), - AuthorizationError::InvalidCredentials => { - message(StatusCode::UNAUTHORIZED, "Bad credentials") - } - AuthorizationError::RepoNotAccessible => message(StatusCode::NOT_FOUND, "Not Found"), - AuthorizationError::PermissionDenied => message( - StatusCode::FORBIDDEN, - "Resource not accessible by integration", - ), - } -} - -fn accepts(headers: &HeaderMap, media_type: &str) -> bool { - headers.get_all(ACCEPT).iter().any(|value| { - value.to_str().is_ok_and(|value| { - value - .split(',') - .any(|candidate| candidate.trim() == media_type) - }) - }) -} - -fn is_exact_commit_sha(value: &str) -> bool { - value.len() == 40 && value.bytes().all(|byte| byte.is_ascii_hexdigit()) -} - -fn message(status: StatusCode, body: &'static str) -> Response { - (status, Json(serde_json::json!({ "message": body }))).into_response() -} - #[cfg(test)] mod tests { + use axum::http::header::ACCEPT; + use super::*; use crate::server::TestServer; use crate::state::AppState; diff --git a/test/twin/github/src/handlers/contents.rs b/test/twin/github/src/handlers/contents.rs index d4a718817..92f60d24a 100644 --- a/test/twin/github/src/handlers/contents.rs +++ b/test/twin/github/src/handlers/contents.rs @@ -1,17 +1,14 @@ use axum::Json; use axum::body::Body; use axum::extract::{Path, Query, State}; -use axum::http::header::{ACCEPT, CONTENT_TYPE}; +use axum::http::header::CONTENT_TYPE; use axum::http::{HeaderMap, StatusCode}; use axum::response::{IntoResponse, Response}; use serde::Deserialize; -use crate::auth::{ - BearerTokenError, InstallationTokenAccessError, authorize_installation_token, - ensure_repo_permission, -}; +use crate::handlers::support::{accepts, authorize_repo_access, is_exact_commit_sha, message}; use crate::server::SharedState; -use crate::state::{AppState, PermissionLevel, TokenPermission}; +use crate::state::{PermissionLevel, TokenPermission}; const RAW_CONTENT_MEDIA_TYPE: &str = "application/vnd.github.raw+json"; @@ -32,25 +29,24 @@ pub async fn get_content( if !accepts(&headers, RAW_CONTENT_MEDIA_TYPE) { return message(StatusCode::NOT_ACCEPTABLE, "Not Acceptable"); } - if let Err(error) = authorize(&headers, &state, &repo) { - return authorization_error(error); + if let Err(response) = authorize_repo_access( + &headers, + &state, + &repo, + TokenPermission::Contents, + PermissionLevel::Read, + ) { + return *response; } if !is_exact_commit_sha(&query.revision) { return message(StatusCode::NOT_FOUND, "Not Found"); } - let Some(repository) = state - .repositories - .iter() - .find(|repository| repository.owner == owner && repository.name == repo) - else { + let Some(repository) = state.find_repository(&owner, &repo) else { return message(StatusCode::NOT_FOUND, "Not Found"); }; - if let Some(contents) = repository - .files - .get(&(query.revision.clone(), path.clone())) - { + if let Some(contents) = repository.file_at(&query.revision, &path) { return ( StatusCode::OK, [(CONTENT_TYPE, RAW_CONTENT_MEDIA_TYPE)], @@ -59,75 +55,19 @@ pub async fn get_content( .into_response(); } - let directory_prefix = format!("{path}/"); - if repository.files.keys().any(|(sha, stored_path)| { - sha == &query.revision && stored_path.starts_with(&directory_prefix) - }) { + // GitHub answers a directory path with a JSON listing rather than raw + // bytes, which is what lets the reader reject non-file targets. + if repository.has_directory_at(&query.revision, &format!("{path}/")) { return (StatusCode::OK, Json(serde_json::json!([]))).into_response(); } message(StatusCode::NOT_FOUND, "Not Found") } -#[derive(Clone, Copy)] -enum AuthorizationError { - MissingCredentials, - InvalidCredentials, - RepoNotAccessible, - PermissionDenied, -} - -fn authorize(headers: &HeaderMap, state: &AppState, repo: &str) -> Result<(), AuthorizationError> { - let token = authorize_installation_token(headers, state).map_err(|error| match error { - BearerTokenError::Missing => AuthorizationError::MissingCredentials, - BearerTokenError::Invalid => AuthorizationError::InvalidCredentials, - })?; - ensure_repo_permission( - &token, - repo, - TokenPermission::Contents, - PermissionLevel::Read, - ) - .map_err(|error| match error { - InstallationTokenAccessError::RepoNotAccessible => AuthorizationError::RepoNotAccessible, - InstallationTokenAccessError::PermissionDenied => AuthorizationError::PermissionDenied, - }) -} - -fn authorization_error(error: AuthorizationError) -> Response { - match error { - AuthorizationError::MissingCredentials => message(StatusCode::UNAUTHORIZED, "Unauthorized"), - AuthorizationError::InvalidCredentials => { - message(StatusCode::UNAUTHORIZED, "Bad credentials") - } - AuthorizationError::RepoNotAccessible => message(StatusCode::NOT_FOUND, "Not Found"), - AuthorizationError::PermissionDenied => message( - StatusCode::FORBIDDEN, - "Resource not accessible by integration", - ), - } -} - -fn accepts(headers: &HeaderMap, media_type: &str) -> bool { - headers.get_all(ACCEPT).iter().any(|value| { - value.to_str().is_ok_and(|value| { - value - .split(',') - .any(|candidate| candidate.trim() == media_type) - }) - }) -} - -fn is_exact_commit_sha(value: &str) -> bool { - value.len() == 40 && value.bytes().all(|byte| byte.is_ascii_hexdigit()) -} - -fn message(status: StatusCode, body: &'static str) -> Response { - (status, Json(serde_json::json!({ "message": body }))).into_response() -} - #[cfg(test)] mod tests { + use axum::http::header::ACCEPT; + use super::*; use crate::server::TestServer; use crate::state::AppState; diff --git a/test/twin/github/src/handlers/mod.rs b/test/twin/github/src/handlers/mod.rs index 8ad15f34d..837db5e8b 100644 --- a/test/twin/github/src/handlers/mod.rs +++ b/test/twin/github/src/handlers/mod.rs @@ -9,6 +9,7 @@ pub mod manifests; pub mod oauth; pub mod pulls; pub mod releases; +pub mod support; pub mod users; use axum::Router; diff --git a/test/twin/github/src/handlers/pulls.rs b/test/twin/github/src/handlers/pulls.rs index 5dcbcbb76..51725da88 100644 --- a/test/twin/github/src/handlers/pulls.rs +++ b/test/twin/github/src/handlers/pulls.rs @@ -3,43 +3,11 @@ use axum::extract::{Path, State}; use axum::http::{HeaderMap, StatusCode}; use axum::response::IntoResponse; -use crate::auth::{ - BearerTokenError, InstallationTokenAccessError, authorize_installation_token, - ensure_repo_permission, -}; +use crate::auth::{authorize_installation_token, ensure_repo_permission}; +use crate::handlers::support::{bearer_token_error_response, repo_permission_error_response}; use crate::server::SharedState; use crate::state::{PermissionLevel, PullRequest, TokenPermission}; -fn bearer_token_error_response(error: BearerTokenError) -> axum::response::Response { - match error { - BearerTokenError::Missing => ( - StatusCode::UNAUTHORIZED, - Json(serde_json::json!({"message": "Unauthorized"})), - ) - .into_response(), - BearerTokenError::Invalid => ( - StatusCode::UNAUTHORIZED, - Json(serde_json::json!({"message": "Bad credentials"})), - ) - .into_response(), - } -} - -fn repo_permission_error_response(error: InstallationTokenAccessError) -> axum::response::Response { - match error { - InstallationTokenAccessError::RepoNotAccessible => ( - StatusCode::NOT_FOUND, - Json(serde_json::json!({"message": "Not Found"})), - ) - .into_response(), - InstallationTokenAccessError::PermissionDenied => ( - StatusCode::FORBIDDEN, - Json(serde_json::json!({"message": "Resource not accessible by integration"})), - ) - .into_response(), - } -} - /// POST /repos/{owner}/{repo}/pulls pub async fn create_pull_request( State(state): State, diff --git a/test/twin/github/src/handlers/support.rs b/test/twin/github/src/handlers/support.rs new file mode 100644 index 000000000..0eaf4d226 --- /dev/null +++ b/test/twin/github/src/handlers/support.rs @@ -0,0 +1,75 @@ +//! Response and authorization helpers shared by the twin's handlers. +//! +//! GitHub answers every rejection with the same `{"message": ...}` envelope, so +//! the mapping from an auth failure to a status lives here once rather than in +//! each endpoint. + +use axum::Json; +use axum::http::header::ACCEPT; +use axum::http::{HeaderMap, StatusCode}; +use axum::response::{IntoResponse, Response}; + +use crate::auth::{ + BearerTokenError, InstallationTokenAccessError, authorize_installation_token, + ensure_repo_permission, +}; +use crate::state::{AppState, PermissionLevel, TokenInfo, TokenPermission}; + +/// Render GitHub's `{"message": ...}` error envelope. +pub fn message(status: StatusCode, body: &'static str) -> Response { + (status, Json(serde_json::json!({ "message": body }))).into_response() +} + +pub fn bearer_token_error_response(error: BearerTokenError) -> Response { + match error { + BearerTokenError::Missing => message(StatusCode::UNAUTHORIZED, "Unauthorized"), + BearerTokenError::Invalid => message(StatusCode::UNAUTHORIZED, "Bad credentials"), + } +} + +pub fn repo_permission_error_response(error: InstallationTokenAccessError) -> Response { + match error { + InstallationTokenAccessError::RepoNotAccessible => { + message(StatusCode::NOT_FOUND, "Not Found") + } + InstallationTokenAccessError::PermissionDenied => message( + StatusCode::FORBIDDEN, + "Resource not accessible by integration", + ), + } +} + +/// Authorize an installation token against one repository permission, +/// rendering GitHub's response shape for every rejection. +/// +/// The rejection is boxed because an `axum` `Response` is far larger than the +/// token it displaces in the `Ok` path. +pub fn authorize_repo_access( + headers: &HeaderMap, + state: &AppState, + repo: &str, + permission: TokenPermission, + level: PermissionLevel, +) -> Result> { + let token = authorize_installation_token(headers, state) + .map_err(|error| Box::new(bearer_token_error_response(error)))?; + ensure_repo_permission(&token, repo, permission, level) + .map_err(|error| Box::new(repo_permission_error_response(error)))?; + Ok(token) +} + +/// Whether the client explicitly listed `media_type` in its `Accept` header. +pub fn accepts(headers: &HeaderMap, media_type: &str) -> bool { + headers.get_all(ACCEPT).iter().any(|value| { + value.to_str().is_ok_and(|value| { + value + .split(',') + .any(|candidate| candidate.trim() == media_type) + }) + }) +} + +/// Whether `value` is an exact 40-character hex commit SHA. +pub fn is_exact_commit_sha(value: &str) -> bool { + value.len() == 40 && value.bytes().all(|byte| byte.is_ascii_hexdigit()) +} diff --git a/test/twin/github/src/state.rs b/test/twin/github/src/state.rs index 3a0fe92ce..be22927c1 100644 --- a/test/twin/github/src/state.rs +++ b/test/twin/github/src/state.rs @@ -40,15 +40,51 @@ pub struct Installation { pub struct Repository { pub owner: String, pub name: String, + /// Ref selector (`heads/main`, `tags/v1.0.0`) to the commit SHA it names. pub refs: HashMap, - pub files: HashMap<(String, String), Vec>, + /// Commit SHA to the repository paths readable at that commit. + pub files: HashMap>>, pub default_branch: String, pub private: bool, pub git_dir: Option, } +impl Repository { + /// Contents of `path` at `sha`, if the fixture seeded one. + pub fn file_at(&self, sha: &str, path: &str) -> Option<&Vec> { + self.files.get(sha).and_then(|paths| paths.get(path)) + } + + /// Whether any path at `sha` sits under `prefix`, i.e. `prefix` names a + /// directory rather than a file. + pub fn has_directory_at(&self, sha: &str, prefix: &str) -> bool { + self.files + .get(sha) + .is_some_and(|paths| paths.keys().any(|stored| stored.starts_with(prefix))) + } + + /// Whether `sha` is a commit this repository can serve content for. + pub fn knows_commit(&self, sha: &str) -> bool { + self.files.contains_key(sha) || self.refs.values().any(|known| known == sha) + } +} + pub const DEFAULT_REPOSITORY_SHA: &str = "0123456789abcdef0123456789abcdef01234567"; +/// The ref selector GitHub uses for a branch. +pub fn heads_selector(branch: &str) -> String { + format!("heads/{branch}") +} + +/// Seed each branch at [`DEFAULT_REPOSITORY_SHA`], the shape every repository +/// fixture starts in. +pub fn head_refs(branches: Vec) -> HashMap { + branches + .into_iter() + .map(|branch| (heads_selector(&branch), DEFAULT_REPOSITORY_SHA.to_string())) + .collect() +} + /// A pull request. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct PullRequest { @@ -393,19 +429,10 @@ impl AppState { branches: Vec, private: bool, ) { - let refs = branches - .into_iter() - .map(|branch| { - ( - format!("heads/{branch}"), - DEFAULT_REPOSITORY_SHA.to_string(), - ) - }) - .collect(); self.repositories.push(Repository { owner: owner.to_string(), name: name.to_string(), - refs, + refs: head_refs(branches), files: HashMap::new(), default_branch: "main".to_string(), private, @@ -413,13 +440,21 @@ impl AppState { }); } - pub fn set_repository_ref(&mut self, owner: &str, name: &str, selector: &str, sha: &str) { - let repository = self - .repositories + pub fn find_repository(&self, owner: &str, name: &str) -> Option<&Repository> { + self.repositories + .iter() + .find(|repository| repository.owner == owner && repository.name == name) + } + + fn repository_mut(&mut self, owner: &str, name: &str) -> &mut Repository { + self.repositories .iter_mut() .find(|repository| repository.owner == owner && repository.name == name) - .expect("repository fixture should exist before adding a ref"); - repository + .expect("repository fixture should exist before seeding refs or files") + } + + pub fn set_repository_ref(&mut self, owner: &str, name: &str, selector: &str, sha: &str) { + self.repository_mut(owner, name) .refs .insert(selector.to_string(), sha.to_string()); } @@ -432,14 +467,11 @@ impl AppState { path: &str, contents: impl Into>, ) { - let repository = self - .repositories - .iter_mut() - .find(|repository| repository.owner == owner && repository.name == name) - .expect("repository fixture should exist before adding a file"); - repository + self.repository_mut(owner, name) .files - .insert((sha.to_string(), path.to_string()), contents.into()); + .entry(sha.to_string()) + .or_default() + .insert(path.to_string(), contents.into()); } pub fn find_installation(&self, owner: &str, repo: &str) -> Option<&Installation> {