chore(managementapi): bulk-delete enqueue seam and River adapter
Why
Bulk delete answers 202 and applies the batch in a job, so a handler has to enqueue. managementapi holds no database handle and no job-backend client on purpose: its Deps is narrow interfaces, and the started *jobsriver.Client lives unexported in the composition root. A bulk request that arrives while River is still building has to answer 503 service_unavailable rather than read as a server defect.
This is Step 28 of the S17 Phase 4 plan, against the bulk-delete Outcome and Error Cases sections of S17. The seam ships inert: the bulk routes that consume it are later steps of the same plan, so nothing wires the adapter into managementapi.Deps yet.
What is worth attention
- The seam takes a batch builder, not built jobs. Only the enqueuer can read the acceptance time off the database clock (
SELECT now(), inside the enqueue transaction), and only the caller knows the payload types a later step introduces.BulkDeleteBatchFuncreceives that instant and returns the jobs, which keeps the seam free of any concrete payload type and lets a chunked selector enqueue atomically. - Which failures carry the unavailable sentinel. An unpublished client, an absent pool, a transaction that will not start, an unreadable clock, and a refused insert all record nothing, so they answer 503 and the caller resubmits the whole batch. A failed
Commitwithholds the sentinel, because the batch's fate is then unknown, and a batch this service built wrong (empty, unregistered kind, over the payload cap) travels as itself, because a retry rebuilds it. - The 503 arm stamps 499 on a dead request. A disconnect finalizes the transaction, and the insert that follows is classified as a backend outage, so the cancellation is gone from the error by the time the helper sees it. The request context is the predicate instead. Left unstamped, the instrumentation writer records the request as 200, and a backend slow enough to time clients out would read as a burst of successes.
wire_jobs.gogains 5 lines, and the accessor ridesbootDeps. Publishing beforeStartis safe because jobsriver refuses a pre-Start enqueue withErrBackendUnavailable, which this seam maps to the 503. UsingbootDepsrather than a newwireJobsparameter keeps the insertion disjoint from Step 29, which edits the same closure's registration loop, so the two branches merge in either order.- Size: 1458 lines, 285 of them production. The test floor is the rest, and it was authored before the code it constrains, so splitting the two would undo that.
Test plan
go test ./internal/managementapi/... ./cmd/artifact-registry/..., plus the enqueue floor against real PostgreSQL: go test -tags=integration -run 'BulkEnqueuer|RiverClientHolder' ./cmd/artifact-registry/.... That floor asserts a two-job batch lands both rows on one database-clock acceptance time, a batch whose second job fails records neither, an empty batch records nothing, an unstarted client answers unavailable, and the River build closure publishes the client. Lint is clean in both tag sets, integration tags included.
Spec coverage
Scoped to the bulk-delete enqueue path, which is the spec surface this step ships. Every other S17 row belongs to another step of the plan and travels with that step's MR.
Acceptance criteria
| # | Criterion | Tests |
|---|---|---|
| AC-15 | Every error path returns the S01 envelope with the correlation request_id |
TestEnqueueBulkDelete_ResponseMatrix (the 503 and 500 arms decode the envelope). request_id is stamped by transport.WriteError and covered in internal/transport. |
| AC-39 | No bulk request returns 409 or 422 on any route |
TestEnqueueBulkDelete_ResponseMatrix (the helper's whole status vocabulary is 202, 503, 500) |
| AC-44 | No bulk response body reports per-entry outcomes | TestEnqueueBulkDelete_ResponseMatrix (the accepted row asserts an empty body) |
| AC-51 | Bulk delete applies its entries in a job on the S27 foundation | TestIntegration_BulkEnqueuer_EnqueueFloor/a_two-job_batch_lands_both_rows_on_one_database-clock_acceptance_time. The enqueue half only, since job application is the worker steps. |
| AC-55 | A batch of exactly the cap returns 202 |
Partial: TestEnqueueBulkDelete_ResponseMatrix (the 202) and TestEnqueueBulkDelete_ChunkedBatch_IsOneEnqueue (a multi-job batch is one enqueue). The cap itself is Step 27. |
| AC-62 | An artifact newer than a delete_all acceptance time survives it |
TestIntegration_BulkEnqueuer_EnqueueFloor/a_two-job_batch_lands_both_rows_on_one_database-clock_acceptance_time (the time's provenance and its single-instant property). The predicate is the worker steps. |
Error cases
| Condition | Status | Code | Tests |
|---|---|---|---|
| Bulk delete: job backend unavailable at enqueue | 503 |
service_unavailable |
TestEnqueueBulkDelete_ResponseMatrix (bare and wrapped sentinel), TestBulkEnqueuer_UnpublishedClient_IsUnavailable, TestBulkEnqueuer_NilHolder_IsUnavailable, TestBulkEnqueuer_NilDatabaseHandle_IsUnavailable, TestIntegration_BulkEnqueuer_EnqueueFloor/a_built_but_unstarted_client_is_unavailable |
| Bulk delete: no job recorded when the enqueue fails | 503 |
service_unavailable |
TestIntegration_BulkEnqueuer_EnqueueFloor/a_built_but_unstarted_client_is_unavailable, .../a_batch_whose_second_job_fails_records_neither |
| All: unexpected server failure | 500 |
internal_server_error |
TestEnqueueBulkDelete_ResponseMatrix (unrelated-error row), TestEnqueueBulkDelete_CancelledRequest_WritesNothing, TestEmptyBulkDeleteBatch_IsNotTheUnavailableSentinel, TestIntegration_BulkEnqueuer_EnqueueFloor/an_empty_batch_is_an_error_and_records_nothing |
Bulk delete: the 400 selector and body rows, 413 size cap |
- | - | Owned by Step 27 (selector decode). This helper runs after validation, so no arm of it can answer them. |
Artifact writes: the 404 and 405 route rows |
- | - | Owned by Step 8 (write-route scaffold). The seam holds no route, no slug, and no path value. |
Security considerations
| Concern | Tests |
|---|---|
| Echoed input: no failure response repeats a submitted identifier | TestEnqueueBulkDelete_ResponseMatrix (the 503 and 500 envelopes carry static messages, and the seam never receives a selector) |
| Bounded blast radius: the batch cap | Owned by Step 27 (selector decode), which checks the cap before the namespace lookup. |
| Authentication, authorization, existence hiding, tenant isolation | Not reached by this seam: it holds no namespace, no selector, and no query. Covered by the route and handler steps. |
Context for LLM agents
Rejected alternatives
- A
[]jobs.Argsparameter with the acceptance time stamped by the caller. The caller cannot read the database clock, and a time taken from an application host is what thedelete_allboundary rules out ("read from the database clock so the boundary does not depend on an application host's"). - A second seam method that returns the acceptance time. The clock read would then land in a different transaction from the inserts, so the boundary a worker resolves against and the batch it resolves could disagree.
- A new
wireJobsparameter for the accessor, mirroringperiodicRegistrations. Correct in isolation, but Step 29 adds its own parameter to the same signature and the same call line, so both branches would conflict there.bootDepsalready reacheswireJobsand already carries a non-factory hook (fatal), and it doubles as the override seam the composition test uses to observe what the build closure published. - Widening the
riverClientinterface the River component holds.jobsriver.EnqueueTxis a generic free function over the concrete client, so no interface method can carry it. - A retry inside the adapter. The spec makes the caller resubmit the whole batch, and a server-side retry would hold a connection through an outage for a request that has already lost its client.
Non-goals
- Wiring the adapter into
managementapi.Depsor any route. Steps 32, 34, and 37 own those, and the plan orders workers before routes because a live enqueue route whose worker kind is unregistered strands accepted jobs. - The bulk job payload types and the byte-budget chunker (Step 29), the batch cap and selector validation (Step 27), and the background-jobs documentation (Step 29).
- The plan's Status table. Nine step branches are in flight from this one plan, so the rows land together in a single follow-up commit rather than nine conflicting edits.
- An upper bound on batch size inside the seam. The cap lives at decode and in Step 29's chunker, so a second one here would duplicate it.
Carried forward for the route steps
- Nothing yet ties a consumer to the
bootDepsholder, so a route step that constructsnewBulkEnqueuer(db, &riverClientHolder{})would read nil forever and every bulk delete would answer 503 while the boot-window comment made that look expected. Add a boot-level composition test that enqueues through the wiredmanagementapi.DepsafterStartand asserts a job row lands. Tracked in Boot-level composition test for the bulk enqueu... (#603) • Hayley Swimelar. internal/managementapi/contract_test.go'semittablelist omitsservice_unavailable, andapi/openapi/v1.yamlonmainends its Error code enum atinternal_server_error, so the code this surface answers with is today neither documented nor asserted. Its open diff adds both halves, so the gap closes with feat(managementapi): declare the artifact delet... (!1445 - merged) • Hayley Swimelar • 19.3.- On a done request context this surface's shared 500 path (
logAndWriteInternalError) still returns without stamping a status, so those requests are recorded as 200. The 503 arm added here stampstransport.StatusClientClosedRequest. Aligning the 500 path is its own change, inresolve.go.
Related to #313 (closed)