From e934df5f7dd7ebc52e8c74033d478c88fa638224 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= <biwanczuk@gmail.com>
Date: Sun, 24 May 2020 19:20:40 +0200
Subject: [PATCH] fix: create HTTP cache lazily (#5795)

---
 cli/deno_dir.rs     |  2 +-
 cli/disk_cache.rs   | 19 +++++++++++--------
 cli/global_state.rs |  1 -
 cli/http_cache.rs   | 29 +++++++++++++++++++++++------
 4 files changed, 35 insertions(+), 16 deletions(-)

diff --git a/cli/deno_dir.rs b/cli/deno_dir.rs
index 3224fbf467..1447682ec6 100644
--- a/cli/deno_dir.rs
+++ b/cli/deno_dir.rs
@@ -40,7 +40,7 @@ impl DenoDir {
       root,
       gen_cache: DiskCache::new(&gen_path),
     };
-    deno_dir.gen_cache.ensure_location()?;
+    deno_dir.gen_cache.ensure_dir_exists(&gen_path)?;
 
     Ok(deno_dir)
   }
diff --git a/cli/disk_cache.rs b/cli/disk_cache.rs
index 828ca90ca6..58744acd8f 100644
--- a/cli/disk_cache.rs
+++ b/cli/disk_cache.rs
@@ -31,14 +31,14 @@ impl DiskCache {
   }
 
   /// Ensures the location of the cache.
-  pub fn ensure_location(&self) -> io::Result<()> {
-    if self.location.is_dir() {
+  pub fn ensure_dir_exists(&self, path: &Path) -> io::Result<()> {
+    if path.is_dir() {
       return Ok(());
     }
-    fs::create_dir_all(&self.location).map_err(|e| {
+    fs::create_dir_all(&path).map_err(|e| {
       io::Error::new(e.kind(), format!(
         "Could not create TypeScript compiler cache location: {:?}\nCheck the permission of the directory.",
-        self.location
+        path
       ))
     })
   }
@@ -129,8 +129,7 @@ impl DiskCache {
   pub fn set(&self, filename: &Path, data: &[u8]) -> std::io::Result<()> {
     let path = self.location.join(filename);
     match path.parent() {
-      Some(ref parent) => fs::create_dir_all(parent)
-        .map_err(|e| with_io_context(&e, format!("{:#?}", &path))),
+      Some(ref parent) => self.ensure_dir_exists(parent),
       None => Ok(()),
     }?;
     deno_fs::write_file(&path, data, 0o666)
@@ -154,7 +153,9 @@ mod tests {
     let mut cache_path = cache_location.path().to_owned();
     cache_path.push("foo");
     let cache = DiskCache::new(&cache_path);
-    cache.ensure_location().expect("Testing expect:");
+    cache
+      .ensure_dir_exists(&cache.location)
+      .expect("Testing expect:");
     assert!(cache_path.is_dir());
   }
 
@@ -166,7 +167,9 @@ mod tests {
     cache_location.push("foo");
     assert_eq!(cache_location.is_dir(), false);
     let cache = DiskCache::new(&cache_location);
-    cache.ensure_location().expect("Testing expect:");
+    cache
+      .ensure_dir_exists(&cache.location)
+      .expect("Testing expect:");
     assert_eq!(cache_location.is_dir(), true);
   }
 
diff --git a/cli/global_state.rs b/cli/global_state.rs
index 90961ed627..8bd01f438f 100644
--- a/cli/global_state.rs
+++ b/cli/global_state.rs
@@ -51,7 +51,6 @@ impl GlobalState {
     let dir = deno_dir::DenoDir::new(custom_root)?;
     let deps_cache_location = dir.root.join("deps");
     let http_cache = http_cache::HttpCache::new(&deps_cache_location);
-    http_cache.ensure_location()?;
 
     let file_fetcher = SourceFileFetcher::new(
       http_cache,
diff --git a/cli/http_cache.rs b/cli/http_cache.rs
index 2a9882376a..23b482840d 100644
--- a/cli/http_cache.rs
+++ b/cli/http_cache.rs
@@ -113,16 +113,16 @@ impl HttpCache {
   }
 
   /// Ensures the location of the cache.
-  pub fn ensure_location(&self) -> io::Result<()> {
-    if self.location.is_dir() {
+  fn ensure_dir_exists(&self, path: &Path) -> io::Result<()> {
+    if path.is_dir() {
       return Ok(());
     }
-    fs::create_dir_all(&self.location).map_err(|e| {
+    fs::create_dir_all(&path).map_err(|e| {
       io::Error::new(
         e.kind(),
         format!(
           "Could not create remote modules cache location: {:?}\nCheck the permission of the directory.",
-          self.location
+          path
         ),
       )
     })
@@ -163,7 +163,7 @@ impl HttpCache {
     let parent_filename = cache_filename
       .parent()
       .expect("Cache filename should have a parent dir");
-    fs::create_dir_all(parent_filename)?;
+    self.ensure_dir_exists(parent_filename)?;
     // Cache content
     deno_fs::write_file(&cache_filename, content, 0o666)?;
 
@@ -187,8 +187,25 @@ mod tests {
     let dir = TempDir::new().unwrap();
     let mut cache_path = dir.path().to_owned();
     cache_path.push("foobar");
+    // HttpCache should be created lazily on first use:
+    // when zipping up a local project with no external dependencies
+    // "$DENO_DIR/deps" is empty. When unzipping such project
+    // "$DENO_DIR/deps" might not get restored and in situation
+    // when directory is owned by root we might not be able
+    // to create that directory. However if it's not needed it
+    // doesn't make sense to return error in such specific scenarios.
+    // For more details check issue:
+    // https://github.com/denoland/deno/issues/5688
     let cache = HttpCache::new(&cache_path);
-    assert!(cache.ensure_location().is_ok());
+    assert!(!cache.location.exists());
+    cache
+      .set(
+        &Url::parse("http://example.com/foo/bar.js").unwrap(),
+        HeadersMap::new(),
+        b"hello world",
+      )
+      .expect("Failed to add to cache");
+    assert!(cache.ensure_dir_exists(&cache.location).is_ok());
     assert!(cache_path.is_dir());
   }