refactor(npm): hosted read seam 2/3, the inline-build arm (S31 plan: 8/19)
Hosted document read seam, part 2 of 3: the inline-build arm
The stack
Merge in order. !1983 (merged) has merged, so this MR now targets main and the diff below is only its own.
| # | MR | What it does | Reviewable LOC |
|---|---|---|---|
| 1 | !1983 (merged) (merged) | Seam types and the cache arm | 1234 |
| 2 | !1984 (merged) (this MR) | The inline-build arm and its collapse group | 1239 |
| 3 | !1985 (merged) | Both hosted handlers migrate onto the seam | 1214 |
Rebased onto main
This branch was based on !1983 (merged)'s first commit, and !1983 (merged) gained three more commits during review before it merged. The branch has been rebased onto main with --onto, and the three resolutions worth reading are:
- The miss sentinel. !1983 (merged) split it into
errHostedDocumentCacheMissplus a wrappingerrHostedDocumentCacheBlobGone. This part answers the miss, soerrHostedDocumentCacheMissgoes anderrHostedDocumentCacheBlobGonestays as the one internal signal that routes the dist-tags self-heal into the build. It never leavesRead. - The 307 validator. !1983 (merged) made a redirect carry the ETag for dist-tags only, since a packument's 307 names a representation it does not send. That divergence is kept, and
TestHostedDocumentReader_Read_RedirectsPerKindnow asserts it per kind. - The cache-arm fixture. !1983 (merged) stripped the version and tag rows out of
hostedDocFixture, because its arm reads neither. This part reads both, so they come back.
Two of !1983 (merged)'s tests moved with the behavior:
TestHostedDocumentReader_Read_UncachedPackageIsAMissis dropped.TestHostedDocumentReader_Read_InlineBuildsPerKindcovers the same cold read per kind and asserts the document it now produces, so it is strictly stronger.TestHostedDocumentReader_Read_AnotherKindsFreshRowIsAMissbecomes..._AnotherKindsFreshRowDoesNotAnswer, keeping its per-kind coverage and asserting the build instead of the sentinel.
What this part does
The seam can now answer a cache miss, so errHostedDocumentCacheMiss goes and Read resolves every request the two hosted routes will hand it once !1985 (merged) migrates them.
The arm renders the requested variant from npm_versions and npm_tags and collapses concurrent misses for one (namespace, package, kind) onto a single build. It reuses collapseInlineBuild, so this seam and the handler arms it will replace cannot drift on the detach, the deadline, or the in-flight cap. A successful build dispatches the cache refill, including for a conditional request, which paid for the same build.
The dist-tags self-heal now completes: a fresh row whose blob is gone rebuilds from the tables instead of surfacing the miss. The packument arm still refuses to self-heal, and both sides keep the comments saying not to converge them.
The handlers are still untouched, so this part changes no behavior either.
Three signature changes, and where they come from
Each follows from the arm rather than standing on its own, and each edits call sites !1983 (merged) introduced.
- The constructor takes the version and tag listers a render reads.
Readtakes theResponseWriter. A build may run toinlineBuildTimeoutand outlive the write deadline Go armed when the request headers were read, so the connection is re-armed before the build starts. Without it a slow build's response reaches a connection the server has already given up on.HostedDocumentRequestgainsPackageNameandBaseURL, the two inputs only a render consumes.
Testing
The inline-build arm per kind against the real generators, both load-shedding classifications with mutual NotErrorIs assertions, the abandoned build, a plain datastore fault as neither 503, the self-heal and its four negative boundaries, and two cross-variant negatives: a fresh full-kind row does not answer an abbreviated read, and per kind, another kind's fresh row does not answer either.
TestHostedDocumentReader_Read_CollapsesConcurrentBuildsOnOneReader is the composition test the sharing contract needs, and it is the one to look at closely. HostedDocumentReader's doc states that one reader is one collapse group, but no constructor signature can catch a caller that builds one per request, and the only symptom is halved coalescing plus a doubled fill cap, which no ordinary unit test fails on. Eight concurrent Reads on one reader, rendezvoused at the cache read and gated inside the build, assert exactly one build ran and eight cache reads did.
It uses the same settle-and-log shape as the existing TestDistTagsHandler_Singleflight_CollapsesInlineBuilds, because singleflight exposes no waiter count.
Diff size
1239 reviewable LOC: production 144 added / 31 removed, tests 928 added / 136 removed. The removals are the call-site churn the three signature changes cause in the tests !1983 (merged) left on main. The count is higher than the 1013 quoted before the rebase because the diff is now measured against main rather than against !1983 (merged)'s branch.
Guardrails
- Behavior-preserving
refactor, so exempt from theRelated to <issue>line and from the e2e scenario catalog. - No ADR conflict; no schema, migration, config, OpenAPI, or Bruno change.