Skip to content

Commit 24a60c9

Browse files
committed
feat: convert tarball to tarball_manager
1 parent 0e3c275 commit 24a60c9

File tree

2 files changed

+103
-80
lines changed

2 files changed

+103
-80
lines changed

crates/registry/src/lib.rs

+19-15
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use std::path::{Path, PathBuf};
77
use async_recursion::async_recursion;
88
use futures_util::future::join_all;
99
use pacquet_package_json::{DependencyGroup, PackageJson};
10-
use pacquet_tarball::{download_dependency, get_package_store_folder_name};
10+
use pacquet_tarball::{get_package_store_folder_name, TarballManager};
1111

1212
use crate::{error::RegistryError, http_client::HttpClient};
1313

@@ -16,6 +16,7 @@ pub struct RegistryManager {
1616
node_modules_path: PathBuf,
1717
store_path: PathBuf,
1818
package_json: Box<PackageJson>,
19+
tarball_manager: Box<TarballManager>,
1920
}
2021

2122
impl RegistryManager {
@@ -29,6 +30,7 @@ impl RegistryManager {
2930
node_modules_path: node_modules_path.into(),
3031
store_path: store_path.into(),
3132
package_json: Box::new(PackageJson::create_if_needed(&package_json_path.into())?),
33+
tarball_manager: Box::new(TarballManager::new()),
3234
})
3335
}
3436

@@ -51,13 +53,14 @@ impl RegistryManager {
5153
let package_node_modules_path =
5254
self.store_path.join(dependency_store_folder_name).join("node_modules");
5355

54-
download_dependency(
55-
name,
56-
latest_version.get_tarball_url(),
57-
&package_node_modules_path.join(name),
58-
&self.node_modules_path.join(name),
59-
)
60-
.await?;
56+
self.tarball_manager
57+
.download_dependency(
58+
name,
59+
latest_version.get_tarball_url(),
60+
&package_node_modules_path.join(name),
61+
&self.node_modules_path.join(name),
62+
)
63+
.await?;
6164

6265
if let Some(dependencies) = latest_version.dependencies.as_ref() {
6366
join_all(
@@ -97,13 +100,14 @@ impl RegistryManager {
97100

98101
// TODO: We shouldn't call this function for multiple same packages.
99102
// There needs to be some sort of thread safety.
100-
download_dependency(
101-
name,
102-
package_version.get_tarball_url(),
103-
&package_node_modules_path.join(name),
104-
&symlink_path.join(&package.name),
105-
)
106-
.await?;
103+
self.tarball_manager
104+
.download_dependency(
105+
name,
106+
package_version.get_tarball_url(),
107+
&package_node_modules_path.join(name),
108+
&symlink_path.join(&package.name),
109+
)
110+
.await?;
107111

108112
if let Some(dependencies) = package_version.dependencies.as_ref() {
109113
join_all(

crates/tarball/src/lib.rs

+84-65
Original file line numberDiff line numberDiff line change
@@ -24,78 +24,94 @@ pub enum TarballError {
2424
Io(#[from] std::io::Error),
2525
}
2626

27-
pub fn get_package_store_folder_name(input: &str, version: &str) -> String {
28-
format!("{0}@{1}", input.replace('/', "+"), version)
29-
}
27+
#[derive(Debug)]
28+
pub struct TarballManager;
3029

31-
#[instrument]
32-
pub async fn download_tarball(url: &str, tarball_path: &Path) -> Result<(), TarballError> {
33-
let mut stream = reqwest::get(url).await?.bytes_stream();
34-
let mut file = File::create(tarball_path)?;
35-
event!(Level::DEBUG, "downloading tarball to {}", tarball_path.display());
36-
37-
while let Some(item) = stream.next().await {
38-
let chunk = item.map_err(TarballError::Network)?;
39-
file.write_all(&chunk)?;
30+
impl Default for TarballManager {
31+
fn default() -> Self {
32+
Self::new()
4033
}
41-
42-
Ok(())
4334
}
4435

45-
#[instrument]
46-
pub fn extract_tarball(tarball_path: &Path, extract_path: &Path) -> Result<(), TarballError> {
47-
let unpack_path = env::temp_dir().join(Uuid::new_v4().to_string());
48-
event!(Level::DEBUG, "unpacking tarball to {}", unpack_path.display());
49-
let tar_gz = File::open(tarball_path)?;
50-
let tar = GzDecoder::new(tar_gz);
51-
let mut archive = Archive::new(tar);
52-
archive.unpack(&unpack_path)?;
53-
fs::remove_file(tarball_path)?;
54-
fs::create_dir_all(extract_path)?;
55-
// Check if extract_path is empty or not.
56-
// This is due by a race condition causing 2 different threads to download and
57-
// extract to the same folder. Likely be fixed with mutex locks.
58-
// TODO: Remove this when we have some sort of thread safety
59-
if extract_path.read_dir()?.next().is_none() {
60-
fs::rename(unpack_path.join("package"), extract_path)?;
36+
impl TarballManager {
37+
pub fn new() -> Self {
38+
TarballManager {}
6139
}
62-
fs::remove_dir_all(&unpack_path)?;
63-
Ok(())
64-
}
6540

66-
#[instrument]
67-
pub async fn download_dependency(
68-
name: &str,
69-
url: &str,
70-
save_path: &Path,
71-
symlink_to: &Path,
72-
) -> Result<(), TarballError> {
73-
// If name contains `/` such as @fastify/error, we need to make sure that @fastify folder
74-
// exists before we symlink to that directory.
75-
if name.contains('/') {
76-
fs::create_dir_all(symlink_to.parent().unwrap())?;
41+
#[instrument]
42+
async fn download(&self, url: &str, tarball_path: &Path) -> Result<(), TarballError> {
43+
let mut stream = reqwest::get(url).await?.bytes_stream();
44+
let mut file = File::create(tarball_path)?;
45+
event!(Level::DEBUG, "downloading tarball to {}", tarball_path.display());
46+
47+
while let Some(item) = stream.next().await {
48+
let chunk = item.map_err(TarballError::Network)?;
49+
file.write_all(&chunk)?;
50+
}
51+
52+
Ok(())
7753
}
7854

79-
// Do not try to install dependency if this version already exists in package.json
80-
if save_path.exists() {
81-
if !symlink_to.is_symlink() {
82-
symlink_dir(&save_path.to_path_buf(), &symlink_to.to_path_buf())?;
55+
#[instrument]
56+
fn extract(&self, tarball_path: &Path, extract_path: &Path) -> Result<(), TarballError> {
57+
let unpack_path = env::temp_dir().join(Uuid::new_v4().to_string());
58+
event!(Level::DEBUG, "unpacking tarball to {}", unpack_path.display());
59+
let tar_gz = File::open(tarball_path)?;
60+
let tar = GzDecoder::new(tar_gz);
61+
let mut archive = Archive::new(tar);
62+
archive.unpack(&unpack_path)?;
63+
fs::remove_file(tarball_path)?;
64+
fs::create_dir_all(extract_path)?;
65+
// Check if extract_path is empty or not.
66+
// This is due by a race condition causing 2 different threads to download and
67+
// extract to the same folder. Likely be fixed with mutex locks.
68+
// TODO: Remove this when we have some sort of thread safety
69+
if extract_path.read_dir()?.next().is_none() {
70+
fs::rename(unpack_path.join("package"), extract_path)?;
8371
}
84-
return Ok(());
72+
fs::remove_dir_all(&unpack_path)?;
73+
Ok(())
8574
}
8675

87-
let tarball_path = env::temp_dir().join(Uuid::new_v4().to_string());
88-
download_tarball(url, &tarball_path).await?;
76+
#[instrument]
77+
pub async fn download_dependency(
78+
&self,
79+
name: &str,
80+
url: &str,
81+
save_path: &Path,
82+
symlink_to: &Path,
83+
) -> Result<(), TarballError> {
84+
// If name contains `/` such as @fastify/error, we need to make sure that @fastify folder
85+
// exists before we symlink to that directory.
86+
if name.contains('/') {
87+
fs::create_dir_all(symlink_to.parent().unwrap())?;
88+
}
89+
90+
// Do not try to install dependency if this version already exists in package.json
91+
if save_path.exists() {
92+
if !symlink_to.is_symlink() {
93+
symlink_dir(&save_path.to_path_buf(), &symlink_to.to_path_buf())?;
94+
}
95+
return Ok(());
96+
}
8997

90-
extract_tarball(&tarball_path, save_path)?;
98+
let tarball_path = env::temp_dir().join(Uuid::new_v4().to_string());
99+
self.download(url, &tarball_path).await?;
91100

92-
// TODO: Currently symlink paths are absolute paths.
93-
// If you move the root folder to a different path, all symlinks will be broken.
94-
if !symlink_to.is_symlink() {
95-
symlink_dir(&save_path.to_path_buf(), &symlink_to.to_path_buf())?;
101+
self.extract(&tarball_path, save_path)?;
102+
103+
// TODO: Currently symlink paths are absolute paths.
104+
// If you move the root folder to a different path, all symlinks will be broken.
105+
if !symlink_to.is_symlink() {
106+
symlink_dir(&save_path.to_path_buf(), &symlink_to.to_path_buf())?;
107+
}
108+
109+
Ok(())
96110
}
111+
}
97112

98-
Ok(())
113+
pub fn get_package_store_folder_name(input: &str, version: &str) -> String {
114+
format!("{0}@{1}", input.replace('/', "+"), version)
99115
}
100116

101117
#[cfg(test)]
@@ -132,14 +148,17 @@ mod tests {
132148
let save_path = store_path.join("@fastify+error@3.3.0");
133149
let symlink_path = node_modules_path.join("@fastify/error");
134150

135-
download_dependency(
136-
"@fastify/error",
137-
"https://registry.npmjs.org/@fastify/error/-/error-3.3.0.tgz",
138-
&save_path.to_path_buf(),
139-
&symlink_path.to_path_buf(),
140-
)
141-
.await
142-
.unwrap();
151+
let manager = TarballManager::new();
152+
153+
manager
154+
.download_dependency(
155+
"@fastify/error",
156+
"https://registry.npmjs.org/@fastify/error/-/error-3.3.0.tgz",
157+
&save_path.to_path_buf(),
158+
&symlink_path.to_path_buf(),
159+
)
160+
.await
161+
.unwrap();
143162

144163
// Validate if we delete the tar.gz file
145164
assert!(!store_path.join("@fastify+error@3.3.0.tar.gz").exists());

0 commit comments

Comments
 (0)