manifest: update get to return FileMetadata copy

Summary:
There is no good reason to return a reference for FileMetadata. It is a relatively
small object that implements the `Copy` trait so returning a copy is
approapriate.

Reviewed By: quark-zju

Differential Revision: D16571838

fbshipit-source-id: 0c315c9f405e425832d39da5c67809dd15b4ab5e
This commit is contained in:
Stefan Filip 2019-08-01 10:30:45 -07:00 committed by Facebook Github Bot
parent 427ad4416e
commit a585faf2db
3 changed files with 22 additions and 22 deletions

View File

@ -78,7 +78,7 @@ py_class!(class treemanifest |py| {
let tree = self.underlying(py).borrow();
let result = match tree.get(&repo_path).map_pyerr::<exc::RuntimeError>(py)? {
None => None,
Some(file_metadata) => Some(file_metadata_to_py_tuple(py, file_metadata)?),
Some(file_metadata) => Some(file_metadata_to_py_tuple(py, &file_metadata)?),
};
Ok(result)
}

View File

@ -27,7 +27,7 @@ use types::{Node, RepoPath, RepoPathBuf};
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: &RepoPath) -> 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.

View File

@ -69,11 +69,11 @@ impl Tree {
}
impl Manifest for Tree {
fn get(&self, path: &RepoPath) -> Fallible<Option<&FileMetadata>> {
fn get(&self, path: &RepoPath) -> Fallible<Option<FileMetadata>> {
match self.get_link(path)? {
None => Ok(None),
Some(link) => {
if let Leaf(file_metadata) = link {
if let &Leaf(file_metadata) = link {
Ok(Some(file_metadata))
} else {
Err(format_err!("Encountered directory where file was expected"))
@ -622,17 +622,17 @@ mod tests {
fn test_insert() {
let mut tree = Tree::ephemeral(Arc::new(TestStore::new()));
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("foo/bar")).unwrap(), Some(meta("10")));
assert_eq!(tree.get(repo_path("baz")).unwrap(), None);
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")));
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(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")));
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")));
}
#[test]
@ -657,13 +657,13 @@ mod tests {
.unwrap();
let mut tree = Tree::durable(Arc::new(store), node("1"));
assert_eq!(tree.get(repo_path("foo/bar")).unwrap(), Some(&meta("11")));
assert_eq!(tree.get(repo_path("baz")).unwrap(), Some(&meta("20")));
assert_eq!(tree.get(repo_path("foo/bar")).unwrap(), Some(meta("11")));
assert_eq!(tree.get(repo_path("baz")).unwrap(), Some(meta("20")));
tree.insert(repo_path_buf("foo/bat"), meta("12")).unwrap();
assert_eq!(tree.get(repo_path("foo/bat")).unwrap(), Some(&meta("12")));
assert_eq!(tree.get(repo_path("foo/bar")).unwrap(), Some(&meta("11")));
assert_eq!(tree.get(repo_path("baz")).unwrap(), Some(&meta("20")));
assert_eq!(tree.get(repo_path("foo/bat")).unwrap(), Some(meta("12")));
assert_eq!(tree.get(repo_path("foo/bar")).unwrap(), Some(meta("11")));
assert_eq!(tree.get(repo_path("baz")).unwrap(), Some(meta("20")));
}
#[test]
@ -721,11 +721,11 @@ mod tests {
assert!(tree.remove(RepoPath::empty()).is_err());
assert_eq!(tree.get(repo_path("a1/b1/c1/d1")).unwrap(), None);
assert_eq!(tree.get(repo_path("a1/b1/c1")).unwrap(), None);
assert_eq!(tree.get(repo_path("a1/b2")).unwrap(), Some(&meta("20")));
assert_eq!(tree.get(repo_path("a1/b2")).unwrap(), Some(meta("20")));
tree.remove(repo_path("a1/b2")).unwrap();
assert_eq!(tree.get_link(repo_path("a1")).unwrap(), None);
assert_eq!(tree.get(repo_path("a2/b2/c2")).unwrap(), Some(&meta("30")));
assert_eq!(tree.get(repo_path("a2/b2/c2")).unwrap(), Some(meta("30")));
tree.remove(repo_path("a2/b2/c2")).unwrap();
assert_eq!(tree.get(repo_path("a2")).unwrap(), None);
@ -756,13 +756,13 @@ mod tests {
assert!(tree.remove(repo_path("a1")).is_err());
tree.remove(repo_path("a1/b1")).unwrap();
assert_eq!(tree.get(repo_path("a1/b1")).unwrap(), None);
assert_eq!(tree.get(repo_path("a1/b2")).unwrap(), Some(&meta("12")));
assert_eq!(tree.get(repo_path("a1/b2")).unwrap(), Some(meta("12")));
tree.remove(repo_path("a1/b2")).unwrap();
assert_eq!(tree.get(repo_path("a1/b2")).unwrap(), None);
assert_eq!(tree.get(repo_path("a1")).unwrap(), None);
assert_eq!(tree.get_link(repo_path("a1")).unwrap(), None);
assert_eq!(tree.get(repo_path("a2")).unwrap(), Some(&meta("20")));
assert_eq!(tree.get(repo_path("a2")).unwrap(), Some(meta("20")));
tree.remove(repo_path("a2")).unwrap();
assert_eq!(tree.get(repo_path("a2")).unwrap(), None);
@ -783,10 +783,10 @@ mod tests {
let tree = Tree::durable(store.clone(), node);
assert_eq!(
tree.get(repo_path("a1/b1/c1/d1")).unwrap(),
Some(&meta("10"))
Some(meta("10"))
);
assert_eq!(tree.get(repo_path("a1/b2")).unwrap(), Some(&meta("20")));
assert_eq!(tree.get(repo_path("a2/b2/c2")).unwrap(), Some(&meta("30")));
assert_eq!(tree.get(repo_path("a1/b2")).unwrap(), Some(meta("20")));
assert_eq!(tree.get(repo_path("a2/b2/c2")).unwrap(), Some(meta("30")));
assert_eq!(tree.get(repo_path("a2/b1")).unwrap(), None);
}