feat(npm): dist-tags kind=1 cache fast-path (S11 Step 10, MR 3/5)

📦 What

S11 Step 10 (dist-tags GET endpoint) is a five-MR stack, in dependency order (each targets the one above; GitLab auto-retargets to main as they merge):

  • !622 (merged) - dist-tags handler primitives + in-package unit tests.
  • !623 (merged) - inline-build handler (resolve -> 404 -> build {tag:version} -> serve).
  • !624 (merged) - kind=1 npm_metadata_files cache fast-path + conditional-GET 304-without-blob-read. <- this MR
  • !625 (merged) - inline-build singleflight (collapse concurrent cache misses).
  • !626 (merged) - integration suite (real datastore finders + stub BlobStore).

This MR targets dm/npm-local-step-10b (!623 (merged)); GitLab auto-retargets it to main as the parents merge.

Slice 3 - layers the kind=1 npm_metadata_files cache in front of the inline build: a fresh cache row serves the dist-tags blob via BlobStore.OpenBlob; a cache miss (absent or expired row, surfaced as datastore.ErrNotFound) falls through to the inline build from !623 (merged).

  • ServeHTTP now reads the kind=1 cache row after resolving the package and visibility, switching on cache hit / miss / error.
  • serveCacheHit derives the strong "sha256-<hex>" ETag from the row's blob_sha256 and serves the blob (200 stream or 307 redirect; S06 owns the wire format). A matching If-None-Match returns 304 before any OpenBlob call - the digest comes from the row, not the blob. The validator and Cache-Control/Vary directives are set only on the 200/304/307 success paths (after the storage open succeeds), so a Reader / RedirectURL / non-ErrBlobNotFound OpenBlob failure returns a 500 that is not advertised as cacheable, matching the OCI blob handler. A fresh row whose blob is gone (OpenBlob -> storage.ErrBlobNotFound) instead self-heals by rebuilding inline and increments npm_disttags_cache_blob_missing_total; cross-references mark the deliberate divergence from packument_get.go (which 500s on every OpenBlob error).
  • NewDistTagsHandler gains the cache finder and blob store dependencies (now six, all nil-checked). The inline-build singleflight is the next slice.

Spec coverage

Behaviour Test
Cache hit serves the blob with the strong ETag via exactly one OpenBlob TestDistTagsHandler_CacheHit_ServesBlobWithETag
If-None-Match match -> 304 with zero cache-blob reads (counting stub) TestDistTagsHandler_ConditionalGet_304WithoutBlobRead
Non-matching If-None-Match -> 200 + body (one OpenBlob) TestDistTagsHandler_ConditionalGet_NonMatchServesBody
Cache-Control by visibility on the cache-hit path TestDistTagsHandler_CacheControlByVisibility (cache-hit rows)
Redirect delivery -> 307 + Location + ETag/Cache-Control, no body TestDistTagsHandler_CacheHit_RedirectDelivery
ErrBlobNotFound self-heal rebuilds inline and is metered by npm_disttags_cache_blob_missing_total (no increment on a generic OpenBlob 500) TestDistTagsHandler_CacheHit_BlobGone_RebuildsInline, TestDistTagsHandler_CacheHit_OpenBlobError_500

⚠️ Known gap - handler not yet wired into the route table

DistTagsHandler is implemented and tested, but nothing mounts it: internal/format/npm/handler.go still serves the 501 Not Implemented placeholder for the dist-tags route, and the tests construct the handler directly with an injected Resolution. This is an unassigned plan-level gap (the npm hosted plan creates handler.go with placeholders in Step 8 and never assigns the placeholder->real-handler swap to a handler step); it surfaces no later than Step 23 (real npm-CLI conformance) and is closed via a plan amendment, not bolted onto this stack. Deliberately out of scope.

🧪 Testing

  • go test ./internal/format/npm/... - pass
  • golangci-lint run ./internal/format/npm/... - 0 issues
  • go vet -tags integration ./internal/format/npm/... - clean (integration build compiles)

Related to #122 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading