feat(managementapi): npm version stats (S17 Phase 8 plan: 20/13)

Why

The npm arm of .../{format}/versions/{version_id}/statistics answered 501. This serves it: the version's visible file count, the distinct bytes those files occupy, and its stored last_downloaded_at. files_count is the figure the scope exists for, because a files page carries no total and no column holds one.

size_bytes derives per request rather than reading npm_versions.size_bytes. That column holds the same footprint, and unlike hosted Maven's it has no flush window: the publish commit writes it and a hard delete refolds it in the delete's own transaction. Neither column has a reconciliation loop, so one that has drifted stays drifted, and its specified correction is this same walk run again. The derived figure is exact at read time whatever the column holds, which is what the read demonstrates by answering over a stamped-wrong one.

Step 19 landed first

The family-shared VersionStatistics DTO, the versionStatisticsScope verdict type, and the handleVersionStatistics dispatch all landed in feat(managementapi): maven version stats (S17 P... (!2021 - merged) • Hayley Swimelar • 19.4, whose npm arm was the placeholder this fills.

This branch was stacked on that MR and now targets main. The parent merged as a squash, which leaves no patch-id match against it, so the rebase used --onto against refs/merge-requests/2021/head: nine parent commits dropped, the five here kept. The Deps seam pin was re-derived from the walk's own failure on the rebased tree rather than by arithmetic, because sibling steps moved main's count while this branch was open.

The footprint's join, and the gate over it

The plan entry for this step names one external gate. ADR-007 declines the blob_storage_blobs_by_namespace shadow for a per-version walk, on the premise that a walk holding its own digests prunes the base table, and the spec's version scope fixes the shadow as the join target (Spec: Phase 8 statistics reads).

The measurement that settles it is delivered on Measure the artifact-level size lookup: base ta... (#564) • Hayley Swimelar and its disposition recorded on S17 Phase 8: docs corrections from wave-1 build... (#916) • Hayley Swimelar • 19.4: the shadow join wins on PostgreSQL 16, 17, and 18, and npm's base-table form prunes nothing, opening all 64 partitions on every call and beating the shadow in none of the 123 cells measured. The verdict is the shadow, and both version steps were released against it.

Two document fixes remain, and neither gates this code. While Amend AR ADR-007: artifact-level walks read the... (gitlab-com/content-sites/handbook!20894 - merged) • Hayley Swimelar is open the docs/adr/ mirror carries ADR-007's pre-amendment conclusion, and once it merges the daily sync:adrs job brings the amended text over. The spec's version scope keeps its conclusion and swaps its stated reason for the measured one.

What is not obvious

The two base-table walks stay where they are. NpmFileStore answers this footprint twice already: SumDistinctNpmFileSizesByVersion as a pool read with no production caller, and RecomputeNpmVersionSizeBytesTx as the fold that keeps npm_versions.size_bytes current on a hard delete. Neither is reused. The fold is an UPDATE and returns no figure, and the pool twin returns the right bytes on the wrong path: blob_storage_blobs is partitioned by HASH(sha256), which a runtime digest set cannot prune. Both belong to the storage accounting workstream and keep their shape, so the package now answers one question twice, across npm_files.go and npm_versions.go, which the plan's Research Findings names as a deliberate near-miss. This walk composes the shared shadow-joining sum instead, writing no join, no aggregate, and no alias of its own.

The arm keeps its own hosted verdict. It does not reuse hostedVersionFileRepository, the gate the Phase 3 version reads ride, and the reason is what that gate becomes rather than what it answers: the remote artifact reads narrow it to virtual, while statistics stay hosted-only. The two answer the same bytes today, since writeRemoteArtifactReadPending calls writeRepositoryNotFound, so no test can tell them apart and none claims to.

npm has no package-level file row. npm_files.npm_version_id is NOT NULL and the table carries no package column, so Maven's third scope, the maven-metadata.xml row a version walk must skip, does not exist here. The scope fixtures seed a neighbouring package's version instead, which keeps the same three-way leak detectable: dropping the version predicate answers 1111, reaching one neighbour alone 1011 or 111.

The mirrored format-mismatch row lands here, because the Maven arm's addressing matrix could not drive a Maven repository under the npm segment while this arm answered the placeholder.

One Deps field, four literals. handler_test.go, pact_provider_test.go, read_handlers_integration_test.go, and cmd/artifact-registry/wire_management_test.go each construct one, and requireNonNilSeams panics at construction on a nil leaf, so all four move together. The seam count pin moves by one, re-derived from the walk's own failure message after each rebase rather than by arithmetic, because sibling steps add their own fields. No hand-written nil guard: the reflective walk covers every field.

Eight //nolint:dupl directives, four of them on the Maven twins. dupl reports both halves of a pair, so a per-format suite cannot suppress it from one side alone. Each was measured by dropping it and re-running. The same measurement is why four //nolint:nilnil tokens leave fake_version_file_readers_test.go in this MR: that linter sits in .golangci.yaml's disable list, so they suppressed nothing, and the file comes out consistent with the measured-token rule rather than with its neighbours.

Diff size

Past the 500-LOC guideline against main, and the overrun is test mass. Splitting the reads from the handler was interrogated when the plan was written and rejected, and the measurement holds here: the split lands at roughly 988 and 903 LOC, so neither half clears 500 and it buys a duplicated fixture and nothing else.

Group Added + removed
Production, internal/datastore 101
Production, internal/managementapi 121
Production, cmd/artifact-registry 2
Tests, internal/datastore 887
Tests, internal/managementapi 750
Tests, cmd/artifact-registry 30
Total 1891

Test plan

  • go test ./internal/managementapi/ ./internal/datastore/ ./cmd/artifact-registry/
  • go test -tags=integration ./internal/datastore/ ./internal/managementapi/, both packages whole rather than filtered to this step, against a PostgreSQL 16 carrying CI's own .pg-service-options (max_locks_per_transaction=1024 included, without which the 64-partition tables exhaust the lock table and unrelated suites fail in their namespace-delete cleanup)
  • go-lint-ci ./internal/... ./cmd/..., plus the integration-tagged run with --max-same-issues=0 --max-issues-per-linter=0 --uniq-by-line=false, which is the only run that compiles the //go:build integration files at all

Both plan pins are measured, not reasoned, and neither names an index. The count plans as an Index Only Scan over one npm_files partition with both scoping columns in the Index Cond. Every index that can answer its predicate leads with that pair, so the assertion holds whichever the planner picks.

The footprint cannot promise that, and the plan's Acceptance line asks it to. The line names "index-assisted on index_npm_files_on_ns_id_ver_id", which this walk cannot bind: that index carries no digest column, so DISTINCT blob_sha256 would pay a sort, and measured on PostgreSQL 16 the planner takes (namespace_id, blob_sha256) and applies the version as a Filter. Maven's twin can name its index only because index_maven_files_on_ns_id_ver_id is three columns wide. So the pin asserts what every correct plan must bind and nothing more: one partition, the namespace in the Index Cond, and the version at that same scan, as an Index Cond term or as its Filter. Reading the npm_files scan node alone is what keeps that honest, because a predicate rendered above the scan narrows a range already read, and npmFilesScanNode carries a five-case test, two cases of which exist to kill one break condition each. Nothing asserts the absence of a sequential scan: explainListPlan sets enable_seqscan=off, so that assertion could never fail. A step MR cannot edit the plan, so this is where the correction is recorded.

Both queries were mutation-tested. Tautology-swapping either namespace predicate, either version predicate, the count's soft-delete predicate, or the digest subquery's DISTINCT fails at least one behavioural subtest in every case. The shadow join's own namespace predicate survives this step's subtests and is killed by TestSumDistinctBlobSizes, which is the seam that owns it.

No e2e scenario is added or affected. The statistics consumers are the monolith UI slices, whose journeys land with those slices, and no protocol-client catalog scenario reads statistics, which the plan's Testing Strategy records. No protocol behavior changes, so conformance does not apply, and the MR adds no configuration, no migration, and no OpenAPI operation.

Spec coverage

Spec coverage: acceptance criteria, error cases, and security considerations

Spec coverage

Spec: docs/specs/S17-rest-management-api.md

Acceptance criteria

# Criterion Tests
AC-116 files_count matches the version's visible file list TestNpmVersionStore_CountVisibleNpmFilesByVersion (the count read against ListNpmFilesByVersion's own page, a marked row, a sibling version's row and another package's version's row excluded, an empty version reads 0, a cross-namespace id reads 0), TestNpmVersionVisibleFilesCountStmt_BindsBothScopesAndTheMark, TestNpmVersionStatistics_SerializesBothFiguresAndTheStoredInstant
AC-116 size_bytes counts a blob two of the version's files share once TestNpmVersionStore_SumNpmVersionFootprint (a digest two of the version's files share counts once asserts 4096 rather than 8192, and asserts the count 2 beside it), TestNpmVersionFootprintDigestsStmt_ScopesTheWalkToOneVersion (the DISTINCT and the alias the shared sum joins on)
AC-116 The figure is derived, so it may differ from the stored column. The spec sentence names maven_versions.size_bytes, and the npm arm derives the same way, against npm_versions.size_bytes TestNpmVersionStore_SumNpmVersionFootprint (the derived figure answers where the stored column disagrees, stamping 999999 against a 64-byte fixture)
AC-117 A blob two versions share counts once in each, so the two footprints sum to more than the distinct bytes TestNpmVersionStore_SumNpmVersionFootprint (a digest two versions share counts in full in each, which asserts both figures and that their sum exceeds the distinct bytes)
AC-119 A tombstoned file's bytes stay in the live size_bytes walk while every live count drops it from the mark onward - version-scope half TestNpmVersionStore_SumNpmVersionFootprint (a marked file's bytes stay while its row leaves the count, asserting 137 bytes against 1 file in one fixture), TestNpmVersionFootprintDigestsStmt_KeepsMarkedFilesInTheWalk (the absent predicate, which no EXPLAIN can show). The stored-counter half is S22's own settlement rule.
AC-120 Phase 3 addressing: a missing, foreign, malformed, or soft-deleted target returns 404 TestNpmVersionStatistics_AddressingMatrix (unresolved slug, missing repository, the repository under another slug, soft-deleted repository, a non-UUID, unhyphenated, and nil-UUID version id, and an id addressing no row), TestNpmVersionStatistics_SoftDeletedVersionIs404, TestNpmVersionStatistics_VersionUnderAnotherRepositoryIs404
AC-120 A :format segment differing from the repository's format returns 404 TestNpmVersionStatistics_AddressingMatrix (maven repository under the npm format segment, the direction the Maven arm's row could not drive while this arm answered the placeholder, and docker repository under its own format segment)
AC-120 The route on a virtual or remote repository returns 404 TestNpmVersionStatistics_NonHostedRepositoryIs404 (both kinds, asserting the repository-class message and that neither figure nor the chain read ran), TestFindNpmVersionInScope_RejectsNonHostedScope, and the family-shared TestVersionStatisticsScopeFor_StoresTheHostedVerdict and TestVersionStatisticsScope_ZeroValueIsNotHosted
AC-121 Every statistics error path returns the S01 envelope with the correlation request_id assertErrorEnvelope inside every 404 and 500 case above, plus TestStatisticsNpmVersion_ResponsesMatchOpenAPIContract (the 404 and 500 rows through validateAgainstContract)
AC-113, AC-114, AC-115, AC-118, AC-122, AC-123 Other statistics scopes, the withdrawn per-artifact paths, and the connection test Other steps of this plan, or other phases. This step serves the npm version arm alone, which drains the last getVersionStatistics entry from contractPendingOperations. 24 entries remain, the 20 upstream-repository operations and the 4 other statistics scopes.

Error cases

Condition Tests
Version statistics: any Artifact-routes condition (missing, foreign, malformed, soft-deleted, format mismatch) is 404 not_found TestNpmVersionStatistics_AddressingMatrix, TestNpmVersionStatistics_SoftDeletedVersionIs404, TestNpmVersionStatistics_VersionUnderAnotherRepositoryIs404
Version statistics: repository kind not hosted is 404, even on the remote repositories the artifact routes serve TestNpmVersionStatistics_NonHostedRepositoryIs404. The remote body is byte-identical to the artifact routes' own remote answer, so the case pins the shared shape and the durable reason travels in prose: remote artifact reads narrow that gate to virtual, statistics stay hosted-only
A statistics read that fails is a logged 500, never a masked 404 TestNpmVersionStatistics_SeamFailureReturns500 (both figures, the child-row resolve, the chain read, and a nil row with no error, each with its own log line)
Store argument guards: nil context, zero namespace, zero version id TestNpmVersionStore_StatisticsReads_ArgumentGuards, TestNpmVersionStore_StatisticsReads_RejectTheZeroIDsSeparately

Security considerations

Concern Tests
Existence hiding: a break at the repository is indistinguishable from a missing one, and a break below it from a missing artifact TestNpmVersionStatistics_AddressingMatrix asserts the message class per row, so no response separates a well-formed guess from a malformed one within its class
Tenant isolation: every query is scoped by the resolved namespace_id, and the parent chain is verified down from the repository in the URL TestNpmVersionStore_CountVisibleNpmFilesByVersion (a version id under another namespace counts nothing), TestNpmVersionStore_SumNpmVersionFootprint (a version id under another namespace sums nothing, which seeds the foreign digest under the addressed namespace at a different size so the walk's own predicate is the only thing holding the answer at zero), TestNpmVersionStatistics_VersionUnderAnotherRepositoryIs404, and the ids asserted on both recorded reads in TestNpmVersionStatistics_SerializesBothFiguresAndTheStoredInstant
Read cost: the npm walks prune to one partition and bind both scopes at their scan TestNpmVersionStore_CountVisibleNpmFilesByVersion_PlanBindsBothScopingColumns (one partition, both columns in one Index Cond), TestNpmVersionStore_SumNpmVersionFootprint_PlanPrunesAndBindsTheVersion (one partition, the namespace in an Index Cond, the version at the same scan). Neither names an index. The footprint walk does not require the version in the Index Cond: no npm_files index carries both the version and the digest, so DISTINCT trades a free ordering against a narrower range, and measured on PostgreSQL 16 it takes the ordering index at this fixture's row count. Both pins read the npm_files scan node alone, so a predicate applied above the scan fails them
Statistics authorization: the version scope binds to read_artifact S09's enforcement step owns it. No authorization runs on this route yet, so nothing here can assert it.
Attribution exposure, echoed input, bounded blast radius, aggregate disclosure Not reachable at this scope: the response carries three figures and no name, identifier, or submitted value.

Related to #316

Edited by Hayley Swimelar

Merge request reports

Loading
Loading