From 9b82a95b7ecf8db94790fd207a990d0ef9590a63 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 13 Sep 2026 20:52:32 -0400 Subject: [PATCH] feat: Settings can re-download the Discord images the None naming broke (3999) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The operator chose a clean re-download over relinking in place. The ~1,600 broken files can't be relinked reliably: their message ids are gone, and their sidecars collided. Settings → Maintenance → "Repair Discord downloads" previews, then applies: - Deletes every image whose path is `…/discord/None/<8 digits>_None_…`. Both the folder and the name are required, because that pair is only what the bug produced. It reuses cleanup_service.delete_images for the record and file deletes. - Sweeps the leftover collided sidecars from those folders and removes the emptied folders. - Only then clears gallery-dl's archive rows `discord%`, excluding `discordasset_%`. Upstream keys message attachments as `discord{message_id}_{num}`. Since the broken files lost their message ids, per-source forgetting is impossible. Every pre-fix Discord download is broken, and files fetched after the fix still exist on disk, so gallery-dl's `skip` won't re-fetch them. - Arms a fresh backfill on every Discord source. The apply defaults to preview at both the route and the task, runs on maintenance_long, and is never on a beat. The card uses the confirm-dialog pattern of AttachmentReclaimCard. Supporting refactors, with no behaviour change: - gallery_dl.archive_path() is the single definition of the archive location. - source_service.arm_backfill() is the mutation start_backfill already did, now shared with the sync repair. Tests (tests/test_discord_repair.py): - The archive clear leaves other platforms and Discord assets alone, and counting mutates nothing. - Case-twin artist folders are both found. - The folder sweep works. - An integration run shows only the broken image goes. A correctly named Discord file and a `None` folder under Patreon survive, and only Discord sources are re-armed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SHQB1YukL3VyvMK8rcbmV9 --- backend/app/api/admin.py | 14 ++ backend/app/services/discord_repair.py | 183 ++++++++++++++++++ backend/app/services/gallery_dl.py | 12 +- backend/app/services/source_service.py | 21 +- backend/app/tasks/admin.py | 22 +++ .../components/settings/DiscordRepairCard.vue | 118 +++++++++++ .../components/settings/MaintenancePanel.vue | 2 + tests/test_discord_repair.py | 170 ++++++++++++++++ 8 files changed, 534 insertions(+), 8 deletions(-) create mode 100644 backend/app/services/discord_repair.py create mode 100644 frontend/src/components/settings/DiscordRepairCard.vue create mode 100644 tests/test_discord_repair.py diff --git a/backend/app/api/admin.py b/backend/app/api/admin.py index 9f676be..41b865f 100644 --- a/backend/app/api/admin.py +++ b/backend/app/api/admin.py @@ -475,6 +475,20 @@ async def trigger_reclaim_attachments(): return _queued(async_result) +@admin_bp.route("/maintenance/repair-discord-downloads", methods=["POST"]) +async def trigger_repair_discord_downloads(): + """Clean re-download of the Discord files broken by the `None` naming + (#3999). Body {"dry_run": bool}; dry_run is the DEFAULT, because the apply + deletes files and makes gallery-dl forget every Discord download. Returns the + Celery task id — poll /maintenance/task-result/ for the summary.""" + from ..tasks.admin import repair_discord_downloads_task + + body = await request.get_json(silent=True) or {} + dry_run = bool(body.get("dry_run", True)) + async_result = repair_discord_downloads_task.delay(dry_run=dry_run) + return _queued(async_result) + + @admin_bp.route("/maintenance/dedup-videos", methods=["POST"]) async def trigger_dedup_videos(): """Tier-1 video dedup (#871). Body {"dry_run": bool}: dry_run=true previews diff --git a/backend/app/services/discord_repair.py b/backend/app/services/discord_repair.py new file mode 100644 index 0000000..891e13a --- /dev/null +++ b/backend/app/services/discord_repair.py @@ -0,0 +1,183 @@ +"""Repair the Discord downloads made before the naming fix (issue #3999). + +Until dc840fe, gallery-dl's Discord patterns asked for keys the extractor never +emits, so every Discord download landed as +`/discord/None/_None_`, next to a sidecar named +after the attachment's ORIGINAL name. That broke two things: + +* **No Post, no date.** `find_sidecar` can never pair those names, so these + files were imported as loose images with no Post, and a card shows the + download time. +* **No trustworthy metadata to relink from.** Every `image.png` in a channel + wrote the same `image.json`, so the surviving sidecar describes whichever + message was written last. The message id is gone from the filename too. + +The operator chose a clean re-download (2026-09-13) over relinking in place: +delete the broken files and their records, make gallery-dl forget it fetched +them, and backfill every Discord source again under the fixed naming. + +## Why the archive is cleared for ALL of Discord + +gallery-dl records a download as `discord{message_id}_{num}` (upstream +`DiscordExtractor.archive_fmt`, prefixed with the category). The broken files +lost their message ids, so there is no way to forget one source's entries and +not another's. Every Discord download made before the fix is broken, so +forgetting all of them is exactly right. Anything downloaded AFTER the fix still +exists on disk under its correct name, and gallery-dl's `skip` sees the file and +does not fetch it again. + +## What it does not touch + +Discord posts FC grouped itself (#388 E2) are built from Posts, and these files +never had one, so there is nothing grouped to unwind. Images outside a +`discord/None/` directory are never selected: the path filter requires both the +`None` directory and the `_None_` filename, the pair only the bug produced. + +Operator-triggered only (Settings, preview first). Never on a beat. +""" + +from __future__ import annotations + +import logging +import sqlite3 +from pathlib import Path + +from sqlalchemy import func, select +from sqlalchemy.orm import Session + +from ..models import ImageRecord, Source +from .cleanup_service import delete_images +from .gallery_dl import archive_path +from .source_service import arm_backfill + +log = logging.getLogger(__name__) + +# `%` and `_` are LIKE wildcards, so the literal underscores around None are +# escaped. `________` is the eight-digit date prefix the old pattern wrote. +_BROKEN_PATH_LIKE = r"%/discord/None/________\_None\_%" + +# Upstream keys asset downloads as `asset_{server_id}_{id}`. FC never fetches +# server assets, but excluding them keeps this to exactly the message +# attachments the bug mangled. +_ARCHIVE_SQL_MATCH = r"entry LIKE 'discord%' AND entry NOT LIKE 'discordasset\_%' ESCAPE '\'" +_COUNT_SQL = "SELECT COUNT(*) FROM archive WHERE " + _ARCHIVE_SQL_MATCH +_DELETE_SQL = "DELETE FROM archive WHERE " + _ARCHIVE_SQL_MATCH + + +def broken_directories(images_root: Path) -> list[Path]: + """Every `/discord/None` directory. Artist folders that differ only by + case (`Conto` and `conto`) are separate directories and both are found.""" + return sorted(d for d in Path(images_root).glob("*/discord/None") if d.is_dir()) + + +def count_archive_entries(archive: Path) -> int: + return _archive(archive, delete=False) + + +def forget_archive_entries(archive: Path) -> int: + return _archive(archive, delete=True) + + +def _archive(archive: Path, *, delete: bool) -> int: + if not archive.is_file(): + return 0 + # A download running at the same moment holds this file briefly. Waiting + # 30s for its lock beats failing the repair over a transient contention. + conn = sqlite3.connect(str(archive), timeout=30) + try: + has_table = conn.execute( + "SELECT 1 FROM sqlite_master WHERE type='table' AND name='archive'" + ).fetchone() + if not has_table: + return 0 + if not delete: + return conn.execute(_COUNT_SQL).fetchone()[0] + cur = conn.execute(_DELETE_SQL) + conn.commit() + return cur.rowcount + finally: + conn.close() + + +def _sweep_directory(directory: Path) -> tuple[int, bool]: + """Remove what the record deletes left behind: the collided sidecars, plus any + file that never became a record (a quarantined or rejected download). Then + the directory itself, if it is empty. Returns (files removed, dir removed).""" + removed = 0 + for f in directory.iterdir(): + if f.is_file(): + try: + f.unlink() + removed += 1 + except OSError as exc: + log.warning("discord repair: could not remove %s: %s", f, exc) + try: + directory.rmdir() + return removed, True + except OSError: + return removed, False + + +def repair_discord_downloads( + session: Session, *, images_root: Path, dry_run: bool, +) -> dict: + images_root = Path(images_root) + archive = archive_path(images_root) + + broken = select(ImageRecord.id, ImageRecord.size_bytes).where( + ImageRecord.path.like(_BROKEN_PATH_LIKE, escape="\\") + ) + rows = session.execute(broken).all() + image_ids = [r.id for r in rows] + directories = broken_directories(images_root) + sources = session.execute( + select(Source).where(Source.platform == "discord") + ).scalars().all() + + summary = { + "images": len(image_ids), + "bytes": sum(r.size_bytes or 0 for r in rows), + "directories": len(directories), + "sources": len(sources), + "enabled_sources": sum(1 for s in sources if s.enabled), + } + + if dry_run: + summary["archive_entries"] = count_archive_entries(archive) + return summary + + deleted = delete_images(session, image_ids=image_ids, images_root=images_root) + + swept = 0 + directories_removed = 0 + for d in directories: + n, gone = _sweep_directory(d) + swept += n + directories_removed += int(gone) + + # Only after the files are gone. Forgetting first and failing half way would + # leave gallery-dl free to re-fetch into a directory still full of the old + # copies. + forgotten = forget_archive_entries(archive) + + for source in sources: + arm_backfill(source) + session.commit() + + remaining = session.execute( + select(func.count(ImageRecord.id)).where( + ImageRecord.path.like(_BROKEN_PATH_LIKE, escape="\\") + ) + ).scalar_one() + + summary.update( + images_deleted=deleted["images_deleted"], + files_failed=deleted["files_failed"], + leftover_files_removed=swept, + directories_removed=directories_removed, + archive_entries=forgotten, + backfills_started=len(sources), + remaining=remaining, + ) + log.info("discord repair applied: %s", summary) + return summary diff --git a/backend/app/services/gallery_dl.py b/backend/app/services/gallery_dl.py index 83e8aaa..b07fb1b 100644 --- a/backend/app/services/gallery_dl.py +++ b/backend/app/services/gallery_dl.py @@ -136,6 +136,16 @@ def metadata_postprocessor(filename: str) -> dict: return {"name": "metadata", "mode": "json", "directory": ".", "filename": filename} +def archive_path(images_root: Path) -> Path: + """gallery-dl's download archive: the record of what it has already fetched. + + One definition, because the Discord repair (services/discord_repair.py) has + to find the same file the downloader writes, without constructing a service + whose __init__ creates directories. + """ + return Path(images_root) / ".gallery-dl" / "archive.sqlite3" + + @dataclass class SourceConfig: """Per-source overrides loaded from Source.config_overrides JSON. @@ -447,7 +457,7 @@ class GalleryDLService: config = { "extractor": { "base-directory": str(self.images_root), - "archive": str(self._config_dir / "archive.sqlite3"), + "archive": str(archive_path(self.images_root)), "skip": True, "sleep": self._rate_limit, "sleep-request": max(0.5, self._rate_limit / 4), diff --git a/backend/app/services/source_service.py b/backend/app/services/source_service.py index 22a49ba..034eeb5 100644 --- a/backend/app/services/source_service.py +++ b/backend/app/services/source_service.py @@ -158,6 +158,19 @@ def _is_app_managed(key: str) -> bool: BACKFILL_MAX_CHUNKS = 200 +def arm_backfill(source: Source) -> None: + """Arm a fresh run-until-done backfill on `source` (plan #693). Mutation + only — the caller commits. Shared by `SourceService.start_backfill` and the + sync Discord repair, so both clear exactly the same resume state.""" + co = dict(source.config_overrides or {}) + co["_backfill_state"] = "running" + for k in ("_backfill_cursor", "_backfill_cursor_stalls", "_backfill_chunks", + "_backfill_posts"): + co.pop(k, None) + source.config_overrides = co + source.backfill_runs_remaining = BACKFILL_MAX_CHUNKS + + class SourceService: def __init__(self, session: AsyncSession): self.session = session @@ -473,13 +486,7 @@ class SourceService: )).scalar_one_or_none() if source is None: raise LookupError(f"source id={source_id} not found") - co = dict(source.config_overrides or {}) - co["_backfill_state"] = "running" - for k in ("_backfill_cursor", "_backfill_cursor_stalls", "_backfill_chunks", - "_backfill_posts"): - co.pop(k, None) - source.config_overrides = co - source.backfill_runs_remaining = BACKFILL_MAX_CHUNKS + arm_backfill(source) await self.session.commit() return await self._row_to_record(source) diff --git a/backend/app/tasks/admin.py b/backend/app/tasks/admin.py index 73ed1d7..571da4a 100644 --- a/backend/app/tasks/admin.py +++ b/backend/app/tasks/admin.py @@ -122,6 +122,28 @@ def prune_missing_file_records_task(self) -> dict: return {"checked": checked, "missing": len(missing_ids), "deleted": deleted} +@celery.task( + name="backend.app.tasks.admin.repair_discord_downloads_task", + bind=True, + autoretry_for=(OperationalError, DBAPIError), + retry_backoff=15, retry_backoff_max=180, max_retries=1, + soft_time_limit=1800, time_limit=2400, # 30 min / 40 min +) +def repair_discord_downloads_task(self, dry_run: bool = True) -> dict: + """Clean re-download of the Discord files broken by the `None` naming + (#3999). dry_run (the default) returns the projection; apply deletes the + broken images and their files, makes gallery-dl forget every Discord + download, and restarts every Discord source's backfill. Defaults to the SAFE + preview because the apply deletes files. Operator-triggered only.""" + from ..services.discord_repair import repair_discord_downloads + + SessionLocal = _sync_session_factory() + with SessionLocal() as session: + return repair_discord_downloads( + session, images_root=IMAGES_ROOT, dry_run=dry_run, + ) + + @celery.task( name="backend.app.tasks.admin.dedup_videos_task", bind=True, diff --git a/frontend/src/components/settings/DiscordRepairCard.vue b/frontend/src/components/settings/DiscordRepairCard.vue new file mode 100644 index 0000000..a17e916 --- /dev/null +++ b/frontend/src/components/settings/DiscordRepairCard.vue @@ -0,0 +1,118 @@ + + + diff --git a/frontend/src/components/settings/MaintenancePanel.vue b/frontend/src/components/settings/MaintenancePanel.vue index d0b3773..3931384 100644 --- a/frontend/src/components/settings/MaintenancePanel.vue +++ b/frontend/src/components/settings/MaintenancePanel.vue @@ -15,6 +15,7 @@ + @@ -85,6 +86,7 @@ import VideoEmbeddingCard from './VideoEmbeddingCard.vue' import CropProposersCard from './CropProposersCard.vue' import HeadsCard from './HeadsCard.vue' import DiscordGroupingCard from './DiscordGroupingCard.vue' +import DiscordRepairCard from './DiscordRepairCard.vue' import MembershipRosterCard from './MembershipRosterCard.vue' import MembershipSuggestionsCard from './MembershipSuggestionsCard.vue' import PostAssociationsCard from './PostAssociationsCard.vue' diff --git a/tests/test_discord_repair.py b/tests/test_discord_repair.py new file mode 100644 index 0000000..340d3a4 --- /dev/null +++ b/tests/test_discord_repair.py @@ -0,0 +1,170 @@ +"""The Discord repair (#3999): forget the broken downloads, delete them, backfill again. + +The danger in a repair like this is reach. It deletes files and clears +gallery-dl's memory of what it fetched, so most of these tests pin what it must +NOT touch: other platforms' archive entries, Discord server assets, images +outside a `discord/None/` folder, and a correctly named Discord file that +happens to share a folder name. +""" + +import sqlite3 + +import pytest +from sqlalchemy import func, select + +from backend.app.models import Artist, ImageRecord, Source +from backend.app.services import discord_repair +from backend.app.services.gallery_dl import archive_path +from backend.app.services.source_service import BACKFILL_MAX_CHUNKS + + +def _archive(tmp_path, entries): + path = archive_path(tmp_path) + path.parent.mkdir(parents=True, exist_ok=True) + conn = sqlite3.connect(path) + # The table gallery-dl itself creates (gallery_dl/archive.py). + conn.execute("CREATE TABLE archive (entry TEXT PRIMARY KEY) WITHOUT ROWID") + conn.executemany("INSERT INTO archive (entry) VALUES (?)", [(e,) for e in entries]) + conn.commit() + conn.close() + return path + + +def _entries(path): + conn = sqlite3.connect(path) + try: + return sorted(r[0] for r in conn.execute("SELECT entry FROM archive")) + finally: + conn.close() + + +# --- the archive ------------------------------------------------------------ + + +def test_only_discord_message_entries_are_forgotten(tmp_path): + path = _archive(tmp_path, [ + "discord300000000000000003_1", + "discord300000000000000004_2", + "discordasset_400000000000000004_9", + "hentaifoundry12345", + ]) + assert discord_repair.count_archive_entries(path) == 2 + assert discord_repair.forget_archive_entries(path) == 2 + assert _entries(path) == ["discordasset_400000000000000004_9", "hentaifoundry12345"] + + +def test_counting_changes_nothing(tmp_path): + path = _archive(tmp_path, ["discord1_1", "discord2_1"]) + discord_repair.count_archive_entries(path) + assert _entries(path) == ["discord1_1", "discord2_1"] + + +def test_a_missing_archive_or_table_is_zero_not_an_error(tmp_path): + assert discord_repair.count_archive_entries(tmp_path / "nope.sqlite3") == 0 + empty = tmp_path / "empty.sqlite3" + sqlite3.connect(empty).close() + assert discord_repair.forget_archive_entries(empty) == 0 + + +# --- the folders ------------------------------------------------------------ + + +def test_broken_directories_finds_every_artist_folder_including_case_twins(tmp_path): + for artist in ("Conto", "conto", "knuxy"): + (tmp_path / artist / "discord" / "None").mkdir(parents=True) + (tmp_path / "conto" / "discord" / "general").mkdir(parents=True) + (tmp_path / "conto" / "patreon" / "None").mkdir(parents=True) + found = [ + p.relative_to(tmp_path).as_posix() + for p in discord_repair.broken_directories(tmp_path) + ] + assert found == ["Conto/discord/None", "conto/discord/None", "knuxy/discord/None"] + + +def test_sweep_removes_leftover_sidecars_and_the_empty_folder(tmp_path): + d = tmp_path / "conto" / "discord" / "None" + d.mkdir(parents=True) + (d / "image.json").write_text("{}") + (d / "20240716_None_rejected.png").write_bytes(b"x") + removed, gone = discord_repair._sweep_directory(d) + assert (removed, gone) == (2, True) + assert not d.exists() + + +# --- the whole repair, against the database ----------------------------------- + + +def _image(db_sync, artist, path, sha): + img = ImageRecord( + artist_id=artist.id, path=str(path), sha256=sha * 64, size_bytes=100, + mime="image/png", origin="downloaded", + ) + db_sync.add(img) + return img + + +@pytest.mark.integration +def test_repair_deletes_only_broken_discord_images_and_rearms_discord_backfills( + db_sync, tmp_path, +): + artist = Artist(name="Conto", slug="conto") + db_sync.add(artist) + db_sync.flush() + discord = Source( + artist_id=artist.id, platform="discord", url="https://discord.com/channels/1/2", + enabled=True, config_overrides={"_backfill_state": "complete", "_backfill_cursor": "x"}, + ) + patreon = Source( + artist_id=artist.id, platform="patreon", url="https://www.patreon.com/conto", + enabled=True, config_overrides={}, + ) + db_sync.add_all([discord, patreon]) + + broken_dir = tmp_path / "conto" / "discord" / "None" + good_dir = tmp_path / "conto" / "discord" / "general" + patreon_dir = tmp_path / "conto" / "patreon" / "None" + for d in (broken_dir, good_dir, patreon_dir): + d.mkdir(parents=True) + broken = broken_dir / "20240716_None_image.png" + good = good_dir / "20240716_300000000000000003_01_image.png" + # A `None` folder under another platform, and a `_None_` name outside one. + # Neither is the bug's pair, so neither may be touched. + elsewhere = patreon_dir / "20240716_None_image.png" + for f in (broken, good, elsewhere): + f.write_bytes(b"x") + (broken_dir / "image.json").write_text("{}") + _image(db_sync, artist, broken, "a") + _image(db_sync, artist, good, "b") + _image(db_sync, artist, elsewhere, "c") + db_sync.commit() + _archive(tmp_path, ["discord300000000000000003_1", "hentaifoundry1"]) + + preview = discord_repair.repair_discord_downloads(db_sync, images_root=tmp_path, dry_run=True) + assert preview["images"] == 1 + assert preview["archive_entries"] == 1 + assert broken.exists() + + result = discord_repair.repair_discord_downloads(db_sync, images_root=tmp_path, dry_run=False) + assert result["images_deleted"] == 1 + assert result["remaining"] == 0 + assert result["backfills_started"] == 1 + + assert not broken.exists() and not broken_dir.exists() + assert good.exists() and elsewhere.exists() + assert _entries(archive_path(tmp_path)) == ["hentaifoundry1"] + + kept = db_sync.execute(select(func.count(ImageRecord.id))).scalar_one() + assert kept == 2 + + rows = { + platform: (runs, overrides) + for platform, runs, overrides in db_sync.execute( + select(Source.platform, Source.backfill_runs_remaining, Source.config_overrides) + ).all() + } + runs, overrides = rows["discord"] + assert runs == BACKFILL_MAX_CHUNKS + assert overrides["_backfill_state"] == "running" + assert "_backfill_cursor" not in overrides + # Only Discord sources are re-armed. + assert rows["patreon"][1] == {}