diff --git a/tests/test_refetch_service.py b/tests/test_refetch_service.py new file mode 100644 index 0000000..59811a7 --- /dev/null +++ b/tests/test_refetch_service.py @@ -0,0 +1,324 @@ +"""Layer-2 auto-refetch remediation — `services/refetch_service.py` (#3071). + +This module was the only one under `backend/app/services/` with no test +file, which matters more than a coverage gap normally would: it runs +UNATTENDED off the recovery sweep (`tasks/maintenance.py`, gated by +FC_AUTO_REFETCH_CORRUPT) and it DELETES a file from disk before asking a +downloader for a fresh copy. The frontend cites it by name as the reason +the Import tab could be retired at all (`stores/import.js`: imports +"heal themselves"). + +`test_api_import_admin.py` already drives the happy path end-to-end +through `POST /api/import/tasks//refetch` — file deleted, task +flagged, one dispatch, second attempt a no-op. What it CANNOT reach is +the branching inside `resolve_refetch_source`, and it never proves the +negative that actually protects the operator's data: that a file whose +source does NOT resolve is still on disk afterwards. Its `no_source` +case points at a path that never existed, so nothing survives to check. + +Those two things are what this module covers. +""" + +import json +from pathlib import Path + +import pytest +from sqlalchemy import select + +from backend.app.models import Artist, ImportBatch, ImportTask, Source +from backend.app.services.refetch_service import ( + attempt_refetch, + resolve_refetch_source, +) + +pytestmark = pytest.mark.integration + + +# --- fixtures / helpers ---------------------------------------------------- + +@pytest.fixture +def import_root(tmp_path): + root = tmp_path / "import" + root.mkdir() + return root + + +def _media(import_root: Path, artist_dir: str, name: str = "post.jpg") -> Path: + """A corrupt-import stand-in at import_root//.""" + d = import_root / artist_dir if artist_dir else import_root + d.mkdir(parents=True, exist_ok=True) + m = d / name + m.write_bytes(b"corrupt-bytes") + return m + + +def _sidecar(media: Path, payload) -> Path: + """gallery-dl writes `.json` beside the media file.""" + sc = media.with_suffix(".json") + sc.write_text(payload if isinstance(payload, str) else json.dumps(payload)) + return sc + + +def _artist(session, name: str) -> Artist: + a = Artist(name=name, slug=name.lower()) + session.add(a) + session.flush() + return a + + +def _source(session, artist, platform="patreon", url=None, enabled=True) -> Source: + s = Source( + artist_id=artist.id, + platform=platform, + url=url if url is not None else f"https://www.{platform}.com/{artist.slug}", + enabled=enabled, + config_overrides={}, + ) + session.add(s) + session.flush() + return s + + +def _task(session, media: Path, refetched: bool = False) -> ImportTask: + batch = ImportBatch( + triggered_by="manual", source_path=str(media.parent), scan_mode="quick", + ) + session.add(batch) + session.flush() + t = ImportTask( + batch_id=batch.id, source_path=str(media), task_type="media", + status="failed", refetched=refetched, + ) + session.add(t) + session.flush() + return t + + +@pytest.fixture +def no_dispatch(monkeypatch): + """Capture download_source.delay instead of queueing a real re-check. + + refetch_service imports the task lazily (inside attempt_refetch, to + dodge a tasks->services->tasks cycle), so patching the attribute on + the module is enough — the import resolves at call time. + """ + from backend.app.tasks import download as download_mod + + calls = [] + monkeypatch.setattr(download_mod.download_source, "delay", calls.append) + return calls + + +# --- resolve_refetch_source: what counts as re-pollable -------------------- + +def test_resolve_finds_enabled_source_matching_the_sidecar_platform(db_sync, import_root): + m = _media(import_root, "Alice") + _sidecar(m, {"category": "patreon", "post_id": 1}) + artist = _artist(db_sync, "Alice") + src = _source(db_sync, artist) + + assert resolve_refetch_source(db_sync, str(m), import_root).id == src.id + + +def test_resolve_skips_a_disabled_source(db_sync, import_root): + """A disabled Source is not re-pollable: the operator turned it off, + and a sweep must not reach past that to delete their file.""" + m = _media(import_root, "Alice") + _sidecar(m, {"category": "patreon"}) + _source(db_sync, _artist(db_sync, "Alice"), enabled=False) + + assert resolve_refetch_source(db_sync, str(m), import_root) is None + + +def test_resolve_rejects_a_synthetic_sidecar_anchor_url(db_sync, import_root): + """`sidecar::` is a bookkeeping anchor for files that + arrived on disk, not a feed. Re-polling it is impossible, so it must + not qualify — otherwise the file is deleted for a fetch that can + never happen.""" + m = _media(import_root, "Alice") + _sidecar(m, {"category": "patreon"}) + _source(db_sync, _artist(db_sync, "Alice"), url="sidecar:patreon:alice") + + assert resolve_refetch_source(db_sync, str(m), import_root) is None + + +def test_resolve_needs_a_source_on_the_sidecars_own_platform(db_sync, import_root): + m = _media(import_root, "Alice") + _sidecar(m, {"category": "patreon"}) + _source(db_sync, _artist(db_sync, "Alice"), platform="pixiv") + + assert resolve_refetch_source(db_sync, str(m), import_root) is None + + +def test_resolve_picks_the_lowest_id_when_several_sources_qualify(db_sync, import_root): + m = _media(import_root, "Alice") + _sidecar(m, {"category": "patreon"}) + artist = _artist(db_sync, "Alice") + first = _source(db_sync, artist, url="https://www.patreon.com/alice-one") + _source(db_sync, artist, url="https://www.patreon.com/alice-two") + + # Deterministic choice, not "whichever the planner returned first" — + # the pick decides which downloader runs. + assert resolve_refetch_source(db_sync, str(m), import_root).id == first.id + + +def test_resolve_reads_the_gallery_dl_numbered_sidecar(db_sync, import_root): + """gallery-dl prefixes media with `NN_` for in-post ordering but + writes the sidecar under the UNPREFIXED stem. Refetch resolves real + downloaded files, so it has to follow that convention.""" + m = _media(import_root, "Alice", name="01_post.jpg") + (import_root / "Alice" / "post.json").write_text(json.dumps({"category": "patreon"})) + src = _source(db_sync, _artist(db_sync, "Alice")) + + assert resolve_refetch_source(db_sync, str(m), import_root).id == src.id + + +# --- resolve_refetch_source: every way it declines ------------------------- + +def test_resolve_declines_without_a_sidecar(db_sync, import_root): + m = _media(import_root, "Alice") + _source(db_sync, _artist(db_sync, "Alice")) + + assert resolve_refetch_source(db_sync, str(m), import_root) is None + + +def test_resolve_declines_on_unreadable_sidecar_json(db_sync, import_root): + m = _media(import_root, "Alice") + _sidecar(m, "{not valid json") + _source(db_sync, _artist(db_sync, "Alice")) + + assert resolve_refetch_source(db_sync, str(m), import_root) is None + + +def test_resolve_declines_when_the_sidecar_is_not_an_object(db_sync, import_root): + # A bare JSON list parses fine but has no `category` to read. + m = _media(import_root, "Alice") + _sidecar(m, ["patreon"]) + _source(db_sync, _artist(db_sync, "Alice")) + + assert resolve_refetch_source(db_sync, str(m), import_root) is None + + +def test_resolve_declines_when_the_sidecar_names_no_platform(db_sync, import_root): + m = _media(import_root, "Alice") + _sidecar(m, {"post_id": 1}) + _source(db_sync, _artist(db_sync, "Alice")) + + assert resolve_refetch_source(db_sync, str(m), import_root) is None + + +def test_resolve_declines_for_a_file_sitting_directly_in_import_root(db_sync, import_root): + """No artist folder means no artist bucket to resolve — the + filesystem-only drop case.""" + m = _media(import_root, "") + _sidecar(m, {"category": "patreon"}) + _source(db_sync, _artist(db_sync, "Alice")) + + assert resolve_refetch_source(db_sync, str(m), import_root) is None + + +def test_resolve_declines_when_no_artist_row_matches_the_folder(db_sync, import_root): + m = _media(import_root, "Nobody") + _sidecar(m, {"category": "patreon"}) + _source(db_sync, _artist(db_sync, "Alice")) + + assert resolve_refetch_source(db_sync, str(m), import_root) is None + + +# --- attempt_refetch: the destructive half --------------------------------- + +def test_attempt_refetch_deletes_the_file_and_queues_one_recheck( + db_sync, import_root, no_dispatch, +): + m = _media(import_root, "Alice") + _sidecar(m, {"category": "patreon"}) + src = _source(db_sync, _artist(db_sync, "Alice")) + task = _task(db_sync, m) + + result = attempt_refetch(db_sync, task, import_root) + + assert result == {"status": "refetch_queued", "source_id": src.id} + assert not m.exists() # the bad copy is gone... + assert no_dispatch == [src.id] # ...and exactly one re-check was queued + assert task.refetched is True + + +def test_attempt_refetch_leaves_the_file_alone_when_nothing_resolves( + db_sync, import_root, no_dispatch, +): + """THE assertion this module exists for. `no_source` is the common + case on a filesystem-only library, and the file on disk is then the + operator's ONLY copy — deleting it without a downloader that can + replace it destroys the thing the remediation was meant to repair. + + The route-level `no_source` test cannot catch a regression here: its + path never existed, so an unconditional unlink would pass it. + """ + m = _media(import_root, "Alice") + _sidecar(m, {"category": "patreon"}) + _source(db_sync, _artist(db_sync, "Alice"), enabled=False) + task = _task(db_sync, m) + + assert attempt_refetch(db_sync, task, import_root) == {"status": "no_source"} + assert m.exists() + assert m.read_bytes() == b"corrupt-bytes" + assert no_dispatch == [] + assert task.refetched is False # not consumed — a real fix can still run + + +def test_attempt_refetch_is_bounded_to_a_single_attempt( + db_sync, import_root, no_dispatch, +): + """The `refetched` bound is what stops SOURCE-side corruption from + looping: re-downloading a file that is broken upstream returns the + same bytes forever. The check must come FIRST — a second call has to + leave the (re-downloaded) file untouched, not delete it again. + """ + m = _media(import_root, "Alice") + _sidecar(m, {"category": "patreon"}) + _source(db_sync, _artist(db_sync, "Alice")) + task = _task(db_sync, m, refetched=True) + + assert attempt_refetch(db_sync, task, import_root) == {"status": "already_refetched"} + assert m.exists() + assert no_dispatch == [] + + +def test_attempt_refetch_proceeds_when_the_file_is_already_gone( + db_sync, import_root, no_dispatch, +): + """`missing_ok=True`: the sweep races an operator who deleted the bad + file by hand. The re-check is still the right next move.""" + m = _media(import_root, "Alice") + _sidecar(m, {"category": "patreon"}) + src = _source(db_sync, _artist(db_sync, "Alice")) + task = _task(db_sync, m) + m.unlink() + + assert attempt_refetch(db_sync, task, import_root)["status"] == "refetch_queued" + assert no_dispatch == [src.id] + + +def test_attempt_refetch_survives_an_unlink_failure( + db_sync, import_root, no_dispatch, +): + """An unremovable path is logged and stepped over, not raised — this + runs unattended, and a raise would abort the whole recovery sweep for + every OTHER poison-pill row in the batch. + + A directory standing where the media file should be produces a + genuine IsADirectoryError (an OSError) without patching pathlib, so + the handler is exercised rather than simulated. + """ + d = import_root / "Alice" / "post.jpg" + d.mkdir(parents=True) + (import_root / "Alice" / "post.json").write_text(json.dumps({"category": "patreon"})) + src = _source(db_sync, _artist(db_sync, "Alice")) + task = _task(db_sync, d) + + assert attempt_refetch(db_sync, task, import_root)["status"] == "refetch_queued" + assert d.exists() # removal genuinely failed... + assert no_dispatch == [src.id] # ...and the sweep carried on anyway + assert db_sync.execute( + select(ImportTask.refetched).where(ImportTask.id == task.id) + ).scalar_one() is True