refactor: thread a Store through config.rs/account.rs and server callers
commit
63ccf40refactor: thread a Store through config.rs/account.rs and server callers
Completes the _with-variant pattern for the two remaining modules that hand-rolled their own Store::open per call. pre_receive and the settings-edit handler each did two/three sequential opens for what is one logical operation; both now open a single Store and pass it through.
feat: add config::load_with/store_with/store_to_ref_with/store_to_ref_authored_with feat: add account::load_with/store_with/is_account_repo_with feat: add revocations::fingerprints_with refactor: share one Store across pre_receive’s member and revocation reads refactor: share one Store across edit_config’s member lookup and config read Assisted-by: Claude:claude-sonnet-5
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
Cargo.lock
@@ -1043,6 +1043,7 @@
"facet",
"getrandom",
"git-ents",
+ "git-store",
"gix-actor",
"gix-date",
"gix-hash",
crates/git-ents-server/Cargo.toml
@@ -16,6 +16,7 @@
clap_mangen = { workspace = true }
facet = { workspace = true }
getrandom = { workspace = true }
+git-store = { workspace = true }
gix-actor = { workspace = true }
gix-date = { workspace = true }
gix-hash = { workspace = true }
crates/git-ents-server/src/verify.rs
@@ -22,8 +22,9 @@
/// environment git populates for the hook.
pub fn pre_receive() -> Result<(), String> {
let repo = std::env::current_dir().map_err(|e| format!("cannot resolve repository: {e}"))?;
- let members =
- members::load_all(&repo).map_err(|e| format!("could not read authorized signers: {e}"))?;
+ let store = git_store::Store::open(&repo).map_err(|e| format!("cannot open store: {e}"))?;
+ let members = members::load_all_with(&store)
+ .map_err(|e| format!("could not read authorized signers: {e}"))?;
if members.is_empty() {
// No trust list pushed yet: stay open so the first signer can be added.
// Revocation is keyed on member refs existing, so revoking every member's
@@ -31,8 +32,8 @@
// bootstrap window.
return Ok(());
}
- let revoked =
- revocations::fingerprints(&repo).map_err(|e| format!("could not read revocations: {e}"))?;
+ let revoked = revocations::fingerprints_with(&store)
+ .map_err(|e| format!("could not read revocations: {e}"))?;
let authorized = members::without_revoked(members, &revoked);
let cert_oid = env("GIT_PUSH_CERT")
crates/git-ents/src/account.rs
@@ -29,20 +29,36 @@
pub created_at: u64,
}
-/// Load the account profile at [`ACCOUNT_REF`] in `repo`, or `None` when the ref
-/// is absent — i.e. when `repo` is not an account repo.
+/// Load the account profile at [`ACCOUNT_REF`] from an already-open `store`,
+/// or `None` when the ref is absent — i.e. when the repository is not an
+/// account repo.
+pub fn load_with(store: &git_store::Store) -> Result<Option<Account>, git_store::Error> {
+ store.load::<Account>(ACCOUNT_REF)
+}
+
+/// Load the account profile at [`ACCOUNT_REF`] in `repo`. See [`load_with`].
pub fn load(repo: &Path) -> Result<Option<Account>, git_store::Error> {
- git_store::Store::open(repo)?.load::<Account>(ACCOUNT_REF)
+ load_with(&git_store::Store::open(repo)?)
}
-/// Write `account` to [`ACCOUNT_REF`], replacing any existing value, as a new
-/// commit.
+/// Write `account` to [`ACCOUNT_REF`] through an already-open `store`,
+/// replacing any existing value as a new commit.
+pub fn store_with(store: &git_store::Store, account: &Account) -> Result<(), git_store::Error> {
+ store.store(ACCOUNT_REF, account, "Update account")
+}
+
+/// Write `account` to [`ACCOUNT_REF`]. See [`store_with`].
pub fn store(repo: &Path, account: &Account) -> Result<(), git_store::Error> {
- git_store::Store::open(repo)?.store(ACCOUNT_REF, account, "Update account")?;
- Ok(())
+ store_with(&git_store::Store::open(repo)?, account)
}
-/// Whether `repo` is an account repo — whether it carries [`ACCOUNT_REF`].
+/// Whether an already-open `store` is an account repo — whether it carries
+/// [`ACCOUNT_REF`].
+pub fn is_account_repo_with(store: &git_store::Store) -> Result<bool, git_store::Error> {
+ Ok(load_with(store)?.is_some())
+}
+
+/// Whether `repo` is an account repo. See [`is_account_repo_with`].
pub fn is_account_repo(repo: &Path) -> Result<bool, git_store::Error> {
Ok(load(repo)?.is_some())
}
crates/git-ents/src/config.rs
@@ -27,52 +27,76 @@
pub topics: Vec<String>,
}
-/// Load the configuration recorded at [`CONFIG_REF`] in `repo`.
+/// Load the configuration recorded at [`CONFIG_REF`] from an already-open
+/// `store`.
///
/// An absent ref yields [`Config::default`], as on a repository whose metadata
/// has not been set yet. A present but unreadable ref is an error so callers can
/// distinguish corruption from "no configuration set".
+pub fn load_with(store: &git_store::Store) -> Result<Config, git_store::Error> {
+ Ok(store.load::<Config>(CONFIG_REF)?.unwrap_or_default())
+}
+
+/// Load the configuration recorded at [`CONFIG_REF`] in `repo`. See
+/// [`load_with`].
pub fn load(repo: &Path) -> Result<Config, git_store::Error> {
- Ok(git_store::Store::open(repo)?
- .load::<Config>(CONFIG_REF)?
- .unwrap_or_default())
+ load_with(&git_store::Store::open(repo)?)
}
-/// Write `config` to [`CONFIG_REF`], replacing any existing value, as a new
-/// commit.
+/// Write `config` to [`CONFIG_REF`] through an already-open `store`,
+/// replacing any existing value as a new commit.
+pub fn store_with(store: &git_store::Store, config: &Config) -> Result<(), git_store::Error> {
+ store_to_ref_with(store, CONFIG_REF, config)
+}
+
+/// Write `config` to [`CONFIG_REF`]. See [`store_with`].
pub fn store(repo: &Path, config: &Config) -> Result<(), git_store::Error> {
- store_to_ref(repo, CONFIG_REF, config)
+ store_with(&git_store::Store::open(repo)?, config)
}
-/// Build the configuration commit on `refname` — chaining on that ref's own tip
-/// — without touching [`CONFIG_REF`].
+/// Build the configuration commit on `refname` — chaining on that ref's own
+/// tip — without touching [`CONFIG_REF`], through an already-open `store`.
///
/// The web write path stages an edit on a throwaway ref pointed at the current
/// config tip, then lands it onto [`CONFIG_REF`] through a signed push, so the
/// `pre-receive` gate judges the change rather than this writing the live ref
/// directly.
-pub fn store_to_ref(repo: &Path, refname: &str, config: &Config) -> Result<(), git_store::Error> {
- git_store::Store::open(repo)?.store(refname, config, "Update configuration")?;
- Ok(())
+pub fn store_to_ref_with(
+ store: &git_store::Store,
+ refname: &str,
+ config: &Config,
+) -> Result<(), git_store::Error> {
+ store.store(refname, config, "Update configuration")
}
-/// Like [`store_to_ref`], but recording `author` (a `(name, email)` pair) as the
-/// commit's author while the committer stays the git-ents system identity. The
-/// web write path uses this so an edit landed by the server still names the human
-/// who made it.
+/// Build the configuration commit on `refname` in `repo`. See
+/// [`store_to_ref_with`].
+pub fn store_to_ref(repo: &Path, refname: &str, config: &Config) -> Result<(), git_store::Error> {
+ store_to_ref_with(&git_store::Store::open(repo)?, refname, config)
+}
+
+/// Like [`store_to_ref_with`], but recording `author` (a `(name, email)` pair)
+/// as the commit's author while the committer stays the git-ents system
+/// identity. The web write path uses this so an edit landed by the server
+/// still names the human who made it.
+pub fn store_to_ref_authored_with(
+ store: &git_store::Store,
+ refname: &str,
+ config: &Config,
+ author: (&str, &str),
+) -> Result<(), git_store::Error> {
+ store.store_authored(refname, config, "Update configuration", author)
+}
+
+/// Like [`store_to_ref`], recording `author` as the commit's author. See
+/// [`store_to_ref_authored_with`].
pub fn store_to_ref_authored(
repo: &Path,
refname: &str,
config: &Config,
author: (&str, &str),
) -> Result<(), git_store::Error> {
- git_store::Store::open(repo)?.store_authored(
- refname,
- config,
- "Update configuration",
- author,
- )?;
- Ok(())
+ store_to_ref_authored_with(&git_store::Store::open(repo)?, refname, config, author)
}
#[cfg(test)]
crates/git-ents/src/revocations.rs
@@ -107,15 +107,21 @@
store_with(&git_store::Store::open(repo)?, revocations)
}
-/// The set of revoked fingerprints recorded at [`REVOKED_REF`] in `repo`, for the
-/// verifier to subtract from the trust set.
-pub fn fingerprints(repo: &Path) -> Result<BTreeSet<String>, git_store::Error> {
- Ok(load(repo)?
+/// The set of revoked fingerprints recorded at [`REVOKED_REF`] from an
+/// already-open `store`, for the verifier to subtract from the trust set.
+pub fn fingerprints_with(store: &git_store::Store) -> Result<BTreeSet<String>, git_store::Error> {
+ Ok(load_with(store)?
.into_iter()
.map(|revocation| revocation.fingerprint)
.collect())
}
+/// The set of revoked fingerprints recorded at [`REVOKED_REF`] in `repo`. See
+/// [`fingerprints_with`].
+pub fn fingerprints(repo: &Path) -> Result<BTreeSet<String>, git_store::Error> {
+ fingerprints_with(&git_store::Store::open(repo)?)
+}
+
#[cfg(test)]
mod tests {
#![allow(
crates/git-ents-server/src/web/write.rs
@@ -202,11 +202,12 @@
.clone()
};
- let username = member_for_public_key(repo, &public_key)
+ let store = git_store::Store::open(repo).map_err(|e| format!("cannot open store: {e}"))?;
+ let username = member_for_public_key_with(&store, &public_key)
.ok_or_else(|| "your web key is not a member of this repository".to_owned())?;
let mut config =
- git_ents::config::load(repo).map_err(|e| format!("could not read config: {e}"))?;
+ git_ents::config::load_with(&store).map_err(|e| format!("could not read config: {e}"))?;
config.description = edit.description.clone();
config.homepage = edit.homepage.clone();
config.topics = edit.topics.clone();
@@ -315,11 +316,15 @@
.success())
}
-/// The username of the member whose web key matches `public_key`, if any. The
-/// match is on the key type and body, ignoring any trailing comment.
-pub(super) fn member_for_public_key(repo: &Path, public_key: &str) -> Option<String> {
+/// The username of the member whose web key matches `public_key`, if any, from
+/// an already-open `store`. The match is on the key type and body, ignoring
+/// any trailing comment.
+pub(super) fn member_for_public_key_with(
+ store: &git_store::Store,
+ public_key: &str,
+) -> Option<String> {
let wanted = normalize_key(public_key);
- let members = git_ents::members::load_all(repo).ok()?;
+ let members = git_ents::members::load_all_with(store).ok()?;
members.into_iter().find_map(|member| {
member
.keys()
@@ -329,6 +334,12 @@
})
}
+/// The username of the member whose web key matches `public_key`, if any. See
+/// [`member_for_public_key_with`].
+pub(super) fn member_for_public_key(repo: &Path, public_key: &str) -> Option<String> {
+ member_for_public_key_with(&git_store::Store::open(repo).ok()?, public_key)
+}
+
/// A public key reduced to its type and body, dropping the comment so two lines
/// for the same key compare equal.
fn normalize_key(line: &str) -> String {