diff --git a/cli/cache/http_cache.rs b/cli/cache/http_cache.rs index e98f4bad74..c659600863 100644 --- a/cli/cache/http_cache.rs +++ b/cli/cache/http_cache.rs @@ -12,7 +12,6 @@ use deno_core::serde::Serialize; use deno_core::serde_json; use deno_core::url::Url; use std::fs; -use std::fs::File; use std::io; use std::path::Path; use std::path::PathBuf; @@ -73,32 +72,42 @@ pub fn url_to_filename(url: &Url) -> Option { } /// Cached metadata about a url. -#[derive(Serialize, Deserialize)] +#[derive(Debug, Serialize, Deserialize, PartialEq, Eq)] pub struct CachedUrlMetadata { pub headers: HeadersMap, pub url: String, - #[serde(default = "SystemTime::now")] - pub now: SystemTime, + #[serde(default = "SystemTime::now", rename = "now")] + pub time: SystemTime, } -impl CachedUrlMetadata { - pub fn write(&self, cache_filename: &Path) -> Result<(), AnyError> { - let metadata_filename = Self::filename(cache_filename); - let json = serde_json::to_string_pretty(self)?; - util::fs::atomic_write_file(&metadata_filename, json, CACHE_PERM)?; - Ok(()) +// DO NOT make the path public. The fact that this is stored in a file +// is an implementation detail. +pub struct MaybeHttpCacheItem(PathBuf); + +impl MaybeHttpCacheItem { + #[cfg(test)] + pub fn read_to_string(&self) -> Result, AnyError> { + let Some(bytes) = self.read_to_bytes()? else { + return Ok(None); + }; + Ok(Some(String::from_utf8(bytes)?)) } - pub fn read(cache_filename: &Path) -> Result { - let metadata_filename = Self::filename(cache_filename); - let metadata = fs::read_to_string(metadata_filename)?; - let metadata: Self = serde_json::from_str(&metadata)?; - Ok(metadata) + pub fn read_to_bytes(&self) -> Result>, AnyError> { + match std::fs::read(&self.0) { + Ok(s) => Ok(Some(s)), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(err) => Err(err.into()), + } } - /// Ex: $DENO_DIR/deps/https/deno.land/c885b7dcf1d6936e33a9cc3a2d74ec79bab5d733d3701c85a029b7f7ec9fbed4.metadata.json - pub fn filename(cache_filename: &Path) -> PathBuf { - cache_filename.with_extension("metadata.json") + pub fn read_metadata(&self) -> Result, AnyError> { + let metadata_filepath = self.0.with_extension("metadata.json"); + match fs::read_to_string(metadata_filepath) { + Ok(metadata) => Ok(Some(serde_json::from_str(&metadata)?)), + Err(err) if err.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(err) => Err(err.into()), + } } } @@ -131,26 +140,64 @@ impl HttpCache { }) } - pub fn get_cache_filename(&self, url: &Url) -> Option { - Some(self.location.join(url_to_filename(url)?)) + pub fn get_modified_time( + &self, + url: &Url, + ) -> Result, AnyError> { + let filepath = self.get_cache_filepath_internal(url)?; + match fs::metadata(filepath) { + Ok(metadata) => Ok(Some(metadata.modified()?)), + Err(err) if err.kind() == io::ErrorKind::NotFound => Ok(None), + Err(err) => Err(err.into()), + } + } + + // DEPRECATED: Where the file is stored and how it's stored should be an implementation + // detail of the cache. + #[deprecated(note = "Do not assume the cache will be stored at a file path.")] + pub fn get_cache_filepath(&self, url: &Url) -> Result { + self.get_cache_filepath_internal(url) + } + + fn get_cache_filepath_internal( + &self, + url: &Url, + ) -> Result { + Ok( + self.location.join( + url_to_filename(url) + .ok_or_else(|| generic_error("Can't convert url to filename."))?, + ), + ) + } + + #[cfg(test)] + pub fn write_metadata( + &self, + url: &Url, + meta_data: &CachedUrlMetadata, + ) -> Result<(), AnyError> { + let cache_path = self.get_cache_filepath_internal(url)?; + self.write_metadata_at_path(&cache_path, meta_data) + } + + fn write_metadata_at_path( + &self, + path: &Path, + meta_data: &CachedUrlMetadata, + ) -> Result<(), AnyError> { + let cache_path = path.with_extension("metadata.json"); + let json = serde_json::to_string_pretty(meta_data)?; + util::fs::atomic_write_file(&cache_path, json, CACHE_PERM)?; + Ok(()) } // TODO(bartlomieju): this method should check headers file // and validate against ETAG/Last-modified-as headers. // ETAG check is currently done in `cli/file_fetcher.rs`. - pub fn get( - &self, - url: &Url, - ) -> Result<(File, HeadersMap, SystemTime), AnyError> { - let cache_filename = self.location.join( - url_to_filename(url) - .ok_or_else(|| generic_error("Can't convert url to filename."))?, - ); - let metadata_filename = CachedUrlMetadata::filename(&cache_filename); - let file = File::open(cache_filename)?; - let metadata = fs::read_to_string(metadata_filename)?; - let metadata: CachedUrlMetadata = serde_json::from_str(&metadata)?; - Ok((file, metadata.headers, metadata.now)) + pub fn get(&self, url: &Url) -> Result { + let cache_filepath = self.get_cache_filepath_internal(url)?; + Ok(MaybeHttpCacheItem(cache_filepath)) } pub fn set( @@ -159,24 +206,30 @@ impl HttpCache { headers_map: HeadersMap, content: &[u8], ) -> Result<(), AnyError> { - let cache_filename = self.location.join( - url_to_filename(url) - .ok_or_else(|| generic_error("Can't convert url to filename."))?, - ); + let cache_filepath = self.get_cache_filepath_internal(url)?; // Create parent directory - let parent_filename = cache_filename + let parent_filename = cache_filepath .parent() .expect("Cache filename should have a parent dir"); self.ensure_dir_exists(parent_filename)?; // Cache content - util::fs::atomic_write_file(&cache_filename, content, CACHE_PERM)?; + util::fs::atomic_write_file(&cache_filepath, content, CACHE_PERM)?; let metadata = CachedUrlMetadata { - now: SystemTime::now(), + time: SystemTime::now(), url: url.to_string(), headers: headers_map, }; - metadata.write(&cache_filename) + self.write_metadata_at_path(&cache_filepath, &metadata)?; + + Ok(()) + } + + pub fn contains(&self, url: &Url) -> bool { + let Ok(cache_filepath) = self.get_cache_filepath_internal(url) else { + return false + }; + cache_filepath.is_file() } } @@ -184,7 +237,6 @@ impl HttpCache { mod tests { use super::*; use std::collections::HashMap; - use std::io::Read; use test_util::TempDir; #[test] @@ -228,11 +280,9 @@ mod tests { let r = cache.set(&url, headers, content); eprintln!("result {r:?}"); assert!(r.is_ok()); - let r = cache.get(&url); - assert!(r.is_ok()); - let (mut file, headers, _) = r.unwrap(); - let mut content = String::new(); - file.read_to_string(&mut content).unwrap(); + let cache_item = cache.get(&url).unwrap(); + let content = cache_item.read_to_string().unwrap().unwrap(); + let headers = cache_item.read_metadata().unwrap().unwrap().headers; assert_eq!(content, "Hello world"); assert_eq!( headers.get("content-type").unwrap(), diff --git a/cli/cache/mod.rs b/cli/cache/mod.rs index 772ba10a53..94ccb42e40 100644 --- a/cli/cache/mod.rs +++ b/cli/cache/mod.rs @@ -84,6 +84,7 @@ impl Loader for FetchCacher { return None; } + #[allow(deprecated)] let local = self.file_fetcher.get_local_path(specifier)?; if local.is_file() { let emit = self diff --git a/cli/file_fetcher.rs b/cli/file_fetcher.rs index 0413e77a08..2426ed0a54 100644 --- a/cli/file_fetcher.rs +++ b/cli/file_fetcher.rs @@ -37,7 +37,6 @@ use std::collections::HashMap; use std::env; use std::fs; use std::future::Future; -use std::io::Read; use std::path::PathBuf; use std::pin::Pin; use std::sync::Arc; @@ -246,24 +245,19 @@ impl FileFetcher { return Err(custom_error("Http", "Too many redirects.")); } - let (mut source_file, headers, _) = match self.http_cache.get(specifier) { - Err(err) => { - if let Some(err) = err.downcast_ref::() { - if err.kind() == std::io::ErrorKind::NotFound { - return Ok(None); - } - } - return Err(err); - } - Ok(cache) => cache, + let cache_item = self.http_cache.get(specifier)?; + let Some(metadata) = cache_item.read_metadata()? else { + return Ok(None); }; + let headers = metadata.headers; if let Some(redirect_to) = headers.get("location") { let redirect = deno_core::resolve_import(redirect_to, specifier.as_str())?; return self.fetch_cached(&redirect, redirect_limit - 1); } - let mut bytes = Vec::new(); - source_file.read_to_end(&mut bytes)?; + let Some(bytes) = cache_item.read_to_bytes()? else { + return Ok(None); + }; let file = self.build_remote_file(specifier, bytes, &headers)?; Ok(Some(file)) @@ -379,10 +373,12 @@ impl FileFetcher { ); } - let maybe_etag = match self.http_cache.get(specifier) { - Ok((_, headers, _)) => headers.get("etag").cloned(), - _ => None, - }; + let maybe_etag = self + .http_cache + .get(specifier) + .ok() + .and_then(|item| item.read_metadata().ok()?) + .and_then(|metadata| metadata.headers.get("etag").cloned()); let maybe_auth_token = self.auth_tokens.get(specifier); let specifier = specifier.clone(); let client = self.http_client.clone(); @@ -437,13 +433,18 @@ impl FileFetcher { CacheSetting::ReloadAll => false, CacheSetting::Use | CacheSetting::Only => true, CacheSetting::RespectHeaders => { - if let Ok((_, headers, cache_time)) = self.http_cache.get(specifier) { - let cache_semantics = - CacheSemantics::new(headers, cache_time, SystemTime::now()); - cache_semantics.should_use() - } else { - false - } + let Ok(item) = self.http_cache.get(specifier) else { + return false; + }; + let Ok(Some(metadata)) = item.read_metadata() else { + return false; + }; + let cache_semantics = CacheSemantics::new( + metadata.headers, + metadata.time, + SystemTime::now(), + ); + cache_semantics.should_use() } CacheSetting::ReloadSome(list) => { let mut url = specifier.clone(); @@ -515,6 +516,15 @@ impl FileFetcher { } } + // DEPRECATED: Where the file is stored and how it's stored should be an implementation + // detail of the cache. + // + // todo(dsheret): remove once implementing + // * https://github.com/denoland/deno/issues/17707 + // * https://github.com/denoland/deno/issues/17703 + #[deprecated( + note = "There should not be a way to do this because the file may not be cached at a local path in the future." + )] pub fn get_local_path(&self, specifier: &ModuleSpecifier) -> Option { // TODO(@kitsonk) fix when deno_graph does not query cache for synthetic // modules @@ -523,13 +533,14 @@ impl FileFetcher { } else if specifier.scheme() == "file" { specifier.to_file_path().ok() } else { - self.http_cache.get_cache_filename(specifier) + #[allow(deprecated)] + self.http_cache.get_cache_filepath(specifier).ok() } } /// Get the location of the current HTTP cache associated with the fetcher. - pub fn get_http_cache_location(&self) -> PathBuf { - self.http_cache.location.clone() + pub fn get_http_cache_location(&self) -> &PathBuf { + &self.http_cache.location } /// A synchronous way to retrieve a source file, where if the file has already @@ -656,7 +667,6 @@ async fn fetch_once<'a>( #[cfg(test)] mod tests { - use crate::cache::CachedUrlMetadata; use crate::http_util::HttpClient; use crate::version; @@ -725,9 +735,11 @@ mod tests { let result: Result = file_fetcher .fetch_remote(specifier, PermissionsContainer::allow_all(), 1, None) .await; - assert!(result.is_ok()); - let (_, headers, _) = file_fetcher.http_cache.get(specifier).unwrap(); - (result.unwrap(), headers) + let cache_item = file_fetcher.http_cache.get(specifier).unwrap(); + ( + result.unwrap(), + cache_item.read_metadata().unwrap().unwrap().headers, + ) } async fn test_fetch_remote_encoded( @@ -1000,7 +1012,7 @@ mod tests { fn test_get_http_cache_location() { let (file_fetcher, temp_dir) = setup(CacheSetting::Use, None); let expected = temp_dir.path().join("deps").to_path_buf(); - let actual = file_fetcher.get_http_cache_location(); + let actual = file_fetcher.get_http_cache_location().to_path_buf(); assert_eq!(actual, expected); } @@ -1075,16 +1087,21 @@ mod tests { ); assert_eq!(file.media_type, MediaType::TypeScript); - let cache_filename = file_fetcher + let mut metadata = file_fetcher .http_cache - .get_cache_filename(&specifier) + .get(&specifier) + .unwrap() + .read_metadata() + .unwrap() .unwrap(); - let mut metadata = CachedUrlMetadata::read(&cache_filename).unwrap(); metadata.headers = HashMap::new(); metadata .headers .insert("content-type".to_string(), "text/javascript".to_string()); - metadata.write(&cache_filename).unwrap(); + file_fetcher + .http_cache + .write_metadata(&specifier, &metadata) + .unwrap(); let result = file_fetcher_01 .fetch(&specifier, PermissionsContainer::allow_all()) @@ -1099,13 +1116,23 @@ mod tests { // the value above. assert_eq!(file.media_type, MediaType::JavaScript); - let (_, headers, _) = file_fetcher_02.http_cache.get(&specifier).unwrap(); + let headers = file_fetcher_02 + .http_cache + .get(&specifier) + .unwrap() + .read_metadata() + .unwrap() + .unwrap() + .headers; assert_eq!(headers.get("content-type").unwrap(), "text/javascript"); metadata.headers = HashMap::new(); metadata .headers .insert("content-type".to_string(), "application/json".to_string()); - metadata.write(&cache_filename).unwrap(); + file_fetcher_02 + .http_cache + .write_metadata(&specifier, &metadata) + .unwrap(); let result = file_fetcher_02 .fetch(&specifier, PermissionsContainer::allow_all()) @@ -1146,50 +1173,68 @@ mod tests { let _http_server_guard = test_util::http_server(); let temp_dir = TempDir::new(); let location = temp_dir.path().join("deps").to_path_buf(); - let file_fetcher_01 = FileFetcher::new( - HttpCache::new(location.clone()), - CacheSetting::Use, - true, - Arc::new(HttpClient::new(None, None)), - Default::default(), - None, - ); let specifier = resolve_url("http://localhost:4545/subdir/mismatch_ext.ts").unwrap(); - let cache_filename = file_fetcher_01 - .http_cache - .get_cache_filename(&specifier) - .unwrap(); - let result = file_fetcher_01 - .fetch(&specifier, PermissionsContainer::allow_all()) - .await; - assert!(result.is_ok()); + let file_modified_01 = { + let file_fetcher = FileFetcher::new( + HttpCache::new(location.clone()), + CacheSetting::Use, + true, + Arc::new(HttpClient::new(None, None)), + Default::default(), + None, + ); - let metadata_filename = CachedUrlMetadata::filename(&cache_filename); - let metadata_file = fs::File::open(metadata_filename).unwrap(); - let metadata_file_metadata = metadata_file.metadata().unwrap(); - let metadata_file_modified_01 = metadata_file_metadata.modified().unwrap(); + let result = file_fetcher + .fetch(&specifier, PermissionsContainer::allow_all()) + .await; + assert!(result.is_ok()); + ( + file_fetcher + .http_cache + .get_modified_time(&specifier) + .unwrap(), + file_fetcher + .http_cache + .get(&specifier) + .unwrap() + .read_metadata() + .unwrap() + .unwrap(), + ) + }; - let file_fetcher_02 = FileFetcher::new( - HttpCache::new(location), - CacheSetting::Use, - true, - Arc::new(HttpClient::new(None, None)), - Default::default(), - None, - ); - let result = file_fetcher_02 - .fetch(&specifier, PermissionsContainer::allow_all()) - .await; - assert!(result.is_ok()); + let file_modified_02 = { + let file_fetcher = FileFetcher::new( + HttpCache::new(location), + CacheSetting::Use, + true, + Arc::new(HttpClient::new(None, None)), + Default::default(), + None, + ); + let result = file_fetcher + .fetch(&specifier, PermissionsContainer::allow_all()) + .await; + assert!(result.is_ok()); - let metadata_filename = CachedUrlMetadata::filename(&cache_filename); - let metadata_file = fs::File::open(metadata_filename).unwrap(); - let metadata_file_metadata = metadata_file.metadata().unwrap(); - let metadata_file_modified_02 = metadata_file_metadata.modified().unwrap(); + ( + file_fetcher + .http_cache + .get_modified_time(&specifier) + .unwrap(), + file_fetcher + .http_cache + .get(&specifier) + .unwrap() + .read_metadata() + .unwrap() + .unwrap(), + ) + }; - assert_eq!(metadata_file_modified_01, metadata_file_modified_02); + assert_eq!(file_modified_01, file_modified_02); } #[tokio::test] @@ -1199,17 +1244,9 @@ mod tests { let specifier = resolve_url("http://localhost:4546/subdir/redirects/redirect1.js") .unwrap(); - let cached_filename = file_fetcher - .http_cache - .get_cache_filename(&specifier) - .unwrap(); let redirected_specifier = resolve_url("http://localhost:4545/subdir/redirects/redirect1.js") .unwrap(); - let redirected_cached_filename = file_fetcher - .http_cache - .get_cache_filename(&redirected_specifier) - .unwrap(); let result = file_fetcher .fetch(&specifier, PermissionsContainer::allow_all()) @@ -1218,24 +1255,40 @@ mod tests { let file = result.unwrap(); assert_eq!(file.specifier, redirected_specifier); - assert_eq!( - fs::read_to_string(cached_filename).unwrap(), - "", - "redirected files should have empty cached contents" - ); - let (_, headers, _) = file_fetcher.http_cache.get(&specifier).unwrap(); - assert_eq!( - headers.get("location").unwrap(), - "http://localhost:4545/subdir/redirects/redirect1.js" - ); + { + let cache_item = file_fetcher.http_cache.get(&specifier).unwrap(); + assert_eq!( + cache_item.read_to_string().unwrap().unwrap(), + "", + "redirected files should have empty cached contents" + ); + assert_eq!( + cache_item + .read_metadata() + .unwrap() + .unwrap() + .headers + .get("location") + .unwrap(), + "http://localhost:4545/subdir/redirects/redirect1.js" + ); + } - assert_eq!( - fs::read_to_string(redirected_cached_filename).unwrap(), - "export const redirect = 1;\n" - ); - let (_, headers, _) = - file_fetcher.http_cache.get(&redirected_specifier).unwrap(); - assert!(headers.get("location").is_none()); + { + let cache_item = + file_fetcher.http_cache.get(&redirected_specifier).unwrap(); + assert_eq!( + cache_item.read_to_string().unwrap().unwrap(), + "export const redirect = 1;\n" + ); + assert!(cache_item + .read_metadata() + .unwrap() + .unwrap() + .headers + .get("location") + .is_none()); + } } #[tokio::test] @@ -1245,24 +1298,12 @@ mod tests { let specifier = resolve_url("http://localhost:4548/subdir/redirects/redirect1.js") .unwrap(); - let cached_filename = file_fetcher - .http_cache - .get_cache_filename(&specifier) - .unwrap(); let redirected_01_specifier = resolve_url("http://localhost:4546/subdir/redirects/redirect1.js") .unwrap(); - let redirected_01_cached_filename = file_fetcher - .http_cache - .get_cache_filename(&redirected_01_specifier) - .unwrap(); let redirected_02_specifier = resolve_url("http://localhost:4545/subdir/redirects/redirect1.js") .unwrap(); - let redirected_02_cached_filename = file_fetcher - .http_cache - .get_cache_filename(&redirected_02_specifier) - .unwrap(); let result = file_fetcher .fetch(&specifier, PermissionsContainer::allow_all()) @@ -1271,40 +1312,64 @@ mod tests { let file = result.unwrap(); assert_eq!(file.specifier, redirected_02_specifier); - assert_eq!( - fs::read_to_string(cached_filename).unwrap(), - "", - "redirected files should have empty cached contents" - ); - let (_, headers, _) = file_fetcher.http_cache.get(&specifier).unwrap(); - assert_eq!( - headers.get("location").unwrap(), - "http://localhost:4546/subdir/redirects/redirect1.js" - ); + { + let cache_item = file_fetcher.http_cache.get(&specifier).unwrap(); + assert_eq!( + cache_item.read_to_string().unwrap().unwrap(), + "", + "redirected files should have empty cached contents" + ); + assert_eq!( + cache_item + .read_metadata() + .unwrap() + .unwrap() + .headers + .get("location") + .unwrap(), + "http://localhost:4546/subdir/redirects/redirect1.js" + ); + } - assert_eq!( - fs::read_to_string(redirected_01_cached_filename).unwrap(), - "", - "redirected files should have empty cached contents" - ); - let (_, headers, _) = file_fetcher - .http_cache - .get(&redirected_01_specifier) - .unwrap(); - assert_eq!( - headers.get("location").unwrap(), - "http://localhost:4545/subdir/redirects/redirect1.js" - ); + { + let cache_item = file_fetcher + .http_cache + .get(&redirected_01_specifier) + .unwrap(); + assert_eq!( + cache_item.read_to_string().unwrap().unwrap(), + "", + "redirected files should have empty cached contents" + ); + assert_eq!( + cache_item + .read_metadata() + .unwrap() + .unwrap() + .headers + .get("location") + .unwrap(), + "http://localhost:4545/subdir/redirects/redirect1.js" + ); + } - assert_eq!( - fs::read_to_string(redirected_02_cached_filename).unwrap(), - "export const redirect = 1;\n" - ); - let (_, headers, _) = file_fetcher - .http_cache - .get(&redirected_02_specifier) - .unwrap(); - assert!(headers.get("location").is_none()); + { + let cache_item = file_fetcher + .http_cache + .get(&redirected_02_specifier) + .unwrap(); + assert_eq!( + cache_item.read_to_string().unwrap().unwrap(), + "export const redirect = 1;\n" + ); + assert!(cache_item + .read_metadata() + .unwrap() + .unwrap() + .headers + .get("location") + .is_none()); + } } #[tokio::test] @@ -1312,52 +1377,69 @@ mod tests { let _http_server_guard = test_util::http_server(); let temp_dir = TempDir::new(); let location = temp_dir.path().join("deps").to_path_buf(); - let file_fetcher_01 = FileFetcher::new( - HttpCache::new(location.clone()), - CacheSetting::Use, - true, - Arc::new(HttpClient::new(None, None)), - Default::default(), - None, - ); let specifier = resolve_url("http://localhost:4548/subdir/mismatch_ext.ts").unwrap(); let redirected_specifier = resolve_url("http://localhost:4546/subdir/mismatch_ext.ts").unwrap(); - let redirected_cache_filename = file_fetcher_01 - .http_cache - .get_cache_filename(&redirected_specifier) - .unwrap(); - let result = file_fetcher_01 - .fetch(&specifier, PermissionsContainer::allow_all()) - .await; - assert!(result.is_ok()); + let metadata_file_modified_01 = { + let file_fetcher = FileFetcher::new( + HttpCache::new(location.clone()), + CacheSetting::Use, + true, + Arc::new(HttpClient::new(None, None)), + Default::default(), + None, + ); - let metadata_filename = - CachedUrlMetadata::filename(&redirected_cache_filename); - let metadata_file = fs::File::open(metadata_filename).unwrap(); - let metadata_file_metadata = metadata_file.metadata().unwrap(); - let metadata_file_modified_01 = metadata_file_metadata.modified().unwrap(); + let result = file_fetcher + .fetch(&specifier, PermissionsContainer::allow_all()) + .await; + assert!(result.is_ok()); - let file_fetcher_02 = FileFetcher::new( - HttpCache::new(location), - CacheSetting::Use, - true, - Arc::new(HttpClient::new(None, None)), - Default::default(), - None, - ); - let result = file_fetcher_02 - .fetch(&redirected_specifier, PermissionsContainer::allow_all()) - .await; - assert!(result.is_ok()); + ( + file_fetcher + .http_cache + .get_modified_time(&redirected_specifier) + .unwrap(), + file_fetcher + .http_cache + .get(&redirected_specifier) + .unwrap() + .read_metadata() + .unwrap() + .unwrap(), + ) + }; - let metadata_filename = - CachedUrlMetadata::filename(&redirected_cache_filename); - let metadata_file = fs::File::open(metadata_filename).unwrap(); - let metadata_file_metadata = metadata_file.metadata().unwrap(); - let metadata_file_modified_02 = metadata_file_metadata.modified().unwrap(); + let metadata_file_modified_02 = { + let file_fetcher = FileFetcher::new( + HttpCache::new(location), + CacheSetting::Use, + true, + Arc::new(HttpClient::new(None, None)), + Default::default(), + None, + ); + let result = file_fetcher + .fetch(&redirected_specifier, PermissionsContainer::allow_all()) + .await; + assert!(result.is_ok()); + + ( + file_fetcher + .http_cache + .get_modified_time(&redirected_specifier) + .unwrap(), + file_fetcher + .http_cache + .get(&redirected_specifier) + .unwrap() + .read_metadata() + .unwrap() + .unwrap(), + ) + }; assert_eq!(metadata_file_modified_01, metadata_file_modified_02); } @@ -1395,17 +1477,9 @@ mod tests { "http://localhost:4550/REDIRECT/subdir/redirects/redirect1.js", ) .unwrap(); - let cached_filename = file_fetcher - .http_cache - .get_cache_filename(&specifier) - .unwrap(); let redirected_specifier = resolve_url("http://localhost:4550/subdir/redirects/redirect1.js") .unwrap(); - let redirected_cached_filename = file_fetcher - .http_cache - .get_cache_filename(&redirected_specifier) - .unwrap(); let result = file_fetcher .fetch(&specifier, PermissionsContainer::allow_all()) @@ -1414,24 +1488,40 @@ mod tests { let file = result.unwrap(); assert_eq!(file.specifier, redirected_specifier); - assert_eq!( - fs::read_to_string(cached_filename).unwrap(), - "", - "redirected files should have empty cached contents" - ); - let (_, headers, _) = file_fetcher.http_cache.get(&specifier).unwrap(); - assert_eq!( - headers.get("location").unwrap(), - "/subdir/redirects/redirect1.js" - ); + { + let cache_item = file_fetcher.http_cache.get(&specifier).unwrap(); + assert_eq!( + cache_item.read_to_string().unwrap().unwrap(), + "", + "redirected files should have empty cached contents" + ); + assert_eq!( + cache_item + .read_metadata() + .unwrap() + .unwrap() + .headers + .get("location") + .unwrap(), + "/subdir/redirects/redirect1.js" + ); + } - assert_eq!( - fs::read_to_string(redirected_cached_filename).unwrap(), - "export const redirect = 1;\n" - ); - let (_, headers, _) = - file_fetcher.http_cache.get(&redirected_specifier).unwrap(); - assert!(headers.get("location").is_none()); + { + let cache_item = + file_fetcher.http_cache.get(&redirected_specifier).unwrap(); + assert_eq!( + cache_item.read_to_string().unwrap().unwrap(), + "export const redirect = 1;\n" + ); + assert!(cache_item + .read_metadata() + .unwrap() + .unwrap() + .headers + .get("location") + .is_none()); + } } #[tokio::test] @@ -1488,8 +1578,8 @@ mod tests { .await; assert!(result.is_err()); let err = result.unwrap_err(); - assert_eq!(get_custom_error_class(&err), Some("NotCached")); assert_eq!(err.to_string(), "Specifier not found in cache: \"http://localhost:4545/run/002_hello.ts\", --cached-only is specified."); + assert_eq!(get_custom_error_class(&err), Some("NotCached")); let result = file_fetcher_02 .fetch(&specifier, PermissionsContainer::allow_all()) diff --git a/cli/lsp/cache.rs b/cli/lsp/cache.rs index 9ce47a11b4..e14497156f 100644 --- a/cli/lsp/cache.rs +++ b/cli/lsp/cache.rs @@ -1,7 +1,7 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. -use crate::cache::CachedUrlMetadata; use crate::cache::HttpCache; +use crate::util::path::specifier_to_file_path; use deno_core::parking_lot::Mutex; use deno_core::ModuleSpecifier; @@ -12,8 +12,21 @@ use std::path::PathBuf; use std::sync::Arc; use std::time::SystemTime; +pub fn calculate_fs_version( + cache: &HttpCache, + specifier: &ModuleSpecifier, +) -> Option { + match specifier.scheme() { + "npm" | "node" | "data" | "blob" => None, + "file" => specifier_to_file_path(specifier) + .ok() + .and_then(|path| calculate_fs_version_at_path(&path)), + _ => calculate_fs_version_in_cache(cache, specifier), + } +} + /// Calculate a version for for a given path. -pub fn calculate_fs_version(path: &Path) -> Option { +pub fn calculate_fs_version_at_path(path: &Path) -> Option { let metadata = fs::metadata(path).ok()?; if let Ok(modified) = metadata.modified() { if let Ok(n) = modified.duration_since(SystemTime::UNIX_EPOCH) { @@ -26,6 +39,22 @@ pub fn calculate_fs_version(path: &Path) -> Option { } } +fn calculate_fs_version_in_cache( + cache: &HttpCache, + specifier: &ModuleSpecifier, +) -> Option { + match cache.get_modified_time(specifier) { + Ok(Some(modified)) => { + match modified.duration_since(SystemTime::UNIX_EPOCH) { + Ok(n) => Some(n.as_millis().to_string()), + Err(_) => Some("1".to_string()), + } + } + Ok(None) => None, + Err(_) => Some("1".to_string()), + } +} + /// Populate the metadata map based on the supplied headers fn parse_metadata( headers: &HashMap, @@ -49,7 +78,7 @@ struct Metadata { version: Option, } -#[derive(Debug, Default, Clone)] +#[derive(Debug, Clone)] pub struct CacheMetadata { cache: HttpCache, metadata: Arc>>, @@ -75,10 +104,7 @@ impl CacheMetadata { ) { return None; } - let version = self - .cache - .get_cache_filename(specifier) - .and_then(|ref path| calculate_fs_version(path)); + let version = calculate_fs_version_in_cache(&self.cache, specifier); let metadata = self.metadata.lock().get(specifier).cloned(); if metadata.as_ref().and_then(|m| m.version.clone()) != version { self.refresh(specifier).map(|m| m.values) @@ -94,10 +120,10 @@ impl CacheMetadata { ) { return None; } - let cache_filename = self.cache.get_cache_filename(specifier)?; - let specifier_metadata = CachedUrlMetadata::read(&cache_filename).ok()?; + let specifier_metadata = + self.cache.get(specifier).ok()?.read_metadata().ok()??; let values = Arc::new(parse_metadata(&specifier_metadata.headers)); - let version = calculate_fs_version(&cache_filename); + let version = calculate_fs_version_in_cache(&self.cache, specifier); let mut metadata_map = self.metadata.lock(); let metadata = Metadata { values, version }; metadata_map.insert(specifier.clone(), metadata.clone()); diff --git a/cli/lsp/completions.rs b/cli/lsp/completions.rs index 17a00010b0..73ec5a9187 100644 --- a/cli/lsp/completions.rs +++ b/cli/lsp/completions.rs @@ -519,7 +519,8 @@ mod tests { source_fixtures: &[(&str, &str)], location: &Path, ) -> Documents { - let mut documents = Documents::new(location.to_path_buf()); + let cache = HttpCache::new(location.to_path_buf()); + let mut documents = Documents::new(cache); for (specifier, source, version, language_id) in fixtures { let specifier = resolve_url(specifier).expect("failed to create specifier"); diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 415fe142df..b202e47d61 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -1169,6 +1169,7 @@ async fn generate_deno_diagnostics( #[cfg(test)] mod tests { use super::*; + use crate::cache::HttpCache; use crate::lsp::config::ConfigSnapshot; use crate::lsp::config::Settings; use crate::lsp::config::SpecifierSettings; @@ -1187,7 +1188,8 @@ mod tests { location: &Path, maybe_import_map: Option<(&str, &str)>, ) -> StateSnapshot { - let mut documents = Documents::new(location.to_path_buf()); + let cache = HttpCache::new(location.to_path_buf()); + let mut documents = Documents::new(cache); for (specifier, source, version, language_id) in fixtures { let specifier = resolve_url(specifier).expect("failed to create specifier"); @@ -1209,7 +1211,12 @@ mod tests { StateSnapshot { documents, maybe_import_map, - ..Default::default() + assets: Default::default(), + cache_metadata: cache::CacheMetadata::new(HttpCache::new( + location.to_path_buf(), + )), + maybe_node_resolver: None, + maybe_npm_resolver: None, } } @@ -1242,7 +1249,7 @@ mod tests { async fn test_enabled_then_disabled_specifier() { let temp_dir = TempDir::new(); let specifier = ModuleSpecifier::parse("file:///a.ts").unwrap(); - let (snapshot, _) = setup( + let (snapshot, cache_location) = setup( &temp_dir, &[( "file:///a.ts", @@ -1256,7 +1263,8 @@ let c: number = "a"; None, ); let snapshot = Arc::new(snapshot); - let ts_server = TsServer::new(Default::default()); + let cache = HttpCache::new(cache_location); + let ts_server = TsServer::new(Default::default(), cache); // test enabled { @@ -1337,7 +1345,7 @@ let c: number = "a"; #[tokio::test] async fn test_cancelled_ts_diagnostics_request() { let temp_dir = TempDir::new(); - let (snapshot, _) = setup( + let (snapshot, cache_location) = setup( &temp_dir, &[( "file:///a.ts", @@ -1348,7 +1356,8 @@ let c: number = "a"; None, ); let snapshot = Arc::new(snapshot); - let ts_server = TsServer::new(Default::default()); + let cache = HttpCache::new(cache_location); + let ts_server = TsServer::new(Default::default(), cache); let config = mock_config(); let token = CancellationToken::new(); diff --git a/cli/lsp/documents.rs b/cli/lsp/documents.rs index f18284db79..11662a8fcd 100644 --- a/cli/lsp/documents.rs +++ b/cli/lsp/documents.rs @@ -1,6 +1,7 @@ // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license. use super::cache::calculate_fs_version; +use super::cache::calculate_fs_version_at_path; use super::text::LineIndex; use super::tsc; use super::tsc::AssetDocument; @@ -9,7 +10,6 @@ use crate::args::package_json; use crate::args::package_json::PackageJsonDeps; use crate::args::ConfigFile; use crate::args::JsxImportSourceConfig; -use crate::cache::CachedUrlMetadata; use crate::cache::FastInsecureHasher; use crate::cache::HttpCache; use crate::file_fetcher::get_source_from_bytes; @@ -656,7 +656,7 @@ fn recurse_dependents( } } -#[derive(Debug, Default)] +#[derive(Debug)] struct SpecifierResolver { cache: HttpCache, redirects: Mutex>, @@ -698,10 +698,12 @@ impl SpecifierResolver { specifier: &ModuleSpecifier, redirect_limit: usize, ) -> Option { - let cache_filename = self.cache.get_cache_filename(specifier)?; - if redirect_limit > 0 && cache_filename.is_file() { - let headers = CachedUrlMetadata::read(&cache_filename) + if redirect_limit > 0 { + let headers = self + .cache + .get(specifier) .ok() + .and_then(|i| i.read_metadata().ok()?) .map(|m| m.headers)?; if let Some(location) = headers.get("location") { let redirect = @@ -732,8 +734,7 @@ impl FileSystemDocuments { let fs_version = if specifier.scheme() == "data" { Some("1".to_string()) } else { - get_document_path(cache, specifier) - .and_then(|path| calculate_fs_version(&path)) + calculate_fs_version(cache, specifier) }; let file_system_doc = self.docs.get(specifier); if file_system_doc.map(|d| d.fs_version().to_string()) != fs_version { @@ -753,8 +754,8 @@ impl FileSystemDocuments { specifier: &ModuleSpecifier, ) -> Option { let doc = if specifier.scheme() == "file" { - let path = get_document_path(cache, specifier)?; - let fs_version = calculate_fs_version(&path)?; + let path = specifier_to_file_path(specifier).ok()?; + let fs_version = calculate_fs_version_at_path(&path)?; let bytes = fs::read(path).ok()?; let maybe_charset = Some(text_encoding::detect_charset(&bytes).to_string()); @@ -776,11 +777,10 @@ impl FileSystemDocuments { resolver, ) } else { - let path = get_document_path(cache, specifier)?; - let fs_version = calculate_fs_version(&path)?; - let bytes = fs::read(path).ok()?; - let cache_filename = cache.get_cache_filename(specifier)?; - let specifier_metadata = CachedUrlMetadata::read(&cache_filename).ok()?; + let fs_version = calculate_fs_version(cache, specifier)?; + let cache_item = cache.get(specifier).ok()?; + let bytes = cache_item.read_to_bytes().ok()??; + let specifier_metadata = cache_item.read_metadata().ok()??; let maybe_content_type = specifier_metadata.headers.get("content-type"); let (_, maybe_charset) = map_content_type(specifier, maybe_content_type); let maybe_headers = Some(specifier_metadata.headers); @@ -799,17 +799,6 @@ impl FileSystemDocuments { } } -fn get_document_path( - cache: &HttpCache, - specifier: &ModuleSpecifier, -) -> Option { - match specifier.scheme() { - "npm" | "node" | "data" | "blob" => None, - "file" => specifier_to_file_path(specifier).ok(), - _ => cache.get_cache_filename(specifier), - } -} - pub struct UpdateDocumentConfigOptions<'a> { pub enabled_urls: Vec, pub document_preload_limit: usize, @@ -831,7 +820,7 @@ pub enum DocumentsFilter { OpenDiagnosable, } -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone)] pub struct Documents { /// The DENO_DIR that the documents looks for non-file based modules. cache: HttpCache, @@ -864,8 +853,7 @@ pub struct Documents { } impl Documents { - pub fn new(location: PathBuf) -> Self { - let cache = HttpCache::new(location); + pub fn new(cache: HttpCache) -> Self { Self { cache: cache.clone(), dirty: true, @@ -990,8 +978,13 @@ impl Documents { if specifier.scheme() == "data" { return true; } - if let Some(path) = get_document_path(&self.cache, &specifier) { - return path.is_file(); + if specifier.scheme() == "file" { + return specifier_to_file_path(&specifier) + .map(|p| p.is_file()) + .unwrap_or(false); + } + if self.cache.contains(&specifier) { + return true; } } false @@ -1859,7 +1852,8 @@ mod tests { fn setup(temp_dir: &TempDir) -> (Documents, PathRef) { let location = temp_dir.path().join("deps"); - let documents = Documents::new(location.to_path_buf()); + let cache = HttpCache::new(location.to_path_buf()); + let documents = Documents::new(cache); (documents, location) } diff --git a/cli/lsp/language_server.rs b/cli/lsp/language_server.rs index 61bd64eefb..d8fa7a7b4f 100644 --- a/cli/lsp/language_server.rs +++ b/cli/lsp/language_server.rs @@ -184,7 +184,7 @@ struct LspConfigFileInfo { pub struct LanguageServer(Arc>); /// Snapshot of the state used by TSC. -#[derive(Debug, Default)] +#[derive(Debug)] pub struct StateSnapshot { pub assets: AssetsSnapshot, pub cache_metadata: cache::CacheMetadata, @@ -610,11 +610,12 @@ impl Inner { http_client.clone(), ); let location = dir.deps_folder_path(); - let documents = Documents::new(location.clone()); let deps_http_cache = HttpCache::new(location); + let documents = Documents::new(deps_http_cache.clone()); let cache_metadata = cache::CacheMetadata::new(deps_http_cache.clone()); let performance = Arc::new(Performance::default()); - let ts_server = Arc::new(TsServer::new(performance.clone())); + let ts_server = + Arc::new(TsServer::new(performance.clone(), deps_http_cache.clone())); let config = Config::new(); let diagnostics_server = DiagnosticsServer::new( client.clone(), diff --git a/cli/lsp/registries.rs b/cli/lsp/registries.rs index 9454f77f54..9586997c5a 100644 --- a/cli/lsp/registries.rs +++ b/cli/lsp/registries.rs @@ -13,7 +13,6 @@ use super::path_to_regex::StringOrVec; use super::path_to_regex::Token; use crate::args::CacheSetting; -use crate::cache::DenoDir; use crate::cache::HttpCache; use crate::file_fetcher::FileFetcher; use crate::http_util::HttpClient; @@ -418,18 +417,6 @@ pub struct ModuleRegistry { file_fetcher: FileFetcher, } -impl Default for ModuleRegistry { - fn default() -> Self { - // This only gets used when creating the tsc runtime and for testing, and so - // it shouldn't ever actually access the DenoDir, so it doesn't support a - // custom root. - let dir = DenoDir::new(None).unwrap(); - let location = dir.registries_folder_path(); - let http_client = Arc::new(HttpClient::new(None, None)); - Self::new(location, http_client) - } -} - impl ModuleRegistry { pub fn new(location: PathBuf, http_client: Arc) -> Self { let http_cache = HttpCache::new(location); diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs index 40c823d5b9..662b480619 100644 --- a/cli/lsp/tsc.rs +++ b/cli/lsp/tsc.rs @@ -22,6 +22,9 @@ use super::urls::LspUrlMap; use super::urls::INVALID_SPECIFIER; use crate::args::TsConfig; +use crate::cache::HttpCache; +use crate::lsp::cache::CacheMetadata; +use crate::lsp::documents::Documents; use crate::lsp::logging::lsp_warn; use crate::tsc; use crate::tsc::ResolveArgs; @@ -96,10 +99,10 @@ type Request = ( pub struct TsServer(mpsc::UnboundedSender); impl TsServer { - pub fn new(performance: Arc) -> Self { + pub fn new(performance: Arc, cache: HttpCache) -> Self { let (tx, mut rx) = mpsc::unbounded_channel::(); let _join_handle = thread::spawn(move || { - let mut ts_runtime = js_runtime(performance); + let mut ts_runtime = js_runtime(performance, cache); let runtime = create_basic_runtime(); runtime.block_on(async { @@ -3242,9 +3245,9 @@ fn op_script_version( /// Create and setup a JsRuntime based on a snapshot. It is expected that the /// supplied snapshot is an isolate that contains the TypeScript language /// server. -fn js_runtime(performance: Arc) -> JsRuntime { +fn js_runtime(performance: Arc, cache: HttpCache) -> JsRuntime { JsRuntime::new(RuntimeOptions { - extensions: vec![deno_tsc::init_ops(performance)], + extensions: vec![deno_tsc::init_ops(performance, cache)], startup_snapshot: Some(tsc::compiler_snapshot()), ..Default::default() }) @@ -3261,11 +3264,19 @@ deno_core::extension!(deno_tsc, op_script_version, ], options = { - performance: Arc + performance: Arc, + cache: HttpCache, }, state = |state, options| { state.put(State::new( - Arc::new(StateSnapshot::default()), + Arc::new(StateSnapshot { + assets: Default::default(), + cache_metadata: CacheMetadata::new(options.cache.clone()), + documents: Documents::new(options.cache.clone()), + maybe_import_map: None, + maybe_node_resolver: None, + maybe_npm_resolver: None, + }), options.performance, )); }, @@ -3897,6 +3908,7 @@ mod tests { use super::*; use crate::cache::HttpCache; use crate::http_util::HeadersMap; + use crate::lsp::cache::CacheMetadata; use crate::lsp::config::WorkspaceSettings; use crate::lsp::documents::Documents; use crate::lsp::documents::LanguageId; @@ -3911,7 +3923,8 @@ mod tests { fixtures: &[(&str, &str, i32, LanguageId)], location: &Path, ) -> StateSnapshot { - let mut documents = Documents::new(location.to_path_buf()); + let cache = HttpCache::new(location.to_path_buf()); + let mut documents = Documents::new(cache.clone()); for (specifier, source, version, language_id) in fixtures { let specifier = resolve_url(specifier).expect("failed to create specifier"); @@ -3924,7 +3937,11 @@ mod tests { } StateSnapshot { documents, - ..Default::default() + assets: Default::default(), + cache_metadata: CacheMetadata::new(cache), + maybe_import_map: None, + maybe_node_resolver: None, + maybe_npm_resolver: None, } } @@ -3935,8 +3952,9 @@ mod tests { sources: &[(&str, &str, i32, LanguageId)], ) -> (JsRuntime, Arc, PathBuf) { let location = temp_dir.path().join("deps").to_path_buf(); + let cache = HttpCache::new(location.clone()); let state_snapshot = Arc::new(mock_state_snapshot(sources, &location)); - let mut runtime = js_runtime(Default::default()); + let mut runtime = js_runtime(Default::default(), cache); start(&mut runtime, debug).unwrap(); let ts_config = TsConfig::new(config); assert_eq!(