diff --git a/backend/app/services/cleanup_service.py b/backend/app/services/cleanup_service.py index 2a34509..d39cdb6 100644 --- a/backend/app/services/cleanup_service.py +++ b/backend/app/services/cleanup_service.py @@ -48,6 +48,47 @@ log = logging.getLogger(__name__) _VIDEO_DURATION_UNKNOWN = -1.0 +# -- artist-cascade predicates (rule 93: ONE definition, preview + apply) --- +# project_artist_cascade (preview) and delete_artist_cascade (apply) both build +# their queries from these. The preview used to re-derive its own — which is how +# it came to count images and stay silent about posts and attachments while the +# apply destroyed both. Same failure shape as the 2026-06-08 fandom-tag +# deletion, where a re-implemented delete predicate diverged from the preview's. +# Returned as condition LISTS spread into `.where(*conds)`, matching +# _unused_tag_conditions / _bare_post_conditions below. + + +def _artist_images_conditions(artist_id: int) -> list: + """Images the cascade deletes (rows AND their on-disk files).""" + return [ImageRecord.artist_id == artist_id] + + +def _artist_posts_conditions(artist_id: int) -> list: + """Posts the cascade destroys. The apply never names these — post.artist_id + is ondelete=CASCADE, so Postgres takes them when the artist row goes — which + is exactly why the preview has to name them: an artist whose posts are + body-only (no images) otherwise previews as `images: 0` and reads as an + empty artist, while every captured body/description/external-link set is + destroyed.""" + return [Post.artist_id == artist_id] + + +def _artist_attachments_conditions(artist_id: int) -> list: + """Attachments the cascade deletes. Matched by artist_id OR by the owning + post's artist: artist_id is nullable (_capture_attachment leaves it NULL + when no artist resolved), so neither arm alone covers every row. The + sha-addressed blobs are NOT unlinked (one blob backs many rows) — these are + row counts, and the bytes are not part of this operation's footprint.""" + return [ + or_( + PostAttachment.artist_id == artist_id, + PostAttachment.post_id.in_( + select(Post.id).where(*_artist_posts_conditions(artist_id)) + ), + ) + ] + + def project_artist_cascade(session: Session, *, slug: str) -> dict: """Read-only projection of what delete_artist_cascade would touch. @@ -56,12 +97,17 @@ def project_artist_cascade(session: Session, *, slug: str) -> dict: "artist": {"id": int, "name": str, "slug": str}, "projected": { "images": int, + "posts": int, # hard-deleted by the post.artist_id CASCADE + "attachments": int, # rows deleted; the sha-addressed blobs stay "sources": int, "thumbs": int, # images with a thumbnail_path set "import_tasks": int, # ImportTask rows referencing the artist's images "bytes_on_disk": int, # SUM(image_record.size_bytes) — column is NOT NULL }, } + Every count is built from the shared `_artist_*_conditions` predicates the + apply uses, so the two halves cannot drift (rule 93). + Raises LookupError if slug not found. No mutations. """ from ..models.import_task import ImportTask @@ -73,36 +119,49 @@ def project_artist_cascade(session: Session, *, slug: str) -> dict: if artist is None: raise LookupError(f"artist slug not found: {slug!r}") + images_conds = _artist_images_conditions(artist.id) + images_count = session.execute( - select(func.count(ImageRecord.id)) - .where(ImageRecord.artist_id == artist.id) + select(func.count(ImageRecord.id)).where(*images_conds) ).scalar_one() + posts_count = session.execute( + select(func.count(Post.id)) + .where(*_artist_posts_conditions(artist.id)) + ).scalar_one() + attachments_count = session.execute( + select(func.count(PostAttachment.id)) + .where(*_artist_attachments_conditions(artist.id)) + ).scalar_one() + # Sources have no shared predicate: the apply never queries them either, it + # gets them from the Artist.sources ORM cascade. Counted directly here. sources_count = session.execute( select(func.count(Source.id)) .where(Source.artist_id == artist.id) ).scalar_one() thumbs_count = session.execute( select(func.count(ImageRecord.id)) - .where(ImageRecord.artist_id == artist.id) + .where(*images_conds) .where(ImageRecord.thumbnail_path.is_not(None)) ).scalar_one() import_tasks_count = session.execute( select(func.count(ImportTask.id)) .where( ImportTask.result_image_id.in_( - select(ImageRecord.id).where(ImageRecord.artist_id == artist.id) + select(ImageRecord.id).where(*images_conds) ) ) ).scalar_one() bytes_on_disk = session.execute( select(func.coalesce(func.sum(ImageRecord.size_bytes), 0)) - .where(ImageRecord.artist_id == artist.id) + .where(*images_conds) ).scalar_one() return { "artist": {"id": artist.id, "name": artist.name, "slug": artist.slug}, "projected": { "images": images_count, + "posts": posts_count, + "attachments": attachments_count, "sources": sources_count, "thumbs": thumbs_count, "import_tasks": import_tasks_count, @@ -291,12 +350,22 @@ def delete_artist_cascade( "files_deleted": 0, "thumbs_deleted": 0, "import_tasks_nulled": 0, + "posts_deleted": 0, "attachments_deleted": 0, "files_failed": 0, }, } artist_info = {"id": artist.id, "name": artist.name, "slug": artist.slug} + # Counted BEFORE the delete: Postgres takes these via the post.artist_id + # CASCADE when the artist row goes, so afterwards there is nothing left to + # count. Reported so the summary can be checked against the preview's + # `posts` — the parity rule 93 asks for is only testable if both halves + # actually state the number. + posts_deleted = session.execute( + select(func.count(Post.id)).where(*_artist_posts_conditions(artist.id)) + ).scalar_one() + images_deleted = 0 files_deleted = 0 thumbs_deleted = 0 @@ -305,7 +374,7 @@ def delete_artist_cascade( while True: rows = session.execute( select(ImageRecord) - .where(ImageRecord.artist_id == artist.id) + .where(*_artist_images_conditions(artist.id)) .limit(500) ).scalars().all() if not rows: @@ -342,24 +411,11 @@ def delete_artist_cascade( # _repoint_post_links guards the identical collision class in the reconcile # path; this is its artist-cascade counterpart. # - # Matched by artist_id OR by the owning post's artist: artist_id is nullable - # and _capture_attachment leaves it NULL when no artist resolved, so neither - # predicate alone covers every row this cascade is about to strand. - # - # The sha-addressed BLOBS are deliberately left on disk. One blob backs many - # rows (attachment_store.store is sha-addressed + idempotent), so unlinking - # needs a refcount pass over the surviving rows — that belongs to the - # attachment-reclamation sweep, not here, and this Tier-C op must not delete - # bytes its own preview never disclosed. + # Which rows count as the artist's — and why the blobs are left on disk — + # is _artist_attachments_conditions, shared with the preview. attachments_deleted = session.execute( - delete(PostAttachment).where( - or_( - PostAttachment.artist_id == artist.id, - PostAttachment.post_id.in_( - select(Post.id).where(Post.artist_id == artist.id) - ), - ) - ) + delete(PostAttachment) + .where(*_artist_attachments_conditions(artist.id)) ).rowcount or 0 session.commit() @@ -373,6 +429,7 @@ def delete_artist_cascade( "files_deleted": files_deleted, "thumbs_deleted": thumbs_deleted, "import_tasks_nulled": import_tasks_nulled, + "posts_deleted": posts_deleted, "attachments_deleted": attachments_deleted, "files_failed": files_failed, }, diff --git a/frontend/src/components/artist/ArtistDangerZone.vue b/frontend/src/components/artist/ArtistDangerZone.vue index 275c4a4..f4da126 100644 --- a/frontend/src/components/artist/ArtistDangerZone.vue +++ b/frontend/src/components/artist/ArtistDangerZone.vue @@ -50,14 +50,19 @@ const projected = ref(null) const projectedCounts = computed(() => projected.value?.projected || null) -const modalDescription = computed( - () => projected.value - ? `Artist “${props.artistName}” — ` - + `${projected.value.projected.images} images, ` - + `${projected.value.projected.sources} sources, ` - + `${Math.round(projected.value.projected.bytes_on_disk / 1_048_576)} MiB on disk` - : '', -) +// `posts` is named here, not left to the counts grid below it: an artist whose +// posts are body-only previews as `images: 0`, and a summary line that says +// only "0 images" reads as "this artist is empty" while the apply destroys +// every captured post body (#3067). Attachments stay in the grid — the grid +// renders every key, so this line carries only what changes the read. +const modalDescription = computed(() => { + const p = projectedCounts.value + return p + ? `Artist “${props.artistName}” — ${p.images} images, ` + + `${p.posts} posts, ${p.sources} sources, ` + + `${Math.round(p.bytes_on_disk / 1_048_576)} MiB on disk` + : '' +}) async function onClick() { loading.value = true diff --git a/tests/test_cleanup_service.py b/tests/test_cleanup_service.py index a9f3d00..7303a2f 100644 --- a/tests/test_cleanup_service.py +++ b/tests/test_cleanup_service.py @@ -68,6 +68,8 @@ def test_project_artist_cascade_returns_zeroes_for_empty_artist(db_sync): assert result["artist"]["slug"] == "empty" assert result["projected"] == { "images": 0, + "posts": 0, + "attachments": 0, "sources": 0, "thumbs": 0, "import_tasks": 0, @@ -92,6 +94,87 @@ def test_project_artist_cascade_counts_images_and_thumbs_and_bytes(db_sync, tmp_ assert result["projected"]["bytes_on_disk"] == 3500 +def test_project_artist_cascade_counts_posts_and_attachments(db_sync, tmp_path): + """The body-only artist: zero images, but posts and attachments that the + apply destroys. Previewing this as `images: 0` alone is what made a + content-only artist read as an empty one (#3067).""" + a = _make_artist(db_sync, slug="bodyonly") + p1 = Post(artist_id=a.id, external_post_id="bo-1", description="a body") + p2 = Post(artist_id=a.id, external_post_id="bo-2", description="another") + db_sync.add_all([p1, p2]) + db_sync.flush() + db_sync.add(PostAttachment( + post_id=p1.id, artist_id=a.id, sha256="b0d1".ljust(64, "0"), + path="/store/b0d1/a.pdf", original_filename="a.pdf", + ext=".pdf", size_bytes=5, + )) + # artist_id NULL, reachable only through its post — the second arm of + # _artist_attachments_conditions. + db_sync.add(PostAttachment( + post_id=p2.id, artist_id=None, sha256="b0d2".ljust(64, "0"), + path="/store/b0d2/b.pdf", original_filename="b.pdf", + ext=".pdf", size_bytes=5, + )) + db_sync.commit() + + projected = cleanup_service.project_artist_cascade( + db_sync, slug="bodyonly", + )["projected"] + assert projected["images"] == 0 + assert projected["posts"] == 2 + assert projected["attachments"] == 2 + + +def test_artist_cascade_preview_matches_apply(db_sync, tmp_path): + """Rule 93: the preview's numbers must be what the apply actually does. + + Guards the drift directly rather than trusting that both halves happen to + use the same predicate — the preview and apply are asserted against each + other on one artist carrying all three row kinds. + """ + a = _make_artist(db_sync, slug="parity") + for i in range(3): + f = tmp_path / f"par{i}.jpg" + f.write_bytes(b"x") + _make_image( + db_sync, artist=a, path=str(f), sha256=f"{i:064x}", size=10, + ) + posts = [ + Post(artist_id=a.id, external_post_id=f"par-{i}") for i in range(4) + ] + db_sync.add_all(posts) + db_sync.flush() + for i, p in enumerate(posts[:2]): + db_sync.add(PostAttachment( + post_id=p.id, artist_id=a.id, sha256=f"par{i}".ljust(64, "0"), + path=f"/store/par{i}/f.zip", original_filename="f.zip", + ext=".zip", size_bytes=9, + )) + db_sync.commit() + artist_id = a.id + + projected = cleanup_service.project_artist_cascade( + db_sync, slug="parity", + )["projected"] + summary = cleanup_service.delete_artist_cascade( + db_sync, artist_id=artist_id, images_root=tmp_path, + )["summary"] + + assert projected["images"] == summary["images_deleted"] == 3 + assert projected["posts"] == summary["posts_deleted"] == 4 + assert projected["attachments"] == summary["attachments_deleted"] == 2 + + # And the apply really did remove them — a matching pair of numbers is + # worth nothing if neither half touched the DB. + assert db_sync.execute( + select(func.count(Post.id)).where(Post.artist_id == artist_id) + ).scalar_one() == 0 + assert db_sync.execute( + select(func.count(PostAttachment.id)) + .where(PostAttachment.artist_id == artist_id) + ).scalar_one() == 0 + + def test_project_artist_cascade_raises_on_unknown_slug(db_sync): with pytest.raises(LookupError): cleanup_service.project_artist_cascade(db_sync, slug="nope")