From e241a1e1f936a0858ca78ff11d634e1dfa322331 Mon Sep 17 00:00:00 2001 From: "Mitch (a.k.a Voz)" Date: Wed, 14 Feb 2024 15:29:42 -0600 Subject: [PATCH] Break up Client trait --- crates/turborepo-api-client/src/lib.rs | 239 +++++++++++----------- crates/turborepo-auth/src/auth/login.rs | 53 +---- crates/turborepo-auth/src/auth/sso.rs | 53 +---- crates/turborepo-cache/src/http.rs | 3 +- crates/turborepo-lib/src/commands/link.rs | 4 +- 5 files changed, 131 insertions(+), 221 deletions(-) diff --git a/crates/turborepo-api-client/src/lib.rs b/crates/turborepo-api-client/src/lib.rs index c804dfb1d3744..fdaf298193499 100644 --- a/crates/turborepo-api-client/src/lib.rs +++ b/crates/turborepo-api-client/src/lib.rs @@ -37,14 +37,29 @@ pub trait Client { async fn get_teams(&self, token: &str) -> Result; async fn get_team(&self, token: &str, team_id: &str) -> Result>; fn add_ci_header(request_builder: RequestBuilder) -> RequestBuilder; - async fn get_caching_status( + async fn get_spaces(&self, token: &str, team_id: Option<&str>) -> Result; + async fn verify_sso_token(&self, token: &str, token_name: &str) -> Result; + async fn handle_403(response: Response) -> Error; + fn make_url(&self, endpoint: &str) -> Result; +} + +#[async_trait] +pub trait CacheClient { + async fn get_artifact( &self, + hash: &str, token: &str, team_id: Option<&str>, team_slug: Option<&str>, - ) -> Result; - async fn get_spaces(&self, token: &str, team_id: Option<&str>) -> Result; - async fn verify_sso_token(&self, token: &str, token_name: &str) -> Result; + method: Method, + ) -> Result>; + async fn fetch_artifact( + &self, + hash: &str, + token: &str, + team_id: Option<&str>, + team_slug: Option<&str>, + ) -> Result>; #[allow(clippy::too_many_arguments)] async fn put_artifact( &self, @@ -56,14 +71,6 @@ pub trait Client { team_id: Option<&str>, team_slug: Option<&str>, ) -> Result<()>; - async fn handle_403(response: Response) -> Error; - async fn fetch_artifact( - &self, - hash: &str, - token: &str, - team_id: Option<&str>, - team_slug: Option<&str>, - ) -> Result>; async fn artifact_exists( &self, hash: &str, @@ -71,15 +78,12 @@ pub trait Client { team_id: Option<&str>, team_slug: Option<&str>, ) -> Result>; - async fn get_artifact( + async fn get_caching_status( &self, - hash: &str, token: &str, team_id: Option<&str>, team_slug: Option<&str>, - method: Method, - ) -> Result>; - fn make_url(&self, endpoint: &str) -> Result; + ) -> Result; } #[async_trait] @@ -164,28 +168,6 @@ impl Client for APIClient { request_builder } - async fn get_caching_status( - &self, - token: &str, - team_id: Option<&str>, - team_slug: Option<&str>, - ) -> Result { - let request_builder = self - .client - .get(self.make_url("/v8/artifacts/status")?) - .header("User-Agent", self.user_agent.clone()) - .header("Content-Type", "application/json") - .header("Authorization", format!("Bearer {}", token)); - - let request_builder = Self::add_team_params(request_builder, team_id, team_slug); - - let response = retry::make_retryable_request(request_builder) - .await? - .error_for_status()?; - - Ok(response.json().await?) - } - async fn get_spaces(&self, token: &str, team_id: Option<&str>) -> Result { // create url with teamId if provided let endpoint = match team_id { @@ -226,64 +208,6 @@ impl Client for APIClient { }) } - #[tracing::instrument(skip_all)] - async fn put_artifact( - &self, - hash: &str, - artifact_body: &[u8], - duration: u64, - tag: Option<&str>, - token: &str, - team_id: Option<&str>, - team_slug: Option<&str>, - ) -> Result<()> { - let mut request_url = self.make_url(&format!("/v8/artifacts/{}", hash))?; - let mut allow_auth = true; - - if self.use_preflight { - let preflight_response = self - .do_preflight( - token, - request_url.clone(), - "PUT", - "Authorization, Content-Type, User-Agent, x-artifact-duration, x-artifact-tag", - ) - .await?; - - allow_auth = preflight_response.allow_authorization_header; - request_url = preflight_response.location.clone(); - } - - let mut request_builder = self - .client - .put(request_url) - .header("Content-Type", "application/octet-stream") - .header("x-artifact-duration", duration.to_string()) - .header("User-Agent", self.user_agent.clone()) - .body(artifact_body.to_vec()); - - if allow_auth { - request_builder = request_builder.header("Authorization", format!("Bearer {}", token)); - } - - request_builder = Self::add_team_params(request_builder, team_id, team_slug); - - request_builder = Self::add_ci_header(request_builder); - - if let Some(tag) = tag { - request_builder = request_builder.header("x-artifact-tag", tag); - } - - let response = retry::make_retryable_request(request_builder).await?; - - if response.status() == StatusCode::FORBIDDEN { - return Err(Self::handle_403(response).await); - } - - response.error_for_status()?; - Ok(()) - } - async fn handle_403(response: Response) -> Error { #[derive(Deserialize)] struct WrappedAPIError { @@ -331,16 +255,57 @@ impl Client for APIClient { } } - #[tracing::instrument(skip_all)] - async fn fetch_artifact( + fn make_url(&self, endpoint: &str) -> Result { + let url = format!("{}{}", self.base_url, endpoint); + Url::parse(&url).map_err(|err| Error::InvalidUrl { url, err }) + } +} + +#[async_trait] +impl CacheClient for APIClient { + async fn get_artifact( &self, hash: &str, token: &str, team_id: Option<&str>, team_slug: Option<&str>, + method: Method, ) -> Result> { - self.get_artifact(hash, token, team_id, team_slug, Method::GET) - .await + let mut request_url = self.make_url(&format!("/v8/artifacts/{}", hash))?; + let mut allow_auth = true; + + if self.use_preflight { + let preflight_response = self + .do_preflight( + token, + request_url.clone(), + "GET", + "Authorization, User-Agent", + ) + .await?; + + allow_auth = preflight_response.allow_authorization_header; + request_url = preflight_response.location; + }; + + let mut request_builder = self + .client + .request(method, request_url) + .header("User-Agent", self.user_agent.clone()); + + if allow_auth { + request_builder = request_builder.header("Authorization", format!("Bearer {}", token)); + } + + request_builder = Self::add_team_params(request_builder, team_id, team_slug); + + let response = retry::make_retryable_request(request_builder).await?; + + match response.status() { + StatusCode::FORBIDDEN => Err(Self::handle_403(response).await), + StatusCode::NOT_FOUND => Ok(None), + _ => Ok(Some(response.error_for_status()?)), + } } #[tracing::instrument(skip_all)] @@ -355,14 +320,29 @@ impl Client for APIClient { .await } - async fn get_artifact( + #[tracing::instrument(skip_all)] + async fn fetch_artifact( &self, hash: &str, token: &str, team_id: Option<&str>, team_slug: Option<&str>, - method: Method, ) -> Result> { + self.get_artifact(hash, token, team_id, team_slug, Method::GET) + .await + } + + #[tracing::instrument(skip_all)] + async fn put_artifact( + &self, + hash: &str, + artifact_body: &[u8], + duration: u64, + tag: Option<&str>, + token: &str, + team_id: Option<&str>, + team_slug: Option<&str>, + ) -> Result<()> { let mut request_url = self.make_url(&format!("/v8/artifacts/{}", hash))?; let mut allow_auth = true; @@ -371,19 +351,22 @@ impl Client for APIClient { .do_preflight( token, request_url.clone(), - "GET", - "Authorization, User-Agent", + "PUT", + "Authorization, Content-Type, User-Agent, x-artifact-duration, x-artifact-tag", ) .await?; allow_auth = preflight_response.allow_authorization_header; - request_url = preflight_response.location; - }; + request_url = preflight_response.location.clone(); + } let mut request_builder = self .client - .request(method, request_url) - .header("User-Agent", self.user_agent.clone()); + .put(request_url) + .header("Content-Type", "application/octet-stream") + .header("x-artifact-duration", duration.to_string()) + .header("User-Agent", self.user_agent.clone()) + .body(artifact_body.to_vec()); if allow_auth { request_builder = request_builder.header("Authorization", format!("Bearer {}", token)); @@ -391,18 +374,42 @@ impl Client for APIClient { request_builder = Self::add_team_params(request_builder, team_id, team_slug); + request_builder = Self::add_ci_header(request_builder); + + if let Some(tag) = tag { + request_builder = request_builder.header("x-artifact-tag", tag); + } + let response = retry::make_retryable_request(request_builder).await?; - match response.status() { - StatusCode::FORBIDDEN => Err(Self::handle_403(response).await), - StatusCode::NOT_FOUND => Ok(None), - _ => Ok(Some(response.error_for_status()?)), + if response.status() == StatusCode::FORBIDDEN { + return Err(Self::handle_403(response).await); } + + response.error_for_status()?; + Ok(()) } - fn make_url(&self, endpoint: &str) -> Result { - let url = format!("{}{}", self.base_url, endpoint); - Url::parse(&url).map_err(|err| Error::InvalidUrl { url, err }) + async fn get_caching_status( + &self, + token: &str, + team_id: Option<&str>, + team_slug: Option<&str>, + ) -> Result { + let request_builder = self + .client + .get(self.make_url("/v8/artifacts/status")?) + .header("User-Agent", self.user_agent.clone()) + .header("Content-Type", "application/json") + .header("Authorization", format!("Bearer {}", token)); + + let request_builder = Self::add_team_params(request_builder, team_id, team_slug); + + let response = retry::make_retryable_request(request_builder) + .await? + .error_for_status()?; + + Ok(response.json().await?) } } diff --git a/crates/turborepo-auth/src/auth/login.rs b/crates/turborepo-auth/src/auth/login.rs index 47592903c7027..b38effd29bbf2 100644 --- a/crates/turborepo-auth/src/auth/login.rs +++ b/crates/turborepo-auth/src/auth/login.rs @@ -104,12 +104,11 @@ mod tests { use std::{assert_matches::assert_matches, sync::atomic::AtomicUsize}; use async_trait::async_trait; - use reqwest::{Method, RequestBuilder, Response}; + use reqwest::{RequestBuilder, Response}; use turborepo_api_client::Client; use turborepo_ui::UI; use turborepo_vercel_api::{ - CachingStatusResponse, Membership, Role, SpacesResponse, Team, TeamsResponse, User, - UserResponse, VerifiedSsoUser, + Membership, Role, SpacesResponse, Team, TeamsResponse, User, UserResponse, VerifiedSsoUser, }; use turborepo_vercel_api_mock::start_test_server; @@ -209,14 +208,6 @@ mod tests { fn add_ci_header(_request_builder: RequestBuilder) -> RequestBuilder { unimplemented!("add_ci_header") } - async fn get_caching_status( - &self, - _token: &str, - _team_id: Option<&str>, - _team_slug: Option<&str>, - ) -> turborepo_api_client::Result { - unimplemented!("get_caching_status") - } async fn get_spaces( &self, _token: &str, @@ -234,49 +225,9 @@ mod tests { team_id: Some("team_id".to_string()), }) } - async fn put_artifact( - &self, - _hash: &str, - _artifact_body: &[u8], - _duration: u64, - _tag: Option<&str>, - _token: &str, - _team_id: Option<&str>, - _team_slug: Option<&str>, - ) -> turborepo_api_client::Result<()> { - unimplemented!("put_artifact") - } async fn handle_403(_response: Response) -> turborepo_api_client::Error { unimplemented!("handle_403") } - async fn fetch_artifact( - &self, - _hash: &str, - _token: &str, - _team_id: Option<&str>, - _team_slug: Option<&str>, - ) -> turborepo_api_client::Result> { - unimplemented!("fetch_artifact") - } - async fn artifact_exists( - &self, - _hash: &str, - _token: &str, - _team_id: Option<&str>, - _team_slug: Option<&str>, - ) -> turborepo_api_client::Result> { - unimplemented!("artifact_exists") - } - async fn get_artifact( - &self, - _hash: &str, - _token: &str, - _team_id: Option<&str>, - _team_slug: Option<&str>, - _method: Method, - ) -> turborepo_api_client::Result> { - unimplemented!("get_artifact") - } fn make_url(&self, endpoint: &str) -> turborepo_api_client::Result { let url = format!("{}{}", self.base_url, endpoint); Url::parse(&url).map_err(|err| turborepo_api_client::Error::InvalidUrl { url, err }) diff --git a/crates/turborepo-auth/src/auth/sso.rs b/crates/turborepo-auth/src/auth/sso.rs index 153fef6f54b61..39bad6b5ac192 100644 --- a/crates/turborepo-auth/src/auth/sso.rs +++ b/crates/turborepo-auth/src/auth/sso.rs @@ -121,12 +121,11 @@ mod tests { use std::sync::atomic::AtomicUsize; use async_trait::async_trait; - use reqwest::{Method, RequestBuilder, Response}; + use reqwest::{RequestBuilder, Response}; use turborepo_api_client::Client; use turborepo_ui::UI; use turborepo_vercel_api::{ - CachingStatusResponse, Membership, Role, SpacesResponse, Team, TeamsResponse, User, - UserResponse, VerifiedSsoUser, + Membership, Role, SpacesResponse, Team, TeamsResponse, User, UserResponse, VerifiedSsoUser, }; use turborepo_vercel_api_mock::start_test_server; @@ -215,14 +214,6 @@ mod tests { fn add_ci_header(_request_builder: RequestBuilder) -> RequestBuilder { unimplemented!("add_ci_header") } - async fn get_caching_status( - &self, - _token: &str, - _team_id: Option<&str>, - _team_slug: Option<&str>, - ) -> turborepo_api_client::Result { - unimplemented!("get_caching_status") - } async fn get_spaces( &self, _token: &str, @@ -240,49 +231,9 @@ mod tests { team_id: Some("team_id".to_string()), }) } - async fn put_artifact( - &self, - _hash: &str, - _artifact_body: &[u8], - _duration: u64, - _tag: Option<&str>, - _token: &str, - _team_id: Option<&str>, - _team_slug: Option<&str>, - ) -> turborepo_api_client::Result<()> { - unimplemented!("put_artifact") - } async fn handle_403(_response: Response) -> turborepo_api_client::Error { unimplemented!("handle_403") } - async fn fetch_artifact( - &self, - _hash: &str, - _token: &str, - _team_id: Option<&str>, - _team_slug: Option<&str>, - ) -> turborepo_api_client::Result> { - unimplemented!("fetch_artifact") - } - async fn artifact_exists( - &self, - _hash: &str, - _token: &str, - _team_id: Option<&str>, - _team_slug: Option<&str>, - ) -> turborepo_api_client::Result> { - unimplemented!("artifact_exists") - } - async fn get_artifact( - &self, - _hash: &str, - _token: &str, - _team_id: Option<&str>, - _team_slug: Option<&str>, - _method: Method, - ) -> turborepo_api_client::Result> { - unimplemented!("get_artifact") - } fn make_url(&self, endpoint: &str) -> turborepo_api_client::Result { let url = format!("{}{}", self.base_url, endpoint); Url::parse(&url).map_err(|err| turborepo_api_client::Error::InvalidUrl { url, err }) diff --git a/crates/turborepo-cache/src/http.rs b/crates/turborepo-cache/src/http.rs index 6803beb9b9a0f..fdb02afe6b471 100644 --- a/crates/turborepo-cache/src/http.rs +++ b/crates/turborepo-cache/src/http.rs @@ -4,7 +4,8 @@ use tracing::debug; use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf, AnchoredSystemPathBuf}; use turborepo_analytics::AnalyticsSender; use turborepo_api_client::{ - analytics, analytics::AnalyticsEvent, APIAuth, APIClient, Client, Response, + analytics::{self, AnalyticsEvent}, + APIAuth, APIClient, CacheClient, Response, }; use crate::{ diff --git a/crates/turborepo-lib/src/commands/link.rs b/crates/turborepo-lib/src/commands/link.rs index f9ad0646da18f..6fd0b80eeea09 100644 --- a/crates/turborepo-lib/src/commands/link.rs +++ b/crates/turborepo-lib/src/commands/link.rs @@ -17,7 +17,7 @@ use dirs_next::home_dir; #[cfg(test)] use rand::Rng; use thiserror::Error; -use turborepo_api_client::Client; +use turborepo_api_client::{CacheClient, Client}; #[cfg(not(test))] use turborepo_ui::CYAN; use turborepo_ui::{BOLD, GREY, UNDERLINE}; @@ -109,7 +109,7 @@ pub(crate) const SPACES_URL: &str = "https://vercel.com/docs/workflow-collaborat /// /// returns: Result<(), Error> pub(crate) async fn verify_caching_enabled<'a>( - api_client: &impl Client, + api_client: &(impl Client + CacheClient), team_id: &str, token: &str, selected_team: Option>,