feat: check a backup on demand with POST /api/backups/{fileName}/verify #138
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feat/verify-a-backup-on-demand"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What's in
POST /api/backups/{fileName}/verifyopens the file through the staging copy on the database volume (never on the backup volume, per ADR 0014 amendment 4), records the verdict beside it, schedules nothing, and answers200with the backup in the same shape the listing uses, carrying the fresh verdict. A file that will not load is a successful check withverification.status == "failed"and the reason indetail— not a409.400for a non-backup or traversing name,404for a missing file,401unauthenticated.BackupRestoreService.CheckAndRecordAsync, which writes the receipt whatever the verdict, soGET /api/backupscan now listfailedfor a file the server looked at and refused (before, onlyverified/unverifiedcould appear there, and a restore-check failure left a stale receipt in place). The enum/DTO docs, endpoint descriptions,IBackupRestoreServicecontract docs,deploy/DEPLOYMENT.md§6.2 table and new §6.8 "Verify a backup", and the README API row say so.verifyaction besidedelete/restore. List rows carry no actions, as before.BackupStagingArea.OverLocalCopynow gives each check its own staging copy (staging/<fileName>.<guid>), so two checks of one file — or a check racing a restore request — can no longer fight over one path and leave a spuriousfailedreceipt beside an intact backup.SweepandDiscardstill cover the suffixed names; VACUUM INTO and the startup restore are untouched.BackupRestoreService, a privateLocate/Refusedpair replaces the three copies of the name/existence guard.VerifyBackupUseCaseis registered inServer/BackupRegistration.cs;backend/openapi/v1.jsonregenerated. Backend only — no app files touched.Verification
Executed:
scripts/verify.sh backendfrom the worktree atd10df11, fresh:== verify OK (backend) ==, exit 0. ApplicationCore.Tests 49/49, IntegrationTests 1/1, Infrastructure.Tests 210/210, RestAdapter.Tests 637/637, no failures or skips.OpenApiSnapshotTestsgreen inside that run (snapshot regenerated withUPDATE_OPENAPI_SNAPSHOT=1in the first commit; the second commit moved nothing on the wire, so no snapshot change).Layers exercised:
BackupVerificationIntegrationTests—200 verifiedwithcheckedAtandcreatedAtmatching the create answer;200 failedwithdetailfor a non-database file, file kept, restore of it still409; theorytest.db -> 400,..%2F..%2Fetc%2Fpasswd -> 400, missing name-> 404with nothing written to the volume; theverifyhypermedia action on the create and verify envelopes; the listing carrying the fresh verdict afterwards.BackupRestoreServiceTests—VerifyBackupAsyncreaches its own verdict by opening the file (same-size garbage under a still-validverifiedreceipt comes backFailed), writes the receipt for both verdicts, writes no restore marker, returnsIsSuccessnotConflict,Locatemapping (traversal-> 400, missing-> 404),CreatedAtequals the creation answer; the restore request now records a failing verdict (listing saysFailedafter a409).BackupStagingTests— both the restore request and the check go throughOverLocalCopy(an unusable staging directory makes both throwIOException); newTwo_Checks_Of_One_Backup_Do_Not_Share_A_Staging_Copynests a secondOverLocalCopyinside the first and asserts the outer copy survives and staging is empty afterwards.Mutation probes (each: mutate, run the filtered suites, restore from the index, confirm working tree == index):
BackupReceipts.WritefromCheckAndRecordAsync-> red: 4 Infrastructure tests + 2 RestAdapter tests.VerifyBackupAsyncreturned the stale receipt instead of callingCheckAndRecordAsync-> red: 3 Infrastructure + 2 RestAdapter.OverLocalCopy(opened the backup-volume path directly) -> red:A_Restore_Request_And_A_Check_Open_The_Backup_Through_The_Database_Volume.verifyentry inBuildBackupActions-> red:HypermediaHelpersTests.BuildBackupActions_Offers_Delete_Verify_And_Restore+A_Backup_Advertises_The_Check_As_An_Action.Conflicton a non-Verifiedverdict -> red:A_Check_Reaches_Its_Own_Verdict_Rather_Than_Repeating_The_Receipt+A_File_That_Will_Not_Load_Is_A_Successful_Check_With_A_Failing_Verdict.Verified(the old restore behaviour) -> red:A_Restore_Request_For_A_File_That_Does_Not_Verify_Is_Refused,A_Check_Reaches_Its_Own_Verdict...,A_File_That_Will_Not_Load....createdAton the verify answer was pinned at no layer (info.CreationTimeUtc -> defaultstayed green everywhere). Fixed in the second commit by assertingCreatedAtin both happy-path tests; the same mutation now goes red: Infrastructure 34/35 (A_Check_Promotes_A_Backup_Nothing_Had_Checked_And_Schedules_Nothing), RestAdapter 9/10 (A_Backup_Nothing_Had_Checked_Is_Verified_On_Request_And_Nothing_Is_Scheduled).OverLocalCopyto the sharedstaging/<fileName>name -> red: onlyTwo_Checks_Of_One_Backup_Do_Not_Share_A_Staging_Copy(34/35). Removing itsfinally { Discard }-> red: the same test only (its "staging empty afterwards" assertion; no pre-existing test pinned staging cleanup after a check). A second-round re-probe atd10df11reproduced every red above.Unverified or confirmed by reading only:
deploy/smoke.shrun against the container image.VerifyBackupUseCase'sServiceResultmapping: it is a one-line pass-through,RestAdapter.Tests/UseCaseshas no backup use-case tests to sit beside, and the mapping is exercised end to end by the integration tests.info.Refresh()removal fromVerifyBackupAsyncintroduces no branch (the prober showed it unobservable), so nothing pins it.500(below) was confirmed by readingLocateandFile.Copy, not by a test.failedverdict from a live-relative check (migration history differs, or the live database holds rows and the backup none) persists until the file is re-checked.Follow-ups
docs/BACKLOG.md(Backend):deploy/smoke.shdoes not exercisePOST /api/backups/{fileName}/verify; one request after its upload step (expect200,verification.status == "verified") would cover the container path.docs/BACKLOG.md(Backend): a backup deleted while it is being checked answers500, not404(Locatesees the file, then the copy into staging throwsFileNotFoundException); pre-existing on the restore request, shared by the check; needs a seam a test can delete through.docs/BACKLOG.md(App): the "Not checked -> Verified" item now says the route exists and only the "Check now" affordance remains, and that a listed backup can now readfailed, so the "Will not load" rendering gets its first real input.docs/BACKLOG.md(keep this PR's reworded item, drop the schedule item). An integration merge of all five batch branches with that hunk resolved passedscripts/verify.sh all.🤖 Generated with Claude Code
Summary
Summary
Coverage
DoujinManager.ApplicationCore - 94.1%
DoujinManager.Infrastructure - 96.4%
on
pshot
Series
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__CoverImageHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__PrimaryTitleHelper
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__RolledBackImageCleanup
E20E2ABA2099901FFAB4904C0513DF077BA6D6CA374__UploadChapterValidator
DoujinManager.RestAdapter - 94.4%
E5CB12CEAC5BBBEA65844E1C05F9ADA4DA38B848075C41304FC6A1ABF77__ValidationAttr
ibuteCache
DoujinManager.Server - 62.5%
🔮 fufu~ Jibril reviewed your code!
"Check this one, change nothing" — the missing verb of the whole backup story, and you gave it the full treatment: one shared
CheckAndRecordAsyncso the check and the restore request cannot drift, GUID-suffixed staging copies so two checks can't duel, and an ADR amendment that argues both directions of the wire change. I read all 20 files of the diff.Verdict: ✅ Approved~ ♡
✅ What I verified myself
d10df11, fresh build in my sandbox: ApplicationCore 49/49, Infrastructure 210/210, Integration 1/1, RestAdapter 633 passed / 0 failed / 4 skipped (the uid-0LosesWritePermissionrows — my sandbox also runs as root) in 5 m 31 s. Matches yourverify.sharithmetic exactly.OverLocalCopyreverted to the sharedstaging/<fileName>name → red on exactlyTwo_Checks_Of_One_Backup_Do_Not_Share_A_Staging_Copy(1 red, 6 green).BackupReceipts.Writedropped fromCheckAndRecordAsync→ red on exactly the 4 receipt-dependent tests (A_Check_Promotes…,A_Check_Reaches_Its_Own_Verdict…,A_Restore_Request_Promotes…,A_Restore_Request_For_A_File_That_Does_Not_Verify…).OverLocalCopycalls deterministically (inner inside outer's work) is the interleaving that matters, and the decisive assertion — outer still there after inner's cleanup — pins precisely the delete-from-beneath fault. Fufu~ elegant.VerifyBackupAsync'sCreatedAtcomes frominfo.CreationTimeUtcand is asserted in both happy paths (service + integration) — the round-1 gap, properly closed, red under thedefaultmutation per your body and consistent with what I read.BackupVerificationStatus.Faileddoc re-write (it no longer lies about never reaching a listing), DTO docs, and the regenerated snapshot all say the same thing. Theverifyhypermedia action mirrors its siblings and is pinned at both the helper and the endpoint layer.💡 Little ideas (non-blocking)~
500and the missing smoke.sh row — agreed on both, nothing to add. The merge-note about #139's BACKLOG hunk is the kind of thing that saves the second merger five minutes; thank you for writing it down.Refused<T>is a cute little functor. If a third refusal shape ever appears, it might deserveServiceResult<T>.From(located)— but at two call sites, the local helper is the right size. Do not grow it preemptively.✅ What I liked~
200. Getting the semantics right (the check succeeded; the file is bad) and then defending them in the endpoint description so the next reader doesn't "fix" it into a 409 — that's the whole game.verifiedtrap (rotted bytes under a same-size receipt) and theunverifiedlimbo in one move; the honesty-guard test that keeps the fixture's receipt valid while the bytes rot is exactly how you prove a check doesn't trust paper.Automated review by Jibril · 2026-09-18
CI coverage: bot comment present for head
d10df11(VerifyBackupUseCase100%). · Local: full backend suite green atd10df11in sandbox (see above); probes re-run fresh this round, trees restored clean.(Verification notes: first RestAdapter attempt in my sandbox hung at discovery for 30 min under the same session's thread pressure — killed, rebuilt, re-ran clean; the 633/637 above is the fresh run. One earlier transient red in
SqliteHostingProbeTests-adjacent suites was traced to stale binaries from crashed builds and did not reproduce on any clean rebuild.)