Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(vfs) Add ability to rename a file with mem_fs #2551

Merged
merged 2 commits into from
Sep 3, 2021
Merged

feat(vfs) Add ability to rename a file with mem_fs #2551

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af6dc30
feat(vfs) Add ability to rename a file with `mem_fs`.
Hywan Sep 2, 2021
3d27db8
feat(vfs) Add test cases for `mem_fs::FileSystem::rename`.
Hywan Sep 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
176 changes: 146 additions & 30 deletions lib/vfs/src/mem_fs/filesystem.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,7 @@ impl crate::FileSystem for FileSystem {
}

fn rename(&self, from: &Path, to: &Path) -> Result<()> {
let (
(position_of_from, inode, inode_of_from_parent),
(inode_of_to_parent, name_of_to_directory),
) = {
let ((position_of_from, inode, inode_of_from_parent), (inode_of_to_parent, name_of_to)) = {
// Read lock.
let fs = self.inner.try_read().map_err(|_| FsError::Lock)?;

Expand All @@ -172,46 +169,56 @@ impl crate::FileSystem for FileSystem {
let parent_of_from = from.parent().ok_or(FsError::BaseNotDirectory)?;
let parent_of_to = to.parent().ok_or(FsError::BaseNotDirectory)?;

// Check the directory names.
let name_of_from_directory = from
// Check the names.
let name_of_from = from
.file_name()
.ok_or(FsError::InvalidInput)?
.to_os_string();
let name_of_to_directory = to.file_name().ok_or(FsError::InvalidInput)?.to_os_string();
let name_of_to = to.file_name().ok_or(FsError::InvalidInput)?.to_os_string();

// Find the parent inodes.
let inode_of_from_parent = fs.inode_of_parent(parent_of_from)?;
let inode_of_to_parent = fs.inode_of_parent(parent_of_to)?;

// Get the child indexes to update in the parent nodes, in
// addition to the inode of the directory to update.
let (position_of_from, inode) = fs.from_parent_get_position_and_inode_of_directory(
inode_of_from_parent,
&name_of_from_directory,
DirectoryMustBeEmpty::No,
)?;
let (position_of_from, inode) = fs
.from_parent_get_position_and_inode(inode_of_from_parent, &name_of_from)?
.ok_or(FsError::NotAFile)?;

(
(position_of_from, inode, inode_of_from_parent),
(inode_of_to_parent, name_of_to_directory),
(inode_of_to_parent, name_of_to),
)
};

{
// Write lock.
let mut fs = self.inner.try_write().map_err(|_| FsError::Lock)?;

// Update the directory name, and update the modified
// time.
fs.update_node_name(inode, name_of_to_directory)?;
// Update the file name, and update the modified time.
fs.update_node_name(inode, name_of_to)?;

// Remove the directory from its parent, and update the
// modified time.
fs.remove_child_from_node(inode_of_from_parent, position_of_from)?;
// The parents are different. Let's update them.
if inode_of_from_parent != inode_of_to_parent {
// Remove the file from its parent, and update the
// modified time.
fs.remove_child_from_node(inode_of_from_parent, position_of_from)?;

// Add the directory to its new parent, and update the
// modified time.
fs.add_child_to_node(inode_of_to_parent, inode)?;
// Add the file to its new parent, and update the modified
// time.
fs.add_child_to_node(inode_of_to_parent, inode)?;
}
// Otherwise, we need to at least update the modified time of the parent.
else {
match fs.storage.get_mut(inode_of_from_parent) {
Some(Node::Directory {
metadata: Metadata { modified, .. },
..
}) => *modified = time(),
_ => return Err(FsError::UnknownError),
}
}
}

Ok(())
Expand Down Expand Up @@ -399,6 +406,35 @@ impl FileSystemInner {
}
}

/// From the inode of a parent node (so, a directory), returns the
/// child index of `name_of` along with its inode, whatever the
/// type of inode is (directory or file).
fn from_parent_get_position_and_inode(
&self,
inode_of_parent: Inode,
name_of: &OsString,
) -> Result<Option<(usize, Inode)>> {
match self.storage.get(inode_of_parent) {
Some(Node::Directory { children, .. }) => children
.iter()
.enumerate()
.filter_map(|(nth, inode)| self.storage.get(*inode).map(|node| (nth, node)))
.find_map(|(nth, node)| match node {
Node::File { inode, name, .. } | Node::Directory { inode, name, .. }
if name.as_os_str() == name_of =>
{
Some(Some((nth, *inode)))
}

_ => None,
})
.or(Some(None))
.ok_or(FsError::InvalidInput),

_ => Err(FsError::BaseNotDirectory),
}
}

/// Set a new name for the node represented by `inode`.
pub(super) fn update_node_name(&mut self, inode: Inode, new_name: OsString) -> Result<()> {
let node = self.storage.get_mut(inode).ok_or(FsError::UnknownError)?;
Expand Down Expand Up @@ -808,10 +844,31 @@ mod test_filesystem {

assert_eq!(fs.create_dir(path!("/bar")), Ok(()));

assert!(
matches!(
fs.new_open_options()
.write(true)
.create_new(true)
.open(path!("/bar/hello1.txt")),
Ok(_),
),
"creating a new file (`hello1.txt`)",
);
assert!(
matches!(
fs.new_open_options()
.write(true)
.create_new(true)
.open(path!("/bar/hello2.txt")),
Ok(_),
),
"creating a new file (`hello2.txt`)",
);

{
let fs_inner = fs.inner.read().unwrap();

assert_eq!(fs_inner.storage.len(), 4, "storage has all directories");
assert_eq!(fs_inner.storage.len(), 6, "storage has all files");
assert!(
matches!(
fs_inner.storage.get(ROOT_INODE),
Expand Down Expand Up @@ -856,24 +913,60 @@ mod test_filesystem {
name,
children,
..
}) if name == "bar" && children.is_empty()
}) if name == "bar" && children == &[4, 5]
),
"`bar` is contains `hello.txt`",
);
assert!(
matches!(
fs_inner.storage.get(4),
Some(Node::File {
inode: 4,
name,
..
}) if name == "hello1.txt"
),
"`bar` is empty",
"`hello1.txt` exists",
);
assert!(
matches!(
fs_inner.storage.get(5),
Some(Node::File {
inode: 5,
name,
..
}) if name == "hello2.txt"
),
"`hello2.txt` exists",
);
}

assert_eq!(
fs.rename(path!("/bar/hello2.txt"), path!("/foo/world2.txt")),
Ok(()),
"renaming (and moving) a file",
);

assert_eq!(
fs.rename(path!("/foo"), path!("/bar/baz")),
Ok(()),
"renaming a directory",
);

assert_eq!(
fs.rename(path!("/bar/hello1.txt"), path!("/bar/world1.txt")),
Ok(()),
"renaming a file (in the same directory)",
);

{
let fs_inner = fs.inner.read().unwrap();

dbg!(&fs_inner);

assert_eq!(
fs_inner.storage.len(),
4,
6,
"storage has still all directories"
);
assert!(
Expand All @@ -896,9 +989,9 @@ mod test_filesystem {
name,
children,
..
}) if name == "baz" && children == &[2]
}) if name == "baz" && children == &[2, 5]
),
"`foo` has been renamed to `baz` and contains `qux`",
"`foo` has been renamed to `baz` and contains `qux` and `world2.txt`",
);
assert!(
matches!(
Expand All @@ -920,9 +1013,31 @@ mod test_filesystem {
name,
children,
..
}) if name == "bar" && children == &[1]
}) if name == "bar" && children == &[4, 1]
),
"`bar` contains `bar` (ex `foo`) and `world1.txt` (ex `hello1`)",
);
assert!(
matches!(
fs_inner.storage.get(4),
Some(Node::File {
inode: 4,
name,
..
}) if name == "world1.txt"
),
"`hello1.txt` has been renamed to `world1.txt`",
);
assert!(
matches!(
fs_inner.storage.get(5),
Some(Node::File {
inode: 5,
name,
..
}) if name == "world2.txt"
),
"`bar` contains `baz` (ex `foo`)",
"`hello2.txt` has been renamed to `world2.txt`",
);
}
}
Expand Down Expand Up @@ -1248,6 +1363,7 @@ mod test_filesystem {
}
}

#[allow(dead_code)] // The `No` variant.
pub(super) enum DirectoryMustBeEmpty {
Yes,
No,
Expand Down