refactor(datastore): extract the shared ListRemotes URL-usable gate

🎯 What this MR does

Extracts the URL-usability gate ContainerRemoteRepositoryStore.ListRemotes applies — refuse a row whose stored URL remote.ParseUpstreamBase rejects, sample up to 5 distinct refusal reasons, log one summary line per call — into a shared refsWithUsableURL helper in the new internal/datastore/remote_repository_enumeration.go. Container is the only caller today; this is prep for an npm-format store that needs the identical gate, landing as a separate MR in the stack below. No behavior changes for container: its message text, field set, and sample bound are byte-for-byte unchanged.

📚 The stack (splitting a single oversized branch into reviewable pieces — see Related to below for the tracking item):

Step MR What it delivers
0 · this MR 👉 !1479 (merged) Shared refsWithUsableURL enumeration helper (this MR)
1 !1481 (merged) npm datastore health seams
2a !1482 (merged) npmremote.Target + UpstreamRequestBuilder
2b !1483 (merged) npmremote.CacheStore + health-sweep glue
3 !1484 (merged) Composition root: wiring, audit sink, breadcrumb

🧩 The change

ContainerRemoteRepositoryStore.listRemotes used to filter and log inline: loop the rows, call remote.ParseUpstreamBase per row, accumulate a bounded sample keyed by refusal reason, and emit one WARN naming the skip count. That logic is now refsWithUsableURL(ctx, tablePrefix, refs): the store maps its jet rows onto remote.RemoteRepositoryRef first, then hands the slice to the shared gate, which returns only the usable ones and logs the same summary it always did — under a tablePrefix argument (containerRemoteTablePrefix here) so each future caller's line stays greppable on its own.

skippedRemoteRow, skippedRemoteRows, logSkippedRemoteRows, and maxListRemotesSkipSample moved verbatim into the new file; logSkippedRemoteRows gained the tablePrefix parameter.

The prefix is a named type, remoteTablePrefix, whose members are declared in the same file, and skipSummaryMessage maps a member to its text rather than interpolating the argument. Its underlying type is an integer, so neither naming a family with a literal nor converting a row value into one compiles — the greppable prefix is declared in this file, not supplied at a call site. A value that names no declared member takes the unprefixed message and carries no text of its own. Container's line is byte-for-byte unchanged. The AppSec review is answered in its own thread.

A note for !1481 (merged): adding npm's caller means adding its remoteTablePrefix member and its skipSummaryMessage arm, and neither can be skipped quietly. A bare literal does not compile, and a member declared without an arm fails the lint: .golangci.yaml enables exhaustive with default-signifies-exhaustive: false, so the default clause does not make the switch look exhaustive. What is left to add there is the store-side message assertion, the way TestContainerRemoteRepositoryStore_ListRemotes_SkipSummary now does — only a store's own suite can show it passes its own prefix rather than a copied one.

Verification

  • Full ContainerRemoteRepositoryStore suite passes (11/11, including TestContainerRemoteRepositoryStore_ListRemotes_SkipSummary, ..._SkipSummary_DedupesByReason, and ..._PlanShape) — the refactor is behavior-preserving by construction and by test. ..._SkipSummary now also asserts the rendered message, which is the one claim neither the compiler nor exhaustive can make: that this store passes its own prefix rather than a copied one or the zero value. Verified by mutation — passing 0 fails the assertion.
  • refsWithUsableURL also has direct coverage now, in internal/datastore/remote_repository_enumeration_test.go (7 tests, no database): the usable refs come back whole and in arrival order, the skip count is exact, the sample stops at maxListRemotesSkipSample across more distinct reasons than the bound, many refs sharing one reason collapse to one entry, a call that skipped nothing or enumerated nothing logs nothing, the container message text is asserted whole, and skipSummaryMessage is pinned for a declared member and for the values that remain reachable once the type is an integer — zero, out of range, negative. The summary is read back through a capturing slog JSON handler, so the sample is asserted against the encoding that ships rather than the Go value.
  • The two guards the type leans on were checked, not assumed: a bare literal prefix and a remoteTablePrefix(row.URL) conversion are both compile errors, and adding a second member without a skipSummaryMessage arm produces missing cases in switch of type datastore.remoteTablePrefix (exhaustive).
  • go build ./..., go vet ./internal/datastore/..., golangci-lint run --max-same-issues=0 --max-issues-per-linter=0 ./internal/datastore/...: clean.
  • golangci-lint run --build-tags=integration --max-same-issues=0 --max-issues-per-linter=0 ./internal/datastore/...: clean on both changed files.

🧪 E2E scenario impact

None: pure internal refactor, no request-path or protocol behavior change. No files under docs/testing/ reference this code path.

Related to #346 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports

Loading
Loading