Files
FabledCurator/tests/test_refetch_service.py
T
bvandeusenandClaude Opus 5 89155478a8
CI / extension-version (push) Successful in 2s
CI / lint (push) Successful in 2s
CI / frontend-build (push) Successful in 24s
CI / backend-lint-and-test (push) Successful in 28s
CI / integration (push) Successful in 3m59s
test(refetch): cover the Layer-2 auto-refetch remediation (#3071)
refetch_service was the only module under backend/app/services/ with no
test file — and not an inert one: it runs unattended off the recovery
sweep and deletes a file from disk before asking a downloader to replace
it. The frontend cites it by name as the reason the Import tab could be
retired ("imports heal themselves").

The ticket described it as having zero direct coverage. That is true of
the module, but not of the code: test_api_import_admin.py already drives
the happy path end-to-end through the refetch route — file deleted, task
flagged, one dispatch, second attempt a no-op. These 17 tests therefore
target what the route tests cannot reach rather than restating them:

  * every branch of resolve_refetch_source — disabled Source, a
    `sidecar:<platform>:<slug>` synthetic anchor, a platform mismatch,
    the gallery-dl `NN_` numbering-prefix sidecar, the lowest-id pick
    among several candidates, and each of the five ways it declines
    (no sidecar, unreadable JSON, non-object JSON, no platform, no
    artist folder / no matching Artist row).

  * that the file SURVIVES when nothing re-pollable resolves. This is
    the assertion the module exists for: `no_source` is the common case
    on a filesystem-only library, where the file on disk is the
    operator's only copy. The route-level no_source test cannot catch a
    regression here — its path never existed, so an unconditional unlink
    would pass it.

  * that the `refetched` bound is checked BEFORE the unlink, so a second
    sweep leaves the re-downloaded file alone rather than deleting it
    again.

  * that an unlink failure is logged and stepped over, not raised —
    a raise would abort the whole sweep for every other poison-pill row
    in the batch. Exercised with a real IsADirectoryError rather than a
    patched pathlib.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-27 07:33:14 -04:00

325 lines
12 KiB
Python

"""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