tsarevvv/rust-ipfs
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: suggestions from code review

Browse Source 
Including mostly comment fixes and removal of an extra &mut.

Co-authored-by: ljedrz <ljedrz@users.noreply.github.com>
This commit is contained in:
Joonas Koivunen 2020-06-17 15:57:51 +03:00 committed by Joonas Koivunen
parent d56058a200
commit 3bfa506f69
8 changed files with 41 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
http/src/v0/root_files.rs
View File

@ -161,7 +161,7 @@ fn walk<Types: IpfsTypes>(
},
ContinuedWalk::Directory(item) => {
// only first instances of directorys will have the metadata
// only first instances of directories will have the metadata
if let Some(metadata) = item.as_entry().metadata() {
let path = item.as_entry().path();
@ -224,7 +224,7 @@ impl fmt::Display for GetError {
Walk(e) => write!(fmt, "{}", e),
Loading(e) => write!(fmt, "loading failed: {}", e),
InvalidFileName(x) => write!(fmt, "filename cannot be put inside tar: {:?}", x),
InvalidLinkName(x) => write!(fmt, "symlin link name cannot be put inside tar: {:?}", x),
InvalidLinkName(x) => write!(fmt, "symlink name cannot be put inside tar: {:?}", x),
}
}
}

14
http/src/v0/root_files/tar_helper.rs
View File

@ -1,10 +1,10 @@
///! Tar helper is internal to `/get` implementation. It uses some private parts of the `tar-rs`
///! crate to provide a BytesMut writing implementation instead of one using `std::io` interfaces.
///! crate to provide a `BytesMut` writing implementation instead of one using `std::io` interfaces.
///!
///! Code was originally taken and modified from the dependency version of `tar-rs`. The most
///! important copied parts are related to the long file name and long link name support. Issue
///! will be opened on the `tar-rs` to discuss if these could be made public, leaving us only the
///! Bytes (copying) code.
///! `Bytes` (copying) code.
use super::GetError;
use bytes::{buf::BufMut, Bytes, BytesMut};
use ipfs::unixfs::ll::Metadata;
@ -181,8 +181,8 @@ impl TarHelper {
return Err(GetError::InvalidLinkName(data.to_vec()));
}
// this is another long header trick, but this time we have different entry type but
// similarly the long file name is written as an separate entry with own headers.
// this is another long header trick, but this time we have a different entry type and
// similarly the long file name is written as a separate entry with its own headers.
self.long_filename_header.set_size(data.len() as u64 + 1);
self.long_filename_header
@ -208,7 +208,7 @@ impl TarHelper {
Ok(ret)
}
/// Content is tar files is padded to 512 byte sectors which might be configurable as well.
/// Content in tar is padded to 512 byte sectors which might be configurable as well.
pub(super) fn pad(&self, total_size: u64) -> Option<Bytes> {
let padding = 512 - (total_size % 512);
if padding < 512 {
@ -291,7 +291,7 @@ fn prepare_long_header<'a>(
}
// we **only** have utf8 paths as protobuf has already parsed this file
// name and all of the previous as utf8.
// name and all of the previous ones as utf8.
let data = path2bytes(path);
@ -301,7 +301,7 @@ fn prepare_long_header<'a>(
return Err(GetError::InvalidFileName(data.to_vec()));
}
// the plus one is documented as compliance to GNU tar, probably the null byte
// the plus one is documented as compliance with GNU tar, probably the null byte
// termination?
long_filename_header.set_size(data.len() as u64 + 1);
long_filename_header.set_entry_type(tar::EntryType::new(b'L'));

6
unixfs/README.md
View File

@ -12,14 +12,14 @@ Status:
* first iteration of resolving IpfsPath segments through directories has been
implemented
* as the HAMTShard structure is not fully understood, all buckets are
searched, however the API is expected to remain same even if more
searched, however the API is expected to remain the same even if more
efficient lookup is implemented
* first iteration of `/get` alike tree walking implemented
* first iteration of `/get`-like tree walking implemented
* creation and alteration of dags has not been implemented
Usage:
* The main entry point to walking anything unixfs should be `ipfs_unixfs::walk::Walker`
* The main entry point to resolving links under dag-pb or unixfs should be `ipfs_unixfs::resolve`
* There is a `ipfs_unixfs::file::visit::FileVisit` facility but it should be
* There is a `ipfs_unixfs::file::visit::FileVisit` utility but it should be
considered superceded by `ipfs_unixfs::walk::Walker`

4
unixfs/examples/get.rs
View File

@ -152,8 +152,8 @@ impl fmt::Display for Error {
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
use Error::*;
match self {
OpeningFailed(e) => write!(fmt, "File opening failed: {}", e),
Other(e) => write!(fmt, "Other file related io error: {}", e),
OpeningFailed(e) => write!(fmt, "Failed to open file: {}", e),
Other(e) => write!(fmt, "A file-related IO error: {}", e),
Walk(e) => write!(fmt, "Walk failed, please report this as a bug: {}", e),
}
}

8
unixfs/examples/resolve.rs
View File

@ -88,8 +88,8 @@ pub struct IpfsPath {
}
impl From<Cid> for IpfsPath {
/// Creates a new IpfsPath from just the Cid, which is the same as parsing from a string
/// representation of a Cid but cannot fail.
/// Creates a new `IpfsPath` from just the `Cid`, which is the same as parsing from a string
/// representation of a `Cid`, but cannot fail.
fn from(root: Cid) -> IpfsPath {
IpfsPath {
root: Some(root),
@ -180,9 +180,9 @@ fn walk(blocks: ShardedBlockStore, mut path: IpfsPath) -> Result<Option<Cid>, Er
NotFound => return Ok(None),
// when we stumble upon a HAMT shard, we'll need to look up other blocks in order to
// find the final link. The current implementation cannot lookup the directory by
// find the final link. The current implementation cannot search for the directory by
// hashing the name and looking it up, but the implementation can be changed underneath
// but the API would stay the same.
// without changes to the API.
//
// HAMTDirecotories or HAMT shards are multi-block directories where the entires are
// bucketed per their hash value.

2
unixfs/src/file/visit.rs
View File

@ -25,7 +25,7 @@ impl IdleFileVisit {
/// Begins the visitation by processing the first block to be visited.
///
/// Returns on success a tuple of file bytes, total file size, any metadata associated, and
/// Returns (on success) a tuple of file bytes, total file size, any metadata associated, and
/// optionally a `FileVisit` to continue the walk.
#[allow(clippy::type_complexity)]
pub fn start(

16
unixfs/src/lib.rs
View File

@ -1,11 +1,11 @@
#![warn(rust_2018_idioms, missing_docs)]
//! ipfs-unixfs: UnixFs tree support in Rust.
//!
//! The crate aims to provide a blockstore implementation independent UnixFs implementation by
//! The crate aims to provide a blockstore implementation independent of the UnixFs implementation by
//! working on slices and not doing any IO operations.
//!
//! The main entry point for extracting information and/or data out of UnixFs trees is
//! `ipfs_unixfs::walk::Walker`. To resolve IpfsPath segments over dag-pb nodes,
//! `ipfs_unixfs::walk::Walker`. To resolve `IpfsPath` segments over dag-pb nodes,
//! `ipfs_unixfs::resolve` should be used.
use std::borrow::Cow;
@ -127,7 +127,7 @@ impl UnexpectedNodeType {
}
}
/// Container for the unixfs metadata, which can be present at the root of the file trees.
/// A container for the UnixFs metadata, which can be present at the root of the file, directory, or symlink trees.
#[derive(Debug, Default, PartialEq, Eq, Clone)]
pub struct Metadata {
mode: Option<u32>,
@ -138,9 +138,9 @@ impl Metadata {
/// Returns the full file mode, if one has been specified.
///
/// The full file mode is originally read through `st_mode` field of `stat` struct defined in
/// `sys/stat.h` and it's defining OpenGroup standard. Lowest 3 bytes will correspond to read,
/// write, and execute rights per user, group, and other and 4th byte determines sticky bits,
/// set user id or set group id. Following two bytes correspond to the different file types, as
/// `sys/stat.h` and its defining OpenGroup standard. The lowest 3 bytes correspond to read,
/// write, and execute rights per user, group, and other, while the 4th byte determines sticky bits,
/// set user id or set group id. The following two bytes correspond to the different file types, as
/// defined by the same OpenGroup standard:
/// https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_stat.h.html
pub fn mode(&self) -> Option<u32> {
@ -149,14 +149,14 @@ impl Metadata {
/// Returns the raw timestamp of last modification time, if specified.
///
/// The timestamp is `(seconds, nanos)` similar to `std::time::Duration` with the exception of
/// The timestamp is `(seconds, nanos)` - similar to `std::time::Duration`, with the exception of
/// allowing seconds to be negative. The seconds are calculated from `1970-01-01 00:00:00` or
/// the common "unix epoch".
pub fn mtime(&self) -> Option<(i64, u32)> {
self.mtime
}
/// Returns the mtime metadata as an `FileTime`. Enabled only on feature `filetime`.
/// Returns the mtime metadata as a `FileTime`. Enabled only in the `filetime` feature.
#[cfg(feature = "filetime")]
pub fn mtime_as_filetime(&self) -> Option<filetime::FileTime> {
self.mtime()

28
unixfs/src/walk.rs
View File

@ -14,24 +14,24 @@ use std::convert::TryFrom;
use std::fmt;
use std::path::{Path, PathBuf};
/// Walker helps with walking an UnixFS tree, including all of the content and files. Created with
/// `Walker` helps with walking a UnixFS tree, including all of the content and files. It is created with
/// `Walker::new` and walked over each block with `Walker::continue_block`. Use
/// `Walker::pending_links` to learn the next [`Cid`] to be loaded and the prefetchable links.
/// `Walker::pending_links` to obtain the next [`Cid`] to be loaded and the prefetchable links.
#[derive(Debug)]
pub struct Walker {
/// This is `None` until the first block has been visited. Failing any unwraps would be logic
/// This is `None` until the first block has been visited. Any failing unwraps would be logic
/// errors.
current: Option<InnerEntry>,
/// On the next call to `continue_walk` this will be the block, unless we have an ongoing file
/// walk in which case we shortcircuit to continue it. Failing any of the unwrappings of
/// `self.next` would be an logic error
/// walk, in which case we short-circuit to continue it. Any failing unwraps of
/// `self.next` would be logic errors.
next: Option<(Cid, String, usize)>,
pending: Vec<(Cid, String, usize)>,
// tried to recycle the names but that was consistently as fast and used more memory than just
// cloning the strings
}
/// Converts a link of a Directory, specifically not a link of HAMTShard.
/// Converts a link of specifically a Directory (and not a link of a HAMTShard).
fn convert_link(
depth: usize,
nth: usize,
@ -51,7 +51,7 @@ fn convert_link(
Ok((cid, name, depth))
}
/// Converts a link of HAMTShard, specifically not a link of Directory.
/// Converts a link of specifically a HAMTShard (and not a link of a Directory).
fn convert_sharded_link(
depth: usize,
nth: usize,
@ -80,7 +80,7 @@ fn convert_sharded_link(
}
impl Walker {
/// Returns a new instance of a walker, ready to start from the given Cid.
/// Returns a new instance of a walker, ready to start from the given `Cid`.
pub fn new(cid: Cid, root_name: String) -> Walker {
// 1 == Path::ancestors().count() for an empty path
let depth = if root_name.is_empty() { 1 } else { 2 };
@ -93,13 +93,13 @@ impl Walker {
}
}
/// Returns a description of a kind of node Walker is currently looking at, if the
/// continue_walk has been called after creating the value.
/// Returns a description of the kind of node the `Walker` is currently looking at, if
/// `continue_walk` has been called after the value was created.
pub fn as_entry(&'_ self) -> Option<Entry<'_>> {
self.current.as_ref().map(|c| c.as_entry())
}
/// Returns the next cid to load and pass content of which to pass to `continue_walk`.
/// Returns the next `Cid` to load and pass its associated content to `continue_walk`.
pub fn pending_links<'a>(&'a self) -> (&'a Cid, impl Iterator<Item = &'a Cid> + 'a) {
use InnerKind::*;
// rev: because we'll pop any of the pending
@ -115,7 +115,7 @@ impl Walker {
let next = self
.next
.as_ref()
.expect("validated in start and continue_walk we have the next");
.expect("we've validated that we have the next in new and continue_walk");
(&next.0, Either::Right(cids))
}
}
@ -125,7 +125,7 @@ impl Walker {
///
/// Returns a descriptor for the next element found as `ContinuedWalk` which includes the means
/// to further continue the walk. `bytes` is the raw data of the next block, `cache` is an
/// optional cache for data structures which can always be substituted for `&mut None`.
/// optional cache for data structures which can always be substituted with `&mut None`.
pub fn continue_walk<'a>(
mut self,
bytes: &'a [u8],
@ -133,7 +133,7 @@ impl Walker {
) -> Result<ContinuedWalk<'a>, Error> {
use InnerKind::*;
if let Some(File(_, visit @ Some(_), _)) = &mut self.current.as_mut().map(|c| &mut c.kind) {
if let Some(File(_, visit @ Some(_), _)) = self.current.as_mut().map(|c| &mut c.kind) {
// we have an ongoing filevisit, the block must be related to it.
let (bytes, step) = visit
.take()