From 24b1d803bbb24298cbb62347fc6b094f86a7ff58 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sat, 24 Jan 2026 13:51:57 -0500 Subject: [PATCH 1/4] Start on sync api --- rs_lib/src/file_fetcher/mod.rs | 380 ++++++++++++++++++++++----------- 1 file changed, 259 insertions(+), 121 deletions(-) diff --git a/rs_lib/src/file_fetcher/mod.rs b/rs_lib/src/file_fetcher/mod.rs index 5e560af..4544c5e 100644 --- a/rs_lib/src/file_fetcher/mod.rs +++ b/rs_lib/src/file_fetcher/mod.rs @@ -5,6 +5,7 @@ use std::collections::HashMap; use std::io::Read; use std::path::Path; use std::path::PathBuf; +use std::thread::AccessError; use std::time::SystemTime; use boxed_error::Boxed; @@ -446,6 +447,18 @@ pub trait HttpClient: std::fmt::Debug + MaybeSend + MaybeSync { url: &Url, headers: HeaderMap, ) -> Result; + + /// Send a request getting the response synchronously. + /// + /// The implementation MUST not follow redirects. Return `SendResponse::Redirect` + /// in that case. + /// + /// The implementation may retry the request on failure. + fn send_no_follow_sync( + &self, + url: &Url, + headers: HeaderMap, + ) -> Result; } #[derive(Debug, Clone)] @@ -462,11 +475,17 @@ impl BlobStore for NullBlobStore { async fn get(&self, _url: &Url) -> std::io::Result> { Ok(None) } + + fn get_sync(&self, _url: &Url) -> std::io::Result> { + Ok(None) + } } #[async_trait::async_trait(?Send)] pub trait BlobStore: std::fmt::Debug + MaybeSend + MaybeSync { async fn get(&self, url: &Url) -> std::io::Result>; + /// Synchronous get when requiring ESM. + fn get_sync(&self, url: &Url) -> std::io::Result>; } #[derive(Debug, Default)] @@ -474,7 +493,7 @@ pub struct FetchNoFollowOptions<'a> { pub local: FetchLocalOptions, pub maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, pub maybe_checksum: Option>, - pub maybe_accept: Option<&'a str>, + pub maybe_accept: Option>, pub maybe_cache_setting: Option<&'a CacheSetting>, } @@ -625,12 +644,26 @@ impl if !self.allow_remote { Err(FetchNoFollowErrorKind::NoRemote(url.clone()).into_box()) } else { - self - .fetch_remote_no_follow( - strategy, + let cache_setting = + options.maybe_cache_setting.unwrap_or(&self.cache_setting); + + if self.should_use_cache(url, cache_setting) + && let Some(value) = strategy + .handle_fetch_cached_no_follow(url, options.maybe_checksum)? + { + return Ok(value); + } + + if *cache_setting == CacheSetting::Only { + return Err( + FetchNoFollowErrorKind::NotCached { url: url.clone() }.into_box(), + ); + } + + strategy + .handle_fetch_remote_no_follow_no_cache( url, options.maybe_accept, - options.maybe_cache_setting.unwrap_or(&self.cache_setting), options.maybe_checksum, options.maybe_auth, ) @@ -720,10 +753,23 @@ impl url: &Url, ) -> Result { debug!("FileFetcher::fetch_blob_url() - specifier: {}", url); - let blob = self - .blob_store - .get(url) - .await + let result = self.blob_store.get(url).await; + self.handle_fetch_blob_url_result(url, result) + } + + /// Get a blob URL. + fn fetch_blob_url_sync(&self, url: &Url) -> Result { + debug!("FileFetcher::fetch_blob_url_sync() - specifier: {}", url); + let result = self.blob_store.get_sync(url); + self.handle_fetch_blob_url_result(url, result) + } + + fn handle_fetch_blob_url_result( + &self, + url: &Url, + result: std::io::Result>, + ) -> Result { + let blob = result .map_err(|err| FetchNoFollowErrorKind::ReadingBlobUrl { url: url.clone(), source: err, @@ -747,26 +793,11 @@ impl &self, strategy: &TStrategy, url: &Url, - maybe_accept: Option<&str>, + maybe_accept: Option>, cache_setting: &CacheSetting, maybe_checksum: Option>, maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, ) -> Result { - debug!("FileFetcher::fetch_remote_no_follow - specifier: {}", url); - - if self.should_use_cache(url, cache_setting) - && let Some(value) = - strategy.handle_fetch_cached_no_follow(url, maybe_checksum)? - { - return Ok(value); - } - - if *cache_setting == CacheSetting::Only { - return Err( - FetchNoFollowErrorKind::NotCached { url: url.clone() }.into_box(), - ); - } - strategy .handle_fetch_remote_no_follow_no_cache( url, @@ -780,11 +811,51 @@ impl async fn fetch_remote_no_follow_no_cache( &self, url: &Url, - maybe_accept: Option<&str>, + maybe_accept: Option>, + maybe_checksum: Option>, + maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, + ) -> Result { + let (maybe_etag_cache_entry, maybe_etag) = + self.get_etag_entry(url, maybe_checksum).unzip(); + let headers = + self.build_headers(url, maybe_accept, maybe_auth, maybe_etag)?; + let result = self.http_client.send_no_follow(url, headers).await; + let response = self.handle_send_result(url, result)?; + self.handle_fetch_remote_response( + url, + response, + maybe_checksum, + maybe_etag_cache_entry, + ) + } + + fn fetch_remote_no_follow_no_cache_sync( + &self, + url: &Url, + maybe_accept: Option>, maybe_checksum: Option>, maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, ) -> Result { - let maybe_etag_cache_entry = self + let (maybe_etag_cache_entry, maybe_etag) = + self.get_etag_entry(url, maybe_checksum).unzip(); + let headers = + self.build_headers(url, maybe_accept, maybe_auth, maybe_etag)?; + let result = self.http_client.send_no_follow_sync(url, headers); + let response = self.handle_send_result(url, result)?; + self.handle_fetch_remote_response( + url, + response, + maybe_checksum, + maybe_etag_cache_entry, + ) + } + + fn get_etag_entry( + &self, + url: &Url, + maybe_checksum: Option>, + ) -> Option<(CacheEntry, String)> { + self .http_cache .cache_item_key(url) .ok() @@ -795,23 +866,64 @@ impl .headers .remove("etag") .map(|etag| (cache_entry, etag)) - }); - - let maybe_auth_token = self.auth_tokens.get(url); - match self - .send_request(SendRequestArgs { - url, - maybe_accept, - maybe_auth: maybe_auth.clone(), - maybe_auth_token, - maybe_etag: maybe_etag_cache_entry - .as_ref() - .map(|(_, etag)| etag.as_str()), }) - .await? - { + } + + fn handle_send_result( + &self, + url: &Url, + send_result: Result, + ) -> Result { + match send_result { + Ok(resp) => match resp { + SendResponse::NotModified => Ok(SendRequestResponse::NotModified), + SendResponse::Redirect(headers) => { + let new_url = + resolve_redirect_from_headers(url, &headers).map_err(|err| { + FetchNoFollowErrorKind::RedirectHeaderParse(*err).into_box() + })?; + Ok(SendRequestResponse::Redirect( + new_url, + response_headers_to_headers_map(headers), + )) + } + SendResponse::Success(headers, body) => Ok(SendRequestResponse::Code( + body, + response_headers_to_headers_map(headers), + )), + }, + Err(err) => match err { + SendError::Failed(err) => Err( + FetchNoFollowErrorKind::FetchingRemote { + url: url.clone(), + source: err, + } + .into_box(), + ), + SendError::NotFound => { + Err(FetchNoFollowErrorKind::NotFound(url.clone()).into_box()) + } + SendError::StatusCode(status_code) => Err( + FetchNoFollowErrorKind::ClientError { + url: url.clone(), + status_code, + } + .into_box(), + ), + }, + } + } + + fn handle_fetch_remote_response( + &self, + url: &Url, + response: SendRequestResponse, + maybe_checksum: Option>, + maybe_etag_cache_entry: Option, + ) -> Result { + match response { SendRequestResponse::NotModified => { - let (cache_entry, _) = maybe_etag_cache_entry.unwrap(); + let cache_entry = maybe_etag_cache_entry.unwrap(); FileOrRedirect::from_deno_cache_entry(url, cache_entry).map_err(|err| { FetchNoFollowErrorKind::RedirectResolution(err).into_box() }) @@ -832,7 +944,7 @@ impl source, }, )?; - if let Some(checksum) = &maybe_checksum { + if let Some(checksum) = maybe_checksum { checksum .check(url, &bytes) .map_err(|err| FetchNoFollowErrorKind::ChecksumIntegrity(*err))?; @@ -891,84 +1003,59 @@ impl } } - /// Asynchronously fetches the given HTTP URL one pass only. - /// If no redirect is present and no error occurs, - /// yields Code(ResultPayload). - /// If redirect occurs, does not follow and - /// yields Redirect(url). - async fn send_request( + fn build_headers( &self, - args: SendRequestArgs<'_>, - ) -> Result { - let mut headers = HeaderMap::with_capacity(3); - - if let Some(etag) = args.maybe_etag { - let if_none_match_val = - HeaderValue::from_str(etag).map_err(|source| { + url: &Url, + maybe_accept: Option>, + maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, + maybe_etag: Option, + ) -> Result { + let maybe_auth_token = self.auth_tokens.get(url); + let capacity = [ + maybe_etag.is_some(), + maybe_auth.is_some() || maybe_auth_token.is_some(), + maybe_accept.is_some(), + ] + .into_iter() + .filter(|v| *v) + .count(); + let mut headers = HeaderMap::with_capacity(capacity); + + if let Some(etag) = maybe_etag { + let if_none_match_val = etag.try_into().map_err(|source| { + FetchNoFollowErrorKind::InvalidHeader { + name: "etag", + source, + } + })?; + headers.insert(IF_NONE_MATCH, if_none_match_val); + } + if let Some(auth_token) = maybe_auth_token { + let authorization_val = + auth_token.to_string().try_into().map_err(|source| { FetchNoFollowErrorKind::InvalidHeader { - name: "etag", + name: "authorization", source, } })?; - headers.insert(IF_NONE_MATCH, if_none_match_val); - } - if let Some(auth_token) = args.maybe_auth_token { - let authorization_val = HeaderValue::from_str(&auth_token.to_string()) - .map_err(|source| FetchNoFollowErrorKind::InvalidHeader { - name: "authorization", - source, - })?; headers.insert(AUTHORIZATION, authorization_val); - } else if let Some((header, value)) = args.maybe_auth { + } else if let Some((header, value)) = maybe_auth { headers.insert(header, value); } - if let Some(accept) = args.maybe_accept { - let accepts_val = HeaderValue::from_str(accept).map_err(|source| { - FetchNoFollowErrorKind::InvalidHeader { + if let Some(accept) = maybe_accept { + let result = match accept { + Cow::Borrowed(accept) => accept.try_into(), + Cow::Owned(accept) => accept.try_into(), + }; + let accepts_val = + result.map_err(|source| FetchNoFollowErrorKind::InvalidHeader { name: "accept", source, - } - })?; + })?; headers.insert(ACCEPT, accepts_val); } - match self.http_client.send_no_follow(args.url, headers).await { - Ok(resp) => match resp { - SendResponse::NotModified => Ok(SendRequestResponse::NotModified), - SendResponse::Redirect(headers) => { - let new_url = resolve_redirect_from_headers(args.url, &headers) - .map_err(|err| { - FetchNoFollowErrorKind::RedirectHeaderParse(*err).into_box() - })?; - Ok(SendRequestResponse::Redirect( - new_url, - response_headers_to_headers_map(headers), - )) - } - SendResponse::Success(headers, body) => Ok(SendRequestResponse::Code( - body, - response_headers_to_headers_map(headers), - )), - }, - Err(err) => match err { - SendError::Failed(err) => Err( - FetchNoFollowErrorKind::FetchingRemote { - url: args.url.clone(), - source: err, - } - .into_box(), - ), - SendError::NotFound => { - Err(FetchNoFollowErrorKind::NotFound(args.url.clone()).into_box()) - } - SendError::StatusCode(status_code) => Err( - FetchNoFollowErrorKind::ClientError { - url: args.url.clone(), - status_code, - } - .into_box(), - ), - }, - } + debug_assert_eq!(headers.capacity(), capacity); + Ok(headers) } /// Fetch a source file from the local file system. @@ -1066,6 +1153,11 @@ trait FetchOrEnsureCacheStrategy { url: &Url, ) -> Result; + fn handle_blob_url_sync( + &self, + url: &Url, + ) -> Result; + fn handle_fetch_cached_no_follow( &self, url: &Url, @@ -1075,7 +1167,15 @@ trait FetchOrEnsureCacheStrategy { async fn handle_fetch_remote_no_follow_no_cache( &self, url: &Url, - maybe_accept: Option<&str>, + maybe_accept: Option>, + maybe_checksum: Option>, + maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, + ) -> Result; + + fn handle_fetch_remote_no_follow_no_cache_sync( + &self, + url: &Url, + maybe_accept: Option>, maybe_checksum: Option>, maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, ) -> Result; @@ -1124,6 +1224,13 @@ impl self.0.fetch_blob_url(url).await.map(FileOrRedirect::File) } + fn handle_blob_url_sync( + &self, + url: &Url, + ) -> Result { + self.0.fetch_blob_url_sync(url).map(FileOrRedirect::File) + } + fn handle_fetch_cached_no_follow( &self, url: &Url, @@ -1135,7 +1242,7 @@ impl async fn handle_fetch_remote_no_follow_no_cache( &self, url: &Url, - maybe_accept: Option<&str>, + maybe_accept: Option>, maybe_checksum: Option>, maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, ) -> Result { @@ -1149,6 +1256,21 @@ impl ) .await } + + fn handle_fetch_remote_no_follow_no_cache_sync( + &self, + url: &Url, + maybe_accept: Option>, + maybe_checksum: Option>, + maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, + ) -> Result { + self.0.fetch_remote_no_follow_no_cache_sync( + url, + maybe_accept, + maybe_checksum, + maybe_auth, + ) + } } struct EnsureCachedStrategy< @@ -1193,6 +1315,13 @@ impl Ok(CachedOrRedirect::Cached) } + fn handle_blob_url_sync( + &self, + _url: &Url, + ) -> Result { + Ok(CachedOrRedirect::Cached) + } + fn handle_fetch_cached_no_follow( &self, url: &Url, @@ -1212,7 +1341,7 @@ impl async fn handle_fetch_remote_no_follow_no_cache( &self, url: &Url, - maybe_accept: Option<&str>, + maybe_accept: Option>, maybe_checksum: Option>, maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, ) -> Result { @@ -1227,6 +1356,24 @@ impl .await .map(|file_or_redirect| file_or_redirect.into()) } + + fn handle_fetch_remote_no_follow_no_cache_sync( + &self, + url: &Url, + maybe_accept: Option>, + maybe_checksum: Option>, + maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, + ) -> Result { + self + .0 + .fetch_remote_no_follow_no_cache_sync( + url, + maybe_accept, + maybe_checksum, + maybe_auth, + ) + .map(|file_or_redirect| file_or_redirect.into()) + } } fn response_headers_to_headers_map(response_headers: HeaderMap) -> HeadersMap { @@ -1302,15 +1449,6 @@ fn resolve_url_from_location( } } -#[derive(Debug)] -struct SendRequestArgs<'a> { - pub url: &'a Url, - pub maybe_accept: Option<&'a str>, - pub maybe_etag: Option<&'a str>, - pub maybe_auth_token: Option<&'a AuthToken>, - pub maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, -} - #[derive(Debug, Eq, PartialEq)] enum SendRequestResponse { Code(Vec, HeadersMap), From 9e6be081707105df68a995d03b8a03e8d27855d1 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sat, 24 Jan 2026 14:01:35 -0500 Subject: [PATCH 2/4] finish adding sync methods --- rs_lib/src/file_fetcher/mod.rs | 137 +++++++++++++++++++++++------- rs_lib/tests/file_fetcher_test.rs | 16 ++++ 2 files changed, 120 insertions(+), 33 deletions(-) diff --git a/rs_lib/src/file_fetcher/mod.rs b/rs_lib/src/file_fetcher/mod.rs index 4544c5e..eec1437 100644 --- a/rs_lib/src/file_fetcher/mod.rs +++ b/rs_lib/src/file_fetcher/mod.rs @@ -5,7 +5,6 @@ use std::collections::HashMap; use std::io::Read; use std::path::Path; use std::path::PathBuf; -use std::thread::AccessError; use std::time::SystemTime; use boxed_error::Boxed; @@ -488,6 +487,12 @@ pub trait BlobStore: std::fmt::Debug + MaybeSend + MaybeSync { fn get_sync(&self, url: &Url) -> std::io::Result>; } +enum FetchNoFollowResultWithDeferred { + Value(TReturn), + Blob, + Https, +} + #[derive(Debug, Default)] pub struct FetchNoFollowOptions<'a> { pub local: FetchLocalOptions, @@ -603,6 +608,19 @@ impl .await } + /// Synchronously fetches without following redirects. + /// + /// You should verify permissions of the specifier before calling this function. + pub fn fetch_no_follow_sync( + &self, + url: &Url, + options: FetchNoFollowOptions<'_>, + ) -> Result { + // note: this debug output is used by the tests + debug!("FileFetcher::fetch_no_follow_sync - specifier: {}", url); + self.fetch_no_follow_with_strategy_sync(&FetchStrategy(self), url, options) + } + /// Ensures the data is cached without following redirects. /// /// You should verify permissions of the specifier before calling this function. @@ -618,6 +636,26 @@ impl .await } + /// Synchronously ensures the data is cached without following redirects. + /// + /// You should verify permissions of the specifier before calling this function. + pub fn ensure_cached_no_follow_sync( + &self, + url: &Url, + options: FetchNoFollowOptions<'_>, + ) -> Result { + // note: this debug output is used by the tests + debug!( + "FileFetcher::ensure_cached_no_follow_sync - specifier: {}", + url + ); + self.fetch_no_follow_with_strategy_sync( + &EnsureCachedStrategy(self), + url, + options, + ) + } + async fn fetch_no_follow_with_strategy< TStrategy: FetchOrEnsureCacheStrategy, >( @@ -626,20 +664,79 @@ impl url: &Url, options: FetchNoFollowOptions<'_>, ) -> Result { + match self + .fetch_no_follow_with_strategy_deferred(strategy, url, &options)? + { + FetchNoFollowResultWithDeferred::Value(value) => Ok(value), + FetchNoFollowResultWithDeferred::Blob => { + strategy.handle_blob_url(url).await + } + FetchNoFollowResultWithDeferred::Https => { + strategy + .handle_fetch_remote_no_follow_no_cache( + url, + options.maybe_accept, + options.maybe_checksum, + options.maybe_auth, + ) + .await + } + } + } + + fn fetch_no_follow_with_strategy_sync< + TStrategy: FetchOrEnsureCacheStrategy, + >( + &self, + strategy: &TStrategy, + url: &Url, + options: FetchNoFollowOptions<'_>, + ) -> Result { + match self + .fetch_no_follow_with_strategy_deferred(strategy, url, &options)? + { + FetchNoFollowResultWithDeferred::Value(value) => Ok(value), + FetchNoFollowResultWithDeferred::Blob => { + strategy.handle_blob_url_sync(url) + } + FetchNoFollowResultWithDeferred::Https => strategy + .handle_fetch_remote_no_follow_no_cache_sync( + url, + options.maybe_accept, + options.maybe_checksum, + options.maybe_auth, + ), + } + } + + fn fetch_no_follow_with_strategy_deferred< + TStrategy: FetchOrEnsureCacheStrategy, + >( + &self, + strategy: &TStrategy, + url: &Url, + options: &FetchNoFollowOptions<'_>, + ) -> Result< + FetchNoFollowResultWithDeferred, + FetchNoFollowError, + > { let scheme = url.scheme(); if let Some(file) = self.memory_files.get(url) { - Ok(strategy.handle_memory_file(file)) + Ok(FetchNoFollowResultWithDeferred::Value( + strategy.handle_memory_file(file), + )) } else if scheme == "file" { match strategy.handle_local(url, &options.local)? { - Some(file) => Ok(file), + Some(file) => Ok(FetchNoFollowResultWithDeferred::Value(file)), None => Err(FetchNoFollowErrorKind::NotFound(url.clone()).into_box()), } } else if scheme == "data" { strategy .handle_data_url(url) + .map(FetchNoFollowResultWithDeferred::Value) .map_err(|e| FetchNoFollowErrorKind::DataUrlDecode(e).into_box()) } else if scheme == "blob" { - strategy.handle_blob_url(url).await + Ok(FetchNoFollowResultWithDeferred::Blob) } else if scheme == "https" || scheme == "http" { if !self.allow_remote { Err(FetchNoFollowErrorKind::NoRemote(url.clone()).into_box()) @@ -651,7 +748,7 @@ impl && let Some(value) = strategy .handle_fetch_cached_no_follow(url, options.maybe_checksum)? { - return Ok(value); + return Ok(FetchNoFollowResultWithDeferred::Value(value)); } if *cache_setting == CacheSetting::Only { @@ -660,14 +757,7 @@ impl ); } - strategy - .handle_fetch_remote_no_follow_no_cache( - url, - options.maybe_accept, - options.maybe_checksum, - options.maybe_auth, - ) - .await + Ok(FetchNoFollowResultWithDeferred::Https) } } else { Err( @@ -789,25 +879,6 @@ impl }) } - async fn fetch_remote_no_follow( - &self, - strategy: &TStrategy, - url: &Url, - maybe_accept: Option>, - cache_setting: &CacheSetting, - maybe_checksum: Option>, - maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, - ) -> Result { - strategy - .handle_fetch_remote_no_follow_no_cache( - url, - maybe_accept, - maybe_checksum, - maybe_auth, - ) - .await - } - async fn fetch_remote_no_follow_no_cache( &self, url: &Url, @@ -1009,7 +1080,7 @@ impl maybe_accept: Option>, maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, maybe_etag: Option, - ) -> Result { + ) -> Result { let maybe_auth_token = self.auth_tokens.get(url); let capacity = [ maybe_etag.is_some(), diff --git a/rs_lib/tests/file_fetcher_test.rs b/rs_lib/tests/file_fetcher_test.rs index 81ddf18..ebec5da 100644 --- a/rs_lib/tests/file_fetcher_test.rs +++ b/rs_lib/tests/file_fetcher_test.rs @@ -34,6 +34,14 @@ async fn test_file_fetcher_redirects() { #[async_trait::async_trait(?Send)] impl HttpClient for TestHttpClient { async fn send_no_follow( + &self, + url: &Url, + headers: HeaderMap, + ) -> Result { + self.send_no_follow_sync(url, headers) + } + + fn send_no_follow_sync( &self, _url: &Url, _headers: HeaderMap, @@ -91,6 +99,14 @@ async fn test_file_fetcher_ensure_cached() { #[async_trait::async_trait(?Send)] impl HttpClient for TestHttpClient { async fn send_no_follow( + &self, + url: &Url, + headers: HeaderMap, + ) -> Result { + self.send_no_follow_sync(url, headers) + } + + fn send_no_follow_sync( &self, url: &Url, _headers: HeaderMap, From 6f7a72fc117e6b6f4ea59b6d8de13ca2212b6ba7 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sat, 24 Jan 2026 14:03:42 -0500 Subject: [PATCH 3/4] add tests --- rs_lib/tests/file_fetcher_test.rs | 146 ++++++++++++++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/rs_lib/tests/file_fetcher_test.rs b/rs_lib/tests/file_fetcher_test.rs index ebec5da..32eb939 100644 --- a/rs_lib/tests/file_fetcher_test.rs +++ b/rs_lib/tests/file_fetcher_test.rs @@ -173,6 +173,152 @@ async fn test_file_fetcher_ensure_cached() { } } +#[test] +fn test_file_fetcher_fetch_no_follow_sync() { + #[derive(Debug)] + struct TestHttpClient; + + #[async_trait::async_trait(?Send)] + impl HttpClient for TestHttpClient { + async fn send_no_follow( + &self, + url: &Url, + headers: HeaderMap, + ) -> Result { + self.send_no_follow_sync(url, headers) + } + + fn send_no_follow_sync( + &self, + _url: &Url, + _headers: HeaderMap, + ) -> Result { + Ok(SendResponse::Redirect(HeaderMap::new())) + } + } + + let sys = InMemorySys::default(); + let file_fetcher = create_file_fetcher(sys.clone(), TestHttpClient); + + // test bad redirect + let result = file_fetcher.fetch_no_follow_sync( + &Url::parse("http://localhost/bad_redirect").unwrap(), + FetchNoFollowOptions::default(), + ); + + match result.unwrap_err().into_kind() { + FetchNoFollowErrorKind::RedirectHeaderParse(err) => { + assert_eq!(err.request_url.as_str(), "http://localhost/bad_redirect"); + } + err => unreachable!("{:?}", err), + } + + // test local file + let time = SystemTime::now(); + sys.set_time(Some(time)); + sys.fs_create_dir_all("/").unwrap(); + sys.fs_write("/some_path.ts", "text").unwrap(); + + for include_mtime in [true, false] { + let result = file_fetcher.fetch_no_follow_sync( + &Url::parse("file:///some_path.ts").unwrap(), + FetchNoFollowOptions { + local: FetchLocalOptions { include_mtime }, + ..Default::default() + }, + ); + match result.unwrap() { + FileOrRedirect::File(file) => { + assert_eq!(file.mtime, if include_mtime { Some(time) } else { None }); + assert_eq!(file.source.to_vec(), b"text"); + } + FileOrRedirect::Redirect(_) => unreachable!(), + } + } +} + +#[test] +fn test_file_fetcher_ensure_cached_no_follow_sync() { + #[derive(Debug)] + struct TestHttpClient; + + #[async_trait::async_trait(?Send)] + impl HttpClient for TestHttpClient { + async fn send_no_follow( + &self, + url: &Url, + headers: HeaderMap, + ) -> Result { + self.send_no_follow_sync(url, headers) + } + + fn send_no_follow_sync( + &self, + url: &Url, + _headers: HeaderMap, + ) -> Result { + if url.path() == "/redirect" { + let mut header_map = HeaderMap::new(); + header_map.insert(http::header::LOCATION, "/home".parse().unwrap()); + Ok(SendResponse::Redirect(header_map)) + } else { + Ok(SendResponse::Success( + HeaderMap::new(), + "hello".to_string().into_bytes(), + )) + } + } + } + + let sys = InMemorySys::default(); + let file_fetcher = create_file_fetcher(sys.clone(), TestHttpClient); + + // test redirect + { + let result = file_fetcher.ensure_cached_no_follow_sync( + &Url::parse("http://localhost/redirect").unwrap(), + FetchNoFollowOptions::default(), + ); + assert_eq!( + result.unwrap(), + CachedOrRedirect::Redirect(Url::parse("http://localhost/home").unwrap()) + ); + } + + // test success + { + let result = file_fetcher.ensure_cached_no_follow_sync( + &Url::parse("http://localhost/other").unwrap(), + FetchNoFollowOptions::default(), + ); + assert_eq!(result.unwrap(), CachedOrRedirect::Cached); + } + + // test local file + sys.fs_create_dir_all("/").unwrap(); + sys.fs_write("/some_path.ts", "text").unwrap(); + { + let result = file_fetcher.ensure_cached_no_follow_sync( + &Url::parse("file:///some_path.ts").unwrap(), + FetchNoFollowOptions::default(), + ); + assert_eq!(result.unwrap(), CachedOrRedirect::Cached); + } + + // test not found + { + let url = Url::parse("file:///not_exists.ts").unwrap(); + let result = + file_fetcher.ensure_cached_no_follow_sync(&url, FetchNoFollowOptions::default()); + match result.unwrap_err().as_kind() { + FetchNoFollowErrorKind::NotFound(not_found_url) => { + assert_eq!(url, *not_found_url) + } + _ => unreachable!(), + } + } +} + fn create_file_fetcher( sys: InMemorySys, client: TClient, From bca830e66f3bb91a18a92113312d22736896e89b Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sat, 24 Jan 2026 14:17:38 -0500 Subject: [PATCH 4/4] update --- rs_lib/src/file_fetcher/mod.rs | 153 +++++++++++++----------------- rs_lib/tests/file_fetcher_test.rs | 4 +- 2 files changed, 69 insertions(+), 88 deletions(-) diff --git a/rs_lib/src/file_fetcher/mod.rs b/rs_lib/src/file_fetcher/mod.rs index eec1437..da6e887 100644 --- a/rs_lib/src/file_fetcher/mod.rs +++ b/rs_lib/src/file_fetcher/mod.rs @@ -886,18 +886,14 @@ impl maybe_checksum: Option>, maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, ) -> Result { - let (maybe_etag_cache_entry, maybe_etag) = - self.get_etag_entry(url, maybe_checksum).unzip(); - let headers = - self.build_headers(url, maybe_accept, maybe_auth, maybe_etag)?; - let result = self.http_client.send_no_follow(url, headers).await; - let response = self.handle_send_result(url, result)?; - self.handle_fetch_remote_response( + let (headers, maybe_etag_cache_entry) = self.build_headers_and_etag( url, - response, + maybe_accept, maybe_checksum, - maybe_etag_cache_entry, - ) + maybe_auth, + )?; + let result = self.http_client.send_no_follow(url, headers).await; + self.handle_send_result(url, result, maybe_checksum, maybe_etag_cache_entry) } fn fetch_remote_no_follow_no_cache_sync( @@ -907,18 +903,14 @@ impl maybe_checksum: Option>, maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, ) -> Result { - let (maybe_etag_cache_entry, maybe_etag) = - self.get_etag_entry(url, maybe_checksum).unzip(); - let headers = - self.build_headers(url, maybe_accept, maybe_auth, maybe_etag)?; - let result = self.http_client.send_no_follow_sync(url, headers); - let response = self.handle_send_result(url, result)?; - self.handle_fetch_remote_response( + let (headers, maybe_etag_cache_entry) = self.build_headers_and_etag( url, - response, + maybe_accept, maybe_checksum, - maybe_etag_cache_entry, - ) + maybe_auth, + )?; + let result = self.http_client.send_no_follow_sync(url, headers); + self.handle_send_result(url, result, maybe_checksum, maybe_etag_cache_entry) } fn get_etag_entry( @@ -944,24 +936,53 @@ impl &self, url: &Url, send_result: Result, - ) -> Result { + maybe_checksum: Option>, + maybe_etag_cache_entry: Option, + ) -> Result { match send_result { Ok(resp) => match resp { - SendResponse::NotModified => Ok(SendRequestResponse::NotModified), + SendResponse::NotModified => { + let cache_entry = maybe_etag_cache_entry.unwrap(); + FileOrRedirect::from_deno_cache_entry(url, cache_entry).map_err( + |err| FetchNoFollowErrorKind::RedirectResolution(err).into_box(), + ) + } SendResponse::Redirect(headers) => { - let new_url = - resolve_redirect_from_headers(url, &headers).map_err(|err| { + let redirect_url = resolve_redirect_from_headers(url, &headers) + .map_err(|err| { FetchNoFollowErrorKind::RedirectHeaderParse(*err).into_box() })?; - Ok(SendRequestResponse::Redirect( - new_url, - response_headers_to_headers_map(headers), - )) + let headers = response_headers_to_headers_map(headers); + self.http_cache.set(url, headers, &[]).map_err(|source| { + FetchNoFollowErrorKind::CacheSave { + url: url.clone(), + source, + } + })?; + Ok(FileOrRedirect::Redirect(redirect_url)) + } + SendResponse::Success(headers, body) => { + let headers = response_headers_to_headers_map(headers); + self.http_cache.set(url, headers.clone(), &body).map_err( + |source| FetchNoFollowErrorKind::CacheSave { + url: url.clone(), + source, + }, + )?; + if let Some(checksum) = maybe_checksum { + checksum + .check(url, &body) + .map_err(|err| FetchNoFollowErrorKind::ChecksumIntegrity(*err))?; + } + Ok(FileOrRedirect::File(File { + url: url.clone(), + mtime: None, + maybe_headers: Some(headers), + #[allow(clippy::disallowed_types)] // ok for source + source: std::sync::Arc::from(body), + loaded_from: LoadedFrom::Remote, + })) } - SendResponse::Success(headers, body) => Ok(SendRequestResponse::Code( - body, - response_headers_to_headers_map(headers), - )), }, Err(err) => match err { SendError::Failed(err) => Err( @@ -985,53 +1006,6 @@ impl } } - fn handle_fetch_remote_response( - &self, - url: &Url, - response: SendRequestResponse, - maybe_checksum: Option>, - maybe_etag_cache_entry: Option, - ) -> Result { - match response { - SendRequestResponse::NotModified => { - let cache_entry = maybe_etag_cache_entry.unwrap(); - FileOrRedirect::from_deno_cache_entry(url, cache_entry).map_err(|err| { - FetchNoFollowErrorKind::RedirectResolution(err).into_box() - }) - } - SendRequestResponse::Redirect(redirect_url, headers) => { - self.http_cache.set(url, headers, &[]).map_err(|source| { - FetchNoFollowErrorKind::CacheSave { - url: url.clone(), - source, - } - })?; - Ok(FileOrRedirect::Redirect(redirect_url)) - } - SendRequestResponse::Code(bytes, headers) => { - self.http_cache.set(url, headers.clone(), &bytes).map_err( - |source| FetchNoFollowErrorKind::CacheSave { - url: url.clone(), - source, - }, - )?; - if let Some(checksum) = maybe_checksum { - checksum - .check(url, &bytes) - .map_err(|err| FetchNoFollowErrorKind::ChecksumIntegrity(*err))?; - } - Ok(FileOrRedirect::File(File { - url: url.clone(), - mtime: None, - maybe_headers: Some(headers), - #[allow(clippy::disallowed_types)] // ok for source - source: std::sync::Arc::from(bytes), - loaded_from: LoadedFrom::Remote, - })) - } - } - } - /// Returns if the cache should be used for a given specifier. fn should_use_cache(&self, url: &Url, cache_setting: &CacheSetting) -> bool { match cache_setting { @@ -1074,6 +1048,20 @@ impl } } + fn build_headers_and_etag( + &self, + url: &Url, + maybe_accept: Option>, + maybe_checksum: Option>, + maybe_auth: Option<(header::HeaderName, header::HeaderValue)>, + ) -> Result<(HeaderMap, Option), FetchNoFollowError> { + let (maybe_etag_cache_entry, maybe_etag) = + self.get_etag_entry(url, maybe_checksum).unzip(); + let headers = + self.build_headers(url, maybe_accept, maybe_auth, maybe_etag)?; + Ok((headers, maybe_etag_cache_entry)) + } + fn build_headers( &self, url: &Url, @@ -1520,13 +1508,6 @@ fn resolve_url_from_location( } } -#[derive(Debug, Eq, PartialEq)] -enum SendRequestResponse { - Code(Vec, HeadersMap), - NotModified, - Redirect(Url, HeadersMap), -} - #[cfg(test)] mod test { use url::Url; diff --git a/rs_lib/tests/file_fetcher_test.rs b/rs_lib/tests/file_fetcher_test.rs index 32eb939..f432925 100644 --- a/rs_lib/tests/file_fetcher_test.rs +++ b/rs_lib/tests/file_fetcher_test.rs @@ -308,8 +308,8 @@ fn test_file_fetcher_ensure_cached_no_follow_sync() { // test not found { let url = Url::parse("file:///not_exists.ts").unwrap(); - let result = - file_fetcher.ensure_cached_no_follow_sync(&url, FetchNoFollowOptions::default()); + let result = file_fetcher + .ensure_cached_no_follow_sync(&url, FetchNoFollowOptions::default()); match result.unwrap_err().as_kind() { FetchNoFollowErrorKind::NotFound(not_found_url) => { assert_eq!(url, *not_found_url)