From dd1bffe91eec992c95616aa0321822c7c805128b Mon Sep 17 00:00:00 2001 From: Lucas Fernandes Nogueira Date: Sat, 27 Jun 2020 15:40:46 -0300 Subject: [PATCH] refactor(api) rewrite readDir API (#722) --- .changes/fs-read-dir-api.md | 5 + cli/tauri.js/api-src/types/fs.ts | 8 +- tauri-api/Cargo.toml | 1 - tauri-api/src/dir.rs | 174 ++++++++++++++--------------- tauri-api/src/dir/utils.rs | 4 - tauri/src/endpoints/file_system.rs | 23 ++-- 6 files changed, 101 insertions(+), 114 deletions(-) create mode 100644 .changes/fs-read-dir-api.md delete mode 100644 tauri-api/src/dir/utils.rs diff --git a/.changes/fs-read-dir-api.md b/.changes/fs-read-dir-api.md new file mode 100644 index 000000000..7f369961b --- /dev/null +++ b/.changes/fs-read-dir-api.md @@ -0,0 +1,5 @@ +--- +"tauri-api": minor +--- + +readDir API refactor. Now returns `path`, `name` and `children`. diff --git a/cli/tauri.js/api-src/types/fs.ts b/cli/tauri.js/api-src/types/fs.ts index a7ca9efe6..391015317 100644 --- a/cli/tauri.js/api-src/types/fs.ts +++ b/cli/tauri.js/api-src/types/fs.ts @@ -35,7 +35,9 @@ export interface FsBinaryFileOption { export interface FileEntry { path: string - // TODO why not camelCase ? - is_dir: boolean - name: string + // name of the directory/file + // can be null if the path terminates with `..` + name?: string + // children of this entry if it's a directory; null otherwise + children?: FileEntry[] } diff --git a/tauri-api/Cargo.toml b/tauri-api/Cargo.toml index 099de3fab..18c3b0bdf 100644 --- a/tauri-api/Cargo.toml +++ b/tauri-api/Cargo.toml @@ -18,7 +18,6 @@ serde = { version = "1.0", features = [ "derive" ] } serde_json = "1.0" serde_repr = "0.1" dirs = "3.0.0" -ignore = "0.4.16" zip = "0.5.6" semver = "0.10" tempfile = "3" diff --git a/tauri-api/src/dir.rs b/tauri-api/src/dir.rs index ec448f62a..0358d5424 100644 --- a/tauri-api/src/dir.rs +++ b/tauri-api/src/dir.rs @@ -1,62 +1,48 @@ -mod utils; - -use ignore::Walk; use serde::Serialize; -use tempfile::{self, tempdir}; - -use utils::get_dir_name_from_path; - use std::fs::{self, metadata}; +use std::path::{Path, PathBuf}; +use tempfile::{self, tempdir}; #[derive(Debug, Serialize)] pub struct DiskEntry { - pub path: String, - pub is_dir: bool, - pub name: String, + pub path: PathBuf, + pub name: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub children: Option>, } -fn is_dir(file_name: String) -> crate::Result { - match metadata(file_name) { - Ok(md) => Result::Ok(md.is_dir()), - Err(err) => Result::Err(err.into()), - } +pub fn is_dir>(path: P) -> crate::Result { + metadata(path).map(|md| md.is_dir()).map_err(|e| e.into()) } -pub fn walk_dir(path_copy: String) -> crate::Result> { - println!("Trying to walk: {}", path_copy.as_str()); +pub fn read_dir>(path: P, recursive: bool) -> crate::Result> { let mut files_and_dirs: Vec = vec![]; - for result in Walk::new(path_copy) { - if let Ok(entry) = result { - let display_value = entry.path().display(); - let _dir_name = display_value.to_string(); + for entry in fs::read_dir(path)? { + let path = entry?.path(); + let path_as_string = path.display().to_string(); - if let Ok(flag) = is_dir(display_value.to_string()) { - files_and_dirs.push(DiskEntry { - path: display_value.to_string(), - is_dir: flag, - name: display_value.to_string(), - }); - } + if let Ok(flag) = is_dir(&path_as_string) { + files_and_dirs.push(DiskEntry { + path: path.clone(), + children: if flag { + Some(if recursive { + read_dir(&path_as_string, true)? + } else { + vec![] + }) + } else { + None + }, + name: path + .file_name() + .map(|name| name.to_string_lossy()) + .map(|name| name.to_string()), + }); } } Result::Ok(files_and_dirs) } -pub fn list_dir_contents(dir_path: String) -> crate::Result> { - let paths = fs::read_dir(dir_path)?; - let mut dirs: Vec = vec![]; - for path in paths { - let dir_path = path.expect("dirpath error").path(); - let _dir_name = dir_path.display(); - dirs.push(DiskEntry { - path: format!("{}", _dir_name), - is_dir: true, - name: get_dir_name_from_path(_dir_name.to_string()), - }); - } - Ok(dirs) -} - pub fn with_temp_dir ()>(callback: F) -> crate::Result<()> { let dir = tempdir()?; callback(&dir); @@ -68,30 +54,41 @@ pub fn with_temp_dir ()>(callback: F) -> crate: mod test { use super::*; use quickcheck_macros::quickcheck; + use std::ffi::OsStr; + use std::path::PathBuf; use totems::assert_ok; // check is dir function by passing in arbitrary strings #[quickcheck] fn qc_is_dir(f: String) -> bool { - // is the string runs through is_dir and comes out as an OK result then it must be a DIR. + // if the string runs through is_dir and comes out as an OK result then it must be a DIR. if let Ok(_) = is_dir(f.clone()) { - std::path::PathBuf::from(f).exists() + PathBuf::from(f).is_dir() } else { true } } + fn name_from_path(path: PathBuf) -> Option { + path + .file_name() + .map(|name| name.to_string_lossy()) + .map(|name| name.to_string()) + } + #[test] - // check the walk_dir function - fn check_walk_dir() { + // check the read_dir function with recursive = true + fn check_read_dir_recursively() { // define a relative directory string test/ - let dir = String::from("test/"); + let dir = PathBuf::from("test/"); // add the files to this directory - let file_one = format!("{}test.txt", &dir).to_string(); - let file_two = format!("{}test_binary", &dir).to_string(); + let mut file_one = dir.clone(); + file_one.push("test.txt"); + let mut file_two = dir.clone(); + file_two.push("test_binary"); // call walk_dir on the directory - let res = walk_dir(dir.clone()); + let res = read_dir(dir.clone(), true); // assert that the result is Ok() assert_ok!(&res); @@ -99,52 +96,45 @@ mod test { // destruct the OK into a vector of DiskEntry Structs if let Ok(vec) = res { // assert that the vector length is only 3 - assert_eq!(vec.len(), 3); + assert_eq!(vec.len(), 2); // get the first DiskEntry let first = &vec[0]; // get the second DiskEntry let second = &vec[1]; - // get the third DiskEntry - let third = &vec[2]; - // check the fields for the first DiskEntry - assert_eq!(first.path, dir); - assert_eq!(first.is_dir, true); - assert_eq!(first.name, dir); - - if second.path.contains(".txt") { - // check the fields for the second DiskEntry - assert_eq!(second.path, file_one); - assert_eq!(second.is_dir, false); - assert_eq!(second.name, file_one); + if first.path.extension() == Some(OsStr::new("txt")) { + // check the fields for the first DiskEntry + assert_eq!(first.path, file_one); + assert_eq!(first.children.is_some(), false); + assert_eq!(first.name, name_from_path(file_one)); // check the fields for the third DiskEntry - assert_eq!(third.path, file_two); - assert_eq!(third.is_dir, false); - assert_eq!(third.name, file_two); + assert_eq!(second.path, file_two); + assert_eq!(second.children.is_some(), false); + assert_eq!(second.name, name_from_path(file_two)); } else { // check the fields for the second DiskEntry - assert_eq!(second.path, file_two); - assert_eq!(second.is_dir, false); - assert_eq!(second.name, file_two); + assert_eq!(first.path, file_two); + assert_eq!(first.children.is_some(), false); + assert_eq!(first.name, name_from_path(file_two)); // check the fields for the third DiskEntry - assert_eq!(third.path, file_one); - assert_eq!(third.is_dir, false); - assert_eq!(third.name, file_one); + assert_eq!(second.path, file_one); + assert_eq!(second.children.is_some(), false); + assert_eq!(second.name, name_from_path(file_one)); } } } #[test] - // check the list_dir_contents function - fn check_list_dir_contents() { - // define a relative directory string test/ - let dir = String::from("test/"); + // check the read_dir function with recursive = false + fn check_read_dir() { + // define a relative directory test/ + let dir = PathBuf::from("test/"); - // call list_dir_contents on the dir string - let res = list_dir_contents(dir); + // call list_dir_contents on the dir + let res = read_dir(dir, false); // assert that the result is Ok() assert_ok!(&res); @@ -158,26 +148,26 @@ mod test { let first = &vec[0]; let second = &vec[1]; - if first.path.contains(".txt") { + if first.path.extension() == Some(OsStr::new("txt")) { // check the fields for the first DiskEntry - assert_eq!(first.path, "test/test.txt".to_string()); - assert_eq!(first.is_dir, true); - assert_eq!(first.name, "test.txt".to_string()); + assert_eq!(first.path, PathBuf::from("test/test.txt")); + assert_eq!(first.children.is_some(), false); + assert_eq!(first.name, Some("test.txt".to_string())); // check the fields for the second DiskEntry - assert_eq!(second.path, "test/test_binary".to_string()); - assert_eq!(second.is_dir, true); - assert_eq!(second.name, "test_binary".to_string()); + assert_eq!(second.path, PathBuf::from("test/test_binary")); + assert_eq!(second.children.is_some(), false); + assert_eq!(second.name, Some("test_binary".to_string())); } else { // check the fields for the first DiskEntry - assert_eq!(second.path, "test/test.txt".to_string()); - assert_eq!(second.is_dir, true); - assert_eq!(second.name, "test.txt".to_string()); + assert_eq!(second.path, PathBuf::from("test/test.txt")); + assert_eq!(second.children.is_some(), false); + assert_eq!(second.name, Some("test.txt".to_string())); // check the fields for the second DiskEntry - assert_eq!(first.path, "test/test_binary".to_string()); - assert_eq!(first.is_dir, true); - assert_eq!(first.name, "test_binary".to_string()); + assert_eq!(first.path, PathBuf::from("test/test_binary")); + assert_eq!(first.children.is_some(), false); + assert_eq!(first.name, Some("test_binary".to_string())); } } } diff --git a/tauri-api/src/dir/utils.rs b/tauri-api/src/dir/utils.rs deleted file mode 100644 index 859569e85..000000000 --- a/tauri-api/src/dir/utils.rs +++ /dev/null @@ -1,4 +0,0 @@ -pub fn get_dir_name_from_path(path: String) -> String { - let path_collect: Vec<&str> = path.split('/').collect(); - path_collect[path_collect.len() - 1].to_string() -} diff --git a/tauri/src/endpoints/file_system.rs b/tauri/src/endpoints/file_system.rs index 0622f8409..8e29ef048 100644 --- a/tauri/src/endpoints/file_system.rs +++ b/tauri/src/endpoints/file_system.rs @@ -26,13 +26,8 @@ pub fn read_dir( } else { (false, None) }; - if recursive { - dir::walk_dir(resolve_path(path, dir)?) - .and_then(|f| serde_json::to_string(&f).map_err(|err| err.into())) - } else { - dir::list_dir_contents(resolve_path(path, dir)?) - .and_then(|f| serde_json::to_string(&f).map_err(|err| err.into())) - } + dir::read_dir(resolve_path(path, dir)?, recursive) + .and_then(|f| serde_json::to_string(&f).map_err(|err| err.into())) }, callback, error, @@ -216,14 +211,14 @@ pub fn write_binary_file( base64::decode(contents) .map_err(|e| e.into()) .and_then(|c| { - File::create(resolve_path(file, options.and_then(|o| o.dir))?) - .map_err(|e| e.into()) - .and_then(|mut f| { - f.write_all(&c) - .map_err(|err| err.into()) - .map(|_| "".to_string()) + File::create(resolve_path(file, options.and_then(|o| o.dir))?) + .map_err(|e| e.into()) + .and_then(|mut f| { + f.write_all(&c) + .map_err(|err| err.into()) + .map(|_| "".to_string()) + }) }) - }) }, callback, error,