Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Commit

Permalink
refactor: remove bucket prefix directory earlier
Browse files Browse the repository at this point in the history
* extract call to strip bucket directory before generating url_safe_path
* update comments about key path fingerprinting format
* rename "generate_url_safe_path_and_hash" -> "generate_path_and_key"
* add url_safe_hash regression test
* add regression test to ensure removing bucket directory
  • Loading branch information
ashleymichal committed Sep 25, 2019
1 parent fbb42ed commit 6e2399d
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 33 deletions.
41 changes: 27 additions & 14 deletions src/commands/kv/bucket/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub fn directory_keys_values(
let b64_value = base64::encode(&value);

let (url_safe_path, key) =
generate_url_safe_key_and_hash(path, directory, Some(b64_value.clone()))?;
generate_path_and_key(path, directory, Some(b64_value.clone()))?;

if verbose {
message::working(&format!("Parsing {}...", key.clone()));
Expand Down Expand Up @@ -66,7 +66,7 @@ fn directory_keys_only(directory: &Path) -> Result<Vec<String>, failure::Error>
// Need to base64 encode value
let b64_value = base64::encode(&value);

let (_, key) = generate_url_safe_key_and_hash(path, directory, Some(b64_value))?;
let (_, key) = generate_path_and_key(path, directory, Some(b64_value))?;

upload_vec.push(key);
}
Expand All @@ -75,10 +75,8 @@ fn directory_keys_only(directory: &Path) -> Result<Vec<String>, failure::Error>
}

// Courtesy of Steve Klabnik's PoC :) Used for bulk operations (write, delete)
fn generate_url_safe_path(path: &Path, directory: &Path) -> Result<String, failure::Error> {
let path = path.strip_prefix(directory).unwrap();

// next, we have to re-build the paths: if we're on Windows, we have paths with
fn generate_url_safe_path(path: &Path) -> Result<String, failure::Error> {
// first, we have to re-build the paths: if we're on Windows, we have paths with
// `\` as separators. But we want to use `/` as separators. Because that's how URLs
// work.
let mut path_with_forward_slash = OsString::new();
Expand All @@ -100,21 +98,26 @@ fn generate_url_safe_path(path: &Path, directory: &Path) -> Result<String, failu
Ok(path.to_string())
}

// Appends the SHA-256 hash of the path's file contents to the url-safe path of a file to
// Adds the SHA-256 hash of the path's file contents to the url-safe path of a file to
// generate a versioned key for the file and its contents. Returns the url-safe path prefix
// for the key, as well as the key with hash appended.
// e.g (sitemap.xml, sitemap.xml-ec717eb2131fdd4fff803b851d2aa5b1dc3e0af36bc3c8c40f2095c747e80d1e)
pub fn generate_url_safe_key_and_hash(
// e.g (sitemap.xml, sitemap.ec717eb2131fdd4fff803b851d2aa5b1dc3e0af36bc3c8c40f2095c747e80d1e.xml)
pub fn generate_path_and_key(
path: &Path,
directory: &Path,
value: Option<String>,
) -> Result<(String, String), failure::Error> {
let url_safe_path = generate_url_safe_path(path, directory)?;
// strip the bucket directory from both paths for ease of reference.
let relative_path = path.strip_prefix(directory).unwrap();

let url_safe_path = generate_url_safe_path(relative_path)?;

let path_with_hash = if let Some(value) = value {
let digest = get_digest(value)?;

generate_path_with_hash(path, digest)?.display().to_string()
generate_path_with_hash(relative_path, digest)?
.display()
.to_string()
} else {
url_safe_path.to_string()
};
Expand Down Expand Up @@ -192,12 +195,22 @@ mod tests {

#[test]
fn it_generates_a_url_safe_hash() {
let os_path = Path::new("./some_stuff/invalid file&name.chars");
let directory = Path::new("./");
let actual_url_safe_path = generate_url_safe_path(os_path, directory).unwrap();
let os_path = Path::new("some_stuff/invalid file&name.chars");
let actual_url_safe_path = generate_url_safe_path(os_path).unwrap();
// TODO: url-encode paths
let expected_url_safe_path = "some_stuff/invalid file&name.chars";

assert_eq!(actual_url_safe_path, expected_url_safe_path);
}

#[test]
fn it_removes_bucket_dir_prefix() {
let path = Path::new("./build/path/to/asset.ext");
let directory = Path::new("./build");
let value = Some("<h1>Hello World!</h1>".to_string());
let (path, key) = generate_path_and_key(path, directory, value).unwrap();

assert!(!path.contains("directory"));
assert!(!key.contains("directory"));
}
}
29 changes: 10 additions & 19 deletions src/commands/kv/bucket/upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,30 +141,21 @@ mod tests {

use cloudflare::endpoints::workerskv::write_bulk::KeyValuePair;

use crate::commands::kv::bucket::generate_url_safe_key_and_hash;
use crate::commands::kv::bucket::generate_path_and_key;
use crate::commands::kv::bucket::upload::filter_unchanged_remote_files;

#[test]
fn it_can_filter_preexisting_files() {
let (_, key_a_old) = generate_url_safe_key_and_hash(
Path::new("/a"),
Path::new("/"),
Some("old".to_string()),
)
.unwrap();
let (_, key_b_old) = generate_url_safe_key_and_hash(
Path::new("/b"),
Path::new("/"),
Some("old".to_string()),
)
.unwrap();
let (_, key_a_old) =
generate_path_and_key(Path::new("/a"), Path::new("/"), Some("old".to_string()))
.unwrap();
let (_, key_b_old) =
generate_path_and_key(Path::new("/b"), Path::new("/"), Some("old".to_string()))
.unwrap();
// Generate new key (using hash of new value) for b when to simulate its value being updated.
let (_, key_b_new) = generate_url_safe_key_and_hash(
Path::new("/b"),
Path::new("/"),
Some("new".to_string()),
)
.unwrap();
let (_, key_b_new) =
generate_path_and_key(Path::new("/b"), Path::new("/"), Some("new".to_string()))
.unwrap();

// Old values found on remote
let mut exclude_keys = HashSet::new();
Expand Down

0 comments on commit 6e2399d

Please sign in to comment.