-
Notifications
You must be signed in to change notification settings - Fork 157
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat: CLI get command improvements (#331)
This focuses on creating tests for the `iroh get` CLI command. The Iroh API is also improved along the way. This required more work in the underlying API to make it more testable. What's tested here is everything *except* the actual IPFS behavior -- the focus is on the behavior of the CLI and API and its interactions with the filesystem. ## trycmd tests In `iroh/tests/cmd` you can see quite a few `.trycmd` files. These describes the command-line interaction. Stuff behind `$ is a command, there's a special `? failed` indicator if the command is considered to have failed, and the output of the command is shown. New are the `.out` and `.in` directories with the same names as the `.trycmd` files. These describe the filesystem before the command runs (may be missing), and the filesystem after the command has run. This way we can describe the effects of the `iroh get` command - directories and files are supposed to be created. We can also test failure scenarios where we refuse to overwrite a directory that already exists. #269 tracks various test cases. ## `get_stream` The `get` high level CLI method has been removed from the mockable `Api` trait (read on to see where it went). Instead, a more low level but still useful API method `get_stream` has been added to the `Api` trait. This gets a stream of relative paths and `OutType`, describing the directories and files you can create. The big difference with what was there before is that it returns relative paths and doesn't calculate final destination paths -- that's up to the user of the API. ## No `async_trait` macro for `Api` trait The interactions between the `Api` trait, `mockall` macro and `async_trait` were getting so hairy I couldn't figure out how to express things anymore once I wanted to add `get_stream`. For my sanity and also to learn better how this really works underneath, I've rewritten the `Api` trait to describe itself explicitly in terms of `(Local)BoxFuture` and `(Local)_BoxStream`. It's more verbose but functionally equivalent and I could express what I wanted. ## `ApiExt` trait The `ApiExt` trait is a trait that implements the high level `get` command. It puts everything together: it handles various error conditions, accesses the stream and then writes the stream to the filesystem. It's basically `iroh get`. The `ApiExt` trait is solely intended to contain default trait methods, and is automatically available when the `Api` contract is fulfilled (if you `use` it). Factored out `save_get_stream` from `getadd.rs` to be solely concerned with turning a stream into files and directories on the files system. That makes it possible to test its behavior in isolation. ## test fixture The `get` test fixture now mocks `get_stream` and returns a fake stream made from a `Vec`. This defines the actual stuff that the CLI writes to disk. ## relative_path Now depend on the [`relative-path` crate](https://crates.io/crates/relative-path) because what the stream returns are clearly relative paths, and we want to force the user to do something with them before being able to actually write stuff.
- Loading branch information
Showing
36 changed files
with
491 additions
and
192 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| use std::path::{Path, PathBuf}; | ||
|
|
||
| use crate::{Api, IpfsPath, OutType}; | ||
| use anyhow::{anyhow, Result}; | ||
| use async_trait::async_trait; | ||
| use futures::Stream; | ||
| use futures::StreamExt; | ||
| use relative_path::RelativePathBuf; | ||
|
|
||
| #[async_trait(?Send)] | ||
| pub trait ApiExt: Api { | ||
| /// High level get, equivalent of CLI `iroh get` | ||
| async fn get<'a>( | ||
| &self, | ||
| ipfs_path: &IpfsPath, | ||
| output_path: Option<&'a Path>, | ||
| ) -> Result<PathBuf> { | ||
| if ipfs_path.cid().is_none() { | ||
| return Err(anyhow!("IPFS path does not refer to a CID")); | ||
| } | ||
| let root_path = get_root_path(ipfs_path, output_path); | ||
| if root_path.exists() { | ||
| return Err(anyhow!( | ||
| "output path {} already exists", | ||
| root_path.display() | ||
| )); | ||
| } | ||
| let blocks = self.get_stream(ipfs_path); | ||
| save_get_stream(&root_path, blocks).await?; | ||
| Ok(root_path) | ||
| } | ||
| } | ||
|
|
||
| impl<T> ApiExt for T where T: Api {} | ||
|
|
||
| /// take a stream of blocks as from `get_stream` and write them to the filesystem | ||
| async fn save_get_stream( | ||
| root_path: &Path, | ||
| blocks: impl Stream<Item = Result<(RelativePathBuf, OutType)>>, | ||
| ) -> Result<()> { | ||
| tokio::pin!(blocks); | ||
| while let Some(block) = blocks.next().await { | ||
| let (path, out) = block?; | ||
| let full_path = path.to_path(root_path); | ||
| match out { | ||
| OutType::Dir => { | ||
| tokio::fs::create_dir_all(full_path).await?; | ||
| } | ||
| OutType::Reader(mut reader) => { | ||
| if let Some(parent) = path.parent() { | ||
| tokio::fs::create_dir_all(parent.to_path(root_path)).await?; | ||
| } | ||
| let mut f = tokio::fs::File::create(full_path).await?; | ||
| tokio::io::copy(&mut reader, &mut f).await?; | ||
| } | ||
| } | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// Given an cid and an optional output path, determine root path | ||
| fn get_root_path(ipfs_path: &IpfsPath, output_path: Option<&Path>) -> PathBuf { | ||
| match output_path { | ||
| Some(path) => path.to_path_buf(), | ||
| None => { | ||
| if ipfs_path.tail().is_empty() { | ||
| PathBuf::from(ipfs_path.cid().unwrap().to_string()) | ||
| } else { | ||
| PathBuf::from(ipfs_path.tail().last().unwrap()) | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
| use std::str::FromStr; | ||
| use tempdir::TempDir; | ||
|
|
||
| #[tokio::test] | ||
| async fn test_save_get_stream() { | ||
| let stream = Box::pin(futures::stream::iter(vec![ | ||
| Ok((RelativePathBuf::from_path("a").unwrap(), OutType::Dir)), | ||
| Ok(( | ||
| RelativePathBuf::from_path("b").unwrap(), | ||
| OutType::Reader(Box::new(std::io::Cursor::new("hello"))), | ||
| )), | ||
| ])); | ||
| let tmp_dir = TempDir::new("test_save_get_stream").unwrap(); | ||
| save_get_stream(tmp_dir.path(), stream).await.unwrap(); | ||
| assert!(tmp_dir.path().join("a").is_dir()); | ||
| assert_eq!( | ||
| std::fs::read_to_string(tmp_dir.path().join("b")).unwrap(), | ||
| "hello" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_get_root_path() { | ||
| let ipfs_path = | ||
| IpfsPath::from_str("/ipfs/QmYyQSo1c1Ym7orWxLYvCrM2EmxFTANf8wXmmE7DWjhx5N").unwrap(); | ||
| assert_eq!( | ||
| get_root_path(&ipfs_path, None), | ||
| PathBuf::from("QmYyQSo1c1Ym7orWxLYvCrM2EmxFTANf8wXmmE7DWjhx5N") | ||
| ); | ||
| assert_eq!( | ||
| get_root_path(&ipfs_path, Some(Path::new("bar"))), | ||
| PathBuf::from("bar") | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_get_root_path_with_tail() { | ||
| let ipfs_path = | ||
| IpfsPath::from_str("/ipfs/QmYyQSo1c1Ym7orWxLYvCrM2EmxFTANf8wXmmE7DWjhx5N/tail") | ||
| .unwrap(); | ||
| assert_eq!(get_root_path(&ipfs_path, None), PathBuf::from("tail")); | ||
| assert_eq!( | ||
| get_root_path(&ipfs_path, Some(Path::new("bar"))), | ||
| PathBuf::from("bar") | ||
| ); | ||
| } | ||
| } |
Oops, something went wrong.