refactor: index members with iddqd instead of a hand-kept BTreeMap
commit
b7d6211refactor: index members with iddqd instead of a hand-kept BTreeMap
BTreeMap<String, Member> duplicated the principal as both the map key
and a struct field, needing a .clone() to build and no guarantee the
two ever agreed. iddqd’s IdOrdMap borrows the key from the value itself.
deps: add iddqd refactor: replace load_all_indexed’s BTreeMap<String, Member> with iddqd::IdOrdMap<Member> Assisted-by: Claude:claude-sonnet-5
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
Cargo.lock
@@ -1025,6 +1025,7 @@
"clap",
"facet",
"git-store",
+ "iddqd",
"tempfile",
"thiserror",
]
Cargo.toml
@@ -45,6 +45,7 @@
gix-odb = "0.81"
gix-actor = "0.41"
gix-date = "0.15"
+iddqd = "0.4"
maud = { version = "0.27", features = ["axum"] }
rstest = "0.26"
tempfile = "3"
crates/git-ents/Cargo.toml
@@ -9,6 +9,7 @@
clap = { workspace = true }
facet = { workspace = true }
git-store = { workspace = true }
+iddqd = { workspace = true }
tempfile = { workspace = true }
thiserror = { workspace = true }
crates/git-ents/src/members.rs
@@ -127,6 +127,16 @@
}
}
+impl iddqd::IdOrdItem for Member {
+ type Key<'a> = &'a str;
+
+ fn key(&self) -> Self::Key<'_> {
+ &self.principal
+ }
+
+ iddqd::id_upcast!();
+}
+
impl Member {
/// A member trusting `keys` with no validity window, admin-registered.
#[must_use]
@@ -227,21 +237,21 @@
/// Prepares the batch path for lookups keyed by principal directly (there is
/// exactly one member per principal, unlike a signing key, which a member may
/// legitimately hold several of — that is why this indexes principals rather
-/// than a `Trust::Keys` bi-map). Not yet wired to any caller: the web layer's
-/// public-key lookup needs a different index (key → member), an O(m×k) linear
-/// scan that stays fine at current scale (see its own doc comment).
+/// than a `Trust::Keys` bi-map). An [`iddqd::IdOrdMap`] rather than a
+/// `BTreeMap<String, Member>` so the principal lives once, on `Member`
+/// itself, instead of also duplicated as a separately-maintained map key. Not
+/// yet wired to any caller: the web layer's public-key lookup needs a
+/// different index (key → member), an O(m×k) linear scan that stays fine at
+/// current scale (see its own doc comment).
pub fn load_all_indexed_with(
store: &git_store::Store,
-) -> Result<BTreeMap<String, Member>, git_store::Error> {
- Ok(load_all_with(store)?
- .into_iter()
- .map(|member| (member.principal.clone(), member))
- .collect())
+) -> Result<iddqd::IdOrdMap<Member>, git_store::Error> {
+ Ok(load_all_with(store)?.into_iter().collect())
}
/// Load every member recorded under [`MEMBER_NS`] in `repo`, keyed by
/// principal. See [`load_all_indexed_with`].
-pub fn load_all_indexed(repo: &Path) -> Result<BTreeMap<String, Member>, git_store::Error> {
+pub fn load_all_indexed(repo: &Path) -> Result<iddqd::IdOrdMap<Member>, git_store::Error> {
load_all_indexed_with(&git_store::Store::open(repo)?)
}