refactor: share meta-ref set plumbing via a MapDoc trait
commit
04f45c7refactor: share meta-ref set plumbing via a MapDoc trait
The signer set, the check set, and a run’s outcomes are each a single-field map of string pairs wrapped in a private document struct. The wrapper stays per-type so its field fixes the on-disk subtree name, but the open/load/store/empty-when-absent plumbing now lives once in git-store.
feat: add git_store::MapDoc with Store::load_entries/store_entries refactor: load/store signers through MapDoc refactor: load/store checks and runs through MapDoc test: cover MapDoc round-trip and replacement in git-store Assisted-by: Claude:claude-opus-4-8
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
Cargo.lock
@@ -1059,6 +1059,7 @@
"facet",
"facet-git-tree",
"gix",
+ "tempfile",
"thiserror",
]
crates/git-store/Cargo.toml
@@ -11,5 +11,8 @@
gix = { workspace = true }
thiserror = { workspace = true }
+[dev-dependencies]
+tempfile = { workspace = true }
+
[lints]
workspace = true
crates/git-ents/src/checks.rs
@@ -13,6 +13,7 @@
use std::path::Path;
use facet::Facet;
+use git_store::MapDoc as _;
/// The ref whose tree holds the configured check set.
pub const CHECKS_REF: &str = "refs/meta/checks";
@@ -24,6 +25,16 @@
checks: BTreeMap<String, String>,
}
+impl git_store::MapDoc for Checks {
+ fn from_entries(entries: BTreeMap<String, String>) -> Self {
+ Self { checks: entries }
+ }
+
+ fn into_entries(self) -> BTreeMap<String, String> {
+ self.checks
+ }
+}
+
/// One configured check recorded in [`CHECKS_REF`].
#[derive(Debug, Clone, PartialEq, Eq, Facet)]
pub struct Check {
@@ -47,12 +58,8 @@
/// been pushed yet. A present but unreadable ref is an error so callers can
/// distinguish corruption from "no checks configured".
pub fn load(repo: &Path) -> Result<Vec<Check>, Error> {
- let store = git_store::Store::open(repo)?;
- let Some(document) = store.load::<Checks>(CHECKS_REF)? else {
- return Ok(Vec::new());
- };
- Ok(document
- .checks
+ Ok(git_store::Store::open(repo)?
+ .load_entries::<Checks>(CHECKS_REF)?
.into_iter()
.map(|(name, command)| Check {
name,
@@ -64,13 +71,11 @@
/// Write `checks` to [`CHECKS_REF`], replacing any existing set, as a new
/// commit.
pub fn store(repo: &Path, checks: &[Check]) -> Result<(), Error> {
- let document = Checks {
- checks: checks
- .iter()
- .map(|check| (check.name.clone(), check.command.clone()))
- .collect(),
- };
- git_store::Store::open(repo)?.store(CHECKS_REF, &document, "Update checks")?;
+ let entries = checks
+ .iter()
+ .map(|check| (check.name.clone(), check.command.clone()))
+ .collect();
+ git_store::Store::open(repo)?.store_entries::<Checks>(CHECKS_REF, entries, "Update checks")?;
Ok(())
}
@@ -88,6 +93,16 @@
results: BTreeMap<String, String>,
}
+impl git_store::MapDoc for RunDoc {
+ fn from_entries(entries: BTreeMap<String, String>) -> Self {
+ Self { results: entries }
+ }
+
+ fn into_entries(self) -> BTreeMap<String, String> {
+ self.results
+ }
+}
+
/// One check's outcome within a [`Run`].
#[derive(Debug, Clone, PartialEq, Eq, Facet)]
pub struct RunOutcome {
@@ -163,7 +178,7 @@
.map(|(at, doc)| Run {
at,
results: doc
- .results
+ .into_entries()
.into_iter()
.map(|(name, outcome)| RunOutcome { name, outcome })
.collect(),
@@ -179,12 +194,12 @@
/// Build a [`RunDoc`] from a run's `outcomes`.
fn run_doc(outcomes: &[RunOutcome]) -> RunDoc {
- RunDoc {
- results: outcomes
+ RunDoc::from_entries(
+ outcomes
.iter()
.map(|outcome| (outcome.name.clone(), outcome.outcome.clone()))
.collect(),
- }
+ )
}
#[cfg(test)]
crates/git-ents/src/signers.rs
@@ -21,6 +21,16 @@
signers: BTreeMap<String, String>,
}
+impl git_store::MapDoc for Auth {
+ fn from_entries(entries: BTreeMap<String, String>) -> Self {
+ Self { signers: entries }
+ }
+
+ fn into_entries(self) -> BTreeMap<String, String> {
+ self.signers
+ }
+}
+
/// One authorized signer recorded in [`AUTH_REF`].
#[derive(Debug, Clone, PartialEq, Eq, Facet)]
pub struct Signer {
@@ -44,12 +54,8 @@
/// not been pushed yet. A present but unreadable ref is an error so callers can
/// fail closed rather than mistake corruption for "no signers".
pub fn load(repo: &Path) -> Result<Vec<Signer>, Error> {
- let store = git_store::Store::open(repo)?;
- let Some(auth) = store.load::<Auth>(AUTH_REF)? else {
- return Ok(Vec::new());
- };
- Ok(auth
- .signers
+ Ok(git_store::Store::open(repo)?
+ .load_entries::<Auth>(AUTH_REF)?
.into_iter()
.map(|(fingerprint, key)| Signer {
fingerprint,
@@ -60,13 +66,15 @@
/// Write `signers` to [`AUTH_REF`], replacing any existing set, as a new commit.
pub fn store(repo: &Path, signers: &[Signer]) -> Result<(), Error> {
- let auth = Auth {
- signers: signers
- .iter()
- .map(|signer| (signer.fingerprint.clone(), signer.key.clone()))
- .collect(),
- };
- git_store::Store::open(repo)?.store(AUTH_REF, &auth, "Update authorized signers")?;
+ let entries = signers
+ .iter()
+ .map(|signer| (signer.fingerprint.clone(), signer.key.clone()))
+ .collect();
+ git_store::Store::open(repo)?.store_entries::<Auth>(
+ AUTH_REF,
+ entries,
+ "Update authorized signers",
+ )?;
Ok(())
}
crates/git-store/src/lib.rs
@@ -9,6 +9,7 @@
//! check set, and the run log all share.
use std::cmp::Reverse;
+use std::collections::BTreeMap;
use std::path::Path;
use facet::Facet;
@@ -42,6 +43,19 @@
Object(String),
}
+/// A meta-ref document that is a single named map of string keys to string
+/// values — the shape the signer set, the check set, and a run's outcomes all
+/// share. The wrapping struct's one field fixes the on-disk subtree name
+/// (`signers/`, `checks/`, `results/`), so each document stays its own type;
+/// this trait is only the bridge that lets them share the load/store plumbing
+/// in [`Store::load_entries`] and [`Store::store_entries`].
+pub trait MapDoc: for<'a> Facet<'a> {
+ /// Wrap `entries` as the document.
+ fn from_entries(entries: BTreeMap<String, String>) -> Self;
+ /// The document's entries, consuming it.
+ fn into_entries(self) -> BTreeMap<String, String>;
+}
+
/// A repository's typed `refs/meta/*` store.
///
/// Refs are read and updated through the high-level [`gix`] API, while all
@@ -104,6 +118,26 @@
self.set_ref(refname, commit)
}
+ /// Load the [`MapDoc`] on `refname` as its `(key, value)` entries, or an
+ /// empty vec when the ref is absent. Centralizes the "missing ref reads
+ /// empty" policy the set documents share.
+ pub fn load_entries<T: MapDoc>(&self, refname: &str) -> Result<Vec<(String, String)>, Error> {
+ Ok(self
+ .load::<T>(refname)?
+ .map(|doc| doc.into_entries().into_iter().collect())
+ .unwrap_or_default())
+ }
+
+ /// Store `entries` as the [`MapDoc`] `T` on `refname` as a new commit.
+ pub fn store_entries<T: MapDoc>(
+ &self,
+ refname: &str,
+ entries: BTreeMap<String, String>,
+ message: &str,
+ ) -> Result<(), Error> {
+ self.store(refname, &T::from_entries(entries), message)
+ }
+
/// The documents on `refname`'s commit chain as `(committer date, value)`
/// pairs, newest first — one entry per commit, following first parents.
pub fn history<T: for<'a> Facet<'a>>(&self, refname: &str) -> Result<Vec<(u64, T)>, Error> {
@@ -217,3 +251,97 @@
parents: Vec<ObjectId>,
seconds: u64,
}
+
+#[cfg(test)]
+mod tests {
+ #![allow(
+ clippy::unwrap_used,
+ clippy::let_underscore_must_use,
+ reason = "unit test"
+ )]
+
+ use std::process::Command;
+
+ use super::*;
+
+ /// A single-field map document, the shape [`MapDoc`] abstracts over.
+ #[derive(Facet)]
+ struct Bag {
+ items: BTreeMap<String, String>,
+ }
+
+ impl MapDoc for Bag {
+ fn from_entries(entries: BTreeMap<String, String>) -> Self {
+ Self { items: entries }
+ }
+
+ fn into_entries(self) -> BTreeMap<String, String> {
+ self.items
+ }
+ }
+
+ fn repo() -> tempfile::TempDir {
+ let dir = tempfile::tempdir().unwrap();
+ let status = Command::new("git")
+ .arg("-C")
+ .arg(dir.path())
+ .args(["init", "-q"])
+ .status()
+ .unwrap();
+ assert!(status.success());
+ dir
+ }
+
+ fn entries(pairs: &[(&str, &str)]) -> BTreeMap<String, String> {
+ pairs
+ .iter()
+ .map(|(k, v)| ((*k).to_owned(), (*v).to_owned()))
+ .collect()
+ }
+
+ #[test]
+ fn absent_ref_loads_no_entries() {
+ let dir = repo();
+ let store = Store::open(dir.path()).unwrap();
+ assert!(
+ store
+ .load_entries::<Bag>("refs/meta/bag")
+ .unwrap()
+ .is_empty()
+ );
+ }
+
+ #[test]
+ fn store_entries_round_trips() {
+ let dir = repo();
+ let store = Store::open(dir.path()).unwrap();
+ let written = entries(&[("a", "1"), ("b", "2")]);
+ store
+ .store_entries::<Bag>("refs/meta/bag", written.clone(), "write")
+ .unwrap();
+ let loaded: BTreeMap<String, String> = store
+ .load_entries::<Bag>("refs/meta/bag")
+ .unwrap()
+ .into_iter()
+ .collect();
+ assert_eq!(loaded, written);
+ }
+
+ #[test]
+ fn store_entries_replaces_the_previous_set() {
+ let dir = repo();
+ let store = Store::open(dir.path()).unwrap();
+ store
+ .store_entries::<Bag>("refs/meta/bag", entries(&[("a", "1")]), "write")
+ .unwrap();
+ store
+ .store_entries::<Bag>("refs/meta/bag", entries(&[("b", "2")]), "write")
+ .unwrap();
+ let loaded: BTreeMap<String, String> = store
+ .load_entries::<Bag>("refs/meta/bag")
+ .unwrap()
+ .into_iter()
+ .collect();
+ assert_eq!(loaded, entries(&[("b", "2")]));
+ }
+}