feat(lsp): registry suggestion cache respects cache headers

Fixes denoland#9931

cli/Cargo.toml

@@ -49,6 +49,7 @@ deno_runtime = { version = "0.36.0", path = "../runtime" }
 atty = "=0.2.14"
 base64 = "=0.13.0"
 clap = "=2.33.3"
+chrono = "=0.4.19"
 data-url = "=0.1.0"
 dissimilar = "=1.0.2"
 dprint-plugin-json = "=0.13.2"
@@ -89,8 +90,6 @@ fwdansi = "=1.1.0"
 winapi = { version = "=0.3.9", features = ["knownfolders", "mswsock", "objbase", "shlobj", "tlhelp32", "winbase", "winerror", "winsock2"] }
 
 [dev-dependencies]
-# Used in benchmark
-chrono = "=0.4.19"
 flaky_test = "=0.1.0"
 os_pipe = "=0.9.2"
 pretty_assertions = "=0.7.2"

cli/file_fetcher.rs

@@ -4,10 +4,12 @@ use crate::auth_tokens::AuthTokens;
 use crate::colors;
 use crate::http_cache::HttpCache;
 use crate::http_util::fetch_once;
+use crate::http_util::CacheSemantics;
 use crate::http_util::FetchOnceArgs;
 use crate::http_util::FetchOnceResult;
 use crate::text_encoding;
 use crate::version::get_user_agent;
+
 use data_url::DataUrl;
 use deno_ast::MediaType;
 use deno_core::error::custom_error;
@@ -34,6 +36,7 @@ use std::io::Read;
 use std::path::PathBuf;
 use std::pin::Pin;
 use std::sync::Arc;
+use std::time::SystemTime;
 
 pub const SUPPORTED_SCHEMES: [&str; 5] =
   ["data", "blob", "file", "http", "https"];
@@ -89,17 +92,34 @@ pub enum CacheSetting {
   /// `--reload=https://deno.land/std` or
   /// `--reload=https://deno.land/std,https://deno.land/x/example`.
   ReloadSome(Vec<String>),
+  /// The usability of a cached value is determined by analyzing the cached
+  /// headers and other metadata associated with a cached response, reloading
+  /// any cached "non-fresh" cached responses.
+  RespectHeaders,
   /// The cached source files should be used for local modules.  This is the
   /// default behavior of the CLI.
   Use,
 }
 
 impl CacheSetting {
   /// Returns if the cache should be used for a given specifier.
-  pub fn should_use(&self, specifier: &ModuleSpecifier) -> bool {
+  pub fn should_use(
+    &self,
+    specifier: &ModuleSpecifier,
+    http_cache: &HttpCache,
+  ) -> bool {
     match self {
       CacheSetting::ReloadAll => false,
       CacheSetting::Use | CacheSetting::Only => true,
+      CacheSetting::RespectHeaders => {
+        if let Ok((_, headers, cache_time)) = http_cache.get(specifier) {
+          let cache_semantics =
+            CacheSemantics::new(headers, cache_time, SystemTime::now());
+          cache_semantics.should_use()
+        } else {
+          false
+        }
+      }
       CacheSetting::ReloadSome(list) => {
         let mut url = specifier.clone();
         url.set_fragment(None);
@@ -312,7 +332,7 @@ impl FileFetcher {
       return Err(custom_error("Http", "Too many redirects."));
     }
 
-    let (mut source_file, headers) = match self.http_cache.get(specifier) {
+    let (mut source_file, headers, _) = match self.http_cache.get(specifier) {
       Err(err) => {
         if let Some(err) = err.downcast_ref::<std::io::Error>() {
           if err.kind() == std::io::ErrorKind::NotFound {
@@ -469,7 +489,7 @@ impl FileFetcher {
       return futures::future::err(err).boxed();
     }
 
-    if self.cache_setting.should_use(specifier) {
+    if self.cache_setting.should_use(specifier, &self.http_cache) {
       match self.fetch_cached(specifier, redirect_limit) {
         Ok(Some(file)) => {
           return futures::future::ok(file).boxed();
@@ -495,7 +515,7 @@ impl FileFetcher {
     info!("{} {}", colors::green("Download"), specifier);
 
     let maybe_etag = match self.http_cache.get(specifier) {
-      Ok((_, headers)) => headers.get("etag").cloned(),
+      Ok((_, headers, _)) => headers.get("etag").cloned(),
       _ => None,
     };
     let maybe_auth_token = self.auth_tokens.get(specifier);
@@ -682,7 +702,7 @@ mod tests {
       .fetch_remote(specifier, &mut Permissions::allow_all(), 1)
       .await;
     assert!(result.is_ok());
-    let (_, headers) = file_fetcher.http_cache.get(specifier).unwrap();
+    let (_, headers, _) = file_fetcher.http_cache.get(specifier).unwrap();
     (result.unwrap(), headers)
   }
 
@@ -1065,7 +1085,7 @@ 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();
     assert_eq!(headers.get("content-type").unwrap(), "text/javascript");
     metadata.headers = HashMap::new();
     metadata
@@ -1194,7 +1214,7 @@ mod tests {
       "",
       "redirected files should have empty cached contents"
     );
-    let (_, headers) = file_fetcher.http_cache.get(&specifier).unwrap();
+    let (_, headers, _) = file_fetcher.http_cache.get(&specifier).unwrap();
     assert_eq!(
       headers.get("location").unwrap(),
       "http://localhost:4545/subdir/redirects/redirect1.js"
@@ -1204,7 +1224,7 @@ mod tests {
       fs::read_to_string(redirected_cached_filename).unwrap(),
       "export const redirect = 1;\n"
     );
-    let (_, headers) =
+    let (_, headers, _) =
       file_fetcher.http_cache.get(&redirected_specifier).unwrap();
     assert!(headers.get("location").is_none());
   }
@@ -1247,7 +1267,7 @@ mod tests {
       "",
       "redirected files should have empty cached contents"
     );
-    let (_, headers) = file_fetcher.http_cache.get(&specifier).unwrap();
+    let (_, headers, _) = file_fetcher.http_cache.get(&specifier).unwrap();
     assert_eq!(
       headers.get("location").unwrap(),
       "http://localhost:4546/subdir/redirects/redirect1.js"
@@ -1258,7 +1278,7 @@ mod tests {
       "",
       "redirected files should have empty cached contents"
     );
-    let (_, headers) = file_fetcher
+    let (_, headers, _) = file_fetcher
       .http_cache
       .get(&redirected_01_specifier)
       .unwrap();
@@ -1271,7 +1291,7 @@ mod tests {
       fs::read_to_string(redirected_02_cached_filename).unwrap(),
       "export const redirect = 1;\n"
     );
-    let (_, headers) = file_fetcher
+    let (_, headers, _) = file_fetcher
       .http_cache
       .get(&redirected_02_specifier)
       .unwrap();
@@ -1392,7 +1412,7 @@ mod tests {
       "",
       "redirected files should have empty cached contents"
     );
-    let (_, headers) = file_fetcher.http_cache.get(&specifier).unwrap();
+    let (_, headers, _) = file_fetcher.http_cache.get(&specifier).unwrap();
     assert_eq!(
       headers.get("location").unwrap(),
       "/subdir/redirects/redirect1.js"
@@ -1402,7 +1422,7 @@ mod tests {
       fs::read_to_string(redirected_cached_filename).unwrap(),
       "export const redirect = 1;\n"
     );
-    let (_, headers) =
+    let (_, headers, _) =
       file_fetcher.http_cache.get(&redirected_specifier).unwrap();
     assert!(headers.get("location").is_none());
   }

cli/http_cache.rs

@@ -17,6 +17,7 @@ use std::fs::File;
 use std::io;
 use std::path::Path;
 use std::path::PathBuf;
+use std::time::SystemTime;
 
 pub const CACHE_PERM: u32 = 0o644;
 
@@ -81,6 +82,8 @@ pub struct HttpCache {
 pub struct Metadata {
   pub headers: HeadersMap,
   pub url: String,
+  #[serde(default = "SystemTime::now")]
+  pub now: SystemTime,
 }
 
 impl Metadata {
@@ -138,7 +141,10 @@ impl HttpCache {
   // 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), AnyError> {
+  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."))?,
@@ -147,7 +153,7 @@ impl HttpCache {
     let file = File::open(cache_filename)?;
     let metadata = fs::read_to_string(metadata_filename)?;
     let metadata: Metadata = serde_json::from_str(&metadata)?;
-    Ok((file, metadata.headers))
+    Ok((file, metadata.headers, metadata.now))
   }
 
   pub fn set(
@@ -169,6 +175,7 @@ impl HttpCache {
     fs_util::atomic_write_file(&cache_filename, content, CACHE_PERM)?;
 
     let metadata = Metadata {
+      now: SystemTime::now(),
       url: url.to_string(),
       headers: headers_map,
     };
@@ -227,7 +234,7 @@ mod tests {
     assert!(r.is_ok());
     let r = cache.get(&url);
     assert!(r.is_ok());
-    let (mut file, headers) = r.unwrap();
+    let (mut file, headers, _) = r.unwrap();
     let mut content = String::new();
     file.read_to_string(&mut content).unwrap();
     assert_eq!(content, "Hello world");