Use RepoPath for path types in Manifest

Summary: `RepoPath` and `PathComponent` were recently added as path abstractions for source control internals. This diff makes use of these abstractions in the manifest crate.

Reviewed By: quark-zju

Differential Revision: D14142925

fbshipit-source-id: 1e0033f4f2aeb5c1d63899adbce198302086bfda

lib/manifest/src/lib.rs

@@ -10,7 +10,7 @@
 //! repository. The file path and file revision are then used to retrieve the contents of the
 //! file thus achieving the reconstruction of the entire repository state.
 use failure::Fallible;
-use types::node::Node;
+use types::{Node, RepoPath, RepoPathBuf};
 
 /// Manifest describes a mapping between file path ([`String`]) and file metadata ([`FileMetadata`]).
 /// Fundamentally it is just a Map<file_path, file_metadata>.
@@ -25,15 +25,15 @@ use types::node::Node;
 pub trait Manifest {
     /// Retrieve the FileMetadata that is associated with a path.
     /// Paths that were not set will return None.
-    fn get(&self, file_path: &str) -> Fallible<Option<&FileMetadata>>;
+    fn get(&self, file_path: &RepoPath) -> Fallible<Option<&FileMetadata>>;
 
     /// Associates a file path with specific file metadata.
     /// A call with a file path that already exists results in an override or the old metadata.
-    fn insert(&mut self, file_path: String, file_metadata: FileMetadata) -> Fallible<()>;
+    fn insert(&mut self, file_path: RepoPathBuf, file_metadata: FileMetadata) -> Fallible<()>;
 
     /// Removes a file from the manifest (equivalent to removing it from the repository).
     /// A call with a file path that does not exist in the manifest is a no-op.
-    fn remove(&mut self, file_path: &str) -> Fallible<()>;
+    fn remove(&mut self, file_path: &RepoPath) -> Fallible<()>;
 }
 
 /// The contents of the Manifest for a file.

lib/manifest/src/tree.rs

@@ -7,6 +7,7 @@ use crate::FileMetadata;
 use crate::Manifest;
 use failure::{bail, Fallible};
 use std::collections::BTreeMap;
+use types::{PathComponent, PathComponentBuf, RepoPath, RepoPathBuf};
 
 /// The Tree implementation of a Manifest dedicates an inner node for each directory in the
 /// repository and a leaf for each file.
@@ -33,7 +34,7 @@ enum Link {
     /// available in memory. They need to be persisted to be available in future. They are the
     /// mutable type of an inner node. They store the contents of a directory that has been
     /// modified.
-    Ephemeral(BTreeMap<String, Link>),
+    Ephemeral(BTreeMap<PathComponentBuf, Link>),
     // TODO: add durable link (reading from storage)
 }
 use self::Link::*;
@@ -47,14 +48,14 @@ impl Link {
         Ephemeral(BTreeMap::new())
     }
 
-    fn child(&self, component: &str) -> Fallible<Option<&Link>> {
+    fn child(&self, component: &PathComponent) -> Fallible<Option<&Link>> {
         match self {
             Leaf(_) => bail!("Encountered file where a directory was expected."),
             Ephemeral(links) => Ok(links.get(component)),
         }
     }
 
-    fn child_mut_or_insert(&mut self, component: String) -> Fallible<&mut Link> {
+    fn child_mut_or_insert(&mut self, component: PathComponentBuf) -> Fallible<&mut Link> {
         match self {
             Leaf(_) => bail!("Encountered file where a directory was expected."),
             Ephemeral(links) => Ok(links.entry(component).or_insert(Link::ephemeral())),
@@ -86,9 +87,9 @@ impl Link {
 }
 
 impl Manifest for Tree {
-    fn get(&self, path: &str) -> Fallible<Option<&FileMetadata>> {
+    fn get(&self, path: &RepoPath) -> Fallible<Option<&FileMetadata>> {
         let mut cursor = &self.root;
-        for component in path.split("/") {
+        for component in path.components() {
             match cursor.child(component)? {
                 None => return Ok(None),
                 Some(link) => cursor = link,
@@ -97,15 +98,15 @@ impl Manifest for Tree {
         Ok(Some(cursor.file_metadata()?))
     }
 
-    fn insert(&mut self, path: String, file_metadata: FileMetadata) -> Fallible<()> {
+    fn insert(&mut self, path: RepoPathBuf, file_metadata: FileMetadata) -> Fallible<()> {
         let mut cursor = &mut self.root;
-        for component in path.split("/") {
-            cursor = cursor.child_mut_or_insert(component.to_string())?;
+        for component in path.components() {
+            cursor = cursor.child_mut_or_insert(component.to_owned())?;
         }
         cursor.set_metadata(file_metadata)
     }
 
-    fn remove(&mut self, _path: &str) -> Fallible<()> {
+    fn remove(&mut self, _path: &RepoPath) -> Fallible<()> {
         // TODO: implement deletion
         Ok(())
     }
@@ -119,21 +120,27 @@ mod tests {
     fn meta(node: u8) -> FileMetadata {
         FileMetadata::regular(Node::from_u8(node))
     }
+    fn repo_path(s: &str) -> &RepoPath {
+        RepoPath::from_str(s).unwrap()
+    }
+    fn repo_path_buf(s: &str) -> RepoPathBuf {
+        RepoPathBuf::from_string(s.to_owned()).unwrap()
+    }
 
     #[test]
     fn test_insert() {
         let mut tree = Tree::new();
-        tree.insert(String::from("foo/bar"), meta(10)).unwrap();
-        assert_eq!(tree.get("foo/bar").unwrap(), Some(&meta(10)));
-        assert_eq!(tree.get("baz").unwrap(), None);
+        tree.insert(repo_path_buf("foo/bar"), meta(10)).unwrap();
+        assert_eq!(tree.get(repo_path("foo/bar")).unwrap(), Some(&meta(10)));
+        assert_eq!(tree.get(repo_path("baz")).unwrap(), None);
 
-        tree.insert(String::from("baz"), meta(20)).unwrap();
-        assert_eq!(tree.get("foo/bar").unwrap(), Some(&meta(10)));
-        assert_eq!(tree.get("baz").unwrap(), Some(&meta(20)));
+        tree.insert(repo_path_buf("baz"), meta(20)).unwrap();
+        assert_eq!(tree.get(repo_path("foo/bar")).unwrap(), Some(&meta(10)));
+        assert_eq!(tree.get(repo_path("baz")).unwrap(), Some(&meta(20)));
 
-        tree.insert(String::from("foo/bat"), meta(30)).unwrap();
-        assert_eq!(tree.get("foo/bat").unwrap(), Some(&meta(30)));
-        assert_eq!(tree.get("foo/bar").unwrap(), Some(&meta(10)));
-        assert_eq!(tree.get("baz").unwrap(), Some(&meta(20)));
+        tree.insert(repo_path_buf("foo/bat"), meta(30)).unwrap();
+        assert_eq!(tree.get(repo_path("foo/bat")).unwrap(), Some(&meta(30)));
+        assert_eq!(tree.get(repo_path("foo/bar")).unwrap(), Some(&meta(10)));
+        assert_eq!(tree.get(repo_path("baz")).unwrap(), Some(&meta(20)));
     }
 }