feat: add Store collection helpers and a HasId trait for Member
commit
7d369b0feat: add Store collection helpers and a HasId trait for Member
Every decomposed-ref collection (members today, checks/issues/comments
later) hand-formats its own {prefix}/{id} ref name at each load/store
call site. load_item/store_item/list_items centralize that, inheriting
store’s CAS-and-merge behavior for free. HasId lets a type that
legitimately stores its own key (a member’s principal is both the ref
segment and an allowed_signers field) avoid passing that key twice.
feat: add Store::load_item/store_item/list_items feat: add Store::HasId and Store::store_keyed feat: implement HasId for Member and use store_keyed/list_items in members.rs refactor: add members::load_with/load_all_with/store_with to thread a Store Assisted-by: Claude:claude-sonnet-5
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
crates/git-ents/src/members.rs
@@ -71,6 +71,12 @@
CertAuthority(String),
}
+impl git_store::HasId for Member {
+ fn id(&self) -> &str {
+ &self.principal
+ }
+}
+
impl Member {
/// A member trusting `keys` with no validity window.
#[must_use]
@@ -116,32 +122,48 @@
}
}
-/// Load the member named `username` in `repo`, or `None` when the ref is absent.
-pub fn load(repo: &Path, username: &str) -> Result<Option<Member>, git_store::Error> {
- git_store::Store::open(repo)?.load::<Member>(&member_ref(username))
+/// Load the member named `username` from an already-open `store`.
+pub fn load_with(
+ store: &git_store::Store,
+ username: &str,
+) -> Result<Option<Member>, git_store::Error> {
+ store.load_item(MEMBER_NS, username)
}
-/// Load every member recorded under [`MEMBER_NS`] in `repo`, newest ref first.
+/// Load the member named `username` in `repo`, or `None` when the ref is absent.
+pub fn load(repo: &Path, username: &str) -> Result<Option<Member>, git_store::Error> {
+ load_with(&git_store::Store::open(repo)?, username)
+}
+
+/// Load every member recorded under [`MEMBER_NS`] from an already-open
+/// `store`, newest ref first.
///
/// An empty result is a fresh server whose trust list has not been pushed yet. A
/// present but unreadable member ref is an error so callers can fail closed
/// rather than mistake corruption for "no members".
-pub fn load_all(repo: &Path) -> Result<Vec<Member>, git_store::Error> {
- let store = git_store::Store::open(repo)?;
- let mut members = Vec::new();
- for refname in store.list(&format!("{MEMBER_NS}/"))? {
- if let Some(member) = store.load::<Member>(&refname)? {
- members.push(member);
- }
- }
- Ok(members)
+pub fn load_all_with(store: &git_store::Store) -> Result<Vec<Member>, git_store::Error> {
+ Ok(store
+ .list_items::<Member>(MEMBER_NS)?
+ .into_iter()
+ .map(|(_id, member)| member)
+ .collect())
}
-/// Write `member` to its `refs/meta/member/<principal>` ref, replacing any prior
-/// value, as a new commit.
+/// Load every member recorded under [`MEMBER_NS`] in `repo`. See
+/// [`load_all_with`].
+pub fn load_all(repo: &Path) -> Result<Vec<Member>, git_store::Error> {
+ load_all_with(&git_store::Store::open(repo)?)
+}
+
+/// Write `member` to its `refs/meta/member/<principal>` ref, replacing any
+/// prior value, as a new commit, through an already-open `store`.
+pub fn store_with(store: &git_store::Store, member: &Member) -> Result<(), git_store::Error> {
+ store.store_keyed(MEMBER_NS, member, "Update member")
+}
+
+/// Write `member` to its `refs/meta/member/<principal>` ref. See [`store_with`].
pub fn store(repo: &Path, member: &Member) -> Result<(), git_store::Error> {
- git_store::Store::open(repo)?.store(&member_ref(&member.principal), member, "Update member")?;
- Ok(())
+ store_with(&git_store::Store::open(repo)?, member)
}
/// Drop every `revoked` fingerprint from `members`, returning the trust set the
crates/git-store/src/lib.rs
@@ -86,6 +86,19 @@
fn into_pair(self) -> (String, String);
}
+/// A document that legitimately stores its own collection key.
+///
+/// Most decomposed-ref collections must *not* implement this: an issue's
+/// stable key is its ref's genesis hash, never a stored field, and duplicating
+/// it inside the document would let the stored copy disagree with the ref. A
+/// member is the exception — its principal is both the ref segment and a
+/// field genuinely used elsewhere (rendering `allowed_signers`) — so `HasId`
+/// lets a caller store one without passing the principal twice.
+pub trait HasId {
+ /// The value's collection key — the segment its ref is stored under.
+ fn id(&self) -> &str;
+}
+
/// A repository's typed `refs/meta/*` store.
///
/// Refs are read and updated through the high-level [`gix`] API, while all
@@ -202,6 +215,58 @@
self.try_set_ref(refname, expected, commit)
}
+ /// Load the item `id` under the collection ref namespace `prefix`
+ /// (`{prefix}/{id}`), or `None` when its ref is absent. The thin wrapper
+ /// every decomposed-ref collection (members, checks, issues, comments, …)
+ /// shares instead of hand-formatting its own ref name.
+ pub fn load_item<T: for<'a> Facet<'a>>(
+ &self,
+ prefix: &str,
+ id: &str,
+ ) -> Result<Option<T>, Error> {
+ self.load(&format!("{prefix}/{id}"))
+ }
+
+ /// Store `value` as item `id` under the collection ref namespace `prefix`
+ /// (`{prefix}/{id}`), inheriting [`store`](Self::store)'s CAS-and-merge
+ /// behavior.
+ pub fn store_item<T: for<'a> Facet<'a>>(
+ &self,
+ prefix: &str,
+ id: &str,
+ value: &T,
+ message: &str,
+ ) -> Result<(), Error> {
+ self.store(&format!("{prefix}/{id}"), value, message)
+ }
+
+ /// Like [`store_item`](Self::store_item), but for a [`HasId`] value that
+ /// carries its own collection key, so the caller does not pass it twice.
+ pub fn store_keyed<T: for<'a> Facet<'a> + HasId>(
+ &self,
+ prefix: &str,
+ value: &T,
+ message: &str,
+ ) -> Result<(), Error> {
+ self.store_item(prefix, value.id(), value, message)
+ }
+
+ /// Every item under the collection ref namespace `prefix`, paired with the
+ /// id (the ref's last path segment) it was stored under, newest first.
+ pub fn list_items<T: for<'a> Facet<'a>>(
+ &self,
+ prefix: &str,
+ ) -> Result<Vec<(String, T)>, Error> {
+ let mut items = Vec::new();
+ for refname in self.list(&format!("{prefix}/"))? {
+ let id = refname.rsplit('/').next().unwrap_or(&refname).to_owned();
+ if let Some(value) = self.load::<T>(&refname)? {
+ items.push((id, value));
+ }
+ }
+ Ok(items)
+ }
+
/// 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.
@@ -490,6 +555,94 @@
assert_eq!(loaded, entries(&[("b", "2")]));
}
+ /// A minimal keyed item, used to exercise `load_item`/`store_item`/
+ /// `list_items`/`store_keyed`.
+ #[derive(Facet, Clone, Debug, PartialEq)]
+ struct Item {
+ id: String,
+ value: String,
+ }
+
+ impl HasId for Item {
+ fn id(&self) -> &str {
+ &self.id
+ }
+ }
+
+ #[test]
+ fn store_item_then_load_item_round_trips() {
+ let dir = repo();
+ let store = Store::open(dir.path()).unwrap();
+ let item = Item {
+ id: "a".into(),
+ value: "1".into(),
+ };
+ store
+ .store_item("refs/meta/items", "a", &item, "write")
+ .unwrap();
+ assert_eq!(
+ store.load_item::<Item>("refs/meta/items", "a").unwrap(),
+ Some(item)
+ );
+ assert_eq!(
+ store
+ .load_item::<Item>("refs/meta/items", "missing")
+ .unwrap(),
+ None
+ );
+ }
+
+ #[test]
+ fn store_keyed_uses_the_value_own_id() {
+ let dir = repo();
+ let store = Store::open(dir.path()).unwrap();
+ let item = Item {
+ id: "a".into(),
+ value: "1".into(),
+ };
+ store
+ .store_keyed("refs/meta/items", &item, "write")
+ .unwrap();
+ assert_eq!(
+ store.load_item::<Item>("refs/meta/items", "a").unwrap(),
+ Some(item)
+ );
+ }
+
+ #[test]
+ fn list_items_returns_every_item_keyed_by_id() {
+ let dir = repo();
+ let store = Store::open(dir.path()).unwrap();
+ store
+ .store_keyed(
+ "refs/meta/items",
+ &Item {
+ id: "a".into(),
+ value: "1".into(),
+ },
+ "write",
+ )
+ .unwrap();
+ store
+ .store_keyed(
+ "refs/meta/items",
+ &Item {
+ id: "b".into(),
+ value: "2".into(),
+ },
+ "write",
+ )
+ .unwrap();
+ let mut ids: Vec<String> = store
+ .list_items::<Item>("refs/meta/items")
+ .unwrap()
+ .into_iter()
+ .map(|(id, _item)| id)
+ .collect();
+ ids.sort();
+ assert_eq!(ids, vec!["a".to_owned(), "b".to_owned()]);
+ }
+
/// A small multi-field, multi-collection document used to exercise the
/// structural merge: two scalar fields plus a scalar-keyed map.
#[derive(Facet, Clone, Debug, PartialEq)]