chore(datastore): add the maven remote row-locked update

Hayley Swimelarrequested to merge
hswimelar/s17-p6-step-5 into main

Why

Maven's remote store has the create and details halves but no update path, so the remote settings PATCH (plan Step 23) has no Maven store to dispatch to. This MR completes the store pair with the row-locked update: field-wise patch semantics, the in-store credential clear on URL change returning remote.CredentialAction, and the in-transaction health reset. Plan: 2026-08-13-s17-phase6-virtual-remote-repositories.md, Step 5. Spec: S17-rest-management-api.md. The create half merged in chore(datastore): add the maven remote create a... (!1579 - merged) • Hayley Swimelar • 19.3.

What's non-obvious

  • A URL change is a normalized-value change: the supplied URL passes through remote.ParseUpstreamBase before both the comparison and the write, so a respelling of the stored value is a no-op success (Changed=false, no clear, no reset, no audit event). npm's landed update is the precedent.
  • The update input carries MetadataCacheValidityHours and SnapshotMetadataAlwaysRevalidate. npm's landed input now carries the metadata window too: 3ef2294f added it to UpdateNpmRemoteRepositoryInput, to isZero(), and to npmRemoteUpdateAssignments. The spec marks both fields mutable for Maven.
  • The store input has no explicit-credential-clear arm: the spec's credentials: null tri-state clear is handler-side composition (Step 23), matching npm's landed store shape. The store clears only on URL change, and it reports remote.CredentialClear on a URL change even when no pair was stored, the remote.CredentialManager.ApplyURLChange semantics.
  • The health reset rides the URL change's own UPDATE and is unconditional on the credential arm: a same-request pair keeps the credentials and still resets health. It is not durable against a probe already in flight, because the compare-and-set carries no url. Nulling last_health_checked_at widens the exposure to every probe rather than causing it, since a probe stamps itself one line before the call and the guard would not have refused it under the old stamp either. The method doc carries the window and names both twins: container documents it, and npm carries the same reset and guard without a note. The failure-counter reset and the eviction enqueue are handler concerns, both Step 34.
  • No actor field on the input, matching the landed create. The management audit seam (plan Step 3) has since merged: it added Actor to remote.CredentialWriteEvent and left remote.RecordCredentialWrite without an actor parameter, so this emit compiles untouched and no merge order enforces the thread either way. The obligation stays recorded on the seam in internal/remote/audit.go and in Complete the S03-A/S13 contract for the credent... (#557) • Unassigned.
  • The URL-change NULLs are built per call with pg.CAST(pg.NULL) rather than pg.StringExp(pg.NULL). Those wrappers call setRoot on what they wrap, so wrapping the one shared pg.NULL mutates a process-global and races under -race. fix(datastore): stop nullable binds from wrappi... (!1710 - merged) • João Pereira • 19.4 converges npm's, container's, and the remaining main sites onto the same call.
  • Error wraps are identifier-free per the create's convention, deliberately diverging from npm's grandfathered identifier-carrying wraps.
  • The new FOR UPDATE lock statement's partition pruning is pinned by TestMavenRemoteRepositoryStore_LockMavenRemoteRepository_PrunesToOnePartition, standing in for posted EXPLAIN output.

Size

1911 added lines: production 550 (maven_remote_repositories_update.go) and 2 (query_names.go), integration tests 1073, unit tests 247, explain pin 39. The plan scopes the update as one step mirroring npm's landed update, and splitting the suite from the store would sever the npm dropped-subtest diff from the code it reviews.

Test plan

Unit suites (TestUpdateMavenRemoteRepositoryStmt SET-order and binding pins, argument guards) and the integration battery (go test -tags=integration -count=1 ./internal/datastore/ -run 'TestMavenRemoteRepositoryStore_'): 16 behavior subtests (clear and reset only on URL change, same-request pair survival, respelled-URL no-op, the two-updater lock race), 12 row gates (every format and kind as a positive ErrNotFound hit, with a same-namespace decoy so an uncorrelated gate fails the suite), 10 credential-audit cases including the caller-rollback strand, 9 argument guards, and the lock-statement pruning pin. Zero dropped subtests against npm's update suites. golangci-lint 2.12 clean in both modes on the step's files.

Spec coverage

Spec: docs/specs/S17-rest-management-api.md Plan: docs/plans/2026-08-13-s17-phase6-virtual-remote-repositories.md, Step 5

Scope: the internal/datastore Maven remote-repository update. Acceptance criteria are numbered by position (123 total); the Phase 6 block opens at 68.

Acceptance criteria

# Criterion (store-layer share) Tests
AC-73 A create or update with credentials stores them and never the values; a one-sided pair and an over-2048 value are refused Update share: TestMavenRemoteRepositoryStore_UpdateMavenRemoteRepository ("a new pair supplied with the url change replaces instead of clearing", "replaces the pair without a url change"), ..._RowGates ("rejects an over-cap credential value before the row is touched"), ..._UpdateMavenRemoteRepository_ArgumentGuards (empty pair, half pair, over-cap, all through validateMavenRemoteCredentialPair). The 400 mapping and has_credentials serialization are Steps 11, 12, and 24; the create share is Step 4
AC-75 PATCH merges settings by field: a field absent from the body is unchanged ..._UpdateMavenRemoteRepository ("writes an explicit cache validity", "writes an explicit metadata window", "writes the snapshot revalidation flag", "keeps the stored cache validity across a url change", where create-time resolution is not re-run on update), ..._UpdateCredentialWriteAudit ("an update that leaves the credential columns alone emits nothing"), TestUpdateMavenRemoteRepositoryStmt (a window-only patch renders only that column). The credentials: null explicit clear is handler-side composition, Steps 23-24
AC-76 PATCH changing url without credentials clears the stored pair in the same transaction and emits the audit event; changing both stores the new pair and reports no clear ..._UpdateMavenRemoteRepository ("clears the credential pair on a url change with no new pair", "a new pair supplied with the url change replaces instead of clearing", "a respelled url is not a change", "reports a clear on a url change even when no pair was stored", "the locked read serializes two updaters onto fresh state" (one clear, not two), "a rolled-back transaction leaves the row untouched"), ..._UpdateCredentialWriteAudit (both clear rows), TestUpdateMavenRemoteRepositoryStmt (the clear shares the url write's statement). has_credentials: false serialization is Steps 12 and 24 and credentials_cleared: true is Step 34; the container auth-memo clear is Step 7
AC-77 PATCH changing url resets last_health_status to unknown and last_health_checked_at to null in the same transaction ..._UpdateMavenRemoteRepository ("a url change resets the stored health verdict", "a url change with a same-request pair still resets health", "a non-url patch leaves the health columns untouched", "a respelled url leaves the health columns untouched"), TestUpdateMavenRemoteRepositoryStmt (the reset rides the url change's own SET list). The unprobed response body is Steps 23-24, and the post-commit failure-counter reset is Step 34
AC-79 Update emits one audit event naming the actor; every credential write emits S13's credential event Store share (the update's credential events): ..._UpdateCredentialWriteAudit (clear with and without a stored pair, set, replace, empty and half-empty stored pairs read absent so a supply is a set, nothing on a window-only write, nothing on a no-op, nothing on a refusal); the two lock-race subtests pin set-versus-replace under concurrency. The repository-update event with the actor and the previous and new URL values is Steps 3 and 24; the create-side credential events are Step 4's suite
AC-83 An invalid url returns 400 on update Store share: ..._UpdateMavenRemoteRepository_ArgumentGuards ("invalid url": a query string is refused through remote.ParseUpstreamBase before the row lock). The reject set is the merged remote suite's; the 400 is Steps 11 and 24
AC-74, AC-84 Details reads and the create-time Central pin Step 4 (merged); the update's "keeps the stored cache validity across a url change" subtest pins that the pin is not re-derived here
AC-1 to AC-72, AC-78, AC-80 to AC-82, AC-85 to AC-123 Phases 1-4, the Phase 6 OpenAPI document, creates, the every-kind cap, eviction enqueue on URL change, associations, connection test, remote reads and evictions, and Phase 8 statistics Not this step. Step 5 is the Maven remote datastore update only; the container twins are Steps 6-7, npm's update backfill (metadata window and health reset) is Step 8, handlers are Steps 10-12 and 23-24

Error cases

Condition Tests
Detail, Update, Delete: repository missing returns 404 Update share (ErrNotFound): ..._RowGates (missing binding, soft-deleted parent, npm-, docker-, and oci-format parents, hosted and virtual kinds, cross-namespace; every gated row keeps its stored pair, and the parent-gate cases seed a valid same-namespace decoy so an uncorrelated gate fails the suite)
Update: invalid url, a one-sided or over-long credential value, an out-of-range cache window return 400 Store share (the sentinels the handler maps): ..._UpdateMavenRemoteRepository_ArgumentGuards, ..._RowGates ("maps a CHECK refusal without leaking the failing row", "maps a metadata-window CHECK refusal to the same sentinel", "a transient failure is not misreported as ErrNotFound"). The envelope and status are Steps 11 and 24
Every other row (auth, routing, list, delete, PATCH body validation, associations, connection test, statistics) Not this step: HTTP-layer conditions owned by their route steps

Security considerations

Concern Tests
Injection: all queries use the Jet builder with bound parameters TestUpdateMavenRemoteRepositoryStmt asserts every value binds as an ordered arg and the partition key sits in the predicate
Tenant isolation: every query scoped by namespace_id, the partition key "does not update a binding from a different namespace", the WHERE pin in TestUpdateMavenRemoteRepositoryStmt
Upstream credentials: write-only, cleared on URL change, audited on every write, absent from logs and error bodies The clearing subtests and ..._UpdateCredentialWriteAudit; "maps a CHECK refusal without leaking the failing row" (requireNoPgErrorInChain plus the undisclosed-value assertion); every refused update asserts the stored pair survived
SSRF: the static covers run at update through remote.ParseUpstreamBase ..._UpdateMavenRemoteRepository_ArgumentGuards "invalid url" (refused before the row lock); the cover set itself is the merged remote suite's
URL retarget and the cache: the eviction pass on a committed URL change Not this step: the enqueue and its 503 rule are Steps 23-24. The store share is the normalized URL-change comparison the pass keys on, pinned above
Auth, existence hiding, write authorization, statistics authorization Handler and platform layers; not this step
Context for LLM agents

Rationale

  • An explicit-clear arm on the store input. Rejected: npm's landed store routes the tri-state null through the handler (Step 23), and diverging here would fork the per-format store contract the handler dispatches over.
  • Re-running the Central-pin resolution when the URL changes to a known-immutable host. Rejected: the spec words the pin as a create-time default, and the update's field-wise contract writes only what the caller supplied.
  • An actor field on the update input. Deferred, not rejected: remote.RecordCredentialWrite still takes no actor parameter, and the merged audit seam did not add one, so threading the actor through every credential write path stays open work rather than something a merge order forces.

Non-goals

  • Handler, route, or OpenAPI wiring. Steps 11, 12, 23, 24, and 34 consume this store.
  • The failure-counter reset and the eviction enqueue on URL change. Handler-side, Step 34.
  • npm's metadata-window backfill and health reset. Step 8.
  • The container twins. Steps 6 and 7.
  • The plan's Status-table row. A dedicated single-writer MR fills it for the whole fan-out.
  • The endemic contextcheck class in pre-existing sibling integration suites. CI-lint-blind, untouched.

Database Review Evidence

Queries

Note

Plans are from EXPLAIN (ANALYZE, BUFFERS) against an ephemeral PostgreSQL 17.10 container (matching GL_PG_CURR_VERSION from .gitlab-ci-other-versions.yml), with synthesized seed data rolled back per query and the container torn down at the end of the run. Numbers reflect moderate cardinality and do not capture production-scale effects. See Database review evidence for seed sizing, methodology, and the anomalies the skill flags. Expand each row's details for the seed shape, rendered SQL, bound args, and raw plan.

Method Plan node Index Rows (plan / actual) Cost Time Buffers (hit / read) Partitions
datastore.lockMavenRemoteRepositoryStmt LockRows unique_maven_remote_repositories_namespace_id_and_repository_id, index_repositories_on_namespace_id_id_format_remotes 1 / 1 16.64 0.024ms 7 / 0 1/64 maven_remote_repositories, 1/64 repositories
datastore.updateMavenRemoteRepositoryStmt.UrlChangeClear Update n/a (Seq Scan, see notes) 0 / 0 1.75 0.085ms 16 / 0 1/64 maven_remote_repositories
datastore.updateMavenRemoteRepositoryStmt.WindowOnly Update n/a (Seq Scan, see notes) 0 / 0 1.75 0.087ms 16 / 0 1/64 maven_remote_repositories

Query notes:

  • Both Update rows report 0 / 0 because the statement returns no rows. The driving scan matched 1 row of the 50 seeded, in the pruned partition.
  • The Seq Scan on both Update rows is the write-target seed size (1 target row plus 49 siblings), not a missing index. Re-running the url-change statement against 5000 seeded rows switches the plan to an Index Scan on pk_maven_remote_repositories (cost 8.30, actual 0.013 ms), still pruned to one partition.
  • The Trigger for constraint fk_maven_remote_repositories_* lines in the two Update plans are an artifact of seeding inside the measured transaction. PostgreSQL forces the referential check when the row was inserted by the current transaction, even though this UPDATE changes neither repository_id nor namespace_id. Committing the seed first and measuring the UPDATE in its own transaction, which is the production shape, emits no trigger lines and drops execution from 0.852 ms to 0.231 ms. Read the trigger time (0.6 ms to 8.5 ms across runs, dominated by the RI check's own plan cache) as measurement overhead rather than statement cost.
datastore.lockMavenRemoteRepositoryStmt

Summary: Plan matches the method's intent: the FOR UPDATE read resolves through the unique index on (namespace_id, repository_id) and the parent gate runs as a Nested Loop Semi Join over index_repositories_on_namespace_id_id_format_remotes, with both tables pruned to one of 64 partitions. Estimates match reality (1 / 1) and execution is 0.045 ms against 5000 seeded bindings. LockRows sits above the join, so the lock covers the maven_remote_repositories row only, which is what choosing an EXISTS predicate over the reads' JOIN buys. No anomalies.

Seed shape: namespaces=1, repositories=5000, maven_remote_repositories=5000

Rendered SQL:

SELECT maven_remote_repositories.id AS "maven_remote_repositories.id",
     maven_remote_repositories.url AS "maven_remote_repositories.url",
     ((((maven_remote_repositories.tmp_plaintext_username IS NOT NULL) AND (maven_remote_repositories.tmp_plaintext_username != $1::text)) AND (maven_remote_repositories.tmp_plaintext_password IS NOT NULL)) AND (maven_remote_repositories.tmp_plaintext_password != $2::text)) AS "has_credentials"
FROM public.maven_remote_repositories
WHERE ((maven_remote_repositories.namespace_id = $3::uuid) AND (maven_remote_repositories.repository_id = $4::uuid)) AND (EXISTS (
           SELECT $5
           FROM public.repositories
           WHERE ((((repositories.namespace_id = $6::uuid) AND (repositories.format = $7)) AND (repositories.kind = $8)) AND (repositories.soft_deleted_at IS NULL)) AND (repositories.id = maven_remote_repositories.repository_id)
      ))
FOR UPDATE;

Bound args: ['', '', '9065602d-b3a1-422f-8b0e-7010cb193cf7', 'a2d89716-e162-456d-a650-5ed30b84057b', 1, '9065602d-b3a1-422f-8b0e-7010cb193cf7', 1, 2]

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

LockRows  (cost=0.56..16.64 rows=1 width=95) (actual time=0.023..0.024 rows=1 loops=1)
  Buffers: shared hit=7
  ->  Nested Loop Semi Join  (cost=0.56..16.62 rows=1 width=95) (actual time=0.020..0.020 rows=1 loops=1)
        Buffers: shared hit=6
        ->  Index Scan using maven_remote_repositories_p50_namespace_id_repository_id_idx on maven_remote_repositories_p50 maven_remote_repositories  (cost=0.28..8.30 rows=1 width=104) (actual time=0.009..0.010 rows=1 loops=1)
              Index Cond: ((namespace_id = '9065602d-b3a1-422f-8b0e-7010cb193cf7'::uuid) AND (repository_id = 'a2d89716-e162-456d-a650-5ed30b84057b'::uuid))
              Buffers: shared hit=3
        ->  Index Scan using repositories_p50_namespace_id_id_format_idx on repositories_p50 repositories  (cost=0.28..8.31 rows=1 width=26) (actual time=0.008..0.008 rows=1 loops=1)
              Index Cond: ((namespace_id = '9065602d-b3a1-422f-8b0e-7010cb193cf7'::uuid) AND (id = 'a2d89716-e162-456d-a650-5ed30b84057b'::uuid) AND (format = '1'::smallint))
              Filter: ((soft_deleted_at IS NULL) AND (kind = '2'::smallint))
              Buffers: shared hit=3
Planning:
  Buffers: shared hit=254
Planning Time: 1.126 ms
Execution Time: 0.045 ms

Timings: planning 1.126ms, execution 0.045ms, total 1.171ms.

datastore.updateMavenRemoteRepositoryStmt.UrlChangeClear

Summary: Plan matches intent: the partition key prunes to one of 64 partitions, and the url write, the credential zeroing, and the health reset render as one statement over one row. The Seq Scan is the seed size rather than a missing index. A write target is seeded with 50 rows, which fits a single page, and a supplementary 5000-row run of this same statement chose an Index Scan on pk_maven_remote_repositories (cost 8.30, 0.013 ms). No anomalies.

Seed shape: namespaces=1, repositories=50, maven_remote_repositories=50

Rendered SQL:

UPDATE public.maven_remote_repositories
SET url = $1::text,
    tmp_plaintext_username = NULL,
    tmp_plaintext_password = NULL,
    last_health_status = $2,
    last_health_checked_at = NULL
WHERE (maven_remote_repositories.namespace_id = $3::uuid) AND (maven_remote_repositories.id = $4::uuid);

Bound args: ['https://repo.example.test/maven2', 0, 'aa64f563-08c9-4a6c-8bbd-763301bfe897', '6385d734-e058-4a23-ab06-9d5ccd1b8ee9']

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

Update on maven_remote_repositories  (cost=0.00..1.75 rows=0 width=0) (actual time=0.085..0.085 rows=0 loops=1)
  Update on maven_remote_repositories_p34 maven_remote_repositories_1
  Buffers: shared hit=16
  ->  Seq Scan on maven_remote_repositories_p34 maven_remote_repositories_1  (cost=0.00..1.75 rows=1 width=116) (actual time=0.004..0.006 rows=1 loops=1)
        Filter: ((namespace_id = 'aa64f563-08c9-4a6c-8bbd-763301bfe897'::uuid) AND (id = '6385d734-e058-4a23-ab06-9d5ccd1b8ee9'::uuid))
        Rows Removed by Filter: 49
        Buffers: shared hit=1
Planning:
  Buffers: shared hit=45
Planning Time: 0.290 ms
Trigger for constraint fk_maven_remote_repositories_namespace_id_namespaces on maven_remote_repositories_p34: time=0.021 calls=1
Trigger for constraint fk_maven_remote_repositories_repository_id_repositories on maven_remote_repositories_p34: time=0.593 calls=1
Execution Time: 0.852 ms

Timings: planning 0.290ms, execution 0.852ms, total 1.142ms.

datastore.updateMavenRemoteRepositoryStmt.WindowOnly

Summary: The narrowest SET list the builder produces, rendered against the same WHERE. Identical access path and cost to the url-change arm, which is the point of running both: the dynamic SET list changes what is written, not how the row is found. Pruned to one of 64 partitions. No anomalies.

Seed shape: namespaces=1, repositories=50, maven_remote_repositories=50

Rendered SQL:

UPDATE public.maven_remote_repositories
SET metadata_cache_validity_hours = $1
WHERE (maven_remote_repositories.namespace_id = $2::uuid) AND (maven_remote_repositories.id = $3::uuid);

Bound args: [6, '87039fa0-64ec-4b54-8dca-5b8dc8fcad9c', '1f6e2531-5d79-4fd5-8ed9-f2609c6bace9']

Plan (EXPLAIN (ANALYZE, BUFFERS) output):

Update on maven_remote_repositories  (cost=0.00..1.75 rows=0 width=0) (actual time=0.086..0.087 rows=0 loops=1)
  Update on maven_remote_repositories_p33 maven_remote_repositories_1
  Buffers: shared hit=16
  ->  Seq Scan on maven_remote_repositories_p33 maven_remote_repositories_1  (cost=0.00..1.75 rows=1 width=12) (actual time=0.004..0.006 rows=1 loops=1)
        Filter: ((namespace_id = '87039fa0-64ec-4b54-8dca-5b8dc8fcad9c'::uuid) AND (id = '1f6e2531-5d79-4fd5-8ed9-f2609c6bace9'::uuid))
        Rows Removed by Filter: 49
        Buffers: shared hit=1
Planning:
  Buffers: shared hit=46
Planning Time: 0.291 ms
Trigger for constraint fk_maven_remote_repositories_namespace_id_namespaces on maven_remote_repositories_p33: time=0.022 calls=1
Trigger for constraint fk_maven_remote_repositories_repository_id_repositories on maven_remote_repositories_p33: time=0.597 calls=1
Execution Time: 0.858 ms

Timings: planning 0.291ms, execution 0.858ms, total 1.149ms.

Related to #314

Edited by Hayley Swimelar

Merge request reports