feat(npm): publish handler commit transaction (S11 Step 16, 1/2)

Dzmitry (Dima) Meshcharakourequested to merge
dm/npm-local-step-16 into main

Part 1 of 2 of the npm hosted plan — Step 16: publish handler — commit transaction.

🗂️ Stacked MRs

Split into 2 stacked MRs to keep each within the review size limit (~600 ideal reviewable LoC). Each part targets the previous one (part 1 → main); review and merge bottom-up.

📦 What this part adds

Persists the npm rows and responds for a publish whose tarball is already staged and committed to CAS (Spec publish-flow Step 5 "persist npm rows" + Step 6 "respond"). The rows-and-cache transaction runs behind the datastore.NpmPublishCommitter seam, which owns the *sql.Tx and keeps database/sql out of the format layer (ADR 023, mirroring NpmPackageUnpublishDeleter): in one transaction it upserts the package, inserts the version (ON CONFLICT DO NOTHING -> 409 version_exists for the concurrent-publish race loser), writes the attachment and file rows, upserts one npm_tags row per dist-tag, and force-expires the packument cache. The publisher's per-version object is filtered to the allow-list before persistence, so scripts and lifecycle hooks never land. After the transaction commits, the handler fires the post-commit packument rebuild (Step 12) exactly once and emits the buffered counter events (Step 24) off the request path.

✅ Spec coverage

Acceptance criterion Test
AC 1 — happy publish writes all rows, 201, fires one rebuild TestPublishCommitIntegration_HappyPath_WritesAllRowsAndFiresOneRebuild
AC 3 — concurrent same-version: one 201, one 409 version_exists TestPublishCommitIntegration_ConcurrentSameVersion
AC 10 — denylisted package.json fields filtered TestPublishCommitIntegration_DenylistedFieldsFiltered
AC 11 — no resolved context persists no rows (401/403 is S08/S09-owned) TestPublishCommitIntegration_MissingResolution_NoRows
AC 19 — empty dist-tags writes zero npm_tags rows TestPublishCommitIntegration_EmptyDistTags_NoTagRows
AC 20 — scoped publish preserves the @ sigil TestPublishCommitIntegration_ScopedPublish_PreservesScopePrefix
AC 27 — the write force-expires the packument cache TestPublishCommitIntegration_ForceExpiresCache
AC 41 — filtered package.json over the cap -> 422 TestPublishCommitIntegration_FilteredPackageJSONTooLarge
AC 57 — tx rollback -> 500, no rows, orphan blob, no rebuild TestPublishCommitIntegration_TxRollback_NoRowsNoRebuild
AC 61 — cross-namespace / cross-repo blob dedup TestPublishCommitIntegration_CrossNamespaceRepoDedup
Buffered counter values after commit TestPublishCommitIntegration_BufferedCountersIncremented
Publish-path dist-tag validation — semver name → 422 tag_name_invalid, mismatched value → 422 dist_tag_version_mismatch, no rows/no session TestPublishCommitIntegration_SemverTagName_Rejected, TestPublishCommitIntegration_MismatchedDistTagValue_Rejected
Multi-entry dist-tags bind to published version; tags_count counts new names only (rebind doesn't increment) TestPublishCommitIntegration_MultiTagPublish_BindsToPublishedAndCountsNewOnly

Database Review Evidence

Queries

Note

Plans are from EXPLAIN (ANALYZE, BUFFERS) against an ephemeral PostgreSQL 17.10 container (matching GL_PG_CURR_VERSION: "17" 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. All three are new point-update methods keyed on indexed columns. Each target partition was seeded with 5000 rows (above the skill's write-target default of 50) so the planner surfaces the production index path — at 50 rows these tiny point-update tables plan as a Seq Scan purely on cardinality. Numbers reflect moderate cardinality and do not capture production-scale effects.

Method Plan node Index Rows (plan / actual) Cost Time Buffers (hit / read) Partitions
datastore.IncrementRepositoryPublishCounters Update → Index Scan repositories_p44_pkey 1 / 1 8.31 0.23ms 62 / 0 1
datastore.IncrementNpmPackagePublishCounters Update → Index Scan npm_packages_p37_pkey 1 / 1 8.31 1.20ms 40 / 0 1
datastore.ForceExpireNpmMetadataTx Update → Index Scan npm_metadata_files_..._namespace_id_npm_package_id_kind_idx 1 / 1 8.30 1.02ms 24 / 0 1
datastore.IncrementRepositoryPublishCounters

Summary: Plan matches intent — an Update driven by an Index Scan over the partition primary key, with the namespace_id literal pruning to one of the 64 hash partitions. The (id, namespace_id) predicate is the PK, so the point update stays O(1) at any cardinality. Actual rows match the estimate (1 / 1) and execution is ~0.23ms at 5000 seeded rows. No anomalies.

Seed shape: namespaces=1, repositories=5000

Rendered SQL:

UPDATE public.repositories
SET (artifacts_count, size_bytes, last_updated_at) = ((repositories.artifacts_count + $1), (repositories.size_bytes + $2), GREATEST(repositories.last_updated_at, NOW()))
WHERE (repositories.namespace_id = $3::uuid) AND (repositories.id = $4::uuid);

Bound args: [1, 1024, 6c52c09c-ec86-4366-b855-b315e056ef4e, ec071453-ae91-443f-9a3d-8f9f0c8a5ce0]

Plan (EXPLAIN (ANALYZE, BUFFERS)):

 Update on repositories  (cost=0.28..8.31 rows=0 width=0) (actual time=0.131..0.131 rows=0 loops=1)
   Update on repositories_p44 repositories_1
   Buffers: shared hit=62
   ->  Index Scan using repositories_p44_pkey on repositories_p44 repositories_1  (cost=0.28..8.31 rows=1 width=34) (actual time=0.005..0.006 rows=1 loops=1)
         Index Cond: ((id = 'ec071453-ae91-443f-9a3d-8f9f0c8a5ce0'::uuid) AND (namespace_id = '6c52c09c-ec86-4366-b855-b315e056ef4e'::uuid))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=155
 Planning Time: 0.355 ms
 Trigger for constraint fk_repositories_namespace_id_namespaces on repositories_p44: time=0.010 calls=1
 Execution Time: 0.232 ms

Timings: planning 0.355ms, execution 0.232ms, total 0.587ms.

datastore.IncrementNpmPackagePublishCounters

Summary: Plan matches intent — an Update driven by an Index Scan over the partition primary key, pruned to one partition by namespace_id. The (id, namespace_id) PK predicate keeps it a point update. Actual rows match the estimate (1 / 1); execution ~1.2ms is dominated by the FK-validation triggers, not the scan. No anomalies.

Seed shape: namespaces=1, repositories=1, npm_repositories=1, npm_packages=5000

Rendered SQL:

UPDATE public.npm_packages
SET (versions_count, tags_count) = ((npm_packages.versions_count + $1), (npm_packages.tags_count + $2))
WHERE (npm_packages.namespace_id = $3::uuid) AND (npm_packages.id = $4::uuid);

Bound args: [1, 1, 4aa1a51d-e296-4a1f-99e9-ca08fa00fdf3, 8c1aec8c-f34a-434b-bbc2-1c7b931606bc]

Plan (EXPLAIN (ANALYZE, BUFFERS)):

 Update on npm_packages  (cost=0.28..8.31 rows=0 width=0) (actual time=0.089..0.089 rows=0 loops=1)
   Update on npm_packages_p37 npm_packages_1
   Buffers: shared hit=40
   ->  Index Scan using npm_packages_p37_pkey on npm_packages_p37 npm_packages_1  (cost=0.28..8.31 rows=1 width=18) (actual time=0.006..0.006 rows=1 loops=1)
         Index Cond: ((id = '8c1aec8c-f34a-434b-bbc2-1c7b931606bc'::uuid) AND (namespace_id = '4aa1a51d-e296-4a1f-99e9-ca08fa00fdf3'::uuid))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=75
 Planning Time: 0.206 ms
 Trigger for constraint fk_npm_packages_namespace_id_namespaces on npm_packages_p37: time=0.014 calls=1
 Trigger for constraint fk_npm_packages_npm_repository_id_npm_repositories on npm_packages_p37: time=0.819 calls=1
 Execution Time: 1.198 ms

Timings: planning 0.206ms, execution 1.198ms, total 1.404ms.

datastore.ForceExpireNpmMetadataTx

Summary: Plan matches intent — an Update driven by an Index Scan over the composite (namespace_id, npm_package_id, kind) index, pruned to one partition. The WHERE keys on (namespace_id, npm_package_id), the index's leading columns, so it force-expires every cache kind for one package (≤3 rows) without a scan. Actual rows match the estimate (1 / 1); execution ~1ms is FK-trigger-dominated. No anomalies.

Seed shape: namespaces=1, repositories=1, npm_repositories=1, blob_storage_blobs=1, blob_storage_attachments=1, npm_packages=5000, npm_metadata_files=5000

Rendered SQL:

UPDATE public.npm_metadata_files SET expires_at = NOW()
WHERE (npm_metadata_files.namespace_id = $1::uuid) AND (npm_metadata_files.npm_package_id = $2::uuid);

Bound args: [911620d1-8a01-4eb7-9925-2a54e57f9e0b, d88abbfc-2815-4309-b6f9-0153ad0d987f]

Plan (EXPLAIN (ANALYZE, BUFFERS)):

 Update on npm_metadata_files  (cost=0.28..8.30 rows=0 width=0) (actual time=0.062..0.062 rows=0 loops=1)
   Update on npm_metadata_files_p50 npm_metadata_files_1
   Buffers: shared hit=24
   ->  Index Scan using npm_metadata_files_p50_namespace_id_npm_package_id_kind_idx on npm_metadata_files_p50 npm_metadata_files_1  (cost=0.28..8.30 rows=1 width=18) (actual time=0.005..0.006 rows=1 loops=1)
         Index Cond: ((namespace_id = '911620d1-8a01-4eb7-9925-2a54e57f9e0b'::uuid) AND (npm_package_id = 'd88abbfc-2815-4309-b6f9-0153ad0d987f'::uuid))
         Buffers: shared hit=3
 Planning:
   Buffers: shared hit=262
 Planning Time: 0.738 ms
 Trigger for constraint fk_npm_metadata_files_blob_storage_attachment_id_bsa on npm_metadata_files_p50: time=0.681 calls=1
 Trigger for constraint fk_npm_metadata_files_namespace_id_namespaces on npm_metadata_files_p50: time=0.009 calls=1
 Trigger for constraint fk_npm_metadata_files_npm_package_id_npm_packages on npm_metadata_files_p50: time=0.135 calls=1
 Execution Time: 1.023 ms

Timings: planning 0.738ms, execution 1.023ms, total 1.761ms.

📚 References

Related to #134 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports