Merge remote-tracking branch 'origin/storage-tests'

* origin/storage-tests:
  git-storage: add ODB tests

Signed-off-by: Fintan Halpenny <fintan.halpenny@gmail.com>

git-storage/src/backend/read.rs

@@ -3,7 +3,7 @@
 // This file is part of radicle-link, distributed under the GPLv3 with Radicle
 // Linking Exception. For full terms see the included LICENSE file.
 
-use std::path::Path;
+use std::{fmt, path::Path};
 
 use git_ext::{error::is_not_found_err, Oid};
 use std_ext::result::ResultExt as _;
@@ -46,6 +46,14 @@ impl Read {
     }
 }
 
+impl fmt::Debug for Read {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("Read")
+            .field("raw", &self.raw.path())
+            .finish()
+    }
+}
+
 pub mod error {
     use thiserror::Error;
 

git-storage/src/backend/write.rs

@@ -36,6 +36,7 @@ pub mod error;
 /// For write access to the refdb see [`refdb::Write`].
 ///
 /// To construct the `Write` storage use [`Read::open`].
+#[derive(Debug)]
 pub struct Write {
     inner: Read,
     info: UserInfo,
@@ -568,7 +569,19 @@ impl odb::Write for Write {
             .map(Oid::from)
     }
 
-    fn write_tree(&self, builder: git2::TreeBuilder) -> Result<Oid, Self::WriteTree> {
-        builder.write().map(Oid::from)
+    fn write_tree(&self, builder: odb::TreeBuilder) -> Result<Oid, Self::WriteTree> {
+        let repo = self.as_raw();
+        let mut tree = repo.treebuilder(None)?;
+        for entry in builder.iter() {
+            match entry {
+                odb::TreeEntry::Insert {
+                    name,
+                    oid,
+                    filemode,
+                } => tree.insert(name, (*oid).into(), *filemode).map(|_| ())?,
+                odb::TreeEntry::Remove { name } => tree.remove(name)?,
+            }
+        }
+        tree.write().map(Oid::from)
     }
 }

git-storage/src/odb.rs

@@ -20,7 +20,10 @@
 // TODO: this doesn't abstract over the git2 types very well, but it's too much
 // hassle to massage that right now.
 
-use std::error::Error;
+use std::{
+    error::Error,
+    path::{Path, PathBuf},
+};
 
 pub use git2::{Blob, Commit, Object, Tag, Tree};
 
@@ -32,6 +35,71 @@ pub use read::Read;
 pub mod write;
 pub use write::Write;
 
+/// A `TreeBuilder` represents a series of operations to build a git tree, see
+/// *Tree Objects* [here][git-objects].
+///
+/// The [`TreeEntry`] variants represent a limited set of the actions one can
+/// perform [git-objects]: <https://git-scm.com/book/en/v2/Git-Internals-Git-Objects>
+#[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub struct TreeBuilder {
+    entries: Vec<TreeEntry>,
+}
+
+impl TreeBuilder {
+    pub fn new() -> Self {
+        Self {
+            entries: Vec::new(),
+        }
+    }
+
+    /// [`TreeBuilder::insert`] creates a [`TreeEntry::Insert`] entry in the
+    /// `TreeBuilder`.
+    pub fn insert<P, M>(mut self, name: P, oid: Oid, mode: M) -> Self
+    where
+        P: AsRef<Path>,
+        M: Into<i32>,
+    {
+        let name = name.as_ref().to_path_buf();
+        let filemode = mode.into();
+        self.entries.push(TreeEntry::Insert {
+            name,
+            oid,
+            filemode,
+        });
+        self
+    }
+
+    /// [`TreeBuilder::remove`] creates a [`TreeEntry::Remove`] entry in the
+    /// `TreeBuilder`.
+    pub fn remove<P>(mut self, name: P) -> Self
+    where
+        P: AsRef<Path>,
+    {
+        let name = name.as_ref().to_path_buf();
+        self.entries.push(TreeEntry::Remove { name });
+        self
+    }
+
+    /// Iterate over the [`TreeEntry`]'s found in the `TreeBuilder`.
+    pub fn iter(&self) -> impl Iterator<Item = &TreeEntry> {
+        self.entries.iter()
+    }
+}
+
+/// A `TreeEntry` represents a single operation within a [`TreeBuilder`].
+#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
+pub enum TreeEntry {
+    /// Insert the `oid` under the `name` path in the resulting tree.
+    /// The `filemode` determines the file mode for the file found at `name`.
+    Insert {
+        name: PathBuf,
+        oid: Oid,
+        filemode: i32,
+    },
+    /// Remove the `name` path from the resulting tree.
+    Remove { name: PathBuf },
+}
+
 /// Find the [`Object`] corresponding to the given `oid`.
 ///
 /// Will fail if the object does not exist.

git-storage/src/odb/write.rs

@@ -6,7 +6,7 @@ use std::error::Error;
 use git_ext::Oid;
 use git_ref_format::RefStr;
 
-use super::{Commit, Object, Read, Tree};
+use super::{Commit, Object, Read, Tree, TreeBuilder};
 
 /// Read-write access to a git odb.
 ///
@@ -58,9 +58,5 @@ pub trait Write: Read {
         R: AsRef<RefStr>;
 
     /// Write a [`super::Tree`] using the provided `builder`.
-    // XXX: It's annoying that this exposes git2 but the effort of abstracting the
-    // tree builder is too high
-    // TODO: instead of passing the builder, we could use an enum of operations to
-    // modify a builder
-    fn write_tree(&self, builder: git2::TreeBuilder) -> Result<Oid, Self::WriteTree>;
+    fn write_tree(&self, builder: TreeBuilder) -> Result<Oid, Self::WriteTree>;
 }

git-storage/t/Cargo.toml

@@ -0,0 +1,47 @@
+[package]
+name = "git-storage-test"
+version = "0.1.0"
+edition = "2021"
+license = "GPL-3.0-or-later"
+
+publish = false
+
+[lib]
+doctest = false
+test = true
+doc = false
+
+[features]
+test = []
+
+[dependencies]
+proptest = "1"
+
+[dependencies.git-storage]
+path = ".."
+
+[dependencies.git2]
+version = "0.15.0"
+default-features = false
+features = ["vendored-libgit2"]
+
+[dependencies.radicle-git-ext]
+path = "../../radicle-git-ext/"
+
+[dependencies.test-helpers]
+path = "../../test/test-helpers"
+
+[dev-dependencies.uuid]
+version = "1"
+features = ["v4"]
+
+[dev-dependencies.git-ext-test]
+path = "../../radicle-git-ext/t"
+features = ["test"]
+
+[dev-dependencies.git-ref-format]
+path = "../../git-ref-format"
+
+[dev-dependencies.git-ref-format-test]
+path = "../../git-ref-format/t"
+features = ["test"]

git-storage/t/src/gen.rs

@@ -0,0 +1,108 @@
+// Copyright © 2022 The Radicle Link Contributors
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+use std::path::PathBuf;
+
+use git_storage::{
+    odb::{self, write::Write as _},
+    signature::UserInfo,
+    Write,
+};
+use proptest::prelude::*;
+use radicle_git_ext::Oid;
+
+use git2::FileMode;
+
+/// Represents a file in the git tree but without linking it to the repo yet
+#[derive(Clone, Debug)]
+pub struct File {
+    pub path: PathBuf,
+    pub inner: Vec<u8>,
+    pub mode: git2::FileMode,
+    pub oid: git2::Oid,
+}
+
+/// Represents a Tree to be written with the ODB writer.
+///
+/// This Tree does not have an explicit link to a repository, linking is
+/// performed by writing it to the repo using the ODB writer.
+///
+/// Used as a replacement of git2::TreeBuilder, which is more complicated to
+/// build since it requires a repository from the beginning.
+#[derive(Clone, Debug)]
+pub struct Tree {
+    builder: odb::TreeBuilder,
+    files: Vec<File>,
+}
+
+impl Tree {
+    /// Write the files to the filesystem and the repository `storage`
+    pub fn write(self, storage: &Write) -> Result<Oid, git2::Error> {
+        self.write_files(storage)?;
+        storage.write_tree(self.builder)
+    }
+
+    /// Write the files to the filesystem
+    pub fn write_files(&self, storage: &Write) -> Result<(), git2::Error> {
+        for file in &self.files {
+            storage.write_blob(&file.inner)?;
+        }
+        Ok(())
+    }
+}
+
+/// Any valid filename
+pub fn trivial() -> impl Strategy<Value = String> {
+    "[a-zA-Z0-9]+"
+}
+
+pub fn gen_signature() -> impl Strategy<Value = UserInfo> {
+    trivial().prop_map(move |name| {
+        UserInfo {
+            name: name.clone(),
+            // TODO: is it worth to make this more realistic?
+            email: format!("{}@{}.com", &name, &name),
+        }
+    })
+}
+
+pub fn gen_bytes() -> impl Strategy<Value = Vec<u8>> {
+    any::<Vec<u8>>()
+}
+
+pub fn gen_mode() -> impl Strategy<Value = FileMode> {
+    prop_oneof![Just(FileMode::Blob), Just(FileMode::BlobExecutable)]
+}
+
+prop_compose! {
+    pub fn gen_file()
+                    (path in trivial(),
+                     blob in gen_bytes(),
+                     mode in gen_mode())
+                    -> File {
+        let oid = git2::Oid::hash_object(git2::ObjectType::Blob, &blob).unwrap();
+        File {
+            path: PathBuf::from(path),
+            inner: blob,
+            mode,
+            oid,
+        }
+    }
+}
+
+pub fn gen_file_set(max_size: u8) -> impl Strategy<Value = Vec<File>> {
+    prop::collection::vec(gen_file(), 0..(max_size as usize))
+}
+
+/// Generates a [`odb::TreeBuilder`] and a set of [`File`]s.
+///
+/// To write the resulting `Tree`, you MUST write the [`File::oid`] to the
+/// repository first.
+pub fn gen_tree(max_size: u8) -> impl Strategy<Value = Tree> {
+    gen_file_set(max_size).prop_map(move |files| {
+        let builder = files.iter().fold(odb::TreeBuilder::new(), |tree, file| {
+            tree.insert(file.path.clone(), file.oid.into(), file.mode)
+        });
+        Tree { builder, files }
+    })
+}

git-storage/t/src/lib.rs

@@ -0,0 +1,9 @@
+// Copyright © 2022 The Radicle Link Contributors
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+#[cfg(any(test, feature = "test"))]
+pub mod gen;
+#[cfg(test)]
+mod properties;
+#[cfg(any(test, feature = "test"))]
+pub mod tmp;

git-storage/t/src/properties.rs

@@ -0,0 +1,4 @@
+// Copyright © 2022 The Radicle Link Contributors
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+mod odb;

git-storage/t/src/properties/odb.rs

@@ -0,0 +1,108 @@
+// Copyright © 2022 The Radicle Link Contributors
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+use git2::ObjectType;
+
+use git_ref_format::RefString;
+use git_ref_format_test::gen::valid;
+use git_storage::{
+    odb::{Read as _, Write as _},
+    signature::UserInfo,
+};
+
+use proptest::prelude::*;
+
+use crate::{gen, tmp};
+
+// NOTE: It's enough to check the `Oid`s. If the contents were different the
+// hash would be different.
+pub mod prop {
+    use super::*;
+
+    pub fn roundtrip_blob(user: UserInfo, bytes: &[u8]) {
+        let writer = tmp::writer(user);
+
+        let oid = writer.write_blob(bytes).unwrap();
+
+        let readback = writer.find_blob(oid).unwrap().unwrap();
+        assert_eq!(oid, readback.id().into());
+
+        let readback = writer.find_object(oid).unwrap().unwrap();
+        assert_eq!(readback.kind(), Some(ObjectType::Blob));
+    }
+
+    pub fn roundtrip_tree(user: UserInfo, tree: gen::Tree) {
+        let writer = tmp::writer(user);
+        let oid = tree.write(&writer).unwrap();
+        let readback = writer.find_tree(oid).unwrap().unwrap();
+        assert_eq!(oid, readback.id().into());
+
+        let readback = writer.find_object(oid).unwrap().unwrap();
+        assert_eq!(readback.kind(), Some(ObjectType::Tree));
+    }
+
+    pub fn roundtrip_commit(user: UserInfo, tree: gen::Tree, message: &str) {
+        let writer = tmp::writer(user);
+        let tree_oid = tree.write(&writer).unwrap();
+        let tree = writer.find_tree(tree_oid).unwrap().unwrap();
+        let commit_oid = writer.write_commit(&tree, &[], message).unwrap();
+
+        let readback = writer.find_commit(commit_oid).unwrap().unwrap();
+        assert_eq!(readback.id(), commit_oid.into());
+
+        let readback = writer.find_object(commit_oid).unwrap().unwrap();
+        assert_eq!(readback.kind(), Some(ObjectType::Commit));
+    }
+
+    pub fn roundtrip_tag(user: UserInfo, tree: gen::Tree, tag_name: RefString, message: &str) {
+        let writer = tmp::writer(user);
+        let tree_oid = tree.write(&writer).unwrap();
+        let tree = writer.find_tree(tree_oid).unwrap().unwrap();
+        let commit_oid = writer.write_commit(&tree, &[], message).unwrap();
+        let commit_object = writer.find_object(commit_oid).unwrap().unwrap();
+
+        let tag_oid = writer.write_tag(tag_name, &commit_object, message).unwrap();
+
+        let readback = writer.find_tag(tag_oid).unwrap().unwrap();
+        assert_eq!(tag_oid, readback.id().into());
+
+        let readback = writer.find_object(tag_oid).unwrap().unwrap();
+        assert_eq!(readback.kind(), Some(ObjectType::Tag));
+    }
+}
+
+proptest! {
+    #[test]
+    fn roundtrip_blob(user in gen::gen_signature(), bytes in gen::gen_bytes()) {
+        prop::roundtrip_blob(user, &bytes)
+    }
+
+
+    #[test]
+    fn roundtrip_tree(
+        user in gen::gen_signature(),
+        tree in gen::gen_tree(10),
+    ) {
+        prop::roundtrip_tree(user, tree)
+    }
+
+    #[test]
+    fn roundtrip_commit(
+        user in gen::gen_signature(),
+        tree in gen::gen_tree(10),
+        message in gen::trivial()
+    ) {
+        prop::roundtrip_commit(user, tree, &message)
+    }
+
+    #[test]
+    fn roundtrip_tag(
+        user in gen::gen_signature(),
+        tree in gen::gen_tree(10),
+        message in gen::trivial(),
+        tag_name in valid()
+    ) {
+        let tag_name = RefString::try_from(tag_name).unwrap();
+        prop::roundtrip_tag(user, tree, tag_name, &message);
+    }
+}

git-storage/t/src/tmp.rs

@@ -0,0 +1,14 @@
+use git2::Repository;
+use git_storage::{signature::UserInfo, Write};
+use test_helpers::tempdir::WithTmpDir;
+
+pub type TmpWriter = WithTmpDir<Write>;
+pub type TmpRepo = WithTmpDir<Repository>;
+
+pub fn writer(user: UserInfo) -> TmpWriter {
+    WithTmpDir::new(|path| {
+        Write::open(path, user)
+            .map_err(|e| std::io::Error::new(std::io::ErrorKind::Other, format!("{}", e)))
+    })
+    .unwrap()
+}

test/Cargo.toml

@@ -33,3 +33,8 @@ features = ["test"]
 [dev-dependencies.radicle-surf-test]
 path = "../radicle-surf/t"
 features = ["test"]
+
+[dev-dependencies.git-storage-test]
+path = "../git-storage/t"
+features = ["test"]
+