feat(managementapi): container repo stats (S17 Phase 8 plan: 10/13)
Summary
Plan Step 10 fills the container arm of the repository statistics route. A docker or oci repository's response carries the repositories row's three stored counters as read, plus live counts of the repository's visible images and the tags and manifests hanging off them (Spec: Statistics resources).
ContainerImageStore.AggregateContainerRepositoryEntityCounts answers the three counts in one statement, three scalar subqueries over one namespace partition. The kind gate, the existence-hiding 404, and the format dispatch already exist. This repoints the one docker/oci case at an arm that serves.
Worth a second look
manifests_count includes referrers. No subject_digest filter, per the Repository (container) row in Statistics resources. The manifests list excludes referrers unless asked, so this figure can exceed what a caller paging that list with the default sees. The schema says so too.
The children take the image's marker. container_tags and container_manifests carry no soft_deleted_at of their own, so both child counts join their image and take the image gate. A marked image drops its whole subtree from all three counts at the mark, not at the reap. Asserted through both writers: SoftDeleteContainerImage and a raw SQL mark.
tags_count counts one row the tags list drops. The row shape is a tag whose container_image_id names one image while its container_manifest_id names a manifest of a different image, both under this repository. ListContainerTagsPage requires the manifest to sit under the tag's own image, and this count joins the tag to its image only, so it counts such a row and the list does not. Nothing forbids the row: container_tags has two independent foreign keys and no CHECK tying them. No production writer builds one (the tag upsert resolves the manifest within the image), so only direct SQL reaches it.
That is the plan's stated join ("tags and manifests join their image"), kept deliberately. AC #115 (closed) asks that counts match the visible lists, and whether that binds on every representable row or only on rows the writers produce is a spec question, queued for the spec author on #916. If the answer is "representable", the tightening is one extra join. The divergence is recorded at the join and pinned in both directions by ..._CrossImageTag.
Each child subquery binds namespace_id as well as joining on it, and the bind is redundant. Measured on PostgreSQL 16.15 with enable_partitionwise_join off: remove the bind and the plan is byte-identical, still one partition per table, because the planner propagates the images-side bind across the join equality. The clause stays as this side's own bind, and ..._PrunesEveryChildTable pins the outcome rather than the clause. An earlier revision of this branch justified the bind by claiming the join equality could not prune without partitionwise join. That was false, and the three sites repeating it are corrected.
A namespace-predicate mutation survived the whole behavioural suite. Replacing the namespace predicates with IS_NOT_NULL() tautologies left every scoping test green, because each gave its sibling a distinct container_repositories id and the repository bind alone separated them. ..._TwinRepositoryID removes that crutch: pk_container_repositories is (id, namespace_id), so the same repository id under a second namespace is representable, and the driving bind becomes the only thing telling the two subtrees apart. Re-mutating reds it, {1,3,2} against {2,4,3}.
That pin reaches one of the three namespace-predicate sites, and the bound is worth stating. Dropped independently: the images-side bind in visibleContainerImagesIn is killed by the pin. The child-side WHERE bind and the join-side equality each survive the whole suite. Both survivors are semantically redundant given the first, so no behavioural test could catch them, measured rather than inferred. With the join equality removed and the same container_image id seeded under two namespaces, each holding a tag, the count reads 1 rather than 2, because the two WHERE binds already block the cross-namespace join. No test is added for a state the query cannot reach.
Stored figures serve verbatim. artifacts_count and size_bytes are buffered counters moved by signed deltas with no non-negativity constraint, so a dropped increment can carry either below zero until reconciliation corrects it. Nothing clamps. Both floors are gone as of chore(api): drop statistics floors the counter ... (!1995 - merged) • Hayley Swimelar • 19.4, so the negative-counter case runs through validateAgainstContract with the rest of the sweep instead of asserting the body directly. Re-adding either floor reds that one case and no other, measured. That MR's floor rationale in api/openapi/v1.yaml also asserted that no sweep validates a statistics response yet, which this arm falsifies. It is rewritten to state the mechanism rather than which scope answers, so it holds at any merge order and collapses against the identical bytes on feat(managementapi): maven repo statistics (S17... (!2028 - merged) • Hayley Swimelar • 19.4.
Two arm-level tenant pins were deleted, not lost. The resolution gate refuses a row resolved outside the request namespace before any format dispatch, as of chore(managementapi): repository stats gate (S1... (!1994 - merged) • Hayley Swimelar • 19.4. row.NamespaceID therefore equals the request id in every state an arm can see, and which of the two an arm reads stops being observable. TestRepositoryStatistics_ForeignRow_IsRefused owns the behaviour, verified present and green on this base before the deletion landed.
The plan's "guards" have nothing to attach to. refactor(managementapi): derive dependency guar... (!1936 - merged) • Hayley Swimelar • 19.4 replaced the hand-written nil chains with a reflective walk over Deps, which covers the new field for free. The obligation is met by the seam count in TestNewHandler_PanicsOnNilDependency, one higher here than main's, whatever main's reads at rebase time. Two branches bumping it identically merge clean to the wrong number, so re-derive it from the walk's own failure message rather than resolving that literal by picking a side. That has already happened repeatedly on this branch: main first reached the same number on its own through the Tombstoner seam in feat(managementapi): destructive repository DEL... (!1885 - merged) • Pawel Rozlach • 19.4, and every sibling merge since moved it again.
Four test files outside the plan's Files list are edited, each compile-forced by the new Deps field rather than scope creep: handler_test.go, pact_provider_test.go, read_handlers_integration_test.go, and cmd/artifact-registry/wire_management_test.go. api/openapi/v1.yaml is outside it too, comment-only, for the reason above.
statistics_repository_test.go's family-arm helper is relaxed and renamed, not branched. repoFixture defaults to docker, so two of its assertions and one in the integration suite break when this arm serves. requireFamilyArmPlaceholder becomes requireReachedFamilyArm and checks what those tests' own doc comments claim they pin: that the request reached a format-family arm rather than the gate's 404 or the fail-closed 500. That is the split the plan's Step 4 text sets out, and it is what keeps Steps 8 and 9 from touching the file. handler_test.go's repository-named-statistics row moves to 200 for the same reason. Its fixture is docker, so no sibling reaches it.
Governing ADRs
- ADR-007:
container_images,container_tags, andcontainer_manifestsare allHASH(namespace_id)over 64 partitions, and the child tables carry(namespace_id, container_image_id, ...)indexes. Every subquery reaches one partition per table, measured. This is the repository-scope cost class ADR-007 accepts for its reconciliation walks, one join level shallower than the walk it measured there. - ADR-004: caps tags per artifact at 1,000 but neither images per repository nor manifests per image, so two of the three inputs are unbounded. The spec accepts that on the partition-pruned-and-rare grounds, with denormalized per-repository counters as the named fallback.
- ADR-009: the repository statistics route is one of the four scopes it declares.
No deviation, and no amendment needed.
Testing
| Acceptance clause | Test |
|---|---|
| Stored three equal the repository row, negatives included | TestStatisticsContainerRepository_ServesTheStoredThreeAndTheLiveThree, and end to end in ...RepositoryIntegration_ServesFiguresOverRealRows |
| Live three match the visible rows, referrers counted | TestContainerImageStore_AggregateContainerRepositoryEntityCounts (populated, referrer-only), images_count cross-read against ListContainerImages |
| Zeros for an empty repository, every field present | the all-zero contract row, and the empty-repository datastore subtest |
Non-hosted 404 through the Step 4 gate |
..._NonHostedKindsNeverReachTheArm, docker and oci × virtual and remote |
| A marked image excludes its subtree from every count | both marker paths in the datastore suite, and over the route in the integration suite |
| Tenant and repository isolation | ..._TwinRepositoryID (same repository id, two namespaces), the sibling-repository and cross-namespace subtests, ..._CountsAgainstTheResolvedChildRow |
| Only the container cases reach this arm | ..._NonContainerFormatsNeverReachTheArm (maven and npm, zero probes) |
| Response validates against the contract | TestStatisticsContainerRepository_ResponsesMatchOpenAPIContract |
One correction to the commit trail: the spec-coverage table in the test(managementapi) commit body labels the contract-sweep row AC #120 (closed). The criterion it means is AC #111 (closed) (the OpenAPI document defines each scope's response schema and validates in CI). AC #120 (closed) is the version-statistics addressing rule, which this step does not touch. The other four rows are right.
Beyond the plan's list: the partition-pruning EXPLAIN pin, the cross-image tag divergence, both seam failures as 500 with no driver text or row id in the body, and the canceled-context wrap.
No e2e scenario is affected. The plan's Testing Strategy records the same: the statistics consumers are monolith UI slices whose journeys land with those slices, and no protocol-client catalog scenario reads statistics. Nothing under docs/testing/ mentions statistics today.
Diff size
1,376 added lines against the 500-LOC guardrail, split production 186, tests 1,180, and contract prose 10:
| Group | Lines |
|---|---|
internal/datastore/container_image.go |
114 |
internal/managementapi/statistics_repository.go |
41 |
internal/managementapi/handler.go, statistics_resources.go |
24 |
cmd/artifact-registry/wire_management.go, internal/datastore/query_names.go |
7 |
api/openapi/v1.yaml, comment only |
10 |
| Tests, thirteen files | 1,180 |
Neither cut helps. The 186 production lines are one statement, one handler arm, one interface, and one wiring line. Splitting the tests off them leaves the arm untested, and splitting the datastore aggregate from the arm leaves a Deps field with nothing behind it. The test weight is where the review value is: two of the three scoping properties this MR claims were unpinned until a mutation run showed it.
Related to #316