steid

feat: create_repo use case

Owner-only creation of a repository as both a record and a bare repo on disk, with the
compensating transaction from architecture.md: init the directory, save the row, remove
the directory again if the save fails.

Validation precedes both writes, so a bad name leaves no trace in either place — there
are tests for that rather than a comment claiming it. The name normalises on the way in
and the created `Repository` is returned rather than `()`, because someone who typed
`MyRepo` has to be redirected to `myrepo` and only the return value knows that.

Two things the compensation needed reasoning about, both now commented where they live:
deleting by path is safe because `init_bare` just proved nothing was there, so the
loser of a concurrent create can never delete the winner's repo; and the cleanup is
best-effort, because replacing the error that caused it with the cleanup's own error
would hide the cause.

An occupied path with no matching row — an orphan from a create that died mid-way —
is reported to the visitor as "that name is taken". True from outside, and there is no
logging story yet for the operator signal to land in; recorded in current.md against
the reconciliation sweep that fixes it properly.

`is_org_owner` moves to `application/authz.rs` on its second caller. Owner-ness gates
the profile edit, this, and later PATs and push; two copies of an authorization
predicate drift, and they drift open.

`InMemoryGitStorage` joins `DiskGitStorage`, enforcing the same `AlreadyExists` rule —
a permissive fake would let this pass while the real adapter refused. Sixteen new tests
run against it; one runs the real adapter over tempfile to prove the row and the
directory both actually appear.

174 tests, clippy clean.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

JamesPatrickGill authored 24 days agoparent19fbf90Browse files11e7a39711eb9efa0667d854785015bbc2e2fb41

7 files changed+663 −23

plans/current.md+11 −1View file

		@@ -22,7 +22,7 @@ problem twice as interesting, so not in the first pass.
22	22	- [x] Infrastructure: in-memory + SQLite implementations, migration
23	23	- [x] Application: `GitStorage` port — `init_bare`, `remove`, `repo_path`
24	24	- [x] Infrastructure: `DiskGitStorage`, shelling out to `git init --bare`
25		−- [ ] Application: `create_repo` use case — owner only, validates, creates record and
	25	+- [x] Application: `create_repo` use case — owner only, validates, creates record and
26	26	bare repo
27	27	- [ ] Application: `list_repos` / `view_repo` read models — visibility-aware
28	28	- [ ] Web: `/{handle}/repos/new` form, `/{handle}/repos/{name}` page
		@@ -61,6 +61,16 @@ Nothing open. `GitStorage`'s shape and how git is invoked are recorded in
61	61
62	62	### Watch for
63	63
	64	+- An orphaned directory is indistinguishable from a duplicate to the visitor.
	65	+ `create_repo` maps `GitStorageError::AlreadyExists` to "that name is taken", which is
	66	+ true from outside but hides the inconsistency from the operator. There is no logging
	67	+ story yet for it to surface in. The durable fix is the reconciliation sweep in
	68	+ [architecture.md](architecture.md#db-plus-filesystem-writes).
	69	+- The duplicate check races. Two concurrent creates of the same name can both pass
	70	+ `find_by_org_and_name`; the loser is then stopped by `init_bare` or, failing that, by
	71	+ the `unique (org_id, name)` constraint — which surfaces as an opaque storage error
	72	+ rather than "name taken". Correct, just ugly, and single-user for now.
	73	+
64	74	- The database and the filesystem cannot share a transaction. Creating a repo
65	75	writes a row and a directory. Neither previous attempt solved this properly — see
66	76	[architecture.md](architecture.md#db-plus-filesystem-writes). A compensating delete is

plans/progress.md+14 −1View file

		@@ -2,7 +2,7 @@
2	2
3	3	## This attempt (#3, Topcoat)
4	4
5		−158 tests. Active milestone in [current.md](current.md).
	5	+174 tests. Active milestone in [current.md](current.md).
6	6
7	7	### Milestone 0 — Skeleton · done
8	8
		@@ -134,6 +134,19 @@ it. How git is invoked is recorded in
134	134	- `tempfile` for test fixtures, not `target/`. Parallel-safe by construction and
135	135	self-cleaning on panic. Debris under `target/` would be actively harmful here, since
136	136	`init_bare` refuses a path that already exists.
	137	+- `is_org_owner` moved to `application/authz.rs` on its second caller. Owner-ness
	138	+ gates the profile edit, repo creation, and later PATs and push; two copies of an
	139	+ authorization predicate drift, and the direction they drift is open.
	140	+- The compensating transaction is safe to do by path. `remove` after a failed save
	141	+ can only ever delete what `init_bare` just created, because the loser of a concurrent
	142	+ create never gets past `init_bare`. Non-obvious enough that it is commented in the
	143	+ code as well as here.
	144	+- Compensation is best-effort. If the removal also fails, the caller still gets the
	145	+ error that started it — an orphaned directory is the documented failure mode, and
	146	+ replacing the real error with the cleanup's error would hide the cause.
	147	+- `InMemoryGitStorage` enforces `AlreadyExists` too. A permissive fake would let
	148	+ `create_repo` pass while the real adapter refused. The fake mirroring the rule is the
	149	+ point of having two implementations.
137	150	- `Repository::description` stays. Added unrequested and flagged; kept on review
138	151	because this milestone's own "Done when" puts repositories on the profile, which
139	152	makes it a consumer inside the milestone rather than speculation. Worth noting the

src/application/authz.rs+29 −0View file

		@@ -0,0 +1,29 @@
	1	+//! Authorization predicates shared between use cases.
	2	+//!
	3	+//! One answer per question, in one place. A predicate copied into a second use case
	4	+//! is a predicate that will eventually disagree with itself — and an authorization
	5	+//! check that disagrees with itself fails open somewhere.
	6	+
	7	+use crate::domain::{Actor, Organization, Role, repository::MembershipRepository};
	8	+
	9	+use super::error::Result;
	10	+
	11	+/// Whether the actor owns this organisation.
	12	+///
	13	+/// Owner is strictly stronger than membership: a member may read what they can see,
	14	+/// but editing the profile, creating a repository, and pushing are all owner-only.
	15	+/// "Signed in" quietly becoming "allowed" is the usual way this goes wrong.
	16	+pub(crate) async fn is_org_owner(
	17	+ org: &Organization,
	18	+ actor: &Actor,
	19	+ memberships: &impl MembershipRepository,
	20	+) -> Result<bool> {
	21	+ let Some(user_id) = actor.user_id() else {
	22	+ return Ok(false);
	23	+ };
	24	+
	25	+ Ok(memberships
	26	+ .find(&org.id, user_id)
	27	+ .await?
	28	+ .is_some_and(\|membership\| membership.role == Role::Owner))
	29	+}

src/application/mod.rs+3 −0View file

		@@ -3,6 +3,7 @@
3	3	//! Every use case takes an actor or a credential plus the ports it needs, and enforces
4	4	//! the rules before any side effect. Nothing here knows about HTTP or Topcoat.
5	5
	6	+pub(crate) mod authz;
6	7	pub mod claim;
7	8	pub mod config;
8	9	pub mod error;
		@@ -10,6 +11,7 @@ pub mod identity;
10	11	pub mod login;
11	12	pub mod port;
12	13	pub mod profile;
	14	+pub mod repo;
13	15	pub mod session;
14	16
15	17	pub use claim::{OwnerSpec, claim_instance, is_claimed};
		@@ -18,4 +20,5 @@ pub use error::{Error, Result};
18	20	pub use identity::{Identity, describe_identity};
19	21	pub use login::login;
20	22	pub use profile::{PublicProfile, update_profile, view_profile};
	23	+pub use repo::{NewRepo, create_repo};
21	24	pub use session::{SESSION_LIFETIME, end_session, record_session, resolve_actor, sweep_expired};

src/application/profile.rs+5 −21View file

		@@ -1,9 +1,9 @@
1	1	use crate::domain::{
2		− Actor, DomainError, OrgName, Organization, Role,
	2	+ Actor, DomainError, OrgName, Organization,
3	3	repository::{MembershipRepository, OrgRepository},
4	4	};
5	5
6		−use super::error::Result;
	6	+use super::{authz::is_org_owner, error::Result};
7	7
8	8	/// A profile as anyone may see it.
9	9	///
		@@ -58,26 +58,10 @@ pub async fn view_profile(
58	58
59	59	Ok(Some(PublicProfile::of(
60	60	&org,
61		− is_owner(&org, actor, memberships).await?,
	61	+ is_org_owner(&org, actor, memberships).await?,
62	62	)))
63	63	}
64	64
65		−/// Whether the actor owns this organisation.
66		−async fn is_owner(
67		− org: &Organization,
68		− actor: &Actor,
69		− memberships: &impl MembershipRepository,
70		−) -> Result<bool> {
71		− let Some(user_id) = actor.user_id() else {
72		− return Ok(false);
73		− };
74		−
75		− Ok(memberships
76		− .find(&org.id, user_id)
77		− .await?
78		− .is_some_and(\|membership\| membership.role == Role::Owner))
79		−}
80		−
81	65	/// Edits a profile's display name and bio.
82	66	///
83	67	/// Authorization lives here, not in the page: the web form and any future `/api`
		@@ -95,7 +79,7 @@ pub async fn update_profile(
95	79	return Err(DomainError::NotFound { entity: "profile" }.into());
96	80	};
97	81
98		− if !is_owner(&org, actor, memberships).await? {
	82	+ if !is_org_owner(&org, actor, memberships).await? {
99	83	return Err(DomainError::Forbidden.into());
100	84	}
101	85
		@@ -110,7 +94,7 @@ pub async fn update_profile(
110	94	mod tests {
111	95	use super::*;
112	96	use crate::{
113		− domain::{Membership, MembershipId, OrgId, UserId},
	97	+ domain::{Membership, MembershipId, OrgId, Role, UserId},
114	98	infrastructure::repository::{InMemoryMembershipRepo, InMemoryOrgRepo},
115	99	};
116	100

src/application/repo.rs+539 −0View file

		@@ -0,0 +1,539 @@
	1	+use crate::domain::{
	2	+ Actor, DomainError, OrgName, RepoId, Repository, Visibility,
	3	+ repository::{MembershipRepository, OrgRepository, RepoRepository},
	4	+};
	5	+
	6	+use super::{
	7	+ authz::is_org_owner,
	8	+ error::{Error, Result},
	9	+ port::{GitStorage, GitStorageError},
	10	+};
	11	+
	12	+/// The repository a visitor is asking to create.
	13	+///
	14	+/// Raw input: `name` is whatever was typed, and normalising it is
	15	+/// [`Repository::new`]'s job, not the caller's.
	16	+#[derive(Debug, Clone, PartialEq, Eq)]
	17	+pub struct NewRepo {
	18	+ pub name: String,
	19	+ pub description: Option<String>,
	20	+ pub visibility: Visibility,
	21	+}
	22	+
	23	+/// Creates a repository as a record and as a bare repo on disk.
	24	+///
	25	+/// Owner only. A member's read access is not permission to add to someone's portfolio.
	26	+///
	27	+/// Returns the created [`Repository`] rather than `()` because the name normalises on
	28	+/// the way in — someone who typed `MyRepo` has to be redirected to `myrepo`, and only
	29	+/// the returned value knows that.
	30	+///
	31	+/// The two writes cannot share a transaction. The bare repo is created first and
	32	+/// removed again if the record fails to save, per
	33	+/// `plans/architecture.md#db-plus-filesystem-writes`. If the process dies between them
	34	+/// the directory is orphaned; that hole is known and unclosed.
	35	+pub async fn create_repo(
	36	+ actor: &Actor,
	37	+ handle: &OrgName,
	38	+ spec: &NewRepo,
	39	+ orgs: &impl OrgRepository,
	40	+ memberships: &impl MembershipRepository,
	41	+ repos: &impl RepoRepository,
	42	+ storage: &impl GitStorage,
	43	+) -> Result<Repository> {
	44	+ let Some(org) = orgs.find_by_name(handle).await? else {
	45	+ return Err(DomainError::NotFound {
	46	+ entity: "repository owner",
	47	+ }
	48	+ .into());
	49	+ };
	50	+
	51	+ if !is_org_owner(&org, actor, memberships).await? {
	52	+ return Err(DomainError::Forbidden.into());
	53	+ }
	54	+
	55	+ // Validated before anything is written, so a bad name leaves neither a row nor a
	56	+ // directory behind.
	57	+ let repo = Repository::new(
	58	+ RepoId::generate(),
	59	+ org.id.clone(),
	60	+ spec.name.clone(),
	61	+ spec.description.clone(),
	62	+ spec.visibility,
	63	+ )?;
	64	+
	65	+ if repos
	66	+ .find_by_org_and_name(&org.id, &repo.name)
	67	+ .await?
	68	+ .is_some()
	69	+ {
	70	+ return Err(taken());
	71	+ }
	72	+
	73	+ storage
	74	+ .init_bare(handle, &repo.name)
	75	+ .await
	76	+ .map_err(\|error\| {
	77	+ match error {
	78	+ // No record, but something is already on disk: an orphan from a create that
	79	+ // died between the two writes. The name really is unavailable, so this is
	80	+ // what the visitor is told — at the cost of an orphan being
	81	+ // indistinguishable from a duplicate from the outside. The durable fix is
	82	+ // the reconciliation sweep noted in architecture.md.
	83	+ GitStorageError::AlreadyExists => taken(),
	84	+ other => Error::GitStorage(other),
	85	+ }
	86	+ })?;
	87	+
	88	+ if let Err(error) = repos.save(&repo).await {
	89	+ // Compensation. Safe to delete by path because `init_bare` just proved nothing
	90	+ // was there — this can never remove a repository that won a race, because the
	91	+ // loser of that race never gets past `init_bare`.
	92	+ //
	93	+ // Best effort: if the removal also fails the caller still needs the error that
	94	+ // started this, and an orphaned directory is the documented failure mode.
	95	+ let _ = storage.remove(handle, &repo.name).await;
	96	+ return Err(error.into());
	97	+ }
	98	+
	99	+ Ok(repo)
	100	+}
	101	+
	102	+fn taken() -> Error {
	103	+ DomainError::AlreadyExists {
	104	+ entity: "repository",
	105	+ }
	106	+ .into()
	107	+}
	108	+
	109	+#[cfg(test)]
	110	+mod tests {
	111	+ use super::*;
	112	+ use crate::{
	113	+ domain::{
	114	+ Membership, MembershipId, OrgId, Organization, RepoName, Role, UserId,
	115	+ repository::{RepositoryError, RepositoryResult},
	116	+ },
	117	+ infrastructure::{
	118	+ git::{DiskGitStorage, InMemoryGitStorage},
	119	+ repository::{InMemoryMembershipRepo, InMemoryOrgRepo, InMemoryRepoRepo},
	120	+ },
	121	+ };
	122	+
	123	+ /// A `RepoRepository` whose `save` always fails, for exercising compensation.
	124	+ ///
	125	+ /// Test-local on purpose: fault injection does not belong in the shared fake, where
	126	+ /// every other test would have to know about it.
	127	+ #[derive(Debug, Default)]
	128	+ struct FailingRepoRepo;
	129	+
	130	+ impl RepoRepository for FailingRepoRepo {
	131	+ async fn find_by_id(&self, _id: &RepoId) -> RepositoryResult<Option<Repository>> {
	132	+ Ok(None)
	133	+ }
	134	+
	135	+ async fn find_by_org_and_name(
	136	+ &self,
	137	+ _org_id: &OrgId,
	138	+ _name: &RepoName,
	139	+ ) -> RepositoryResult<Option<Repository>> {
	140	+ Ok(None)
	141	+ }
	142	+
	143	+ async fn list_by_org(&self, _org_id: &OrgId) -> RepositoryResult<Vec<Repository>> {
	144	+ Ok(Vec::new())
	145	+ }
	146	+
	147	+ async fn save(&self, _repo: &Repository) -> RepositoryResult<()> {
	148	+ Err(RepositoryError::backend("save failed on purpose"))
	149	+ }
	150	+ }
	151	+
	152	+ struct Fixture {
	153	+ orgs: InMemoryOrgRepo,
	154	+ memberships: InMemoryMembershipRepo,
	155	+ repos: InMemoryRepoRepo,
	156	+ storage: InMemoryGitStorage,
	157	+ owner: Actor,
	158	+ member: Actor,
	159	+ stranger: Actor,
	160	+ handle: OrgName,
	161	+ }
	162	+
	163	+ async fn fixture() -> Fixture {
	164	+ let orgs = InMemoryOrgRepo::new();
	165	+ let memberships = InMemoryMembershipRepo::new();
	166	+
	167	+ let org = Organization::new(OrgId::generate(), "acme", None).expect("valid org");
	168	+ orgs.save(&org).await.expect("save org");
	169	+
	170	+ let owner = UserId::generate();
	171	+ let member = UserId::generate();
	172	+
	173	+ for (user, role) in [(&owner, Role::Owner), (&member, Role::Member)] {
	174	+ memberships
	175	+ .save(&Membership::new(
	176	+ MembershipId::generate(),
	177	+ org.id.clone(),
	178	+ user.clone(),
	179	+ role,
	180	+ ))
	181	+ .await
	182	+ .expect("save membership");
	183	+ }
	184	+
	185	+ Fixture {
	186	+ orgs,
	187	+ memberships,
	188	+ repos: InMemoryRepoRepo::new(),
	189	+ storage: InMemoryGitStorage::new(),
	190	+ owner: Actor::User(owner),
	191	+ member: Actor::User(member),
	192	+ stranger: Actor::User(UserId::generate()),
	193	+ handle: org.name,
	194	+ }
	195	+ }
	196	+
	197	+ fn spec(name: &str) -> NewRepo {
	198	+ NewRepo {
	199	+ name: name.to_owned(),
	200	+ description: None,
	201	+ visibility: Visibility::Public,
	202	+ }
	203	+ }
	204	+
	205	+ impl Fixture {
	206	+ async fn create(&self, actor: &Actor, spec: &NewRepo) -> Result<Repository> {
	207	+ create_repo(
	208	+ actor,
	209	+ &self.handle,
	210	+ spec,
	211	+ &self.orgs,
	212	+ &self.memberships,
	213	+ &self.repos,
	214	+ &self.storage,
	215	+ )
	216	+ .await
	217	+ }
	218	+ }
	219	+
	220	+ #[tokio::test]
	221	+ async fn the_owner_creates_a_record_and_a_bare_repo() {
	222	+ let f = fixture().await;
	223	+
	224	+ let repo = f
	225	+ .create(&f.owner, &spec("steid"))
	226	+ .await
	227	+ .expect("should create");
	228	+
	229	+ assert_eq!(repo.name.as_str(), "steid");
	230	+ assert!(
	231	+ f.repos
	232	+ .find_by_org_and_name(&repo.org_id, &repo.name)
	233	+ .await
	234	+ .expect("lookup")
	235	+ .is_some()
	236	+ );
	237	+ assert!(f.storage.contains(&f.handle, &repo.name));
	238	+ }
	239	+
	240	+ #[tokio::test]
	241	+ async fn an_anonymous_visitor_is_refused() {
	242	+ let f = fixture().await;
	243	+
	244	+ let error = f
	245	+ .create(&Actor::Anonymous, &spec("steid"))
	246	+ .await
	247	+ .expect_err("should refuse");
	248	+
	249	+ assert!(matches!(error, Error::Domain(DomainError::Forbidden)));
	250	+ assert!(f.storage.is_empty());
	251	+ }
	252	+
	253	+ #[tokio::test]
	254	+ async fn a_signed_in_stranger_is_refused() {
	255	+ // Signed in is not the same as allowed.
	256	+ let f = fixture().await;
	257	+
	258	+ let error = f
	259	+ .create(&f.stranger, &spec("steid"))
	260	+ .await
	261	+ .expect_err("should refuse");
	262	+
	263	+ assert!(matches!(error, Error::Domain(DomainError::Forbidden)));
	264	+ assert!(f.storage.is_empty());
	265	+ }
	266	+
	267	+ #[tokio::test]
	268	+ async fn a_non_owner_member_is_refused() {
	269	+ // Membership is read access, not permission to add to someone's portfolio.
	270	+ let f = fixture().await;
	271	+
	272	+ let error = f
	273	+ .create(&f.member, &spec("steid"))
	274	+ .await
	275	+ .expect_err("should refuse");
	276	+
	277	+ assert!(matches!(error, Error::Domain(DomainError::Forbidden)));
	278	+ assert!(f.storage.is_empty());
	279	+ }
	280	+
	281	+ #[tokio::test]
	282	+ async fn an_unknown_handle_is_not_found() {
	283	+ let f = fixture().await;
	284	+ let missing = OrgName::new("nobody").expect("valid handle");
	285	+
	286	+ let error = create_repo(
	287	+ &f.owner,
	288	+ &missing,
	289	+ &spec("steid"),
	290	+ &f.orgs,
	291	+ &f.memberships,
	292	+ &f.repos,
	293	+ &f.storage,
	294	+ )
	295	+ .await
	296	+ .expect_err("should not find");
	297	+
	298	+ assert!(matches!(
	299	+ error,
	300	+ Error::Domain(DomainError::NotFound {
	301	+ entity: "repository owner"
	302	+ })
	303	+ ));
	304	+ }
	305	+
	306	+ #[tokio::test]
	307	+ async fn an_invalid_name_writes_nothing() {
	308	+ // Validation precedes both side effects, so a rejected name leaves no trace.
	309	+ let f = fixture().await;
	310	+
	311	+ let error = f
	312	+ .create(&f.owner, &spec("../escape"))
	313	+ .await
	314	+ .expect_err("should reject");
	315	+
	316	+ assert!(matches!(
	317	+ error,
	318	+ Error::Domain(DomainError::Validation { .. })
	319	+ ));
	320	+ assert!(f.storage.is_empty());
	321	+ }
	322	+
	323	+ #[tokio::test]
	324	+ async fn a_duplicate_name_is_refused_and_changes_nothing() {
	325	+ let f = fixture().await;
	326	+ let first = f
	327	+ .create(&f.owner, &spec("steid"))
	328	+ .await
	329	+ .expect("should create");
	330	+
	331	+ let error = f
	332	+ .create(&f.owner, &spec("steid"))
	333	+ .await
	334	+ .expect_err("should refuse");
	335	+
	336	+ assert!(matches!(
	337	+ error,
	338	+ Error::Domain(DomainError::AlreadyExists {
	339	+ entity: "repository"
	340	+ })
	341	+ ));
	342	+ assert_eq!(f.storage.len(), 1, "the existing repo should be untouched");
	343	+ assert_eq!(
	344	+ f.repos
	345	+ .find_by_org_and_name(&first.org_id, &first.name)
	346	+ .await
	347	+ .expect("lookup")
	348	+ .expect("still there")
	349	+ .id,
	350	+ first.id
	351	+ );
	352	+ }
	353	+
	354	+ #[tokio::test]
	355	+ async fn a_duplicate_is_caught_case_insensitively() {
	356	+ // The name normalises, so `Steid` and `steid` are the same repository.
	357	+ let f = fixture().await;
	358	+ f.create(&f.owner, &spec("steid"))
	359	+ .await
	360	+ .expect("should create");
	361	+
	362	+ let error = f
	363	+ .create(&f.owner, &spec("Steid"))
	364	+ .await
	365	+ .expect_err("should refuse");
	366	+
	367	+ assert!(matches!(
	368	+ error,
	369	+ Error::Domain(DomainError::AlreadyExists {
	370	+ entity: "repository"
	371	+ })
	372	+ ));
	373	+ }
	374	+
	375	+ #[tokio::test]
	376	+ async fn an_orphaned_directory_reads_as_a_taken_name() {
	377	+ // No record, but the path is occupied — a create that died between the writes.
	378	+ // The visitor is told the name is taken, because from outside it is.
	379	+ let f = fixture().await;
	380	+ let name = RepoName::new("steid").expect("valid");
	381	+ f.storage
	382	+ .init_bare(&f.handle, &name)
	383	+ .await
	384	+ .expect("orphan the directory");
	385	+
	386	+ let error = f
	387	+ .create(&f.owner, &spec("steid"))
	388	+ .await
	389	+ .expect_err("should refuse");
	390	+
	391	+ assert!(matches!(
	392	+ error,
	393	+ Error::Domain(DomainError::AlreadyExists {
	394	+ entity: "repository"
	395	+ })
	396	+ ));
	397	+ }
	398	+
	399	+ #[tokio::test]
	400	+ async fn the_name_is_normalised_in_what_comes_back() {
	401	+ // The caller redirects using this, so it has to be the stored form.
	402	+ let f = fixture().await;
	403	+
	404	+ let repo = f
	405	+ .create(&f.owner, &spec(" MyRepo "))
	406	+ .await
	407	+ .expect("should create");
	408	+
	409	+ assert_eq!(repo.name.as_str(), "myrepo");
	410	+ assert!(f.storage.contains(&f.handle, &repo.name));
	411	+ }
	412	+
	413	+ #[tokio::test]
	414	+ async fn visibility_and_description_are_carried_through() {
	415	+ let f = fixture().await;
	416	+
	417	+ let repo = f
	418	+ .create(
	419	+ &f.owner,
	420	+ &NewRepo {
	421	+ name: "steid".to_owned(),
	422	+ description: Some(" A gitforge. ".to_owned()),
	423	+ visibility: Visibility::Private,
	424	+ },
	425	+ )
	426	+ .await
	427	+ .expect("should create");
	428	+
	429	+ assert_eq!(repo.visibility, Visibility::Private);
	430	+ assert_eq!(repo.description.as_deref(), Some("A gitforge."));
	431	+ }
	432	+
	433	+ #[tokio::test]
	434	+ async fn a_blank_description_is_stored_as_unset() {
	435	+ let f = fixture().await;
	436	+
	437	+ let repo = f
	438	+ .create(
	439	+ &f.owner,
	440	+ &NewRepo {
	441	+ name: "steid".to_owned(),
	442	+ description: Some(" ".to_owned()),
	443	+ visibility: Visibility::Public,
	444	+ },
	445	+ )
	446	+ .await
	447	+ .expect("should create");
	448	+
	449	+ assert_eq!(repo.description, None);
	450	+ }
	451	+
	452	+ #[tokio::test]
	453	+ async fn a_failed_save_removes_the_bare_repo() {
	454	+ // The compensating transaction. Without it every failed insert leaks a
	455	+ // directory that then blocks the name forever.
	456	+ let f = fixture().await;
	457	+
	458	+ let error = create_repo(
	459	+ &f.owner,
	460	+ &f.handle,
	461	+ &spec("steid"),
	462	+ &f.orgs,
	463	+ &f.memberships,
	464	+ &FailingRepoRepo,
	465	+ &f.storage,
	466	+ )
	467	+ .await
	468	+ .expect_err("should fail");
	469	+
	470	+ assert!(matches!(error, Error::Repository(_)));
	471	+ assert!(
	472	+ f.storage.is_empty(),
	473	+ "the bare repo should have been compensated away"
	474	+ );
	475	+ }
	476	+
	477	+ #[tokio::test]
	478	+ async fn a_successful_save_keeps_the_bare_repo() {
	479	+ // The other half of the above: compensation must not fire on the happy path.
	480	+ let f = fixture().await;
	481	+
	482	+ let repo = f
	483	+ .create(&f.owner, &spec("steid"))
	484	+ .await
	485	+ .expect("should create");
	486	+
	487	+ assert!(f.storage.contains(&f.handle, &repo.name));
	488	+ }
	489	+
	490	+ #[tokio::test]
	491	+ async fn a_compensated_name_can_be_created_again() {
	492	+ let f = fixture().await;
	493	+ let _ = create_repo(
	494	+ &f.owner,
	495	+ &f.handle,
	496	+ &spec("steid"),
	497	+ &f.orgs,
	498	+ &f.memberships,
	499	+ &FailingRepoRepo,
	500	+ &f.storage,
	501	+ )
	502	+ .await;
	503	+
	504	+ f.create(&f.owner, &spec("steid"))
	505	+ .await
	506	+ .expect("the name should be free again");
	507	+ }
	508	+
	509	+ /// The one test that runs the real adapter. Everything above proves the use case's
	510	+ /// logic against a fake; this proves the record and the directory actually both
	511	+ /// appear when it is wired to disk.
	512	+ #[tokio::test]
	513	+ async fn against_real_disk_storage_both_the_row_and_the_repo_appear() {
	514	+ let f = fixture().await;
	515	+ let dir = tempfile::TempDir::new().expect("temp dir");
	516	+ let storage = DiskGitStorage::new(dir.path());
	517	+
	518	+ let repo = create_repo(
	519	+ &f.owner,
	520	+ &f.handle,
	521	+ &spec("steid"),
	522	+ &f.orgs,
	523	+ &f.memberships,
	524	+ &f.repos,
	525	+ &storage,
	526	+ )
	527	+ .await
	528	+ .expect("should create");
	529	+
	530	+ assert!(
	531	+ f.repos
	532	+ .find_by_id(&repo.id)
	533	+ .await
	534	+ .expect("lookup")
	535	+ .is_some()
	536	+ );
	537	+ assert!(dir.path().join("acme").join("steid.git").is_dir());
	538	+ }
	539	+}

src/infrastructure/git.rs+62 −0View file

		@@ -5,10 +5,12 @@
5	5	//! belong here too rather than growing a second recipe.
6	6
7	7	use std::{
	8	+ collections::HashSet,
8	9	ffi::OsStr,
9	10	io,
10	11	path::PathBuf,
11	12	process::{Output, Stdio},
	13	+ sync::{Arc, Mutex},
12	14	};
13	15
14	16	use tokio::process::Command;
		@@ -145,6 +147,66 @@ where
145	147	Ok(output)
146	148	}
147	149
	150	+/// Bare repositories tracked in memory, for testing use cases without touching disk.
	151	+///
	152	+/// The counterpart to [`DiskGitStorage`], the way `StubHasher` is the counterpart to
	153	+/// the real Argon2 hasher. It enforces the same `AlreadyExists` rule, because a use
	154	+/// case that only passes against a permissive fake proves nothing about the real one.
	155	+#[derive(Debug, Default, Clone)]
	156	+pub struct InMemoryGitStorage {
	157	+ created: Arc<Mutex<HashSet<PathBuf>>>,
	158	+}
	159	+
	160	+impl InMemoryGitStorage {
	161	+ pub fn new() -> Self {
	162	+ Self::default()
	163	+ }
	164	+
	165	+ /// Whether a repository exists, for assertions.
	166	+ pub fn contains(&self, handle: &OrgName, name: &RepoName) -> bool {
	167	+ self.created
	168	+ .lock()
	169	+ .expect("lock poisoned")
	170	+ .contains(&self.repo_path(handle, name))
	171	+ }
	172	+
	173	+ /// How many repositories exist, for asserting that nothing was created.
	174	+ pub fn len(&self) -> usize {
	175	+ self.created.lock().expect("lock poisoned").len()
	176	+ }
	177	+
	178	+ pub fn is_empty(&self) -> bool {
	179	+ self.len() == 0
	180	+ }
	181	+}
	182	+
	183	+impl GitStorage for InMemoryGitStorage {
	184	+ async fn init_bare(&self, handle: &OrgName, name: &RepoName) -> Result<(), GitStorageError> {
	185	+ let mut created = self.created.lock().expect("lock poisoned");
	186	+
	187	+ if !created.insert(self.repo_path(handle, name)) {
	188	+ return Err(GitStorageError::AlreadyExists);
	189	+ }
	190	+
	191	+ Ok(())
	192	+ }
	193	+
	194	+ async fn remove(&self, handle: &OrgName, name: &RepoName) -> Result<(), GitStorageError> {
	195	+ self.created
	196	+ .lock()
	197	+ .expect("lock poisoned")
	198	+ .remove(&self.repo_path(handle, name));
	199	+
	200	+ Ok(())
	201	+ }
	202	+
	203	+ fn repo_path(&self, handle: &OrgName, name: &RepoName) -> PathBuf {
	204	+ PathBuf::from("/in-memory")
	205	+ .join(handle.as_str())
	206	+ .join(format!("{name}.git"))
	207	+ }
	208	+}
	209	+
148	210	#[cfg(test)]
149	211	mod tests {
150	212	use std::path::Path;