From 75149f04b6ad032d61b37ba650ab8fa0adfd10ed Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sat, 10 Aug 2024 16:57:02 -0400 Subject: [PATCH 1/6] perf(cache): store headers and body in same file for remote modules --- Cargo.lock | 2 -- Cargo.toml | 3 +++ cli/cache/mod.rs | 18 ++++++++++++++++++ cli/file_fetcher.rs | 34 +++++++++++++++------------------- cli/lsp/documents.rs | 11 +++++------ cli/lsp/jsr.rs | 3 ++- 6 files changed, 43 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6a758ce09b2e00..00027615734c35 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1313,8 +1313,6 @@ dependencies = [ [[package]] name = "deno_cache_dir" version = "0.10.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b9fa4989e4c6b0409438e2a68a91e4e02858b0d8ba6e5bc6860af6b0d0f385e8" dependencies = [ "deno_media_type", "indexmap", diff --git a/Cargo.toml b/Cargo.toml index 076ebdf4f609bf..ba03a00a58caf5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -388,3 +388,6 @@ opt-level = 3 opt-level = 3 [profile.release.package.zstd-sys] opt-level = 3 + +[patch.crates-io] +deno_cache_dir = { path = "../deno_cache_dir/rs_lib" } diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs index 772d2359d34e90..827a3faffaeb6b 100644 --- a/cli/cache/mod.rs +++ b/cli/cache/mod.rs @@ -10,6 +10,7 @@ use crate::npm::CliNpmResolver; use crate::util::fs::atomic_write_file_with_retries; use deno_ast::MediaType; +use deno_cache_dir::DenoCacheEnvFsFile; use deno_core::futures; use deno_core::futures::FutureExt; use deno_core::ModuleSpecifier; @@ -19,6 +20,7 @@ use deno_graph::source::LoadResponse; use deno_graph::source::Loader; use deno_runtime::deno_permissions::PermissionsContainer; use std::collections::HashMap; +use std::io::Read; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; @@ -58,10 +60,26 @@ pub use parsed_source::ParsedSourceCache; /// Permissions used to save a file in the disk caches. pub const CACHE_PERM: u32 = 0o644; +pub struct RealDenoCacheEnvFsFile(std::fs::File); + +impl DenoCacheEnvFsFile for RealDenoCacheEnvFsFile { + fn read(&mut self, bytes: &mut [u8]) -> std::io::Result { + self.0.read(bytes) + } +} + #[derive(Debug, Clone)] pub struct RealDenoCacheEnv; impl deno_cache_dir::DenoCacheEnv for RealDenoCacheEnv { + fn open_read( + &self, + path: &Path, + ) -> std::io::Result> { + let fs_file = std::fs::OpenOptions::new().read(true).open(path)?; + Ok(Box::new(RealDenoCacheEnvFsFile(fs_file))) + } + fn read_file_bytes(&self, path: &Path) -> std::io::Result>> { match std::fs::read(path) { Ok(s) => Ok(Some(s)), diff --git a/cli/file_fetcher.rs b/cli/file_fetcher.rs index 19acf2e2b8812d..4caeea34a1a1a6 100644 --- a/cli/file_fetcher.rs +++ b/cli/file_fetcher.rs @@ -238,23 +238,22 @@ impl FileFetcher { ); let cache_key = self.http_cache.cache_item_key(specifier)?; // compute this once - let Some(headers) = self.http_cache.read_headers(&cache_key)? else { - return Ok(None); - }; - if let Some(redirect_to) = headers.get("location") { - let redirect = - deno_core::resolve_import(redirect_to, specifier.as_str())?; - return Ok(Some(FileOrRedirect::Redirect(redirect))); - } - let result = self.http_cache.read_file_bytes( + let result = self.http_cache.get( &cache_key, maybe_checksum .as_ref() .map(|c| deno_cache_dir::Checksum::new(c.as_str())), deno_cache_dir::GlobalToLocalCopy::Allow, ); - let bytes = match result { - Ok(Some(bytes)) => bytes, + let cache_data = match result { + Ok(Some(cache_data)) => { + if let Some(redirect_to) = cache_data.metadata.headers.get("location") { + let redirect = + deno_core::resolve_import(redirect_to, specifier.as_str())?; + return Ok(Some(FileOrRedirect::Redirect(redirect))); + } + cache_data + } Ok(None) => return Ok(None), Err(err) => match err { deno_cache_dir::CacheReadFileError::Io(err) => return Err(err.into()), @@ -274,8 +273,8 @@ impl FileFetcher { Ok(Some(FileOrRedirect::File(File { specifier: specifier.clone(), - maybe_headers: Some(headers), - source: Arc::from(bytes), + maybe_headers: Some(cache_data.metadata.headers), + source: Arc::from(cache_data.body), }))) } @@ -1480,13 +1479,10 @@ mod tests { let cache_key = file_fetcher.http_cache.cache_item_key(url).unwrap(); let bytes = file_fetcher .http_cache - .read_file_bytes( - &cache_key, - None, - deno_cache_dir::GlobalToLocalCopy::Allow, - ) + .get(&cache_key, None, deno_cache_dir::GlobalToLocalCopy::Allow) .unwrap() - .unwrap(); + .unwrap() + .body; String::from_utf8(bytes).unwrap() } diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 57e48fbc3a4ed9..748fb574023a8c 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -872,22 +872,21 @@ impl FileSystemDocuments { } else { let http_cache = cache.for_specifier(file_referrer); let cache_key = http_cache.cache_item_key(specifier).ok()?; - let bytes = http_cache - .read_file_bytes(&cache_key, None, LSP_DISALLOW_GLOBAL_TO_LOCAL_COPY) + let cached_file = http_cache + .get(&cache_key, None, LSP_DISALLOW_GLOBAL_TO_LOCAL_COPY) .ok()??; - let specifier_headers = http_cache.read_headers(&cache_key).ok()??; let (_, maybe_charset) = deno_graph::source::resolve_media_type_and_charset_from_headers( specifier, - Some(&specifier_headers), + Some(&cached_file.metadata.headers), ); let content = deno_graph::source::decode_owned_source( specifier, - bytes, + cached_file.body, maybe_charset, ) .ok()?; - let maybe_headers = Some(specifier_headers); + let maybe_headers = Some(cached_file.metadata.headers); Document::new( specifier.clone(), content.into(), diff --git a/cli/lsp/jsr.rs b/cli/lsp/jsr.rs index 9ffcdf9e61fed1..cb55a5edd90f11 100644 --- a/cli/lsp/jsr.rs +++ b/cli/lsp/jsr.rs @@ -258,12 +258,13 @@ fn read_cached_url( cache: &Arc, ) -> Option> { cache - .read_file_bytes( + .get( &cache.cache_item_key(url).ok()?, None, deno_cache_dir::GlobalToLocalCopy::Disallow, ) .ok()? + .map(|f| f.body) } #[derive(Debug)] From ab5dcc8899f7763132964c89bb5fdb0d636ffd02 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sat, 10 Aug 2024 20:03:06 -0400 Subject: [PATCH 2/6] Update tests --- cli/cache/mod.rs | 15 ++++++++------- cli/factory.rs | 7 +++++-- cli/file_fetcher.rs | 27 +++++++++++---------------- cli/lsp/cache.rs | 10 ---------- cli/lsp/documents.rs | 7 ++----- cli/lsp/jsr.rs | 8 ++------ tests/integration/jsr_tests.rs | 25 +++++++++++++++++++------ tests/integration/run_tests.rs | 33 --------------------------------- 8 files changed, 47 insertions(+), 85 deletions(-) diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs index 827a3faffaeb6b..b3b658731d82b3 100644 --- a/cli/cache/mod.rs +++ b/cli/cache/mod.rs @@ -21,6 +21,7 @@ use deno_graph::source::Loader; use deno_runtime::deno_permissions::PermissionsContainer; use std::collections::HashMap; use std::io::Read; +use std::io::Seek; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; @@ -66,6 +67,10 @@ impl DenoCacheEnvFsFile for RealDenoCacheEnvFsFile { fn read(&mut self, bytes: &mut [u8]) -> std::io::Result { self.0.read(bytes) } + + fn seek_relative(&mut self, amount: i64) -> std::io::Result<()> { + self.0.seek_relative(amount) + } } #[derive(Debug, Clone)] @@ -76,16 +81,12 @@ impl deno_cache_dir::DenoCacheEnv for RealDenoCacheEnv { &self, path: &Path, ) -> std::io::Result> { - let fs_file = std::fs::OpenOptions::new().read(true).open(path)?; + let fs_file = std::fs::File::open(path)?; Ok(Box::new(RealDenoCacheEnvFsFile(fs_file))) } - fn read_file_bytes(&self, path: &Path) -> std::io::Result>> { - match std::fs::read(path) { - Ok(s) => Ok(Some(s)), - Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(None), - Err(err) => Err(err), - } + fn read_file_bytes(&self, path: &Path) -> std::io::Result> { + std::fs::read(path) } fn atomic_write_file( diff --git a/cli/factory.rs b/cli/factory.rs index ed288b22f7e03e..3a9fbb608191f5 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -309,8 +309,11 @@ impl CliFactory { let global_cache = self.global_http_cache()?.clone(); match self.cli_options()?.vendor_dir_path() { Some(local_path) => { - let local_cache = - LocalHttpCache::new(local_path.clone(), global_cache); + let local_cache = LocalHttpCache::new( + local_path.clone(), + global_cache, + deno_cache_dir::GlobalToLocalCopy::Allow, + ); Ok(Arc::new(local_cache)) } None => Ok(global_cache), diff --git a/cli/file_fetcher.rs b/cli/file_fetcher.rs index 4caeea34a1a1a6..8d43d9e3dc665c 100644 --- a/cli/file_fetcher.rs +++ b/cli/file_fetcher.rs @@ -11,7 +11,6 @@ use crate::http_util::HttpClientProvider; use crate::util::progress_bar::ProgressBar; use deno_ast::MediaType; -use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::custom_error; use deno_core::error::generic_error; @@ -243,7 +242,6 @@ impl FileFetcher { maybe_checksum .as_ref() .map(|c| deno_cache_dir::Checksum::new(c.as_str())), - deno_cache_dir::GlobalToLocalCopy::Allow, ); let cache_data = match result { Ok(Some(cache_data)) => { @@ -274,7 +272,7 @@ impl FileFetcher { Ok(Some(FileOrRedirect::File(File { specifier: specifier.clone(), maybe_headers: Some(cache_data.metadata.headers), - source: Arc::from(cache_data.body), + source: Arc::from(cache_data.content), }))) } @@ -410,17 +408,14 @@ impl FileFetcher { match file_or_redirect { Some(file_or_redirect) => Ok(file_or_redirect), None => { - // Someone may have deleted the body from the cache since - // it's currently stored in a separate file from the headers, - // so delete the etag and try again - if maybe_etag.is_some() { - debug!("Cache body not found. Trying again without etag."); - maybe_etag = None; - continue; - } else { - // should never happen - bail!("Your deno cache directory is in an unrecoverable state. Please delete it and try again.") - } + // If this happens it means that some other process deleted + // the entry from the cache directory between the cache.read_headers call + // and the fetch_no_follow call, so just try again without an etag + // in order to repopulate the cache. + assert!(maybe_etag.is_some()); + debug!("Cache body not found. Trying again without etag."); + maybe_etag = None; + continue; } } } @@ -1479,10 +1474,10 @@ mod tests { let cache_key = file_fetcher.http_cache.cache_item_key(url).unwrap(); let bytes = file_fetcher .http_cache - .get(&cache_key, None, deno_cache_dir::GlobalToLocalCopy::Allow) + .get(&cache_key, None) .unwrap() .unwrap() - .body; + .content; String::from_utf8(bytes).unwrap() } diff --git a/cli/lsp/cache.rs b/cli/lsp/cache.rs index 5dae38c206f489..d3c05ca91aaba4 100644 --- a/cli/lsp/cache.rs +++ b/cli/lsp/cache.rs @@ -17,16 +17,6 @@ use std::path::Path; use std::sync::Arc; use std::time::SystemTime; -/// In the LSP, we disallow the cache from automatically copying from -/// the global cache to the local cache for technical reasons. -/// -/// 1. We need to verify the checksums from the lockfile are correct when -/// moving from the global to the local cache. -/// 2. We need to verify the checksums for JSR https specifiers match what -/// is found in the package's manifest. -pub const LSP_DISALLOW_GLOBAL_TO_LOCAL_COPY: deno_cache_dir::GlobalToLocalCopy = - deno_cache_dir::GlobalToLocalCopy::Disallow; - pub fn calculate_fs_version( cache: &LspCache, specifier: &ModuleSpecifier, diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index 748fb574023a8c..742cbe3c8fbf94 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -2,7 +2,6 @@ use super::cache::calculate_fs_version; use super::cache::LspCache; -use super::cache::LSP_DISALLOW_GLOBAL_TO_LOCAL_COPY; use super::config::Config; use super::resolver::LspResolver; use super::testing::TestCollector; @@ -872,9 +871,7 @@ impl FileSystemDocuments { } else { let http_cache = cache.for_specifier(file_referrer); let cache_key = http_cache.cache_item_key(specifier).ok()?; - let cached_file = http_cache - .get(&cache_key, None, LSP_DISALLOW_GLOBAL_TO_LOCAL_COPY) - .ok()??; + let cached_file = http_cache.get(&cache_key, None).ok()??; let (_, maybe_charset) = deno_graph::source::resolve_media_type_and_charset_from_headers( specifier, @@ -882,7 +879,7 @@ impl FileSystemDocuments { ); let content = deno_graph::source::decode_owned_source( specifier, - cached_file.body, + cached_file.content, maybe_charset, ) .ok()?; diff --git a/cli/lsp/jsr.rs b/cli/lsp/jsr.rs index cb55a5edd90f11..c0a82383d3915f 100644 --- a/cli/lsp/jsr.rs +++ b/cli/lsp/jsr.rs @@ -258,13 +258,9 @@ fn read_cached_url( cache: &Arc, ) -> Option> { cache - .get( - &cache.cache_item_key(url).ok()?, - None, - deno_cache_dir::GlobalToLocalCopy::Disallow, - ) + .get(&cache.cache_item_key(url).ok()?, None) .ok()? - .map(|f| f.body) + .map(|f| f.content) } #[derive(Debug)] diff --git a/tests/integration/jsr_tests.rs b/tests/integration/jsr_tests.rs index eb951d682cc495..5c50243ba5f7a6 100644 --- a/tests/integration/jsr_tests.rs +++ b/tests/integration/jsr_tests.rs @@ -1,5 +1,7 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use deno_cache_dir::HttpCache; +use deno_core::serde_json; use deno_core::serde_json::json; use deno_core::serde_json::Value; use deno_lockfile::Lockfile; @@ -183,13 +185,24 @@ fn reload_info_not_found_cache_but_exists_remote() { let specifier = Url::parse(&format!("http://127.0.0.1:4250/{}/meta.json", package)) .unwrap(); - let registry_json_path = deno_dir - .path() - .join("deps") - .join(deno_cache_dir::url_to_filename(&specifier).unwrap()); - let mut registry_json = registry_json_path.read_json_value(); + let cache = deno_cache_dir::GlobalHttpCache::new( + deno_dir.path().join("deps").to_path_buf(), + deno_cache_dir::TestRealDenoCacheEnv, + ); + let entry = cache + .get(&cache.cache_item_key(&specifier).unwrap(), None) + .unwrap() + .unwrap(); + let mut registry_json: serde_json::Value = + serde_json::from_slice(&entry.content).unwrap(); remove_version(&mut registry_json, version); - registry_json_path.write_json(®istry_json); + cache + .set( + &specifier, + entry.metadata.headers.clone(), + ®istry_json.to_string().as_bytes(), + ) + .unwrap(); } // This tests that when a local machine doesn't have a version diff --git a/tests/integration/run_tests.rs b/tests/integration/run_tests.rs index 0b3d85deb9be77..0a8113577bb58f 100644 --- a/tests/integration/run_tests.rs +++ b/tests/integration/run_tests.rs @@ -4988,39 +4988,6 @@ console.log(add(3, 4)); output.assert_matches_text("[WILDCARD]5\n7\n"); } -#[test] -fn run_etag_delete_source_cache() { - let test_context = TestContextBuilder::new() - .use_temp_cwd() - .use_http_server() - .build(); - test_context - .temp_dir() - .write("main.ts", "import 'http://localhost:4545/etag_script.ts'"); - test_context - .new_command() - .args("cache main.ts") - .run() - .skip_output_check(); - - // The cache is currently stored unideally in two files where one file has the headers - // and the other contains the body. An issue can happen with the etag header where the - // headers file exists, but the body was deleted. We need to get the cache to gracefully - // handle this scenario. - let deno_dir = test_context.deno_dir().path(); - let etag_script_path = deno_dir.join("deps/http/localhost_PORT4545/26110db7d42c9bad32386735cbc05c301f83e4393963deb8da14fec3b4202a13"); - assert!(etag_script_path.exists()); - etag_script_path.remove_file(); - - test_context - .new_command() - .args("cache --reload --log-level=debug main.ts") - .run() - .assert_matches_text( - "[WILDCARD]Cache body not found. Trying again without etag.[WILDCARD]", - ); -} - #[test] fn code_cache_test() { let test_context = TestContextBuilder::new().use_temp_cwd().build(); From 3592159c7459ab0a7f219832ae2c9b23ef8fbfbf Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sat, 10 Aug 2024 20:43:08 -0400 Subject: [PATCH 3/6] update --- cli/cache/mod.rs | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs index b3b658731d82b3..cc183530d820fc 100644 --- a/cli/cache/mod.rs +++ b/cli/cache/mod.rs @@ -10,7 +10,6 @@ use crate::npm::CliNpmResolver; use crate::util::fs::atomic_write_file_with_retries; use deno_ast::MediaType; -use deno_cache_dir::DenoCacheEnvFsFile; use deno_core::futures; use deno_core::futures::FutureExt; use deno_core::ModuleSpecifier; @@ -20,8 +19,6 @@ use deno_graph::source::LoadResponse; use deno_graph::source::Loader; use deno_runtime::deno_permissions::PermissionsContainer; use std::collections::HashMap; -use std::io::Read; -use std::io::Seek; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; @@ -61,30 +58,10 @@ pub use parsed_source::ParsedSourceCache; /// Permissions used to save a file in the disk caches. pub const CACHE_PERM: u32 = 0o644; -pub struct RealDenoCacheEnvFsFile(std::fs::File); - -impl DenoCacheEnvFsFile for RealDenoCacheEnvFsFile { - fn read(&mut self, bytes: &mut [u8]) -> std::io::Result { - self.0.read(bytes) - } - - fn seek_relative(&mut self, amount: i64) -> std::io::Result<()> { - self.0.seek_relative(amount) - } -} - #[derive(Debug, Clone)] pub struct RealDenoCacheEnv; impl deno_cache_dir::DenoCacheEnv for RealDenoCacheEnv { - fn open_read( - &self, - path: &Path, - ) -> std::io::Result> { - let fs_file = std::fs::File::open(path)?; - Ok(Box::new(RealDenoCacheEnvFsFile(fs_file))) - } - fn read_file_bytes(&self, path: &Path) -> std::io::Result> { std::fs::read(path) } From a44855b901fa03a813666ff7bee5583df4833c0c Mon Sep 17 00:00:00 2001 From: David Sherret Date: Sat, 10 Aug 2024 21:24:46 -0400 Subject: [PATCH 4/6] Get item from cache for etag --- cli/file_fetcher.rs | 94 ++++++++++++++++++++++++++------------------- 1 file changed, 54 insertions(+), 40 deletions(-) diff --git a/cli/file_fetcher.rs b/cli/file_fetcher.rs index 8d43d9e3dc665c..ace4d3c7eefdee 100644 --- a/cli/file_fetcher.rs +++ b/cli/file_fetcher.rs @@ -51,6 +51,25 @@ pub enum FileOrRedirect { Redirect(ModuleSpecifier), } +impl FileOrRedirect { + fn from_deno_cache_entry( + specifier: &ModuleSpecifier, + cache_entry: deno_cache_dir::CacheEntry, + ) -> Result { + if let Some(redirect_to) = cache_entry.metadata.headers.get("location") { + let redirect = + deno_core::resolve_import(redirect_to, specifier.as_str())?; + Ok(FileOrRedirect::Redirect(redirect)) + } else { + Ok(FileOrRedirect::File(File { + specifier: specifier.clone(), + maybe_headers: Some(cache_entry.metadata.headers), + source: Arc::from(cache_entry.content), + })) + } + } +} + /// A structure representing a source file. #[derive(Debug, Clone, Eq, PartialEq)] pub struct File { @@ -243,37 +262,26 @@ impl FileFetcher { .as_ref() .map(|c| deno_cache_dir::Checksum::new(c.as_str())), ); - let cache_data = match result { - Ok(Some(cache_data)) => { - if let Some(redirect_to) = cache_data.metadata.headers.get("location") { - let redirect = - deno_core::resolve_import(redirect_to, specifier.as_str())?; - return Ok(Some(FileOrRedirect::Redirect(redirect))); - } - cache_data - } - Ok(None) => return Ok(None), + match result { + Ok(Some(cache_data)) => Ok(Some(FileOrRedirect::from_deno_cache_entry( + specifier, cache_data, + )?)), + Ok(None) => Ok(None), Err(err) => match err { - deno_cache_dir::CacheReadFileError::Io(err) => return Err(err.into()), + deno_cache_dir::CacheReadFileError::Io(err) => Err(err.into()), deno_cache_dir::CacheReadFileError::ChecksumIntegrity(err) => { // convert to the equivalent deno_graph error so that it // enhances it if this is passed to deno_graph - return Err( + Err( deno_graph::source::ChecksumIntegrityError { actual: err.actual, expected: err.expected, } .into(), - ); + ) } }, - }; - - Ok(Some(FileOrRedirect::File(File { - specifier: specifier.clone(), - maybe_headers: Some(cache_data.metadata.headers), - source: Arc::from(cache_data.content), - }))) + } } /// Convert a data URL into a file, resulting in an error if the URL is @@ -360,12 +368,30 @@ impl FileFetcher { ); } - let maybe_etag = self + let maybe_etag_cache_entry = self .http_cache .cache_item_key(specifier) .ok() - .and_then(|key| self.http_cache.read_headers(&key).ok().flatten()) - .and_then(|headers| headers.get("etag").cloned()); + .and_then(|key| { + self + .http_cache + .get( + &key, + maybe_checksum + .as_ref() + .map(|c| deno_cache_dir::Checksum::new(c.as_str())), + ) + .ok() + .flatten() + }) + .and_then(|cache_entry| { + cache_entry + .metadata + .headers + .get("etag") + .cloned() + .map(|etag| (cache_entry, etag)) + }); let maybe_auth_token = self.auth_tokens.get(specifier); async fn handle_request_or_server_error( @@ -387,7 +413,6 @@ impl FileFetcher { } } - let mut maybe_etag = maybe_etag; let mut retried = false; // retry intermittent failures let result = loop { let result = match self @@ -396,28 +421,17 @@ impl FileFetcher { .fetch_no_follow(FetchOnceArgs { url: specifier.clone(), maybe_accept: maybe_accept.map(ToOwned::to_owned), - maybe_etag: maybe_etag.clone(), + maybe_etag: maybe_etag_cache_entry + .as_ref() + .map(|(_, etag)| etag.clone()), maybe_auth_token: maybe_auth_token.clone(), maybe_progress_guard: maybe_progress_guard.as_ref(), }) .await? { FetchOnceResult::NotModified => { - let file_or_redirect = - self.fetch_cached_no_follow(specifier, maybe_checksum)?; - match file_or_redirect { - Some(file_or_redirect) => Ok(file_or_redirect), - None => { - // If this happens it means that some other process deleted - // the entry from the cache directory between the cache.read_headers call - // and the fetch_no_follow call, so just try again without an etag - // in order to repopulate the cache. - assert!(maybe_etag.is_some()); - debug!("Cache body not found. Trying again without etag."); - maybe_etag = None; - continue; - } - } + let (cache_entry, _) = maybe_etag_cache_entry.unwrap(); + FileOrRedirect::from_deno_cache_entry(specifier, cache_entry) } FetchOnceResult::Redirect(redirect_url, headers) => { self.http_cache.set(specifier, headers, &[])?; From 81c9e412e3e608d182c412d9de97b6f08932e0eb Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 26 Aug 2024 19:11:07 -0400 Subject: [PATCH 5/6] update dep --- Cargo.lock | 4 +++- Cargo.toml | 5 +---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8dcd8cfd1bcd67..2cf3e065281853 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1346,7 +1346,9 @@ dependencies = [ [[package]] name = "deno_cache_dir" -version = "0.10.2" +version = "0.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "405790fa1fa5f05c2e7dca817f820dc1c950ea846f47212ed6d670a3023cb4fe" dependencies = [ "deno_media_type", "indexmap", diff --git a/Cargo.toml b/Cargo.toml index 9aa2bf78b94446..3a89fec1e752d2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -102,7 +102,7 @@ chrono = { version = "0.4", default-features = false, features = ["std", "serde" console_static_text = "=0.8.1" data-encoding = "2.3.3" data-url = "=0.3.0" -deno_cache_dir = "=0.10.2" +deno_cache_dir = "=0.11.0" deno_package_json = { version = "=0.1.1", default-features = false } dlopen2 = "0.6.1" ecb = "=0.1.2" @@ -394,6 +394,3 @@ opt-level = 3 opt-level = 3 [profile.release.package.zstd-sys] opt-level = 3 - -[patch.crates-io] -deno_cache_dir = { path = "../deno_cache_dir/rs_lib" } From eeddac172043b3447b570b03723aadc69bf92e5b Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 26 Aug 2024 19:20:25 -0400 Subject: [PATCH 6/6] lint --- tests/integration/jsr_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration/jsr_tests.rs b/tests/integration/jsr_tests.rs index 0c190deeffb154..f31b53f276cf39 100644 --- a/tests/integration/jsr_tests.rs +++ b/tests/integration/jsr_tests.rs @@ -201,7 +201,7 @@ fn reload_info_not_found_cache_but_exists_remote() { .set( &specifier, entry.metadata.headers.clone(), - ®istry_json.to_string().as_bytes(), + registry_json.to_string().as_bytes(), ) .unwrap(); }