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/<id>/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/<artist_dir>/<name>."""
+    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 `<stem>.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:<platform>:<slug>` 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