feat(datastore): maven virtual upstream associations (S17 Phase 6 plan: 41/42)

Hayley Swimelarrequested to merge
hswimelar/s17-p6-step-41 into main

Why

A Maven virtual repository has had no way to name its upstreams. feat(datastore): npm virtual upstream store and... (!2701 - merged) • Hayley Swimelar • 19.5 built the junction store and the delete-side guard for npm, and feat(managementapi): npm virtual upstream assoc... (!2735 - merged) • Hayley Swimelar • 19.5 built the five association routes over it, leaving Maven and the container formats 404-pending. This is the Maven arm, so those five routes now serve maven, and a repository a Maven virtual repository lists refuses deletion.

It also closes an S17 AC #110 (closed) violation that predates it. The Maven arm of checkRepositoryUpstreamAssociations used to no-op, so a destructive=true delete tombstoned a repository an active Maven virtual list still pointed at.

What (the non-obvious parts)

The five handlers went format-blind rather than gaining a Maven copy. They take a two-field upstreamArm descriptor, and the datastore DTOs went neutral so both concrete stores satisfy one interface. Store types stay per format, which is what the plan's Naming Conventions section fixes.

Maven admits Maven upstreams only, with no family mix. That is the one behavioral rule the container arm will not share.

TestRepositoryStore_Delete_VirtualMavenUpstreamTargetBlocked changes its expectation from ErrRepositoryNotEmpty to ErrRepositoryIsVirtualUpstream. That is the AC #110 (closed) fix above rather than a test bent to fit the code, and feat(datastore): npm virtual upstream store and... (!2701 - merged) • Hayley Swimelar • 19.5 made the identical change to the npm twin.

Nine list-arithmetic helpers sit as per-arm pairs, seven byte-identical and two differing only in the cap constant they gate on. Collapsing them edits a seam feat(datastore): container virtual upstream sto... (!2778 - merged) • Hayley Swimelar • 19.5 is changing right now, so they collapse once after it lands.

Diff size

4663 added lines against 500 reviewable: tests +3337/-230, production Go +1045/-175, and docs +281/-92. maven_virtual_upstreams.go is +859 on its own, a plan-mandated per-format mirror of npm's 847-line store, and the mirrored suites are the bulk of the tests because the testing guidelines require diffing them against npm's for dropped subtests. Splitting the store from the arm would land a delete guard for associations no route can create, which is the window this step exists to close.

Merge order and overlaps

Step 40 landed on main as feat(managementapi): npm virtual upstream assoc... (!2735 - merged) • Hayley Swimelar • 19.5 (6219bf7d), so this MR targets main. Either this MR or feat(datastore): container virtual upstream sto... (!2778 - merged) • Hayley Swimelar • 19.5 may land first. The two branches modify 24 of the same files (19 of them with conflicting hunks), so the second to land rebases over the first.

Test plan

  • Unit and integration suites for both arms against PostgreSQL 16, including concurrent associate/delete pairs on both lock disciplines.
  • golangci-lint with and without --build-tags=integration, go vet -tags=integration ./... repo-wide, and the comment-caps gate against main.
  • driver.sh smoke on an isolated rig: 74 passed, 0 failed. The five Maven routes were then hand-driven there. Associate returns 201 and shifts positions, list 200 in position order, reorder 200, dissociate 204, and the list closes to []. An npm upstream on a Maven virtual repository returns 422 on the format-family message. Deleting a listed upstream returns 409 under destructive=true and destructive=false alike. The five mutations left five audit records.

Related to #314 (closed)

Context for LLM agents

Design rationale and rejected alternatives

Format-blind handlers over a per-format copy. Duplicating the five handlers would have added ~500 lines and forced a third copy when the container arm lands. The plan's Naming Conventions section fixes one association handler file, so a second copy contradicts it. The datastore error sentinels were already format-neutral, which set the precedent one layer down.

Per-format store types kept. The same Naming Conventions section names MavenVirtualUpstreamStore in <format>_virtual_upstreams.go per format. The neutral DTOs stop at the handler boundary.

A registerUpstreamAssociationRoutes helper was withdrawn. TestNewRouter_BindingSetMatchesRegistrationSites counts static handle CallExprs by AST, so one helper invoked twice yields ten bindings against five sites. Ten explicit rows are what that test admits. Do not "clean this up" into a loop.

The mutation read carries no LIMIT, mirroring npm. Deliberate, with the rationale at the call site.

One ADR-004 cap constant per format. npm's maxNpmVirtualUpstreams was renamed maxVirtualUpstreams and the Maven store first read it, which split Maven's write gate from its read bound: a raise would have left every over-cap Maven list failing closed on maxMavenVirtualUpstreams at the old value. The Maven arm now reads maxMavenVirtualUpstreams throughout, so a raise moves a format's two halves together.

Non-goals and known gaps

  • Allow and deny rule CRUD is not here. Associations ship rule-less. The only reference to maven_virtual_upstream_rules this branch adds is the cascade assertion, which seeds a rule row to watch the database remove it.
  • The audit record carries no format field. Open and untracked. Format is derivable from the database by a join but not from the log stream, so this is a real operator gap deferred to S03-A rather than a convenience.
  • The repair WARN names no parent repository id. Threading one adds a third adjacent uuid.UUID to four functions per arm, the swap hazard upstreamScope exists to prevent. It wants a scope value, which is a follow-up.
  • No Maven management-API integration suite. The handlers are format-blind and the Maven-specific surface is covered by unit arm tables plus the datastore integration suite.
  • The child-row lock wait is unbounded. Pre-existing class, The repository DELETE's hard-delete phase waits... (#902) • Unassigned. No lock_timeout or statement_timeout is set, so a request-path caller owes these calls a context deadline. This step widens the class to a second format rather than changing it. Not refiled.
  • Rolling-deploy skew. During a rollout an old pod without this guard arm can still tombstone a repository an active Maven virtual list references. The window closes when the fleet is fully rolled.

Coordination

maxMavenVirtualUpstreams in maven_virtual_resolution.go and maxVirtualUpstreams in npm_virtual_resolution.go are both 20 and are deliberately separate, one per format, so neither format's write gate can drift from its own read bound. Whichever of this MR and feat(datastore): container virtual upstream sto... (!2778 - merged) • Hayley Swimelar • 19.5 rebases second adds the container arm's own, not a fourth reader of npm's.

Edited by Hayley Swimelar

Merge request reports