Skip to content

Commit cbfa5be

Browse files
arcanisarcanis
committed
Improves APIs
1 parent 98d5992 commit cbfa5be

File tree

5 files changed

+81
-70
lines changed

5 files changed

+81
-70
lines changed

‎README.md

+17-25
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@ fn example() {
3232
);
3333

3434
match resolution {
35-
Ok(Resolution::Package(path, subpath)) => {
35+
Ok(Resolution::Resolved(path, subpath)) => {
3636
// path = "/path/to/lodash.zip"
3737
// subpath = "cloneDeep"
3838
},
39-
Ok(Resolution::Specifier(specifier)) => {
39+
Ok(Resolution::Skipped) => {
4040
// This is returned when the PnP resolver decides that it shouldn't
4141
// handle the resolution for this particular specifier. In that case,
4242
// the specifier should be forwarded to the default resolver.
@@ -64,20 +64,15 @@ use pnp::fs::{VPath, open_zip_via_read};
6464

6565
fn read_file(p: PathBuf) -> std::io::Result<String> {
6666
match VPath::from(&p).unwrap() {
67+
// The path was virtual and stored within a zip file; we need to read from the zip file
68+
// Note that this opens the zip file every time, which is expensive; we'll see how to optimize that
69+
VPath::Zip(info) => {
70+
open_zip_via_read(info.physical_base_path()).unwrap().read_to_string(&zip_path)
71+
},
72+
73+
// The path was virtual but not a zip file; we just need to read from the provided location
6774
VPath::Virtual(info) => {
68-
let physical_path
69-
= info.physical_base_path();
70-
71-
match &info.zip_path {
72-
// The path was virtual and stored within a zip file; we need to read from the zip file
73-
// Note that this opens the zip file every time, which is expensive; we'll see how to optimize that
74-
Some(zip_path) => open_zip_via_read(&physical_path)
75-
.unwrap()
76-
.read_to_string(&zip_path),
77-
78-
// The path was virtual but not a zip file; we just need to read from the provided location
79-
None => std::fs::read_to_string(info.physical_base_path())
80-
}
75+
std::fs::read_to_string(info.physical_base_path())
8176
},
8277

8378
// Nothing special to do, it's a regular path
@@ -102,17 +97,14 @@ const ZIP_CACHE: Lazy<LruZipCache<Vec<u8>>> = Lazy::new(|| {
10297

10398
fn read_file(p: PathBuf) -> std::io::Result<String> {
10499
match VPath::from(&p).unwrap() {
105-
VPath::Virtual(info) => {
106-
let physical_path
107-
= info.physical_base_path();
108-
109-
match &info.zip_path {
110-
// The path was virtual and stored within a zip file; we need to read from the zip file
111-
Some(zip_path) => ZIP_CACHE.read_to_string(info.physical_base_path()),
100+
// The path was virtual and stored within a zip file; we need to read from the zip file
101+
VPath::Zip(info) => {
102+
ZIP_CACHE.read_to_string(info.physical_base_path(), &zip_path)
103+
},
112104

113-
// The path was virtual but not a zip file; we just need to read from the provided location
114-
None => std::fs::read_to_string(info.physical_base_path())
115-
}
105+
// The path was virtual but not a zip file; we just need to read from the provided location
106+
VPath::Virtual(info) => {
107+
std::fs::read_to_string(info.physical_base_path())
116108
},
117109

118110
// Nothing special to do, it's a regular path

‎src/fs.rs

+44-29
Original file line numberDiff line numberDiff line change
@@ -5,27 +5,51 @@ use std::{path::{Path, PathBuf}, str::Utf8Error};
55

66
use crate::zip::Zip;
77

8+
#[derive(Clone, Debug, PartialEq, Eq)]
9+
pub enum FileType {
10+
File,
11+
Directory,
12+
}
13+
814
#[derive(Clone, Debug, Deserialize, PartialEq)]
915
#[serde(rename_all = "camelCase")]
10-
pub struct VPathInfo {
16+
pub struct ZipInfo {
1117
pub base_path: String,
1218
pub virtual_segments: Option<(String, String)>,
13-
pub zip_path: Option<String>,
19+
pub zip_path: String,
20+
}
21+
22+
#[derive(Clone, Debug, Deserialize, PartialEq)]
23+
#[serde(rename_all = "camelCase")]
24+
pub struct VirtualInfo {
25+
pub base_path: String,
26+
pub virtual_segments: (String, String),
1427
}
1528

16-
impl VPathInfo {
17-
pub fn physical_base_path(&self) -> PathBuf {
29+
pub trait VPathInfo {
30+
fn physical_base_path(&self) -> PathBuf;
31+
}
32+
33+
impl VPathInfo for ZipInfo {
34+
fn physical_base_path(&self) -> PathBuf {
1835
match &self.virtual_segments {
1936
None => PathBuf::from(&self.base_path),
2037
Some(segments) => PathBuf::from(&self.base_path).join(&segments.1),
2138
}
2239
}
2340
}
2441

42+
impl VPathInfo for VirtualInfo {
43+
fn physical_base_path(&self) -> PathBuf {
44+
PathBuf::from(&self.base_path).join(&self.virtual_segments.1)
45+
}
46+
}
47+
2548
#[derive(Clone, Debug, Deserialize, PartialEq)]
2649
#[serde(untagged)]
2750
pub enum VPath {
28-
Virtual(VPathInfo),
51+
Zip(ZipInfo),
52+
Virtual(VirtualInfo),
2953
Native(PathBuf),
3054
}
3155

@@ -107,11 +131,7 @@ pub trait ZipCache<Storage>
107131
where Storage: AsRef<[u8]> + Send + Sync {
108132
fn act<T, P: AsRef<Path>, F : FnOnce(&Zip<Storage>) -> T>(&self, p: P, cb: F) -> Result<T, std::io::Error>;
109133

110-
fn canonicalize<P: AsRef<Path>, S: AsRef<str>>(&self, zip_path: P, sub: S) -> Result<PathBuf, std::io::Error>;
111-
112-
fn is_dir<P: AsRef<Path>, S: AsRef<str>>(&self, zip_path: P, sub: S) -> bool;
113-
fn is_file<P: AsRef<Path>, S: AsRef<str>>(&self, zip_path: P, sub: S) -> bool;
114-
134+
fn file_type<P: AsRef<Path>, S: AsRef<str>>(&self, zip_path: P, sub: S) -> Result<FileType, std::io::Error>;
115135
fn read<P: AsRef<Path>, S: AsRef<str>>(&self, zip_path: P, sub: S) -> Result<Vec<u8>, std::io::Error>;
116136
fn read_to_string<P: AsRef<Path>, S: AsRef<str>>(&self, zip_path: P, sub: S) -> Result<String, std::io::Error>;
117137
}
@@ -143,18 +163,8 @@ where Storage: AsRef<[u8]> + Send + Sync {
143163
Ok(cb(zip.value()))
144164
}
145165

146-
fn canonicalize<P: AsRef<Path>, S: AsRef<str>>(&self, zip_path: P, sub: S) -> Result<PathBuf, std::io::Error> {
147-
let res = std::fs::canonicalize(zip_path)?;
148-
149-
Ok(res.join(sub.as_ref()))
150-
}
151-
152-
fn is_dir<P: AsRef<Path>, S: AsRef<str>>(&self, zip_path: P, p: S) -> bool {
153-
self.act(zip_path, |zip| zip.is_dir(p.as_ref())).unwrap_or(false)
154-
}
155-
156-
fn is_file<P: AsRef<Path>, S: AsRef<str>>(&self, zip_path: P, p: S) -> bool {
157-
self.act(zip_path, |zip| zip.is_file(p.as_ref())).unwrap_or(false)
166+
fn file_type<P: AsRef<Path>, S: AsRef<str>>(&self, zip_path: P, p: S) -> Result<FileType, std::io::Error> {
167+
self.act(zip_path, |zip| zip.file_type(p.as_ref()))?
158168
}
159169

160170
fn read<P: AsRef<Path>, S: AsRef<str>>(&self, zip_path: P, p: S) -> Result<Vec<u8>, std::io::Error> {
@@ -261,13 +271,18 @@ fn vpath(p: &Path) -> std::io::Result<VPath> {
261271
return Ok(VPath::Native(PathBuf::from(p_str)));
262272
}
263273

264-
Ok(VPath::Virtual(VPathInfo {
265-
base_path: io_bytes_to_str(base_path_u8)?.to_string(),
266-
virtual_segments,
267-
zip_path: zip_path_u8.map(|data| {
268-
io_bytes_to_str(data).map(|str| str.to_string())
269-
}).transpose()?,
270-
}))
274+
if let Some(zip_path_u8) = zip_path_u8 {
275+
Ok(VPath::Zip(ZipInfo {
276+
base_path: io_bytes_to_str(base_path_u8)?.to_string(),
277+
virtual_segments,
278+
zip_path: io_bytes_to_str(zip_path_u8)?.to_string(),
279+
}))
280+
} else {
281+
Ok(VPath::Virtual(VirtualInfo {
282+
base_path: io_bytes_to_str(base_path_u8)?.to_string(),
283+
virtual_segments: virtual_segments.unwrap(),
284+
}))
285+
}
271286
}
272287

273288
#[cfg(test)]

‎src/lib.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ impl ToString for Error {
7373

7474
#[derive(Debug)]
7575
pub enum Resolution {
76-
Specifier(String),
77-
Package(PathBuf, Option<String>),
76+
Resolved(PathBuf, Option<String>),
77+
Skipped,
7878
}
7979

8080
pub struct ResolutionHost {
@@ -409,7 +409,7 @@ pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(manifest: &Manifest,
409409
PackageDependency::Alias(name, reference) => get_package(manifest, &PackageLocator { name, reference }),
410410
}?;
411411

412-
Ok(Resolution::Package(dependency_pkg.package_location.clone(), module_path))
412+
Ok(Resolution::Resolved(dependency_pkg.package_location.clone(), module_path))
413413
} else {
414414
let broken_ancestors = find_broken_peer_dependencies(&specifier, parent_locator);
415415

@@ -450,15 +450,15 @@ pub fn resolve_to_unqualified_via_manifest<P: AsRef<Path>>(manifest: &Manifest,
450450
})
451451
}
452452
} else {
453-
Ok(Resolution::Specifier(specifier.to_string()))
453+
Ok(Resolution::Skipped)
454454
}
455455
}
456456

457457
pub fn resolve_to_unqualified<P: AsRef<Path>>(specifier: &str, parent: P, config: &ResolutionConfig) -> Result<Resolution, Error> {
458458
if let Some(manifest) = (config.host.find_pnp_manifest)(parent.as_ref())? {
459459
resolve_to_unqualified_via_manifest(&manifest, specifier, &parent)
460460
} else {
461-
Ok(Resolution::Specifier(specifier.to_string()))
461+
Ok(Resolution::Skipped)
462462
}
463463
}
464464

‎src/lib_tests.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,11 @@ mod tests {
4545
);
4646

4747
match resolution {
48-
Ok(Resolution::Package(_path, _subpath)) => {
48+
Ok(Resolution::Resolved(_path, _subpath)) => {
4949
// path = "/path/to/lodash.zip"
5050
// subpath = "cloneDeep"
5151
},
52-
Ok(Resolution::Specifier(_specifier)) => {
52+
Ok(Resolution::Skipped) => {
5353
// This is returned when the PnP resolver decides that it shouldn't
5454
// handle the resolution for this particular specifier. In that case,
5555
// the specifier should be forwarded to the default resolver.
@@ -105,11 +105,11 @@ mod tests {
105105
let resolution = resolve_to_unqualified(specifier, parent, &config);
106106

107107
match resolution {
108-
Ok(Resolution::Package(path, _subpath)) => {
108+
Ok(Resolution::Resolved(path, _subpath)) => {
109109
assert_eq!(path.to_string_lossy(), test.expected, "{}", test.it);
110110
},
111-
Ok(Resolution::Specifier(specifier)) => {
112-
assert_eq!(specifier, test.expected, "{}", test.it);
111+
Ok(Resolution::Skipped) => {
112+
assert_eq!(specifier, &test.expected, "{}", test.it);
113113
},
114114
Err(err) => {
115115
assert_eq!(test.expected, "error!", "{}: {}", test.it, err.to_string());

‎src/zip.rs

+10-6
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ use std::error::Error;
44
use byteorder::{ReadBytesExt, LittleEndian};
55
use std::io::Read;
66

7+
use crate::fs::FileType;
8+
79
#[derive(Debug)]
810
pub enum Compression {
911
Uncompressed,
@@ -52,12 +54,14 @@ where T : AsRef<[u8]> {
5254
Ok(zip)
5355
}
5456

55-
pub fn is_dir(&self, p: &str) -> bool {
56-
self.dirs.contains(p)
57-
}
58-
59-
pub fn is_file(&self, p: &str) -> bool {
60-
self.files.contains_key(p)
57+
pub fn file_type(&self, p: &str) -> Result<FileType, std::io::Error> {
58+
if self.dirs.contains(p) {
59+
Ok(FileType::Directory)
60+
} else if self.files.contains_key(p) {
61+
Ok(FileType::File)
62+
} else {
63+
Err(std::io::Error::from(std::io::ErrorKind::NotFound))
64+
}
6165
}
6266

6367
pub fn read(&self, p: &str) -> Result<Vec<u8>, std::io::Error> {

0 commit comments

Comments
 (0)