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 errHostedDocumentCacheMiss plus a wrapping errHostedDocumentCacheBlobGone. This part answers the miss, so errHostedDocumentCacheMiss goes and errHostedDocumentCacheBlobGone stays as the one internal signal that routes the dist-tags self-heal into the build. It never leaves Read.
  • 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_RedirectsPerKind now 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_UncachedPackageIsAMiss is dropped. TestHostedDocumentReader_Read_InlineBuildsPerKind covers the same cold read per kind and asserts the document it now produces, so it is strictly stronger.
  • TestHostedDocumentReader_Read_AnotherKindsFreshRowIsAMiss becomes ..._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.
  • Read takes the ResponseWriter. A build may run to inlineBuildTimeout and 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.
  • HostedDocumentRequest gains PackageName and BaseURL, 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 the Related to <issue> line and from the e2e scenario catalog.
  • No ADR conflict; no schema, migration, config, OpenAPI, or Bruno change.
Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading