From 0708f6f348342836d7c21b83892f221505364f9e Mon Sep 17 00:00:00 2001 From: Jiahao XU Date: Mon, 6 May 2024 23:00:02 +1000 Subject: [PATCH] Ret artifact url in `has_release_artifact` So that we can use it to download from private repositories. Signed-off-by: Jiahao XU --- crates/binstalk-fetchers/src/common.rs | 2 +- .../src/gh_api_client.rs | 21 +++++++++---------- .../src/gh_api_client/request.rs | 13 +++++++++--- crates/binstalk/src/helpers/remote.rs | 2 +- 4 files changed, 22 insertions(+), 16 deletions(-) diff --git a/crates/binstalk-fetchers/src/common.rs b/crates/binstalk-fetchers/src/common.rs index 921fe8d7..28267390 100644 --- a/crates/binstalk-fetchers/src/common.rs +++ b/crates/binstalk-fetchers/src/common.rs @@ -35,7 +35,7 @@ pub(super) async fn does_url_exist( // The future returned has the same size as a pointer match gh_api_client.has_release_artifact(artifact).await? { - HasReleaseArtifact::Yes => return Ok(true), + HasReleaseArtifact::Yes { .. } => return Ok(true), HasReleaseArtifact::No | HasReleaseArtifact::NoSuchRelease => return Ok(false), HasReleaseArtifact::RateLimit { retry_after } => { diff --git a/crates/binstalk-git-repo-api/src/gh_api_client.rs b/crates/binstalk-git-repo-api/src/gh_api_client.rs index 23b9194e..2e4ded9c 100644 --- a/crates/binstalk-git-repo-api/src/gh_api_client.rs +++ b/crates/binstalk-git-repo-api/src/gh_api_client.rs @@ -223,14 +223,10 @@ impl GhApiClient { .await; match res { - Ok(Some(artifacts)) => { - let has_artifact = artifacts.contains(&artifact_name); - Ok(if has_artifact { - HasReleaseArtifact::Yes - } else { - HasReleaseArtifact::No - }) - } + Ok(Some(artifacts)) => Ok(artifacts + .get_artifact_url(&artifact_name) + .map(|url| HasReleaseArtifact::Yes { url }) + .unwrap_or(HasReleaseArtifact::No)), Ok(None) => Ok(HasReleaseArtifact::NoSuchRelease), Err(Error::Unauthorized) => Ok(HasReleaseArtifact::Unauthorized), Err(Error::RateLimit { retry_after }) => { @@ -243,9 +239,12 @@ impl GhApiClient { } } -#[derive(Eq, PartialEq, Copy, Clone, Debug)] +#[derive(Eq, PartialEq, Clone, Debug)] pub enum HasReleaseArtifact { - Yes, + Yes { + /// get url for downloading the artifact using GitHub API (for private repository). + url: CompactString, + }, No, NoSuchRelease, /// GitHub returns 401 requiring a token. @@ -399,7 +398,7 @@ mod test { assert!( matches!( ret, - HasReleaseArtifact::Yes | HasReleaseArtifact::RateLimit { .. } + HasReleaseArtifact::Yes { .. } | HasReleaseArtifact::RateLimit { .. } ), "for '{artifact_name}': answer is {:#?}", ret diff --git a/crates/binstalk-git-repo-api/src/gh_api_client/request.rs b/crates/binstalk-git-repo-api/src/gh_api_client/request.rs index ff73c17a..968103c9 100644 --- a/crates/binstalk-git-repo-api/src/gh_api_client/request.rs +++ b/crates/binstalk-git-repo-api/src/gh_api_client/request.rs @@ -20,6 +20,7 @@ use super::{percent_encode_http_url_path, remote, GhApiError, GhGraphQLErrors, G #[derive(Eq, Deserialize, Debug)] struct Artifact { name: CompactString, + url: CompactString, } // Manually implement PartialEq and Hash to ensure it will always produce the @@ -57,8 +58,11 @@ pub(super) struct Artifacts { } impl Artifacts { - pub(super) fn contains(&self, artifact_name: &str) -> bool { - self.assets.contains(artifact_name) + /// get url for downloading the artifact using GitHub API (for private repository). + pub(super) fn get_artifact_url(&self, artifact_name: &str) -> Option { + self.assets + .get(artifact_name) + .map(|artifact| artifact.url.clone()) } } @@ -201,7 +205,10 @@ query {{ repository(owner:"{owner}",name:"{repo}") {{ release(tagName:"{tag}") {{ releaseAssets({cond}) {{ - nodes {{ name }} + nodes {{ + name + url + }} pageInfo {{ endCursor hasNextPage }} }} }} diff --git a/crates/binstalk/src/helpers/remote.rs b/crates/binstalk/src/helpers/remote.rs index af734f8d..66dc5490 100644 --- a/crates/binstalk/src/helpers/remote.rs +++ b/crates/binstalk/src/helpers/remote.rs @@ -29,7 +29,7 @@ pub async fn does_url_exist( // The future returned has the same size as a pointer match gh_api_client.has_release_artifact(artifact).await? { - HasReleaseArtifact::Yes => return Ok(true), + HasReleaseArtifact::Yes { .. } => return Ok(true), HasReleaseArtifact::No | HasReleaseArtifact::NoSuchRelease => return Ok(false), HasReleaseArtifact::RateLimit { retry_after } => {