Skip to content

Commit

Permalink
refactor: use utf8 strings as mem cache keys (#222)
Browse files Browse the repository at this point in the history
* fix: clippy

* refactor: use utf8 strings as mem cache keys

* refactor: use cleaned_entry_path directly
  • Loading branch information
KSXGitHub authored Nov 28, 2023
1 parent d852b52 commit 5dcb687
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 18 deletions.
3 changes: 1 addition & 2 deletions crates/package-manager/src/create_cas_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use pacquet_npmrc::PackageImportMethod;
use rayon::prelude::*;
use std::{
collections::HashMap,
ffi::OsString,
path::{Path, PathBuf},
};

Expand All @@ -22,7 +21,7 @@ pub enum CreateCasFilesError {
pub fn create_cas_files(
import_method: PackageImportMethod,
dir_path: &Path,
cas_paths: &HashMap<OsString, PathBuf>,
cas_paths: &HashMap<String, PathBuf>,
) -> Result<(), CreateCasFilesError> {
assert_eq!(
import_method,
Expand Down
3 changes: 1 addition & 2 deletions crates/package-manager/src/create_virtual_dir_by_snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use pacquet_lockfile::{DependencyPath, PackageSnapshot};
use pacquet_npmrc::PackageImportMethod;
use std::{
collections::HashMap,
ffi::OsString,
fs, io,
path::{Path, PathBuf},
};
Expand All @@ -14,7 +13,7 @@ use std::{
#[must_use]
pub struct CreateVirtualDirBySnapshot<'a> {
pub virtual_store_dir: &'a Path,
pub cas_paths: &'a HashMap<OsString, PathBuf>,
pub cas_paths: &'a HashMap<String, PathBuf>,
pub import_method: PackageImportMethod,
pub dependency_path: &'a DependencyPath,
pub package_snapshot: &'a PackageSnapshot,
Expand Down
27 changes: 13 additions & 14 deletions crates/tarball/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::{
collections::HashMap,
ffi::OsString,
io::{Cursor, Read},
path::PathBuf,
sync::Arc,
Expand Down Expand Up @@ -77,7 +76,7 @@ pub enum CacheValue {
/// The package is being processed.
InProgress(Arc<Notify>),
/// The package is saved.
Available(Arc<HashMap<OsString, PathBuf>>),
Available(Arc<HashMap<String, PathBuf>>),
}

/// Internal in-memory cache of tarballs.
Expand Down Expand Up @@ -115,7 +114,7 @@ impl<'a> DownloadTarballToStore<'a> {
pub async fn run_with_mem_cache(
self,
mem_cache: &'a MemCache,
) -> Result<Arc<HashMap<OsString, PathBuf>>, TarballError> {
) -> Result<Arc<HashMap<String, PathBuf>>, TarballError> {
let &DownloadTarballToStore { package_url, .. } = &self;

// QUESTION: I see no copying from existing store_dir, is there such mechanism?
Expand Down Expand Up @@ -154,7 +153,7 @@ impl<'a> DownloadTarballToStore<'a> {
}

/// Execute the subroutine without an in-memory cache.
pub async fn run_without_mem_cache(&self) -> Result<HashMap<OsString, PathBuf>, TarballError> {
pub async fn run_without_mem_cache(&self) -> Result<HashMap<String, PathBuf>, TarballError> {
let &DownloadTarballToStore {
http_client,
store_dir,
Expand Down Expand Up @@ -208,7 +207,7 @@ impl<'a> DownloadTarballToStore<'a> {
.filter(|entry| !entry.as_ref().unwrap().header().entry_type().is_dir());

let ((_, Some(capacity)) | (capacity, None)) = entries.size_hint();
let mut cas_paths = HashMap::<OsString, PathBuf>::with_capacity(capacity);
let mut cas_paths = HashMap::<String, PathBuf>::with_capacity(capacity);
let mut pkg_files_idx = PackageFilesIndex { files: HashMap::with_capacity(capacity) };

for entry in entries {
Expand All @@ -222,18 +221,18 @@ impl<'a> DownloadTarballToStore<'a> {
entry.read_to_end(&mut buffer).unwrap();

let entry_path = entry.path().unwrap();
let cleaned_entry_path =
entry_path.components().skip(1).collect::<PathBuf>().into_os_string();
let cleaned_entry_path = entry_path
.components()
.skip(1)
.collect::<PathBuf>()
.into_os_string()
.into_string()
.expect("entry path must be valid UTF-8");
let (file_path, file_hash) = store_dir
.write_cas_file(&buffer, file_is_executable)
.map_err(TarballError::WriteCasFile)?;

let tarball_index_key = cleaned_entry_path
.to_str()
.expect("entry path must be valid UTF-8") // TODO: propagate this error, provide more information
.to_string(); // TODO: convert cleaned_entry_path to String too.

if let Some(previous) = cas_paths.insert(cleaned_entry_path, file_path) {
if let Some(previous) = cas_paths.insert(cleaned_entry_path.clone(), file_path) {
tracing::warn!(?previous, "Duplication detected. Old entry has been ejected");
}

Expand All @@ -247,7 +246,7 @@ impl<'a> DownloadTarballToStore<'a> {
size: file_size,
};

if let Some(previous) = pkg_files_idx.files.insert(tarball_index_key, file_attrs) {
if let Some(previous) = pkg_files_idx.files.insert(cleaned_entry_path, file_attrs) {
tracing::warn!(?previous, "Duplication detected. Old entry has been ejected");
}
}
Expand Down

0 comments on commit 5dcb687

Please sign in to comment.