feat(npm): unpublish handler + tests (npm hosted step 19, part 2/2)

Dzmitry (Dima) Meshcharakourequested to merge
dm/npm-hosted-step-19-part2of2 into main

Part 2 of 2 of the npm hosted plan — Step 19: single-version unpublish (both wire steps).

🗂️ Stacked MRs

Split into 2 stacked MRs to keep each within the review size limit (~600 ideal reviewable LoC). Part 1 has merged; this part is rebased onto main and targets it directly.

🔌 What this part adds

  • UnpublishVersionHandler (step a) for PUT /{slug}/npm/{repo}/{package}/-rev/{rev}: soft-deletes the omitted version, its npm_files, and its npm_tags in one transaction via the part-1 deleter, force-expires the packument cache, then post-commit enqueues one packument rebuild and emits the versions_count / tags_count buffered decrements off the request path (the tags_count decrement is skipped when the version owned no dist-tags). {rev} mismatch → 409 rev_mismatch; missing package → 404 package_not_found; no single omitted version → 404 version_not_found; missing auth resolution → 500 (AC 58). Success → 200 {"ok": true}.
  • UnpublishVersionFileHandler (step b) for DELETE /{slug}/npm/{repo}/{package}/-/{file}/-rev/{rev}: idempotent file cleanup that verifies {rev} and no-ops when step (a) already soft-deleted the file.
  • Unit + boundary tests (fakes, no DB) and the integration suite that drives the real part-1 datastore layer end-to-end.

ℹ️ The routes stay wired to the 501 placeholder in handler.go; mounting the real handlers is a deliberate later step (conformance harness, Step 23), the same pattern used for the other npm handlers.

✅ Spec coverage

Acceptance criterion / case Test
AC 25 — two-step wire flow (PUT then DELETE), step b idempotent TestUnpublishVersionIntegration_TwoStepWireFlow
AC 25 — step (a) soft-deletes omitted version + files + tags in one tx TestUnpublishVersionIntegration_SoftDeletesOmittedVersion
AC 25 — non-matching {rev} → 409 rev_mismatch, rows untouched TestUnpublishVersionIntegration_RevMismatch
AC 26 — step (a) alone leaves the tarball reachable for GC TestUnpublishVersionIntegration_TarballReachableForGC
AC 27 — packument cache force-expired + exactly one rebuild enqueued TestUnpublishVersionIntegration_CacheForceExpired
AC 34 — missing package → 404 package_not_found TestUnpublishVersionIntegration_PackageNotFound
AC 35 — no omitted active version → 404 version_not_found TestUnpublishVersionIntegration_VersionNotFound
AC 35 — >1 omitted version → 404 version_not_found, rows untouched TestUnpublishVersionIntegration_MultipleOmittedVersionNotFound
AC 25 (step b) — file cleanup without step (a); stale {rev} → 409 TestUnpublishVersionFileIntegration_SkippedStepA, ..._RevMismatch
AC 58 — auth contract (no resolution → 500) TestUnpublishVersion{,File}Handler_MissingResolution_Returns500
Error mapping (rev / version / package sentinels → 409 / 404) TestUnpublishVersion{,File}Handler_ErrorMapping
Success dispatch + counter decrements (tags N>0 and N=0 guard) TestUnpublishVersionHandler_Success, ..._Success_NoTagsRemoved
HTTP boundary (invalid {rev}, package name, file-name traversal) TestUnpublishVersionHandler_RevInvalid, ..._PackageNameInvalid, TestUnpublishVersionFileHandler_Boundary
Constructor fail-fast on nil dependencies TestNewUnpublishVersion{,File}Handler_PanicsOnNilDependencies

📚 References

Related to #137 (closed)

Edited by Dzmitry (Dima) Meshcharakou

Merge request reports