Skip to content

Commit

Permalink
http_cache: change .headers.json to .metadata.json (#4175)
Browse files Browse the repository at this point in the history 
Add original URL to metadata. This is so the VS Code Plugin can reverse
look up the URL for cache entries. Ref #4069.
  • Loading branch information
@ry
ry committed Feb 28, 2020
1 parent bc7dbfa commit 0099c28
Show file tree
Hide file tree
Showing 3 changed files with 106 additions and 91 deletions.
135 changes: 54 additions & 81 deletions cli/file_fetcher.rs
View file
Expand Up @@ -298,7 +298,7 @@ impl SourceFileFetcher {
///
/// This is a recursive operation if source file has redirections.
///
/// It will keep reading <filename>.headers.json for information about redirection.
/// It will keep reading <filename>.metadata.json for information about redirection.
/// `module_initial_source_name` would be None on first call,
/// and becomes the name of the very first module that initiates the call
/// in subsequent recursions.
Expand Down Expand Up @@ -753,12 +753,8 @@ mod tests {
Url::parse("http://localhost:4545/cli/tests/subdir/mod2.ts").unwrap();
let module_url_1 = module_url.clone();
let module_url_2 = module_url.clone();
let headers_file_name = fetcher
.http_cache
.get_cache_filename(&module_url)
.with_extension("headers.json");
let headers_file_name_1 = headers_file_name.clone();
let headers_file_name_2 = headers_file_name.clone();

let cache_filename = fetcher.http_cache.get_cache_filename(&module_url);

let result = fetcher
.get_source_file(&module_url, true, false, false)
Expand All @@ -770,13 +766,17 @@ mod tests {
&b"export { printHello } from \"./print_hello.ts\";\n"[..]
);
assert_eq!(&(r.media_type), &msg::MediaType::TypeScript);
assert!(fs::read_to_string(&headers_file_name_1).is_ok());

let mut metadata =
crate::http_cache::Metadata::read(&cache_filename).unwrap();

// Modify .headers.json, write using fs write
let _ = fs::write(
&headers_file_name_1,
"{ \"content-type\": \"text/javascript\" }",
);
metadata.headers = HashMap::new();
metadata
.headers
.insert("content-type".to_string(), "text/javascript".to_string());
metadata.write(&cache_filename).unwrap();

let result2 = fetcher_1
.get_source_file(&module_url, true, false, false)
.await;
Expand All @@ -794,10 +794,12 @@ mod tests {
assert_eq!(headers.get("content-type").unwrap(), "text/javascript");

// Modify .headers.json again, but the other way around
let _ = fs::write(
&headers_file_name_1,
"{ \"content-type\": \"application/json\" }",
);
metadata.headers = HashMap::new();
metadata
.headers
.insert("content-type".to_string(), "application/json".to_string());
metadata.write(&cache_filename).unwrap();

let result3 = fetcher_2
.get_source_file(&module_url_1, true, false, false)
.await;
Expand All @@ -810,9 +812,11 @@ mod tests {
// If get_source_file does not call remote, this should be JavaScript
// as we modified before! (we do not overwrite .headers.json due to no http fetch)
assert_eq!(&(r3.media_type), &msg::MediaType::Json);
assert!(fs::read_to_string(&headers_file_name_2)
.unwrap()
.contains("application/json"));
let metadata = crate::http_cache::Metadata::read(&cache_filename).unwrap();
assert_eq!(
metadata.headers.get("content-type").unwrap(),
"application/json"
);

// let's create fresh instance of DenoDir (simulating another freshh Deno process)
// and don't use cache
Expand All @@ -838,10 +842,8 @@ mod tests {
Url::parse("http://localhost:4545/cli/tests/subdir/mismatch_ext.ts")
.unwrap();
let module_url_1 = module_url.clone();
let headers_file_name = fetcher
.http_cache
.get_cache_filename(&module_url)
.with_extension("headers.json");

let cache_filename = fetcher.http_cache.get_cache_filename(&module_url);

let result = fetcher
.get_source_file(&module_url, true, false, false)
Expand All @@ -855,10 +857,14 @@ mod tests {
assert_eq!(headers.get("content-type").unwrap(), "text/javascript");

// Modify .headers.json
let _ = fs::write(
&headers_file_name,
"{ \"content-type\": \"text/typescript\" }",
);
let mut metadata =
crate::http_cache::Metadata::read(&cache_filename).unwrap();
metadata.headers = HashMap::new();
metadata
.headers
.insert("content-type".to_string(), "text/typescript".to_string());
metadata.write(&cache_filename).unwrap();

let result2 = fetcher
.get_source_file(&module_url, true, false, false)
.await;
Expand All @@ -870,9 +876,10 @@ mod tests {
// as we modified before! (we do not overwrite .headers.json due to no http
// fetch)
assert_eq!(&(r2.media_type), &msg::MediaType::TypeScript);
let metadata = crate::http_cache::Metadata::read(&cache_filename).unwrap();
assert_eq!(
fs::read_to_string(&headers_file_name).unwrap(),
"{ \"content-type\": \"text/typescript\" }",
metadata.headers.get("content-type").unwrap(),
"text/typescript"
);

// let's create fresh instance of DenoDir (simulating another fresh Deno
Expand Down Expand Up @@ -902,15 +909,15 @@ mod tests {
"http://localhost:4545/cli/tests/subdir/mismatch_ext.ts",
)
.unwrap();
let headers_file_name = fetcher
.http_cache
.get_cache_filename(specifier.as_url())
.with_extension("headers.json");
let cache_filename =
fetcher.http_cache.get_cache_filename(&specifier.as_url());

// first download
let r = fetcher.fetch_source_file(&specifier, None).await;
assert!(r.is_ok());

let headers_file_name =
crate::http_cache::Metadata::filename(&cache_filename);
let result = fs::File::open(&headers_file_name);
assert!(result.is_ok());
let headers_file = result.unwrap();
Expand Down Expand Up @@ -1219,57 +1226,24 @@ mod tests {
let module_url =
Url::parse("http://127.0.0.1:4545/cli/tests/subdir/mt_video_mp2t.t3.ts")
.unwrap();
let headers_file_name = fetcher
.http_cache
.get_cache_filename(&module_url)
.with_extension("headers.json");
let result = fetcher
.fetch_remote_source(&module_url, false, false, 10)
.await;
assert!(result.is_ok());
let r = result.unwrap();
assert_eq!(r.source_code, b"export const loaded = true;\n");
assert_eq!(&(r.media_type), &msg::MediaType::TypeScript);
// Modify .headers.json, make sure read from local
let _ = fs::write(
&headers_file_name,
"{ \"content-type\": \"text/javascript\" }",
);
let result2 = fetcher.fetch_cached_remote_source(&module_url);
assert!(result2.is_ok());
let r2 = result2.unwrap().unwrap();
assert_eq!(r2.source_code, b"export const loaded = true;\n");
// Not MediaType::TypeScript due to .headers.json modification
assert_eq!(&(r2.media_type), &msg::MediaType::JavaScript);

drop(http_server_guard);
}

#[tokio::test]
async fn test_fetch_source_1() {
let http_server_guard = crate::test_util::http_server();

let (_temp_dir, fetcher) = test_setup();
let module_url =
Url::parse("http://localhost:4545/cli/tests/subdir/mt_video_mp2t.t3.ts")
.unwrap();
// Modify .metadata.json, make sure read from local
let cache_filename = fetcher.http_cache.get_cache_filename(&module_url);
let mut metadata =
crate::http_cache::Metadata::read(&cache_filename).unwrap();
metadata.headers = HashMap::new();
metadata
.headers
.insert("content-type".to_string(), "text/javascript".to_string());
metadata.write(&cache_filename).unwrap();

let result = fetcher
.fetch_remote_source(&module_url, false, false, 10)
.await;
assert!(result.is_ok());
let r = result.unwrap();
assert_eq!(r.source_code, b"export const loaded = true;\n");
assert_eq!(&(r.media_type), &msg::MediaType::TypeScript);
// Modify .headers.json, make sure read from local
let headers_file_name = fetcher
.http_cache
.get_cache_filename(&module_url)
.with_extension("headers.json");
let _ = fs::write(
&headers_file_name,
"{ \"content-type\": \"text/javascript\" }",
);
let result2 = fetcher.fetch_cached_remote_source(&module_url);
assert!(result2.is_ok());
let r2 = result2.unwrap().unwrap();
Expand Down Expand Up @@ -1630,12 +1604,11 @@ mod tests {
let (_, headers) = fetcher.http_cache.get(&module_url).unwrap();
assert_eq!(headers.get("etag").unwrap(), "33a64df551425fcc55e");

let header_path = fetcher
.http_cache
.get_cache_filename(&module_url)
.with_extension("headers.json");
let metadata_path = crate::http_cache::Metadata::filename(
&fetcher.http_cache.get_cache_filename(&module_url),
);

let modified1 = header_path.metadata().unwrap().modified().unwrap();
let modified1 = metadata_path.metadata().unwrap().modified().unwrap();

// Forcibly change the contents of the cache file and request
// it again with the cache parameters turned off.
Expand All @@ -1648,7 +1621,7 @@ mod tests {
.unwrap();
assert_eq!(cached_source.source_code, b"changed content");

let modified2 = header_path.metadata().unwrap().modified().unwrap();
let modified2 = metadata_path.metadata().unwrap().modified().unwrap();

// Assert that the file has not been modified
assert_eq!(modified1, modified2);
Expand Down
59 changes: 49 additions & 10 deletions cli/http_cache.rs
View file
Expand Up @@ -7,6 +7,8 @@
use crate::fs as deno_fs;
use crate::http_util::HeadersMap;
use deno_core::ErrBox;
use serde::Serialize;
use serde_derive::Deserialize;
use std::fs;
use std::fs::File;
use std::path::Path;
Expand All @@ -16,7 +18,8 @@ use url::Url;
/// Turn base of url (scheme, hostname, port) into a valid filename.
/// This method replaces port part with a special string token (because
/// ":" cannot be used in filename on some platforms).
pub fn base_url_to_filename(url: &Url) -> PathBuf {
/// Ex: $DENO_DIR/deps/https/deno.land/
fn base_url_to_filename(url: &Url) -> PathBuf {
let mut out = PathBuf::new();

let scheme = url.scheme();
Expand Down Expand Up @@ -70,6 +73,33 @@ pub struct HttpCache {
pub location: PathBuf,
}

#[derive(Serialize, Deserialize)]
pub struct Metadata {
pub headers: HeadersMap,
pub url: String,
}

impl Metadata {
pub fn write(&self, cache_filename: &Path) -> Result<(), ErrBox> {
let metadata_filename = Self::filename(cache_filename);
let json = serde_json::to_string_pretty(self)?;
deno_fs::write_file(&metadata_filename, json, 0o666)?;
Ok(())
}

pub fn read(cache_filename: &Path) -> Result<Metadata, ErrBox> {
let metadata_filename = Metadata::filename(&cache_filename);
let metadata = fs::read_to_string(metadata_filename)?;
let metadata: Metadata = serde_json::from_str(&metadata)?;
Ok(metadata)
}

/// Ex: $DENO_DIR/deps/https/deno.land/c885b7dcf1d6936e33a9cc3a2d74ec79bab5d733d3701c85a029b7f7ec9fbed4.metadata.json
pub fn filename(cache_filename: &Path) -> PathBuf {
cache_filename.with_extension("metadata.json")
}
}

impl HttpCache {
/// Returns error if unable to create directory
/// at specified location.
Expand All @@ -89,11 +119,19 @@ impl HttpCache {
// ETAG check is currently done in `cli/file_fetcher.rs`.
pub fn get(&self, url: &Url) -> Result<(File, HeadersMap), ErrBox> {
let cache_filename = self.location.join(url_to_filename(url));
let headers_filename = cache_filename.with_extension("headers.json");
let metadata_filename = Metadata::filename(&cache_filename);
let file = File::open(cache_filename)?;
let headers_json = fs::read_to_string(headers_filename)?;
let headers_map: HeadersMap = serde_json::from_str(&headers_json)?;
Ok((file, headers_map))
let metadata = fs::read_to_string(metadata_filename)?;
let metadata: Metadata = serde_json::from_str(&metadata)?;
Ok((file, metadata.headers))
}

pub fn get_metadata(&self, url: &Url) -> Result<Metadata, ErrBox> {
let cache_filename = self.location.join(url_to_filename(url));
let metadata_filename = Metadata::filename(&cache_filename);
let metadata = fs::read_to_string(metadata_filename)?;
let metadata: Metadata = serde_json::from_str(&metadata)?;
Ok(metadata)
}

pub fn set(
Expand All @@ -103,18 +141,19 @@ impl HttpCache {
content: &[u8],
) -> Result<(), ErrBox> {
let cache_filename = self.location.join(url_to_filename(url));
let headers_filename = cache_filename.with_extension("headers.json");
// Create parent directory
let parent_filename = cache_filename
.parent()
.expect("Cache filename should have a parent dir");
fs::create_dir_all(parent_filename)?;
// Cache content
deno_fs::write_file(&cache_filename, content, 0o666)?;
let serialized_headers = serde_json::to_string(&headers_map)?;
// Cache headers
deno_fs::write_file(&headers_filename, serialized_headers, 0o666)?;
Ok(())

let metadata = Metadata {
url: url.to_string(),
headers: headers_map,
};
metadata.write(&cache_filename)
}
}

Expand Down
3 changes: 3 additions & 0 deletions cli/http_util.rs
View file
Expand Up @@ -79,6 +79,9 @@ fn resolve_url_from_location(base_url: &Url, location: &str) -> Url {
}
}

// TODO(ry) HTTP headers are not unique key, value pairs. There may be more than
// one header line with the same key. This should be changed to something like
// Vec<(String, String)>
pub type HeadersMap = HashMap<String, String>;

#[derive(Debug, PartialEq)]
Expand Down

0 comments on commit 0099c28

Please sign in to comment.