feat(maven): remote cache store — cache-fill writes (S14 Step 9)
What
Lands the write half of the remote.CacheStore implementation for Maven — Step 9 of the S14 Maven remote vertical slice, on Step 8's wrapper/backing-store split. The three interface write methods match the merged S13 signatures and own their transactions through datastore.RunInTx, delegating to unexported qrm.DB-accepting internals a larger transaction can compose (the two-tier shape Step 7 established):
UpsertCacheEntry— the cache fill. Lazily finds-or-creates themaven_remote_packages/maven_remote_versionsparents inside the fill transaction (no auto-create-on-push path exists on a remote; first-cache is the only creation trigger), then upsertsmaven_remote_fileswith the blob reference and all four digest columns. A replacement fill repointsblob_storage_attachment_id/blob_sha256so the superseded blob stops being referenced, rewrites every digest column, and setsupstream_etag/upstream_checked_at.BumpUpstreamCheckedAt— the304revalidation bump; advances onlyupstream_checked_at, never the blob reference or digests, and reports a vanished row asremote.ErrCacheEntryNotFound.BumpLastDownloadedAt— the download-signal bumps for the package and version rows.
The wrapper in internal/format/maven/remote_store.go keeps the seam contract: it parses the path, converts entry.SHA1 to checksum.SHA1Sum, and fails closed on a nil or short SHA-1 before touching the database — the sha1 column is NOT NULL with a length CHECK, so the fetch pipeline's pre-tee call site (which passes zero-value digests) is rejected rather than persisted. That makes S13's tee computation a hard merge-order prerequisite for wiring this store into Fetch, which is Track B's gate, not this step's.
The CacheEntry amendment and the counter deferral
Two operator-approved scope amendments ride this branch, both recorded in the plan and — as of this MR — annotated in the spec:
remote.CacheEntrydigest-field widening (pulled in from S13):SHA1andSHA512as[]byte, nullableMD5as[]byte(nil under FIPS). Only the type widening moves in; S13's tee still computes the digests and remains Track B's external prerequisite. The existing fetch-pipeline call site andremotetestfakes use named-field construction and compile unchanged. The accepted cost: an S14 step shapes an S13 type, so S13's tee work must conform to the field types chosen here.downloads_countdeferred out of the plan. The mergedremote.CacheStore.BumpLastDownloadedAtseam carries no archive-class or HEAD/GET signal, so no store method can express S14's counting rule; thelast_downloaded_atbumps ship as interim directUPDATEs carrying theTODO(s18-buffered-counters)swap marker (AST-enforced), mirroring the merged npm-remote precedent. The counter half of Artifact proxy criteria 1/4/11 and Metadata proxy 8 lands with the buffered-write client work or a seam-widening follow-up.docs/specs/S14-maven-remote.mdis annotated at the cache-hit flow, criterion 4, the counting Resolution, the S18 Dependencies bullet, and the digest-seam Resolution so the spec stops asserting an unamended contract.
Tests
Every acceptance clause has a named asserting test:
| Acceptance clause | Test |
|---|---|
CacheEntry carries the three digest fields; fetch-pipeline call site and fakes compile unchanged; short-SHA1 guard rejects zero-value digests |
TestCacheEntry_DigestFields (internal/remote/cache_entry_test.go); internal/remote/fetch.go and remotetest untouched by the diff; guard pinned by TestRemoteCacheStore_UpsertCacheEntry_PreBackingGuards |
| Fill creates package, version, and file rows lazily and idempotently; second fill creates no duplicates | TestMavenRemoteCacheStore_UpsertCacheEntry_CreatesRowsLazily, _PackageLevelFileHasNoVersion, _SecondFillCreatesNoDuplicates |
| Replacement fill repoints the blob, rewrites all four digest columns, updates etag and checked-at | _ReplacementRepointsBlobAndRewritesDigests, with assertDistinctDigests so the rewrite assertions are not vacuous |
qrm.DB internal driven inside a rolled-back transaction leaves the prior row and blob reference untouched, writes no partial row |
_RolledBackTransactionLeavesRowsUntouched |
304 bump advances upstream_checked_at without touching the blob reference or digests |
TestMavenRemoteCacheStore_BumpUpstreamCheckedAt_AdvancesTimestampWithoutTouchingBlobOrDigests, plus _NoRowReturnsErrCacheEntryNotFound and _RolledBackTransactionLeavesTimestampUntouched |
Entry with no MD5 (FIPS) leaves md5 NULL |
_NilMD5LeavesColumnNull |
| Wrapper maps the widened fields to the right columns end to end | TestRemoteCacheStore_WriteMethods_ThroughTheSeam (distinct random value per column), TestRemoteCacheStore_WriteMethodsParseAtTheSeam, TestMavenRemoteSHA1Sum |
MR size
~3,770 changed lines, over the 500 LOC ceiling in the development model, which asks for a split or a justification here.
Justification: the production surface is ~1,370 lines (maven_remote_cache.go +1020/−25, remote_store.go +292/−25, interfaces.go +20, checksum.go +28/−3, maven_remote_repositories.go +10/−4). The remaining bulk is the same-step test suites the plan template requires (~2,200 lines, most of it integration seed/assert boilerplate — the same pattern Steps 7 and 8 landed — plus the review-driven concurrency and partition-pruning integration suites) and the plan/spec amendments (~200). No split is available that does not separate the write methods from their tests.
End-to-end scenario catalogs
No scenario added or invalidated. docs/testing/ holds no Maven catalog yet — authoring the first one is a separate docs concern, per the plan's testing strategy — and this step's write methods are not wired to any request path (Steps 14–16 wire them through the fill handlers), so there is no observable behavior a scenario could exercise. Step 18's hermetic proxy harness is the automated end-to-end coverage for the S14 paths.
Conformance
No protocol behavior changes: this step is store-layer only and nothing it lands is reachable from a request path yet, so the hosted Maven surface is byte-identical. Conformance for the proxy paths runs with Steps 14–16 and Step 18's harness.
Notes for reviewers
Commit-by-commit:
355682ac4— the plan amendment pulling theCacheEntrywidening into this step (operator-approved; precedes the code commits as declared).75a739281/374f7d09c— the test-first suite and the write methods (the MR's substance).035ba8ebf— the code-simplifier pass over the write internals.b8b19a25a— the plan amendment deferringdownloads_countout of the plan.af2fa4b31,543c48053— comment hygiene (symbol citations, stale-comment corrections).32f06b11d,75553f7e3— review-driven test hardening: the distinct-value md5 rewrite guard, the write-path guard/wrap pins, and the through-the-wrapper mapping test.a030bd2e2— the plan's landed-size record and the corrected pre-tee fail-closed claim.0b358c94b— hoists one subtest's seeding to the test body so the diff adds no newcontextcheckfinding under--build-tags=integration(the file's 13 pre-existing findings from Step 8 are unchanged; CI does not lint integration-tagged files).8d709332c/cfa1e9e9e— the spec annotation and the plan's landed file-set record (this MR's validation fallout).
Pre-push validation (validate-step, five categories): Acceptance and ADR adherence PASS; the three WARNs it raised — plan file-set drift, the new contextcheck finding, and the unamended spec text — are resolved by the last three commits above. Integration lint (golangci-lint --build-tags=integration --max-same-issues=0) is clean for the new code, and the through-the-wrapper integration test passes against testcontainers Postgres.
Related to #286 (closed)