diff --git a/CLAUDE.md b/CLAUDE.md index 5267ad1..04c6f87 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -111,6 +111,17 @@ wander ~20Å across runs). - A web client can NEVER hand the server a directory path (browser sandbox) — so "ingest without copy" is a CLI / Electron / register-path affordance, not a browser one. `source_root` is the single abstraction that expresses all three. +- **Deletion/cleanup mirrors this ownership story** (proposed, not yet built — + [docs/DELETION_AND_CLEANUP.md](docs/DELETION_AND_CLEANUP.md)). The DB cascades + are wired; the *files* are the design. Cleanup is driven off **`Artifact.origin`** + (delete bytes only for `BUILT`/`REFINED` — never IMPORTED in-place trees, which + are the user's) plus **`Run.out_dir` provenance** for analysis output trees, + via a new **`DataStore.delete()`** on the seam (do NOT add an inline resolver). + Note the **submit-time zombie guard**: deleting a project cascades the `Run` + rows but leaves the `out_dir` on disk, and `runservice` does + `mkdir(exist_ok=True)` with DB-resident idempotency — so a fresh analysis + silently writes into the stale tree. The invariant: *a populated `out_dir` is + owned by exactly one live `Run` (or nobody)*; refuse an unowned populated dir. ## Moorhen integration — THE big lesson: Moorhen is Redux-driven diff --git a/docs/DELETION_AND_CLEANUP.md b/docs/DELETION_AND_CLEANUP.md new file mode 100644 index 0000000..62e3102 --- /dev/null +++ b/docs/DELETION_AND_CLEANUP.md @@ -0,0 +1,392 @@ +# ADR: Deletion & cleanup — projects, runs, and the files behind them + +- **Status:** **Implemented** on `feat/delete-cleanup-endpoints`. Chunk 1: + storage-seam `delete()`, `Project.source_managed`, `cleanup.delete_run`, + `DELETE /runs/`, and the submit-side zombie guard. Chunk 2: + `Project.archived`, `cleanup.archive_project`/`purge_project`, + `DELETE /projects/` (archive), `POST /projects//purge`, + `POST /projects//unarchive`. **2026-06-11:** reviewed against Materia's + delete-endpoint brief — §4 records the agreed API shape and five corrections, + all encoded and tested (`inspect_api/tests/test_delete_cleanup.py`). +- **Date:** 2026-06-10 (§4 added 2026-06-11; implemented 2026-06-11). +- **Relates to:** + [DESIGN-artifacts-and-jobs.md](DESIGN-artifacts-and-jobs.md) (the `Artifact` + `origin`/`relpath` model this leans on), + [RUN_LIFECYCLE.md](RUN_LIFECYCLE.md) (the `Run` model and the + `share_path`→`out_dir` invocation contract), + [MULTI_RUN_DATA_MODEL.md](MULTI_RUN_DATA_MODEL.md) (the + `RunDataset`/`Finding` split that makes run-delete non-trivial), and + `inspect_api/storage.py` (the `DataStore` seam that must grow a `delete`). +- **One-line thesis:** the database half of deletion is already free (cascades + are wired); the *file* half is the whole design, and the discriminator that + makes it safe is **`Artifact.origin`** plus **`Run.out_dir` provenance** — + never the filesystem path alone. + +--- + +## Context — what deletion actually has to destroy + +Deleting a `Project` or `Run` is two operations that look like one: + +1. **DB rows.** Every cascade is wired (`on_delete=CASCADE`, + [models.py](../inspect_api/models.py)): deleting a `Project` takes its + `Dataset`s, `Artifact`s, `Shell`s, `Run`s; deleting a `Run` takes its + `RunDataset`s and that run's `Event`s. There is simply no endpoint — adding + `DestroyModelMixin` to `ProjectViewSet`/`RunViewSet` is a few lines. **This + part is solved.** + +2. **Bytes on disk.** Artifacts are referenced **in place** via + `Artifact.relpath` resolved against `project.source_root` — they are *not* + copied into a tree we own (except the zip-`/import` case and our own + derived outputs). So a naïve "delete project → `rm -rf source_root`" would + wipe the user's **primary PanDDA output**. This part is the design. + +The mistake to avoid is treating deletion as a DB-only operation. Runs are a +*disk* operation (§3); if delete only touches rows, the disk silently +out-survives the database and bites the next analysis. + +--- + +## §1 — File cleanup is driven off `Artifact.origin`, never the filesystem + +The field that makes disk cleanup safe already exists: `Artifact.origin` ∈ +`{IMPORTED, BUILT, REFINED}`. It tells us, per artifact, whether the bytes are +**ours** (we wrote them) or **the user's** (we only point at them). + +| origin | bytes live where | ours to delete? | +|---|---|---| +| **IMPORTED** (in-place via `/ingest_path`) | user's external PanDDA tree | **No** — never touch on disk | +| **IMPORTED** (zip via `/import`) | copy under `PANDDA_DATA_ROOT/` | Yes — but only via §2's flag | +| **BUILT** | `/builds//model.pdb` ([buildservice.py:84](../inspect_api/buildservice.py#L84)) | **Yes** — derived, write-once | +| **REFINED** | `/jobs//…` ([jobservice.py:251](../inspect_api/jobservice.py#L251)) | **Yes** — derived job output | +| **LIGAND** (CIF in `contents`) | the DB row itself | free — vanishes with the row | + +**The rule:** + +> Drive file cleanup off the `Artifact` table, never off the filesystem. For +> each artifact being deleted, delete its bytes **only if** +> `origin ∈ {BUILT, REFINED}`, resolved through the storage seam. IMPORTED +> bytes are left on disk. Embedded `contents` need no action. + +This is surgical: we delete exactly the relpaths we wrote (`builds/`, `jobs/`) +and never reason about "is this whole tree mine?" That precision matters +because of a wrinkle — BUILT/REFINED write under `source_root` *when it is set* +(`_project_root`/`_job_root` return `source_root` if present, else +`PANDDA_JOBS_ROOT`). So our derived bytes can sit **nested inside the user's +in-place tree**. A whole-tree `rm` is therefore *never* safe; per-artifact +deletion is the only correct approach. + +### Three pieces of missing machinery + +1. **The seam is read-only.** `LocalFileStore`/`AzureBlobStore` + ([storage.py](../inspect_api/storage.py)) resolve and read but cannot + delete. Cleanup must go through a new `DataStore.delete(relpath)` so the + non-local stores stay correct — do **not** re-introduce an inline + `source_root`/`relpath` resolver (same rule as the read path; CLAUDE.md / + Materia R6). +2. **We don't record import mode.** For the zip-`/import` case we'd also want + to `rm` the copied tree, but nothing on `Project` says "I copied this" vs + "I'm pointing at the user's tree." Add **`Project.source_managed: bool`** + (true from `/import`, false from `/ingest_path`) as the gate for whether the + source tree itself may be removed. +3. **`current_model` guard.** A BUILT/REFINED artifact may be a + `Dataset.current_model` or `Event.current_model`. Blindly deleting one + loses the model of record (SET_NULL nulls the pointer silently). Cleanup + must refuse-or-warn on artifacts still referenced as a current model — + mirroring `reconcile._apply_pointer_policy`. + +### Run it as a Job, not inline + +`rm`-ing `jobs/`/`builds/` trees and issuing blob deletes can be slow. Even +"hard delete + confirm" should fire the file sweep through the existing `Job` +machinery rather than blocking the request. + +--- + +## §2 — Scope semantics: archive the project, hard-delete the run + +Most of what a delete destroys is reconstructible by re-ingesting the PanDDA +tree (Events, RunDatasets, Shells, IMPORTED artifacts). Three things do **not** +survive a re-ingest: + +- **`Finding`s** — the decision/confidence/comment/inspected_by. Hand-entered + human judgement. +- **BUILT models** — ligands a human placed in Moorhen and committed. +- **REFINED outputs** — compute that took real wall-clock time. + +The decision is not "delete vs not" — it is "how much ceremony around +destroying *those three*." The split that fits the data is **asymmetric**: + +> **Soft-archive at the Project level; hard-delete everywhere else.** + +- **Project — soft `archived` flag.** A project is the heavy, decision-dense + container and the thing a user fat-fingers. One manager + one queryset filter + gives undo for free; a separate explicit **purge** runs the §1 file sweep. + Archive doubles as a feature you want anyway (finished projects clutter the + list). *Cost:* every queryset that enumerates projects (the ViewSets, the + `extra_roots` builder, `get_store`) must exclude archived rows or it leaks + ghosts — easy to miss one. +- **Run + the artifact sweep — hard delete + confirm-summary.** A run is cheap + to lose (re-ingestable; `Finding`s are kept — see §2.1). Adding soft-delete + plumbing to the run/event/artifact layer is where the queryset-leak cost + explodes, for little gain. The confirm step reports what's lost ("removes 14 + findings with decisions, 6 built models") — but note it is **advisory only**: + an API/script client can `DELETE` without ever fetching the summary, so the + real safety lives in §1 (files) and §3 (the zombie guard), not the dialog. + +The §1 file-cleanup design is identical either way — archive just delays *when* +the sweep fires. + +### §2.1 — Orphans on run-delete: keep them + +Deleting a `Run` cascades its `RunDataset`s and that run's `Event`s, but: + +- **`Finding`s survive** (run-independent, FK to `Dataset`). A `Finding` left + anchoring no observation is **kept**, not GC'd — it is the durable human + layer ("a decision awaiting re-observation"). A later re-ingest at the same + detection centroid (1.5 Å, per MULTI_RUN_DATA_MODEL) re-links it. Decisions + must never silently vanish. +- **`Dataset`s (Crystals) survive** too; one with zero remaining `RunDataset`s + is an empty husk — kept as the crystal identity, repopulated by re-ingest. + +### §2.2 — `source_root` staleness after run-delete + +`project.source_root` points at the *latest* run's `out_dir`. Delete that run +and dataset-level IMPORTED artifacts (apo pdb/mtz, attached to `Dataset` not +`Run`) still resolve via `extra_roots` (built from surviving `Run.out_dir`s in +`get_store`) — until the last run is gone. Re-ingest fixes it; worth a log line +on delete, not a blocker. + +--- + +## §3 — Run output trees & the submit-time zombie guard + +This is where §1's "leave IMPORTED bytes alone" policy springs a leak. It is a +**DB↔disk desync**: deleting a project cascades the `Run` rows away, but the +analysis output tree on the share is — by §1 — *not ours to delete if it was an +in-place ingest*. **The disk remembers what the database forgot.** + +### Two directories, two keys — don't conflate them + +- **Analysis output tree** — `Run.out_dir`, from + `_default_out_dir(share_path, group)` + ([runservice.py:78-89](../inspect_api/runservice.py#L78-L89)): swaps + `pandda_inputs`→`pandda_results`, else appends `pandda2_out`. **Keyed on + `(share_path, group)` — not the project name.** +- **Import copy** — `PANDDA_DATA_ROOT/` from the zip `/import` + path, **keyed on name**, and *already* guarded (raises if the dir exists, + [importer.py:126](../inspect_api/importer.py#L126)). + +The import case is guarded; the **fresh-analysis trigger is not**. + +### What happens today: silent reuse + +[runservice.py:173-184](../inspect_api/runservice.py#L173-L184) checks only that +the *parent* exists, then `out_dir.mkdir(exist_ok=True)` — a pre-existing, +populated dir is written into without complaint. Three things then conspire: + +1. **The dedup can't save you.** `idempotency_key` + ([models.py:585](../inspect_api/models.py#L585)) is **DB-resident**; a + project delete cascades the `Run` rows away, so there is no surviving key to + hit — a re-submit creates a *fresh* `Run` believing the dir is pristine. +2. **PanDDA2 merges into the polluted tree** (no `--overwrite` is passed; + behaviour is whatever the pinned image does into a non-empty dir). You get + Frankenstein output — stale `pandda_analyse_events.csv` rows with no matching + maps, ghost `processed_datasets/`, the off-by-one event/map counts CLAUDE.md + already warns about. +3. **Ingest faithfully imports the mess.** `_complete()` runs `ingest_pandda2` + against `out_dir` and reconcile clobbers IMPORTED artifacts wholesale, so the + zombie's stale rows become real DB rows. + +### The invariant, enforced from both ends + +> **A populated `out_dir` is owned by exactly one live `Run` (or nobody).** + +Enforce it at both the delete side and the submit side — defense in depth, +because zombies also arise from crashes and manual `rm` mishaps, not only +deletes. + +**Delete-side — purge the trees we launched.** This refines §1's ownership rule. +A project-level `source_managed` flag is too coarse for analysis output; the +right discriminator is **`Run.out_dir` provenance**: a tree we wrote by +launching a run *is* ours to `rmtree` on delete; a tree the user pointed us at +via `/ingest_path` is not. (Do it as a Job — these trees are large.) + +**Submit-side guard — never trust the dir.** Before dispatch, cross-check the +filesystem against the `Run` table: + +| `out_dir` state | a live `Run` references it? | action | +|---|---|---| +| empty / absent | — | proceed (today's happy path) | +| populated | **yes** (`retry_of` / resume / idempotent re-POST) | proceed per existing logic | +| populated | **no** | **zombie → `409`**: *"`` already contains a PanDDA tree owned by no run — purge it or pick a new group."* | + +The third row is the whole point: it is the DB↔disk reconciliation that closes +the desync. **It must consult the filesystem, not just the `Run` table** — +precisely because the idempotency key is DB-resident and is therefore +structurally incapable of seeing on-disk state once the rows are gone. + +The guard keys on the `Run`↔`out_dir` *link*, not mere existence, so that the +legitimate non-empty case — `retry_of` writing into the same dir on purpose +([runservice.py:163-171](../inspect_api/runservice.py#L163-L171)) — is *not* +refused. "Populated" alone never means refuse; "populated **and** unowned" does. + +### Symmetry with `/import` + +After a project delete that left a `source_managed` copy in place, re-`/import` +with the same name would hit the existing `importer.py:126` guard and refuse — +the same zombie biting at a different door. Reinforces the conclusion: delete +must purge the trees it owns, and the write paths (`/runs` submit and `/import`) +must refuse to write onto an unowned tree. **Same invariant, three doors.** + +--- + +## §4 — Materia delete-endpoint brief (2026-06-11): API shape + five corrections + +Materia proposed the concrete HTTP surface that drives §1–§3. We adopt its +shape; this section records it and the five corrections found when checking it +against the actual schema. + +### Adopted shape + +``` +DELETE /api/v1/runs/?delete_outdir= # hard-delete the run +DELETE /api/v1/projects/ # ARCHIVE (soft, reversible) +POST /api/v1/projects//unarchive/ # restore +POST /api/v1/projects//purge/?delete_outdirs= # irreversible +``` +`` ∈ `false` (default, DB-only, returns the on-disk path), `true` +(safe-delete with the orphan check), `force` (`rm` regardless, accept broken +pointers). Response carries an audit summary +(`{run_id, events_deleted, artifacts_deleted, disk_freed_bytes}`). + +**Project shape note (vs the brief):** the brief proposed +`DELETE /projects/?delete_outdirs=…` as a hard delete. Per Q2 (§2 / the +Materia reply) a project DELETE instead **archives** (reversible); the +irreversible cascade + file sweep is the explicit `POST .../purge/` step, which +**requires the project to be archived first** (else `400`). So `delete_outdirs` +lives on `purge`, not on the project `DELETE`. Materia's CLI therefore needs a +separate purge command — the one piece of Materia-side coordination this split +implies. + +### Ownership predicate — `runner_handle`, confirmed + +A Run's `out_dir` is Reinspect-owned **iff `run.runner_handle != ""`** AND +`run.status` is terminal (`succeeded`/`failed`/`cancelled`, +[models.py:536-542](../inspect_api/models.py#L536-L542)). This is §1's "we wrote +it" / §3's "`out_dir` provenance" made concrete with an existing field. +**Confirmed safe:** the synthetic in-place-ingest Run +([reconcile.py:211](../inspect_api/reconcile.py#L211)) never sets +`runner_handle` (stays `""`) and its `out_dir` *is* `source_root` — so the +predicate refuses to `rm` the user's own tree. (`runner_handle` is the per-Run +half; the zip-`/import` copied tree needs the separate project-level +`source_managed` gate — see correction 4.) + +### Correction 1 — the orphan check queried the wrong artifact class + +Dataset-scoped artifacts carry **`project = NULL`**; they reach their project +only via `dataset.project` (proven by `Artifact.owning_project`, +`self.project or (self.dataset.project if self.dataset else None)`, +[models.py:432](../inspect_api/models.py#L432)). So the brief's +`filter(event__isnull=True, project=run.project)` matches only **project-scoped +`report_html`** and never inspects a single `structure`/`data_mtz`. The orphan +query must be: + +```python +Artifact.objects.filter( + dataset__project=run.project, dataset__isnull=False, event__isnull=True +) +``` + +### Correction 2 — embedded ligands & symlinked inputs aren't files in `out_dir` + +The brief's three-class table listed `ligand` as a disk artifact. Ligand +restraint dicts are **embedded in `Artifact.contents`** +([ingest_pandda2.py:408](../inspect_api/management/commands/ingest_pandda2.py#L408)) +— CASCADE-deleted with the row, untouched by `rm_rf(out_dir)`. And the apo +`structure`/`data_mtz` inputs are **symlinks into a sibling `data/` tree**, so +`rm` removes the link, not the target. Corrected table: + +| Artifact class | scope | bytes on disk under `out_dir`? | freed by | +|---|---|---|---| +| `event_map`, `ligand_pose` | event | yes | `rm_rf(out_dir)` (+ DB via Event cascade) | +| `structure`, `data_mtz` | dataset | yes, but apo inputs are **symlinks** (rm drops the link) | `rm_rf(out_dir)`; subject to the orphan check | +| `ligand` | dataset | **no — embedded in `contents`** | DB row delete | +| `report_html` | project | yes | only on `Project.delete()` | + +### Correction 3 — shared `out_dir`s, and `delete_project` inverts the check + +`_default_out_dir` is keyed on `(share_path, group)` and submit does +`mkdir(exist_ok=True)`, so two distinct runs (same group, different +`input_hash`) can **share one `out_dir`**. Two consequences: + +- **Run-delete:** deleting run A excludes B's path from `surviving_relpaths` + (same path) then `rm`s it — destroying B's **event-scoped** maps, which the + orphan check never inspects (`event__isnull=True` only). Before `rm`, + cross-check that **no surviving Run shares this exact `out_dir`**; if one does, + refuse (or DB-delete only). +- **Project-delete:** "apply per-Run" makes surviving roots empty for every run + ⇒ everything flags orphaned ⇒ always refuses unless `force`. Project-delete + must use distinct semantics: all runs are going, so **skip the per-run orphan + check and `rm` every owned `out_dir` in one pass**. + +### Correction 4 — the zip-`/import` copied tree needs `source_managed` + +The brief is entirely `Run.out_dir`-centric. A project from `/import` has its +tree **copied** to `PANDDA_DATA_ROOT/` (importer `copytree`); its +synthetic ingest Run has `runner_handle=""` so per-Run disk delete is refused — +**leaking the copied tree forever** on `delete_project`. Gate that tree on the +project-level **`Project.source_managed`** flag (§1, missing-machinery #2): +`runner_handle` frees triggered `out_dir`s, `source_managed` frees the import +copy. Both are required. + +### Correction 5 — ship the §3 submit-side guard *with* these endpoints + +`delete_outdir=false` is the default, and it is a **zombie factory** for +Materia's own lead use case ("re-run PanDDA and clear the previous run's +artefacts"): DB rows go, `out_dir` stays, the next analysis `mkdir(exist_ok=True)` +merges into the stale tree → Frankenstein output that ingest then imports. The +delete endpoints and §3's submit-side guard (*refuse a populated `out_dir` no +live Run owns*) are two halves of one DB↔disk invariant and must land together. + +### Answers to the brief's four open questions + +1. **Authorization** — same bearer for run-delete; for project-delete keep it + archive-not-purge (Q2) and gate the irreversible purge behind a separate + explicit step, since project-delete is the fat-finger target. +2. **Soft vs hard** — soft-archive Projects (§2), hard-delete Runs. Tombstoning + *runs* is unnecessary: decisions live on run-independent `Finding`s, which a + hard run-delete already preserves (§2.1), so the audit trail is durable + without a `status=deleted` run. +3. **Cascade transaction scope** — DB cascade in one transaction; disk deletion + **outside** it (an `rm` can't roll back). Commit the DB delete, then run a + best-effort disk sweep as a Job, reporting partial failures in the summary. +4. **Concurrent ingest** — a wall-clock recency window on `progress` is fragile + (clocks lie — same failure class as DB-resident idempotency missing on-disk + state). Refuse delete while an **ingest Job for the run is active**. The + terminal-status guard alone won't catch it: `_complete()` ingests *after* + `status=succeeded`, so an explicit ingest lock/flag is needed. + +--- + +## Summary — what to build + +*(All ✅ landed on `feat/delete-cleanup-endpoints`.)* + +1. **`DataStore.delete(relpath)`** on the storage seam (local + azure). +2. **`Project.source_managed: bool`** (migration) — set by `/import` vs + `/ingest_path`. +3. **File sweep**, driven off `Artifact.origin` (BUILT/REFINED only) + + `Run.out_dir` provenance for run trees + `source_managed` copy, with a + `current_model`-reference guard — run as a **Job**. +4. **Project delete** = soft `archived` flag + queryset filtering + a separate + **purge** that fires the sweep. +5. **Run delete** = hard delete + confirm-summary; **keep** orphan + `Finding`s/`Crystal`s. +6. **Submit-time zombie guard** in `runservice.submit_run` — refuse a populated + `out_dir` that no live `Run` owns (`409`). Ships *with* item 7, not after. +7. **Delete endpoints** (§4): `DELETE /runs/` and `/projects/` with + `delete_outdir(s)=false|true|force`. Ownership = `runner_handle != ""` + + terminal status. Orphan query keyed on `dataset__project` (correction 1); + skip the orphan check on project-delete (correction 3); shared-`out_dir` + cross-check before `rm` (correction 3); audit summary in the response. diff --git a/inspect_api/cleanup.py b/inspect_api/cleanup.py new file mode 100644 index 0000000..6951d13 --- /dev/null +++ b/inspect_api/cleanup.py @@ -0,0 +1,309 @@ +""" +Deletion & cleanup service — tear down Runs (and, later, Projects) and the +files behind them. + +The hard part is never the DB rows (the cascades are wired); it is deciding +which *bytes* are ours to remove. Two predicates settle it: + + * ``Run.runner_handle`` — non-empty iff Reinspect's JobRunner dispatched the + run, so we wrote its ``out_dir``. The synthetic in-place-ingest Run leaves + it empty and sets ``out_dir == source_root`` (the user's own tree), so the + predicate refuses to remove a tree we don't own. + * ``Artifact.origin`` / ``Artifact.contents`` — which artifact bytes live on + disk under a run tree vs. embedded in the DB. + +See docs/DELETION_AND_CLEANUP.md (§3 ownership, §4 the corrections this +encodes). DB delete happens first (autocommit); the disk sweep runs after and +is best-effort — a failed ``rm`` is reported, never rolled back into a +half-deleted DB. +""" +import os +import shutil +from pathlib import Path + +from django.conf import settings + +from .models import Artifact, Event, Finding, Run +from .storage import get_store + +# A run is settled (safe to reason about its tree) only in these states; never +# touch the tree of a run the node may still be writing. +TERMINAL_STATUSES = frozenset({ + Run.Status.SUCCEEDED, Run.Status.FAILED, Run.Status.CANCELLED, +}) + + +class CleanupError(Exception): + """A delete was refused (not owned, still running, or would orphan/clobber + surviving artifacts). Maps to HTTP 400.""" + + +def run_owns_outdir(run: Run) -> bool: + """True iff ``run.out_dir`` is a tree Reinspect created and may remove: + we dispatched it (``runner_handle`` set) AND it is terminal.""" + return bool(run.runner_handle) and run.status in TERMINAL_STATUSES + + +def _norm(p) -> str: + return os.path.normpath(str(p)) + + +def _surviving_roots(project, *, exclude_out_dir: str) -> list[Path]: + """Candidate trees that will STILL exist after the delete — every other + run's ``out_dir``, the project ``source_root``, and the data-root landing + dir — minus the tree being removed. Mirrors storage.get_store's root set so + the orphan check sees exactly what artifact serving would resolve against. + """ + exclude = _norm(exclude_out_dir) + roots = set() + if project.source_root: + roots.add(_norm(project.source_root)) + roots.update( + _norm(od) for od in Run.objects.filter(project=project) + .exclude(out_dir="").values_list("out_dir", flat=True) + ) + roots.add(_norm(Path(settings.PANDDA_DATA_ROOT) / project.name)) + roots.discard(exclude) + return [Path(r) for r in roots if Path(r).is_dir()] + + +def _orphaned_dataset_artifacts(run: Run) -> list[str]: + """Dataset-scoped artifacts (structure/data_mtz) that would be left with no + on-disk copy if ``run.out_dir`` is removed. + + Correct scope (DELETION_AND_CLEANUP.md §4 correction 1): dataset-scoped + artifacts carry ``project=NULL`` and reach the project via ``dataset``, so + we filter on ``dataset__project``, NOT ``project``. Embedded artifacts + (ligand CIFs in ``contents``) are excluded (correction 2 — they are not + files). We check existence per-artifact across the surviving roots rather + than walking the (huge) trees to union all relpaths: dataset-scoped + artifacts are few, so O(artifacts x roots) stats beats O(files). + """ + survivors = _surviving_roots(run.project, exclude_out_dir=run.out_dir) + arts = Artifact.objects.filter( + dataset__project=run.project, + dataset__isnull=False, + event__isnull=True, + contents="", + ).values_list("relpath", flat=True) + orphaned = [] + for relpath in arts: + on_disk = any( + (root / relpath).is_file() or (root / relpath).is_symlink() + for root in survivors + ) + if not on_disk: + orphaned.append(relpath) + return orphaned + + +def _may_rm_outdir(run: Run, *, force: bool) -> tuple[bool, str]: + """Decide whether ``run.out_dir`` may be removed. ``force`` skips every + guard (accepts broken pointers). Returns ``(ok, refusal_reason)``.""" + if force: + return True, "" + if not run_owns_outdir(run): + return False, ( + "out_dir is not Reinspect-owned (no runner_handle) or the run is " + "not in a terminal state — pass force to override" + ) + # Shared out_dir: another run reads/writes the SAME tree (same + # share_path+group, different input_hash → identical _default_out_dir). + # Removing it would nuke that run's event-scoped maps, which the orphan + # check never inspects. Refuse. (§4 correction 3.) + sharers = Run.objects.filter( + project=run.project, out_dir=run.out_dir + ).exclude(pk=run.pk) + if run.out_dir and sharers.exists(): + return False, ( + f"out_dir {run.out_dir} is shared with {sharers.count()} other " + "run(s); refusing to remove it — pass force to override" + ) + orphaned = _orphaned_dataset_artifacts(run) + if orphaned: + return False, ( + f"removing out_dir would orphan {len(orphaned)} dataset-scoped " + "artifact(s) with no surviving copy — pass force to override" + ) + return True, "" + + +def _tree_size(path: str) -> int: + """Sum of regular-file sizes under ``path`` (symlinks counted as the link, + not the target — we never follow out of the tree).""" + total = 0 + for dirpath, _dirs, files in os.walk(path, followlinks=False): + for name in files: + fp = Path(dirpath) / name + try: + total += fp.lstat().st_size + except OSError: + pass + return total + + +def _rm_tree(path: str) -> tuple[bool, str]: + """Best-effort recursive remove. Returns ``(removed, error)``.""" + p = Path(path) + if not p.is_dir(): + return False, "" + try: + shutil.rmtree(p) + return True, "" + except OSError as exc: # surfaced in the summary, never re-raised + return False, str(exc) + + +def delete_run(run: Run, *, delete_outdir: bool = False, + force: bool = False) -> dict: + """Delete a Run (DB cascade) and, optionally, its output tree. + + ``delete_outdir=False`` (default): DB-only; the on-disk ``out_dir`` is + left and returned so the caller can clean it up manually. + ``delete_outdir=True``: safe-delete — refuses (CleanupError → 400) if the + tree is not ours, is shared, or would orphan dataset-scoped artifacts. + ``force=True``: remove regardless, accepting broken pointers. + + Findings/Crystals are intentionally NOT touched: a Finding left anchoring + no observation is the durable human layer, kept for re-link on re-ingest + (DELETION_AND_CLEANUP.md §2.1). + """ + run_id = run.id + out_dir = run.out_dir + # Snapshot counts BEFORE the cascade removes the rows. + events_deleted = Event.objects.filter(run_dataset__run=run).count() + artifacts_deleted = Artifact.objects.filter( + event__run_dataset__run=run + ).count() + + rm_outdir = False + if delete_outdir: + rm_outdir, reason = _may_rm_outdir(run, force=force) + if not rm_outdir: + raise CleanupError(reason) + + disk_freed = _tree_size(out_dir) if (rm_outdir and out_dir) else 0 + + run.delete() # CASCADE: RunDataset → this run's Events → event artifacts + + out_dir_removed, rm_error = (False, "") + if rm_outdir and out_dir: + out_dir_removed, rm_error = _rm_tree(out_dir) + if not out_dir_removed: + disk_freed = 0 + + summary = { + "run_id": run_id, + "events_deleted": events_deleted, + "artifacts_deleted": artifacts_deleted, + "disk_freed_bytes": disk_freed, + "out_dir": out_dir, + "out_dir_removed": out_dir_removed, + } + if rm_error: + summary["out_dir_error"] = rm_error + return summary + + +# --- Project archive / purge -------------------------------------------------- + +def project_loss_summary(project) -> dict: + """Counts of what a PURGE would irreversibly destroy — the confirm summary + surfaced at archive AND purge time. The three reconstructible-only-by-hand + classes (decisions, built, refined) are called out separately.""" + events = Event.objects.filter(dataset__project=project) + decided = Finding.objects.filter(dataset__project=project).exclude( + decision=Event.Decision.UNREVIEWED + ) + return { + "n_runs": project.runs.count(), + "n_datasets": project.datasets.count(), + "n_events": events.count(), + "n_findings_with_decisions": decided.count(), + "n_built_models": Artifact.objects.filter( + dataset__project=project, origin=Artifact.Origin.BUILT + ).count(), + "n_refined_models": Artifact.objects.filter( + dataset__project=project, origin=Artifact.Origin.REFINED + ).count(), + } + + +def archive_project(project) -> dict: + """Soft-delete: tombstone the project (reversible). Retains every row and + byte; a later purge does the irreversible work. Idempotent.""" + if not project.archived: + project.archived = True + project.save(update_fields=["archived"]) + return {"id": project.id, "archived": True, **project_loss_summary(project)} + + +def purge_project(project, *, delete_outdirs: bool = False, + force: bool = False) -> dict: + """Hard-delete a project (DB cascade) and, optionally, its files. + + Whole-project teardown, so — unlike run-delete — there is NO per-run orphan + check (every artifact is going) and no current_model guard (every pointer + is going). Disk removal still respects ownership: with ``delete_outdirs`` + we rm only runner-owned ``out_dir`` trees and (if ``source_managed``) the + copied source tree, leaving a user's in-place-ingested tree untouched. + ``force`` removes EVERY out_dir and the source tree regardless — an + explicit nuke that can reach a user's own data. (§4 correction 3 + 4.) + """ + loss = project_loss_summary(project) + store = get_store(project) # capture before the cascade removes the runs + + trees: set[str] = set() + derived: list[str] = [] + source_removed = False + if delete_outdirs: + for run in project.runs.exclude(out_dir=""): + if force or run_owns_outdir(run): + trees.add(_norm(run.out_dir)) + if (project.source_managed or force) and project.source_root: + trees.add(_norm(project.source_root)) + derived = list( + Artifact.objects.filter( + dataset__project=project, + origin__in=(Artifact.Origin.BUILT, Artifact.Origin.REFINED), + ).values_list("relpath", flat=True) + ) + + source_norm = _norm(project.source_root) if project.source_root else None + disk_freed = sum(_tree_size(t) for t in trees) + project_name = project.name + + project.delete() # DB cascade: runs, datasets, events, findings, artifacts + + errors: list[str] = [] + trees_removed = 0 + if delete_outdirs: + for tree in trees: + ok, err = _rm_tree(tree) + if ok: + trees_removed += 1 + if tree == source_norm: + source_removed = True + else: + if err: + errors.append(err) + disk_freed -= 0 # size already counted; rm failed → leave it + # Belt-and-suspenders: sweep any BUILT/REFINED bytes that lived outside + # a removed tree (e.g. under PANDDA_JOBS_ROOT). store paths were + # captured before the cascade, so this still resolves. + for relpath in derived: + try: + store.delete(relpath) + except (OSError, ValueError): + pass + + summary = { + "project": project_name, + **loss, + "trees_removed": trees_removed, + "disk_freed_bytes": disk_freed if delete_outdirs else 0, + "source_tree_removed": source_removed, + } + if errors: + summary["errors"] = errors + return summary diff --git a/inspect_api/importer.py b/inspect_api/importer.py index 8e3d2e7..5efddf8 100644 --- a/inspect_api/importer.py +++ b/inspect_api/importer.py @@ -145,6 +145,11 @@ def import_zip(zip_path: Path, project_name: str) -> dict: "ingest_pandda", project=project_name, root=str(dest) ) project = Project.objects.get(name=project_name) + # We COPIED the tree under PANDDA_DATA_ROOT, so we own it — mark it + # purge-deletable (ingest_path leaves this False). See + # docs/DELETION_AND_CLEANUP.md §4 correction 4. + project.source_managed = True + project.save(update_fields=["source_managed"]) return { "id": project.id, "flavour": flavour, @@ -159,7 +164,7 @@ def import_zip(zip_path: Path, project_name: str) -> dict: crystals_root = manifest.parent shutil.copytree(crystals_root, dest) project = Project.objects.create( - name=project_name, source_root=str(dest) + name=project_name, source_root=str(dest), source_managed=True ) rows = _read_manifest(dest / manifest.name) for row in rows: diff --git a/inspect_api/migrations/0019_project_source_managed.py b/inspect_api/migrations/0019_project_source_managed.py new file mode 100644 index 0000000..0d78285 --- /dev/null +++ b/inspect_api/migrations/0019_project_source_managed.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.30 on 2026-06-11 09:22 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('inspect_api', '0018_remove_event_comment_remove_event_confidence_and_more'), + ] + + operations = [ + migrations.AddField( + model_name='project', + name='source_managed', + field=models.BooleanField(default=False), + ), + ] diff --git a/inspect_api/migrations/0020_project_archived.py b/inspect_api/migrations/0020_project_archived.py new file mode 100644 index 0000000..d103eb8 --- /dev/null +++ b/inspect_api/migrations/0020_project_archived.py @@ -0,0 +1,18 @@ +# Generated by Django 4.2.30 on 2026-06-11 10:09 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('inspect_api', '0019_project_source_managed'), + ] + + operations = [ + migrations.AddField( + model_name='project', + name='archived', + field=models.BooleanField(default=False), + ), + ] diff --git a/inspect_api/models.py b/inspect_api/models.py index 68a4150..1381b83 100644 --- a/inspect_api/models.py +++ b/inspect_api/models.py @@ -28,6 +28,19 @@ class Project(models.Model): # Filesystem location ingested from — the import boundary, not the source # of truth once ingested. source_root = models.CharField(max_length=1024) + # True when WE own the source_root tree — i.e. the zip importer COPIED it + # under PANDDA_DATA_ROOT/ (import_zip). False when the tree is the + # user's own, pointed at in place (ingest_path) or a triggered run's share. + # The gate for whether project-purge may rm source_root itself; the per-run + # out_dir is gated separately on Run.runner_handle. See + # docs/DELETION_AND_CLEANUP.md §1 + §4 correction 4. + source_managed = models.BooleanField(default=False) + # Soft-delete tombstone. DELETE /projects/ sets this (reversible undo); + # a separate explicit POST /projects//purge does the irreversible hard + # delete + file sweep. Archived projects are hidden from the default list + # but still resolve for detail/un-archive/serving. See + # docs/DELETION_AND_CLEANUP.md §2. + archived = models.BooleanField(default=False) ingested_at = models.DateTimeField(auto_now_add=True) def __str__(self): diff --git a/inspect_api/runservice.py b/inspect_api/runservice.py index d81fa64..63fa466 100644 --- a/inspect_api/runservice.py +++ b/inspect_api/runservice.py @@ -89,6 +89,10 @@ def _default_out_dir(share_path: str, group: str) -> str: return str(Path(share_path) / "pandda2_out") +def _is_nonempty_dir(path: Path) -> bool: + return path.is_dir() and any(path.iterdir()) + + # pandda2.analyse flags Reinspect OWNS — a trigger may NOT override these; every # OTHER well-formed pandda2.analyse flag is accepted and passed straight to the # command line. Reserved because they'd break the input/output contract (paths @@ -181,6 +185,20 @@ def submit_run( f"output parent {out_dir.parent} does not exist — is the share " "mounted and share_path correct?" ) + # Zombie guard (DELETION_AND_CLEANUP.md §3): a populated out_dir that NO + # Run row owns is a stale tree — a prior run whose DB rows were deleted but + # whose files were left on disk. mkdir(exist_ok=True) would silently write + # fresh pandda2 output ONTO the stale tree (merge → off-by-one event/map + # counts), which ingest then faithfully imports. Refuse. A populated dir a + # Run DOES reference is legitimate reuse (retry/resume of that run). + if _is_nonempty_dir(out_dir) and not Run.objects.filter( + out_dir=str(out_dir) + ).exists(): + raise RunError( + f"output dir {out_dir} already contains files but is owned by no " + "run (a stale tree from a deleted run?) — remove it or use a " + "different group" + ) out_dir.mkdir(exist_ok=True) project, _ = Project.objects.get_or_create( diff --git a/inspect_api/serializers.py b/inspect_api/serializers.py index 5d19db8..cab87c3 100644 --- a/inspect_api/serializers.py +++ b/inspect_api/serializers.py @@ -273,7 +273,9 @@ class ProjectSerializer(serializers.ModelSerializer): class Meta: model = Project - fields = ["id", "name", "source_root", "ingested_at", "status"] + fields = [ + "id", "name", "source_root", "archived", "ingested_at", "status", + ] @extend_schema_field(serializers.JSONField()) def get_status(self, obj): diff --git a/inspect_api/storage.py b/inspect_api/storage.py index 95d36b4..df8464a 100644 --- a/inspect_api/storage.py +++ b/inspect_api/storage.py @@ -29,6 +29,12 @@ def exists(self, relpath: str) -> bool: ... # to local disk and returns that path; ``local`` returns the resolved path # directly. ``None`` if the ref can't be made a local file. def local_path(self, relpath: str): ... + # Remove the bytes for ONE relpath (the deletion side of the seam, used by + # the artifact cleanup sweep — docs/DELETION_AND_CLEANUP.md §1). Returns + # True if something was removed, False if nothing was there. Removing whole + # run output *trees* (an absolute out_dir we own) is a separate, + # local-filesystem concern handled in cleanup.py, NOT a relpath op here. + def delete(self, relpath: str) -> bool: ... class LocalFileStore: @@ -91,6 +97,22 @@ def local_path(self, relpath: str): p = self._resolve(relpath) return p if p.is_file() else None + def delete(self, relpath: str) -> bool: + """Unlink the bytes for ``relpath``. Removes the LEAF entry itself — + a symlink is unlinked, never its target — so deleting an IMPORTED + symlinked input (``-pandda-input.pdb``) can't reach into the sibling + ``data/`` tree. Tries each candidate root; removes the first match. + (The sweep only ever passes BUILT/REFINED relpaths, which are real + files we wrote, but the symlink-safety holds regardless.) + """ + self._guard(relpath) + for r in self.roots: + candidate = r / relpath + if candidate.is_symlink() or candidate.is_file(): + candidate.unlink() + return True + return False + class AzureBlobStore: """Serve a project's artifacts from an Azure Blob Storage container. @@ -172,6 +194,13 @@ def local_path(self, relpath: str): dest.write_bytes(blob.download_blob().readall()) return dest + def delete(self, relpath: str) -> bool: + blob = self.container.get_blob_client(self._key(relpath)) + if not blob.exists(): + return False + blob.delete_blob() + return True + def get_store(project) -> DataStore: """Return the DataStore for a project's artifacts. diff --git a/inspect_api/tests/test_delete_cleanup.py b/inspect_api/tests/test_delete_cleanup.py new file mode 100644 index 0000000..8d985c3 --- /dev/null +++ b/inspect_api/tests/test_delete_cleanup.py @@ -0,0 +1,463 @@ +""" +Deletion & cleanup: the storage-seam delete primitive, the run-delete service +(ownership predicate, orphan check, shared-out_dir guard, audit summary), the +DELETE /runs/ endpoint modes, and the paired submit-side zombie guard. + +See docs/DELETION_AND_CLEANUP.md (§3 + §4). +""" +import shutil +import tempfile +from pathlib import Path +from unittest import mock + +from django.test import TestCase, override_settings +from rest_framework.test import APIClient + +from inspect_api import runservice +from inspect_api.cleanup import ( + CleanupError, + archive_project, + delete_run, + purge_project, + run_owns_outdir, +) +from inspect_api.models import ( + Artifact, Dataset, Event, Finding, Project, Run, RunDataset, +) +from inspect_api.storage import LocalFileStore + + +class FakeRunner: + """Minimal JobRunner stand-in: submit makes the workdir, returns a handle.""" + + def probe(self): + return {"available": True} + + def submit(self, spec, workdir): + workdir = Path(workdir) + workdir.mkdir(parents=True, exist_ok=True) + return str(workdir) + + def status(self, handle): + return {"state": "running", "exit_code": 0, "outputs": {}} + + def cancel(self, handle): + pass + + +class StorageDeleteTests(TestCase): + def setUp(self): + self.tmp = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) + + def test_delete_file(self): + (self.tmp / "a.pdb").write_text("x", encoding="utf-8") + self.assertTrue(LocalFileStore(self.tmp).delete("a.pdb")) + self.assertFalse((self.tmp / "a.pdb").exists()) + + def test_delete_missing_returns_false(self): + self.assertFalse(LocalFileStore(self.tmp).delete("nope.pdb")) + + def test_delete_symlink_keeps_target(self): + data = self.tmp / "data" + data.mkdir() + target = data / "real.pdb" + target.write_text("x", encoding="utf-8") + (self.tmp / "link.pdb").symlink_to(target) + self.assertTrue(LocalFileStore(self.tmp).delete("link.pdb")) + self.assertFalse((self.tmp / "link.pdb").is_symlink()) + self.assertTrue(target.exists()) # the IMPORTED target is untouched + + def test_delete_guards_traversal(self): + with self.assertRaises(ValueError): + LocalFileStore(self.tmp).delete("../escape.pdb") + + +class _RunFixtureMixin: + """Builds a terminal, Reinspect-owned run with a real out_dir on disk.""" + + def _build(self, *, runner_handle="handle", status=Run.Status.SUCCEEDED, + with_dataset_artifact=False, key="k1", name="P"): + tmp = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, tmp, ignore_errors=True) + out_dir = tmp / "pandda_results" + (out_dir / "processed_datasets" / "ds1").mkdir(parents=True) + project = Project.objects.create(name=name, source_root=str(out_dir)) + run = Run.objects.create( + project=project, group="g", share_path=str(tmp), + out_dir=str(out_dir), idempotency_key=key, status=status, + runner_handle=runner_handle, + ) + ds = Dataset.objects.create(project=project, dtag="ds1") + rd = RunDataset.objects.create(run=run, dataset=ds) + ev = Event.objects.create(dataset=ds, run_dataset=rd, event_num=1) + map_rel = "processed_datasets/ds1/ds1-event_1_map.ccp4" + (out_dir / map_rel).write_text("map-bytes", encoding="utf-8") + Artifact.objects.create( + event=ev, kind=Artifact.Kind.EVENT_MAP, relpath=map_rel, + origin=Artifact.Origin.IMPORTED, + ) + if with_dataset_artifact: + pdb_rel = "processed_datasets/ds1/ds1-pandda-input.pdb" + (out_dir / pdb_rel).write_text("pdb", encoding="utf-8") + Artifact.objects.create( + dataset=ds, kind=Artifact.Kind.STRUCTURE, relpath=pdb_rel, + origin=Artifact.Origin.IMPORTED, + ) + return run, out_dir, ds + + +class RunOwnershipTests(TestCase): + def _run(self, **kw): + p = Project.objects.create(name=kw.pop("name"), source_root="/x") + defaults = dict( + project=p, group="g", share_path="/x", + idempotency_key=kw.pop("key"), status=Run.Status.SUCCEEDED, + runner_handle="h", + ) + defaults.update(kw) + return Run.objects.create(**defaults) + + def test_owned_when_handle_and_terminal(self): + self.assertTrue(run_owns_outdir(self._run(name="A", key="a"))) + + def test_not_owned_without_handle(self): + run = self._run(name="B", key="b", runner_handle="") + self.assertFalse(run_owns_outdir(run)) + + def test_not_owned_when_running(self): + run = self._run(name="C", key="c", status=Run.Status.RUNNING) + self.assertFalse(run_owns_outdir(run)) + + +class DeleteRunServiceTests(_RunFixtureMixin, TestCase): + def test_db_only_keeps_outdir(self): + run, out_dir, ds = self._build() + summary = delete_run(run, delete_outdir=False) + self.assertFalse(Run.objects.filter(pk=summary["run_id"]).exists()) + self.assertTrue(out_dir.is_dir()) # left on disk for manual cleanup + self.assertEqual(summary["events_deleted"], 1) + self.assertEqual(summary["artifacts_deleted"], 1) + self.assertFalse(summary["out_dir_removed"]) + self.assertEqual(summary["out_dir"], str(out_dir)) + + def test_safe_delete_removes_owned_tree(self): + run, out_dir, ds = self._build() + summary = delete_run(run, delete_outdir=True) + self.assertTrue(summary["out_dir_removed"]) + self.assertFalse(out_dir.exists()) + self.assertGreater(summary["disk_freed_bytes"], 0) + + def test_refuse_unowned_tree(self): + # runner_handle="" mimics the in-place-ingest synthetic run. + run, out_dir, ds = self._build(runner_handle="") + with self.assertRaises(CleanupError): + delete_run(run, delete_outdir=True) + self.assertTrue(Run.objects.filter(pk=run.pk).exists()) # not deleted + self.assertTrue(out_dir.exists()) + + def test_force_removes_unowned_tree(self): + run, out_dir, ds = self._build(runner_handle="") + summary = delete_run(run, delete_outdir=True, force=True) + self.assertTrue(summary["out_dir_removed"]) + self.assertFalse(out_dir.exists()) + + def test_refuse_shared_outdir(self): + run, out_dir, ds = self._build() + Run.objects.create( + project=run.project, group="g2", share_path=run.share_path, + out_dir=str(out_dir), idempotency_key="k2", + status=Run.Status.SUCCEEDED, runner_handle="h2", + ) + with self.assertRaises(CleanupError): + delete_run(run, delete_outdir=True) + self.assertTrue(out_dir.exists()) + + def test_refuse_orphan_dataset_artifact(self): + run, out_dir, ds = self._build(with_dataset_artifact=True) + # source_root == out_dir (the only on-disk copy of the pdb); no other + # surviving root holds it → removal would orphan it. + with override_settings( + PANDDA_DATA_ROOT=str(out_dir.parent / "noexist") + ): + with self.assertRaises(CleanupError): + delete_run(run, delete_outdir=True) + self.assertTrue(out_dir.exists()) + + def test_orphan_overridden_by_force(self): + run, out_dir, ds = self._build(with_dataset_artifact=True) + with override_settings( + PANDDA_DATA_ROOT=str(out_dir.parent / "noexist") + ): + summary = delete_run(run, delete_outdir=True, force=True) + self.assertTrue(summary["out_dir_removed"]) + + def test_no_orphan_when_copy_survives(self): + run, out_dir, ds = self._build(with_dataset_artifact=True) + # A surviving second run holds a copy of the dataset artifact. + other = out_dir.parent / "other_results" + (other / "processed_datasets" / "ds1").mkdir(parents=True) + (other / "processed_datasets/ds1/ds1-pandda-input.pdb").write_text( + "pdb", encoding="utf-8" + ) + Run.objects.create( + project=run.project, group="g2", share_path=run.share_path, + out_dir=str(other), idempotency_key="k2", + status=Run.Status.SUCCEEDED, runner_handle="h2", + ) + summary = delete_run(run, delete_outdir=True) + self.assertTrue(summary["out_dir_removed"]) + + def test_finding_and_crystal_survive(self): + run, out_dir, ds = self._build() + finding = Finding.objects.create(dataset=ds, centroid=[1.0, 2.0, 3.0]) + ev = Event.objects.get(run_dataset__run=run) + ev.finding = finding + ev.save(update_fields=["finding"]) + delete_run(run, delete_outdir=True) + self.assertTrue(Finding.objects.filter(pk=finding.pk).exists()) + self.assertTrue(Dataset.objects.filter(pk=ds.pk).exists()) + + +class RunDeleteEndpointTests(_RunFixtureMixin, TestCase): + def setUp(self): + self.client = APIClient() + + def test_delete_default_db_only(self): + run, out_dir, ds = self._build() + resp = self.client.delete(f"/api/v1/runs/{run.id}/") + self.assertEqual(resp.status_code, 200) + self.assertFalse(Run.objects.filter(pk=run.id).exists()) + self.assertTrue(out_dir.is_dir()) + self.assertEqual(resp.json()["events_deleted"], 1) + self.assertFalse(resp.json()["out_dir_removed"]) + + def test_delete_true_removes_tree(self): + run, out_dir, ds = self._build() + resp = self.client.delete( + f"/api/v1/runs/{run.id}/?delete_outdir=true" + ) + self.assertEqual(resp.status_code, 200) + self.assertTrue(resp.json()["out_dir_removed"]) + self.assertFalse(out_dir.exists()) + + def test_delete_bad_mode_400(self): + run, out_dir, ds = self._build() + resp = self.client.delete( + f"/api/v1/runs/{run.id}/?delete_outdir=maybe" + ) + self.assertEqual(resp.status_code, 400) + self.assertTrue(Run.objects.filter(pk=run.id).exists()) + + def test_delete_refusal_400(self): + run, out_dir, ds = self._build(runner_handle="") + resp = self.client.delete( + f"/api/v1/runs/{run.id}/?delete_outdir=true" + ) + self.assertEqual(resp.status_code, 400) + self.assertTrue(Run.objects.filter(pk=run.id).exists()) + self.assertTrue(out_dir.exists()) + + +class ZombieGuardTests(TestCase): + def setUp(self): + self.tmp = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) + self.share = str(self.tmp / "pandda_inputs" / "grp") + Path(self.share, "datasets").mkdir(parents=True) + (self.tmp / "pandda_results").mkdir() # the out_dir parent + + def _submit(self, **kw): + defaults = dict( + project_external_id="P", group="grp", share_path=self.share, + input_hash="h", + ) + defaults.update(kw) + with override_settings(PANDDA_JOBS_ROOT=str(self.tmp)), \ + mock.patch.object( + runservice, "get_runner", return_value=FakeRunner() + ): + return runservice.submit_run(**defaults) + + def test_zombie_outdir_refused(self): + out_dir = self.tmp / "pandda_results" / "grp" + out_dir.mkdir(parents=True) + (out_dir / "stale.csv").write_text("x", encoding="utf-8") + with self.assertRaises(runservice.RunError) as cm: + self._submit() + self.assertIn("owned by no run", str(cm.exception)) + + def test_populated_outdir_owned_by_run_allowed(self): + out_dir = self.tmp / "pandda_results" / "grp" + out_dir.mkdir(parents=True) + (out_dir / "events.csv").write_text("x", encoding="utf-8") + p = Project.objects.create( + name="P", external_id="P", source_root=self.share + ) + Run.objects.create( + project=p, group="grp", share_path=self.share, + out_dir=str(out_dir), idempotency_key="owner", + status=Run.Status.SUCCEEDED, runner_handle="h", + ) + run, created = self._submit(input_hash="different") + self.assertTrue(created) + + def test_empty_outdir_allowed(self): + run, created = self._submit() + self.assertTrue(created) + + +class _ProjectFixtureMixin: + """Builds a project with one run (owned or in-place), a dataset, an + event-scoped artifact, and a real out_dir on disk.""" + + def _build_project(self, *, source_managed=False, owned_run=True, + name="P"): + tmp = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, tmp, ignore_errors=True) + out_dir = tmp / "pandda_results" + (out_dir / "processed_datasets" / "ds1").mkdir(parents=True) + project = Project.objects.create( + name=name, source_root=str(out_dir), + source_managed=source_managed, + ) + run = Run.objects.create( + project=project, group="g", share_path=str(tmp), + out_dir=str(out_dir), idempotency_key=f"{name}-k", + status=Run.Status.SUCCEEDED, + runner_handle=("h" if owned_run else ""), + ) + ds = Dataset.objects.create(project=project, dtag="ds1") + rd = RunDataset.objects.create(run=run, dataset=ds) + ev = Event.objects.create(dataset=ds, run_dataset=rd, event_num=1) + map_rel = "processed_datasets/ds1/ds1-event_1_map.ccp4" + (out_dir / map_rel).write_text("map", encoding="utf-8") + Artifact.objects.create( + event=ev, kind=Artifact.Kind.EVENT_MAP, relpath=map_rel, + origin=Artifact.Origin.IMPORTED, + ) + return project, out_dir, ds + + +class ProjectArchivePurgeTests(_ProjectFixtureMixin, TestCase): + def setUp(self): + self.client = APIClient() + + def _names(self, payload): + rows = payload.get("results", payload) + return [r["name"] for r in rows] + + def test_archive_sets_flag_and_retains(self): + project, out_dir, ds = self._build_project() + resp = self.client.delete(f"/api/v1/projects/{project.id}/") + self.assertEqual(resp.status_code, 200) + project.refresh_from_db() + self.assertTrue(project.archived) + self.assertEqual(resp.json()["n_events"], 1) + self.assertTrue(out_dir.exists()) # nothing deleted on archive + + def test_archived_hidden_from_list_shown_with_flag(self): + self._build_project(name="Active") + archived, _, _ = self._build_project(name="Archived") + archive_project(archived) + listed = self.client.get("/api/v1/projects/").json() + self.assertIn("Active", self._names(listed)) + self.assertNotIn("Archived", self._names(listed)) + incl = self.client.get( + "/api/v1/projects/?include_archived=true" + ).json() + self.assertIn("Archived", self._names(incl)) + + def test_unarchive_restores(self): + project, _, _ = self._build_project() + archive_project(project) + resp = self.client.post( + f"/api/v1/projects/{project.id}/unarchive/" + ) + self.assertEqual(resp.status_code, 200) + project.refresh_from_db() + self.assertFalse(project.archived) + + def test_purge_requires_archived(self): + project, out_dir, _ = self._build_project() + resp = self.client.post(f"/api/v1/projects/{project.id}/purge/") + self.assertEqual(resp.status_code, 400) + self.assertTrue(Project.objects.filter(pk=project.id).exists()) + + def test_purge_db_only_keeps_files(self): + project, out_dir, _ = self._build_project() + archive_project(project) + resp = self.client.post(f"/api/v1/projects/{project.id}/purge/") + self.assertEqual(resp.status_code, 200) + self.assertFalse(Project.objects.filter(pk=project.id).exists()) + self.assertTrue(out_dir.exists()) # files retained (default false) + self.assertEqual(resp.json()["trees_removed"], 0) + + def test_purge_true_removes_owned_tree(self): + project, out_dir, _ = self._build_project(owned_run=True) + archive_project(project) + resp = self.client.post( + f"/api/v1/projects/{project.id}/purge/?delete_outdirs=true" + ) + self.assertEqual(resp.status_code, 200) + self.assertFalse(out_dir.exists()) + self.assertGreaterEqual(resp.json()["trees_removed"], 1) + + def test_purge_true_keeps_inplace_tree(self): + # No runner_handle + not source_managed = the user's in-place tree. + project, out_dir, _ = self._build_project( + owned_run=False, source_managed=False + ) + archive_project(project) + summary = purge_project(project, delete_outdirs=True) + self.assertTrue(out_dir.exists()) # user's data untouched + self.assertEqual(summary["trees_removed"], 0) + + def test_purge_force_removes_inplace_tree(self): + project, out_dir, _ = self._build_project( + owned_run=False, source_managed=False + ) + archive_project(project) + purge_project(project, delete_outdirs=True, force=True) + self.assertFalse(out_dir.exists()) # explicit nuke + + def test_purge_managed_removes_source_tree(self): + project, out_dir, _ = self._build_project( + owned_run=False, source_managed=True + ) + archive_project(project) + summary = purge_project(project, delete_outdirs=True) + self.assertFalse(out_dir.exists()) # source_root==out_dir, managed + self.assertTrue(summary["source_tree_removed"]) + + def test_purge_sweeps_built_under_unremoved_source_root(self): + # source_root != out_dir, unmanaged: the owned out_dir is removed, the + # source tree is NOT, but the BUILT artifact's bytes under it are swept. + tmp = Path(tempfile.mkdtemp()) + self.addCleanup(shutil.rmtree, tmp, ignore_errors=True) + source_root = tmp / "src" + out_dir = tmp / "out" + (source_root / "builds" / "1").mkdir(parents=True) + out_dir.mkdir() + built = source_root / "builds" / "1" / "model.pdb" + built.write_text("pdb", encoding="utf-8") + project = Project.objects.create( + name="P", source_root=str(source_root), source_managed=False + ) + run = Run.objects.create( + project=project, group="g", share_path=str(tmp), + out_dir=str(out_dir), idempotency_key="k", + status=Run.Status.SUCCEEDED, runner_handle="h", + ) + ds = Dataset.objects.create(project=project, dtag="ds1") + RunDataset.objects.create(run=run, dataset=ds) + Artifact.objects.create( + dataset=ds, kind=Artifact.Kind.STRUCTURE, + relpath="builds/1/model.pdb", origin=Artifact.Origin.BUILT, + ) + archive_project(project) + summary = purge_project(project, delete_outdirs=True) + self.assertFalse(built.exists()) # swept via store.delete + self.assertTrue(source_root.is_dir()) # unmanaged tree NOT removed + self.assertFalse(out_dir.exists()) # owned out_dir removed + self.assertEqual(summary["n_built_models"], 1) diff --git a/inspect_api/views.py b/inspect_api/views.py index 99fa190..57843ae 100644 --- a/inspect_api/views.py +++ b/inspect_api/views.py @@ -11,6 +11,12 @@ from rest_framework.response import Response from .buildservice import BuildError, land_built_model +from .cleanup import ( + CleanupError, + archive_project, + delete_run, + purge_project, +) from .identity import identity_from_request from .importer import ImportError_, import_zip, ingest_path from .jobs import get_runner @@ -71,10 +77,79 @@ def _is_local_request(request) -> bool: return request.META.get("REMOTE_ADDR") in _LOOPBACK -class ProjectViewSet(viewsets.ReadOnlyModelViewSet): +class ProjectViewSet( + mixins.RetrieveModelMixin, + mixins.ListModelMixin, + mixins.DestroyModelMixin, + viewsets.GenericViewSet, +): queryset = Project.objects.all() serializer_class = ProjectSerializer + def get_queryset(self): + # Archived projects are tombstoned: hidden from the default list but + # still retrievable (for the un-archive / detail / serving paths) and + # shown when explicitly asked via ?include_archived. See + # docs/DELETION_AND_CLEANUP.md §2. + qs = Project.objects.all() + include = self.request.query_params.get("include_archived") + if self.action == "list" and include not in ("1", "true", "yes"): + qs = qs.filter(archived=False) + return qs + + def destroy(self, request, *args, **kwargs): + """Soft-delete: archive the project (reversible). The irreversible + teardown is the separate POST .../purge step.""" + summary = archive_project(self.get_object()) + return Response(summary, status=200) + + @action(detail=True, methods=["post"]) + def unarchive(self, request, pk=None): + """Restore an archived project.""" + project = self.get_object() + if project.archived: + project.archived = False + project.save(update_fields=["archived"]) + return Response(self.get_serializer(project).data) + + @extend_schema( + responses={ + 200: OpenApiResponse( + description="Purged; body is the audit summary." + ), + 400: OpenApiResponse( + description="Bad delete_outdirs mode, or project is not " + "archived (archive before purging)." + ), + }, + ) + @action(detail=True, methods=["post"]) + def purge(self, request, pk=None): + """Irreversibly hard-delete an ARCHIVED project (DB cascade) and, + per ``?delete_outdirs=false|true|force``, its files. Requires the + project to be archived first — the explicit second step that gates the + irreversible work (docs/DELETION_AND_CLEANUP.md §2/§4).""" + project = self.get_object() + if not project.archived: + return Response( + {"detail": "Archive the project before purging it " + "(DELETE /projects// first)."}, + status=400, + ) + mode = (request.query_params.get("delete_outdirs") or "false").lower() + if mode not in ("false", "true", "force"): + return Response( + {"detail": "delete_outdirs must be one of: false, true, " + "force"}, + status=400, + ) + summary = purge_project( + project, + delete_outdirs=mode in ("true", "force"), + force=mode == "force", + ) + return Response(summary, status=200) + @action(detail=True, methods=["get"]) def reports(self, request, pk=None): """List this project's HTML reports (for the dashboard iframe panel).""" @@ -461,6 +536,7 @@ class RunViewSet( mixins.RetrieveModelMixin, mixins.ListModelMixin, mixins.CreateModelMixin, + mixins.DestroyModelMixin, viewsets.GenericViewSet, ): """ @@ -474,6 +550,11 @@ class RunViewSet( observed success, ingests the produced pandda2_out/ tree (idempotent). * ``GET /runs/?project=&group=…`` — list. * ``POST /runs/{id}/cancel/`` — terminate. + * ``DELETE /runs/{id}/?delete_outdir=false|true|force`` — hard-delete the + run (DB cascade) and optionally its output tree. ``false`` (default) is + DB-only and returns the on-disk path; ``true`` safe-deletes the tree + (refuses if not ours / shared / would orphan); ``force`` removes it + regardless. See docs/DELETION_AND_CLEANUP.md §4. """ serializer_class = RunSerializer @@ -547,3 +628,34 @@ def cancel(self, request, pk=None): run.completed_at = timezone.now() run.save(update_fields=["status", "completed_at"]) return Response(self.get_serializer(run).data) + + @extend_schema( + responses={ + 200: OpenApiResponse( + description="Deleted; body is the audit summary " + "({run_id, events_deleted, artifacts_deleted, " + "disk_freed_bytes, out_dir, out_dir_removed})." + ), + 400: OpenApiResponse( + description="Bad delete_outdir mode, or out_dir removal " + "refused (not Reinspect-owned / shared / would orphan)." + ), + }, + ) + def destroy(self, request, *args, **kwargs): + run = self.get_object() + mode = (request.query_params.get("delete_outdir") or "false").lower() + if mode not in ("false", "true", "force"): + return Response( + {"detail": "delete_outdir must be one of: false, true, force"}, + status=400, + ) + try: + summary = delete_run( + run, + delete_outdir=mode in ("true", "force"), + force=mode == "force", + ) + except CleanupError as exc: + return Response({"detail": str(exc)}, status=400) + return Response(summary, status=200)