feat(managementapi): maven version stats (S17 Phase 8 plan: 19/13)
Why
A version detail page renders a file total the artifact lists cannot supply: a
files page carries no count, and no column stores one. This serves it for Maven,
at .../{format}/versions/{version_id}/statistics, with the version's byte
footprint and its stored last_downloaded_at beside it. The operation and its
schema are declared already, and the Maven arm answered 501 until now.
size_bytes is derived per request, not read from maven_versions.size_bytes.
That column holds the same footprint behind a post-commit recompute, so it lags
its window, and the derived figure is the only exact one available.
The footprint's join, and the gate over it
This step's plan entry 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. The spec's version
scope fixes the shadow as the join target. The measurement that settles it is
delivered on Measure the artifact-level size lookup: base ta... (#564) • Hayley Swimelar: the shadow join wins on
PostgreSQL 16, 17, and 18 across namespaces from 100 to 1,000,000 blobs and
digest sets from 1 to 512, and the bound-array form ADR-007 prescribes wins only
at 1 to 5 digests, below the 4 to 15 files the ADR itself calls typical for
Maven. The verdict is the shadow, and this composes it through the helper the
footprint step landed.
Two document fixes remain, both tracked on
S17 Phase 8: docs corrections from wave-1 build... (#916) • Hayley Swimelar • 19.4, and neither gates this code. The ADR
amendment is open at Amend AR ADR-007: artifact-level walks read the... (gitlab-com/content-sites/handbook!20894 - merged) • Hayley Swimelar: while it is open
the docs/adr/ mirror carries the pre-amendment conclusion, and once it merges
the daily sync:adrs job brings the new text over. The spec's version scope
keeps its conclusion and swaps its stated reason, that runtime digests cannot
prune the base table, which is false for the bound-array form, for the measured
one.
What that measurement does not cover: the walk's input is uncapped. It reads the
version's distinct digests with no LIMIT, nothing caps files per Maven version,
and a long-lived SNAPSHOT version's digest count grows with publish count past
the measured 512. The plan and the spec accept that on the repository scope's
grounds, rare and partition-pruned, with a denormalized per-version column as the
named fallback. Its trigger is a p99 on
database_query_duration_seconds{name="maven_files_select_sum_footprint_by_version"},
which this MR starts emitting.
What is not obvious
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.
versionStatisticsScope stores the verdict at construction because
RepositoryKindHosted is the int16 zero value, so re-deriving it from an unset
field admits every kind.
The near-miss stays in the same file, deliberately.
MavenVersionStore.RecomputeVersionSize is this walk plus a write-back, and it
reads blob_storage_blobs directly with a bound digest array. It is the
counter's maintenance path and it keeps that shape. The statistics walk composes
the shared shadow-joining sum instead, writing no join, no aggregate, and no
alias of its own.
Counts and bytes disagree in one response, twice over. files_count drops a
tombstoned file at the mark and size_bytes keeps its bytes until the reap,
which is the spec's asymmetry, pinned over one fixture so it is not incidental.
The pair is also two autocommit statements, so a commit landing between them
skews them, the read race FindMavenVersionByID documents for its own two
statements and the same trade against a transaction the plan does not open.
The pending-entry drop rewrites its sibling's line. Dropping the Maven
getVersionStatistics entry re-aligns the npm entry under gofmt, the conflict
the plan measured, which the npm step resolves behind its edge on this one. One
addressing-matrix row moved with it: a Maven repository under the npm segment
reaches that pending arm's 501 rather than this one, so the mirrored
format-mismatch direction lands with the npm arm, and the table says so.
Four Deps literals gain the new seam. 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 in handler_test.go goes 84 to 85. No
hand-written nil guard: the reflective walk covers every field.
Diff size
Past the 500-LOC guideline, and the overrun is entirely test mass: the source side landed on the plan's own forecast of 120 to 200 LOC and the test side is past the top of its band. Splitting the reads from the handler was interrogated when the plan was written and rejected, and the measurement holds: split at that seam and the halves are roughly 830 and 1030 LOC, so neither clears 500 and the split buys nothing but a duplicated fixture.
| Group | Added + removed |
|---|---|
Production, internal/datastore |
105 |
Production, internal/managementapi |
170 |
Production, cmd/artifact-registry |
2 |
Tests, internal/datastore |
722 |
Tests, internal/managementapi |
823 |
Tests, cmd/artifact-registry |
33 |
| Total | 1855 |
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=1024included, 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 integrationfiles at all
Both plan pins assert an invariant and name no index, because which index wins is
a cost estimate rather than a property of the statement. CI took
index_maven_files_on_ns_id_attachment_id for the footprint walk and filtered the
version in the heap, failing 1 of 4 full datastore runs, at a measured 12.19
against the winner's 12.17. Each pin now requires one maven_files partition, the
namespace in an Index Cond on that partition's scan node, and the version on the
same node as an Index Cond term or a Filter. The count's own margin is remoter,
8.17 against 12.21, so its pin moved to retire the index-naming shape from the file
rather than to fix a live flake.
mavenFilesScanNode scopes both assertions to that scan node plus a bitmap
access's nested index scans, because the sibling blob scan binds namespace_id too
and would otherwise answer for it. TestMavenFilesScanNode_StopsAtTheNextNode pins
the extraction over five plan shapes, including the bitmap shape CI flaked on.
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 (the plan's Testing Strategy records this). 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 |
TestMavenVersionStore_CountVisibleMavenFilesByVersion (the count read against ListMavenFilesByVersion's own page, a marked row, a package-level row and a sibling version's excluded, an empty version reads 0, a cross-namespace id reads 0), TestMavenVersionStatistics_SerializesBothFiguresAndTheStoredInstant |
| AC-116 | size_bytes counts a blob two of the version's files share once |
TestMavenVersionStore_SumMavenVersionFootprint (a digest two of the version's files share counts once asserts 4096 rather than 8192, and asserts the count 2 beside it), TestMavenVersionFootprintDigestsStmt_ScopesTheWalkToOneVersion (the DISTINCT and the alias the shared sum joins on) |
| AC-116 | A Maven version's figure is derived, so it may differ from the stored maven_versions.size_bytes |
TestMavenVersionStore_SumMavenVersionFootprint (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 | TestMavenVersionStore_SumMavenVersionFootprint (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) |
TestMavenVersionStore_SumMavenVersionFootprint (a marked file's bytes stay while its row leaves the count, asserting 107 bytes against 1 file in one fixture), TestMavenVersionFootprintDigestsStmt_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 |
TestMavenVersionStatistics_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), TestMavenVersionStatistics_SoftDeletedVersionIs404, TestMavenVersionStatistics_VersionUnderAnotherRepositoryIs404 |
| AC-120 | A :format segment differing from the repository's format returns 404 |
TestMavenVersionStatistics_AddressingMatrix (npm repository under the maven format segment, oci repository under its own format segment. The mirrored direction rides the npm segment, whose arm answers the placeholder status until it lands) |
| AC-120 | The route on a virtual or remote repository returns 404 |
TestMavenVersionStatistics_NonHostedRepositoryIs404 (both kinds, asserting the repository-class message and that neither figure was read), TestVersionStatisticsScopeFor_StoresTheHostedVerdict, 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 TestStatisticsMavenVersion_ResponsesMatchOpenAPIContract (the 404 and 500 rows through validateAgainstContract) |
| AC-114, AC-115, AC-118, AC-122 | Other statistics scopes and the withdrawn per-artifact paths | Other steps of this plan. This step serves the Maven version arm alone, and its npm sibling's pending entry still stands in contract_routes_test.go. |
Error cases
| Condition | Tests |
|---|---|
Version statistics: any Artifact-routes condition (missing, foreign, malformed, soft-deleted, format mismatch) is 404 not_found |
TestMavenVersionStatistics_AddressingMatrix, TestMavenVersionStatistics_SoftDeletedVersionIs404, TestMavenVersionStatistics_VersionUnderAnotherRepositoryIs404 |
Version statistics: repository kind not hosted is 404, even on the remote repositories the artifact routes serve |
TestMavenVersionStatistics_NonHostedRepositoryIs404, which pins that a remote row reaches the same repository-class 404 a virtual one does, and costs no query |
A statistics read that fails is a logged 500, never a masked 404 |
TestMavenVersionStatistics_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 | TestMavenVersionStore_StatisticsReads_ArgumentGuards, TestMavenVersionStore_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 | TestMavenVersionStatistics_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 |
TestMavenVersionStore_CountVisibleMavenFilesByVersion and TestMavenVersionStore_SumMavenVersionFootprint (each with a foreign-namespace case, the footprint's seeded on both sides so removing the walk's namespace predicate really fails it), the maven_files.namespace_id = assertion in TestMavenVersionFootprintDigestsStmt_ScopesTheWalkToOneVersion, TestMavenVersionStatistics_VersionUnderAnotherRepositoryIs404, and the ids asserted on both recorded reads in TestMavenVersionStatistics_SerializesBothFiguresAndTheStoredInstant |
| Read cost: the Maven footprint walk binds per version rather than fanning out over the namespace | TestMavenVersionStore_SumMavenVersionFootprint_PlanPrunesAndBindsTheVersion: one maven_files partition, the namespace in an Index Cond on the scan node, and the version bound on that same node. It names no index, for the cost-estimate reason the Test plan gives |
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