fix: an unfinished download is never stored as an attachment, and the temp sweep keeps any file a library row points at
CI and images / lint (push) Successful in 2s
CI and images / extension-version (push) Successful in 3s
CI and images / extension-test (push) Successful in 24s
CI and images / frontend-build (push) Successful in 26s
CI and images / backend-lint-and-test (push) Successful in 35s
CI and images / integration (push) Successful in 2m23s
CI and images / sign-extension (push) Successful in 2s
CI and images / build-agent (push) Successful in 5s
CI and images / build-web (push) Successful in 1m59s
CI and images / smoke-web (push) Successful in 54s
CI and images / promote (push) Successful in 1s
CI and images / refresh-dev (push) Skipped
CI and images / unattended-failure (push) Skipped
CI and images / lint (push) Successful in 2s
CI and images / extension-version (push) Successful in 3s
CI and images / extension-test (push) Successful in 24s
CI and images / frontend-build (push) Successful in 26s
CI and images / backend-lint-and-test (push) Successful in 35s
CI and images / integration (push) Successful in 2m23s
CI and images / sign-extension (push) Successful in 2s
CI and images / build-agent (push) Successful in 5s
CI and images / build-web (push) Successful in 1m59s
CI and images / smoke-web (push) Successful in 54s
CI and images / promote (push) Successful in 1s
CI and images / refresh-dev (push) Skipped
CI and images / unattended-failure (push) Skipped
The importer refuses a staging file (.part, .partial, .megatmp.*) at both entry points, the same way it refuses a sidecar .json, and discard_after_attach never deletes one: the age-gated temp sweep owns them. The sweep checks its stale candidates against image and attachment rows and keeps any a row points at. A Google Drive fetch's half-written file had been stored as <sha>.part; the sweep then deleted it by name and left the row with no file. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -43,6 +43,7 @@ from ..utils.paths import (
|
|||||||
from ..utils.phash import compute_phash, find_similar, fingerprint_path, fingerprints_match
|
from ..utils.phash import compute_phash, find_similar, fingerprint_path, fingerprints_match
|
||||||
from ..utils.sidecar import find_sidecar, parse_sidecar
|
from ..utils.sidecar import find_sidecar, parse_sidecar
|
||||||
from ..utils.slug import slugify
|
from ..utils.slug import slugify
|
||||||
|
from ..utils.staging import is_temp_staging_file
|
||||||
from .archive_extractor import extract_archive, is_archive
|
from .archive_extractor import extract_archive, is_archive
|
||||||
from .attachment_store import AttachmentStore
|
from .attachment_store import AttachmentStore
|
||||||
from .audits import single_color
|
from .audits import single_color
|
||||||
@@ -115,6 +116,21 @@ def is_video(path: Path) -> bool:
|
|||||||
return path.suffix.lower() in VIDEO_EXTS
|
return path.suffix.lower() in VIDEO_EXTS
|
||||||
|
|
||||||
|
|
||||||
|
def _not_content(path: Path) -> ImportResult | None:
|
||||||
|
"""The skip result for a file that is never library content: a sidecar
|
||||||
|
`.json` (metadata) or a downloader's staging file (an unfinished download,
|
||||||
|
which becomes content only under its final name). None for anything else."""
|
||||||
|
if path.suffix.lower() == ".json":
|
||||||
|
error = "sidecar json is metadata, not content"
|
||||||
|
elif is_temp_staging_file(path):
|
||||||
|
error = "unfinished download (staging file), not content"
|
||||||
|
else:
|
||||||
|
return None
|
||||||
|
return ImportResult(
|
||||||
|
status="skipped", skip_reason=SkipReason.invalid_image, error=error,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def _safe_ext(path: Path) -> str:
|
def _safe_ext(path: Path) -> str:
|
||||||
"""Conservatively extract a file extension for PostAttachment.ext
|
"""Conservatively extract a file extension for PostAttachment.ext
|
||||||
(varchar(32)). Thin wrapper over the shared `utils.paths.safe_ext` (kept for
|
(varchar(32)). Thin wrapper over the shared `utils.paths.safe_ext` (kept for
|
||||||
@@ -410,11 +426,8 @@ class Importer:
|
|||||||
media members (one Post via the archive-adjacent sidecar) and
|
media members (one Post via the archive-adjacent sidecar) and
|
||||||
preserve the archive itself. Other non-media → PostAttachment.
|
preserve the archive itself. Other non-media → PostAttachment.
|
||||||
Nothing is skipped for being non-media (FC-2d-iii)."""
|
Nothing is skipped for being non-media (FC-2d-iii)."""
|
||||||
if source.suffix.lower() == ".json":
|
if (refused := _not_content(source)) is not None:
|
||||||
return ImportResult(
|
return refused
|
||||||
status="skipped", skip_reason=SkipReason.invalid_image,
|
|
||||||
error="sidecar json is metadata, not content",
|
|
||||||
)
|
|
||||||
if is_archive(source):
|
if is_archive(source):
|
||||||
return self._import_archive(source)
|
return self._import_archive(source)
|
||||||
if not is_supported(source):
|
if not is_supported(source):
|
||||||
@@ -1219,11 +1232,8 @@ class Importer:
|
|||||||
caller may delete the on-disk original (mirrors duplicate_hash)
|
caller may delete the on-disk original (mirrors duplicate_hash)
|
||||||
- failed → file untouched; caller decides
|
- failed → file untouched; caller decides
|
||||||
"""
|
"""
|
||||||
if path.suffix.lower() == ".json":
|
if (refused := _not_content(path)) is not None:
|
||||||
return ImportResult(
|
return refused
|
||||||
status="skipped", skip_reason=SkipReason.invalid_image,
|
|
||||||
error="sidecar json is metadata, not content",
|
|
||||||
)
|
|
||||||
if is_archive(path):
|
if is_archive(path):
|
||||||
return self._import_archive(
|
return self._import_archive(
|
||||||
path, artist=artist, source_row=source,
|
path, artist=artist, source_row=source,
|
||||||
@@ -1420,7 +1430,11 @@ class Importer:
|
|||||||
|
|
||||||
An imported or superseded media file is kept: it IS the record's file.
|
An imported or superseded media file is kept: it IS the record's file.
|
||||||
So is any path an ImageRecord points at, whatever the status, because a
|
So is any path an ImageRecord points at, whatever the status, because a
|
||||||
re-fetch can write identical bytes over a record's own file."""
|
re-fetch can write identical bytes over a record's own file. A staging
|
||||||
|
file is kept too: it may be a download still being written, and the
|
||||||
|
temp-file sweep removes it once it is old enough to be abandoned."""
|
||||||
|
if is_temp_staging_file(path):
|
||||||
|
return False
|
||||||
if result.status in ("imported", "superseded"):
|
if result.status in ("imported", "superseded"):
|
||||||
redundant = is_archive(path)
|
redundant = is_archive(path)
|
||||||
else:
|
else:
|
||||||
|
|||||||
@@ -20,6 +20,7 @@ from ..models import (
|
|||||||
ImportSettings,
|
ImportSettings,
|
||||||
ImportTask,
|
ImportTask,
|
||||||
LibraryAuditRun,
|
LibraryAuditRun,
|
||||||
|
PostAttachment,
|
||||||
Source,
|
Source,
|
||||||
TaskRun,
|
TaskRun,
|
||||||
)
|
)
|
||||||
@@ -372,23 +373,58 @@ def cleanup_old_tasks() -> int:
|
|||||||
return result.rowcount or 0
|
return result.rowcount or 0
|
||||||
|
|
||||||
|
|
||||||
|
def _paths_with_rows(paths: list[str]) -> set[str]:
|
||||||
|
"""The subset of `paths` that an image (its file or thumbnail) or an
|
||||||
|
attachment row points at."""
|
||||||
|
owned: set[str] = set()
|
||||||
|
SessionLocal = _sync_session_factory()
|
||||||
|
with SessionLocal() as session:
|
||||||
|
for i in range(0, len(paths), 500):
|
||||||
|
chunk = paths[i:i + 500]
|
||||||
|
owned.update(session.execute(
|
||||||
|
select(ImageRecord.path).where(ImageRecord.path.in_(chunk))
|
||||||
|
).scalars())
|
||||||
|
owned.update(session.execute(
|
||||||
|
select(ImageRecord.thumbnail_path).where(ImageRecord.thumbnail_path.in_(chunk))
|
||||||
|
).scalars())
|
||||||
|
owned.update(session.execute(
|
||||||
|
select(PostAttachment.path).where(PostAttachment.path.in_(chunk))
|
||||||
|
).scalars())
|
||||||
|
return owned
|
||||||
|
|
||||||
|
|
||||||
@celery.task(name="backend.app.tasks.maintenance.cleanup_orphaned_temp_files")
|
@celery.task(name="backend.app.tasks.maintenance.cleanup_orphaned_temp_files")
|
||||||
def cleanup_orphaned_temp_files() -> int:
|
def cleanup_orphaned_temp_files() -> int:
|
||||||
"""Delete orphaned staging files (.part, .partial, megatools' .megatmp.*)
|
"""Delete orphaned staging files (.part, .partial, megatools' .megatmp.*)
|
||||||
under the images root, left by a download/import killed mid-write. Only
|
under the images root, left by a download/import killed mid-write. Only
|
||||||
removes files older than ORPHAN_TEMP_MIN_AGE_HOURS so an in-flight
|
removes files older than ORPHAN_TEMP_MIN_AGE_HOURS so an in-flight
|
||||||
download's staging file is never pulled out from under it. Returns the
|
download's staging file is never pulled out from under it, and never a
|
||||||
count removed."""
|
file an image or attachment row points at, whatever its name: that file
|
||||||
|
is library content. Returns the count removed."""
|
||||||
if not IMAGES_ROOT.is_dir():
|
if not IMAGES_ROOT.is_dir():
|
||||||
return 0
|
return 0
|
||||||
cutoff = datetime.now(UTC).timestamp() - ORPHAN_TEMP_MIN_AGE_HOURS * 3600
|
cutoff = datetime.now(UTC).timestamp() - ORPHAN_TEMP_MIN_AGE_HOURS * 3600
|
||||||
removed = 0
|
stale: list[Path] = []
|
||||||
for path in IMAGES_ROOT.rglob("*"):
|
for path in IMAGES_ROOT.rglob("*"):
|
||||||
if not is_temp_staging_file(path) or not path.is_file():
|
if not is_temp_staging_file(path) or not path.is_file():
|
||||||
continue
|
continue
|
||||||
try:
|
try:
|
||||||
if path.stat().st_mtime >= cutoff:
|
if path.stat().st_mtime < cutoff: # fresher may be an active download
|
||||||
continue # too fresh — may be an active download
|
stale.append(path)
|
||||||
|
except OSError as exc:
|
||||||
|
log.warning("cleanup_orphaned_temp_files: %s: %s", path, exc)
|
||||||
|
if not stale:
|
||||||
|
return 0
|
||||||
|
owned = _paths_with_rows([str(p) for p in stale])
|
||||||
|
removed = 0
|
||||||
|
for path in stale:
|
||||||
|
if str(path) in owned:
|
||||||
|
log.warning(
|
||||||
|
"cleanup_orphaned_temp_files: keeping %s: a library row points at it",
|
||||||
|
path,
|
||||||
|
)
|
||||||
|
continue
|
||||||
|
try:
|
||||||
path.unlink()
|
path.unlink()
|
||||||
removed += 1
|
removed += 1
|
||||||
except OSError as exc:
|
except OSError as exc:
|
||||||
|
|||||||
@@ -410,6 +410,31 @@ def test_attach_in_place_invalid_image_returns_skipped(importer):
|
|||||||
assert result.skip_reason.value == "invalid_image"
|
assert result.skip_reason.value == "invalid_image"
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("name", ["film.mp4abc123_.part", "img.jpg.partial", ".megatmp.Xy12"])
|
||||||
|
def test_an_unfinished_download_is_refused_and_left_on_disk(importer, db_sync, name):
|
||||||
|
"""A staging file is a download still being written (or abandoned): it is
|
||||||
|
never stored as an attachment, and the importer never deletes it, so a
|
||||||
|
download in progress is not pulled out from under its writer."""
|
||||||
|
from backend.app.models import PostAttachment
|
||||||
|
|
||||||
|
artist = Artist(name="Hal", slug="hal")
|
||||||
|
db_sync.add(artist)
|
||||||
|
db_sync.flush()
|
||||||
|
staging = importer.images_root / "hal" / "patreon" / "post" / name
|
||||||
|
staging.parent.mkdir(parents=True, exist_ok=True)
|
||||||
|
staging.write_bytes(b"half of a video")
|
||||||
|
|
||||||
|
for result in (importer.attach_in_place(staging, artist=artist),
|
||||||
|
importer.import_one(staging)):
|
||||||
|
assert result.status == "skipped"
|
||||||
|
assert "unfinished download" in result.error
|
||||||
|
assert importer.discard_after_attach(staging, result) is False
|
||||||
|
assert staging.exists()
|
||||||
|
assert db_sync.execute(
|
||||||
|
select(func.count()).where(PostAttachment.original_filename == name)
|
||||||
|
).scalar() == 0
|
||||||
|
|
||||||
|
|
||||||
def test_attach_in_place_non_media_routes_to_attachment(importer, db_sync):
|
def test_attach_in_place_non_media_routes_to_attachment(importer, db_sync):
|
||||||
"""FC-2d-iii dispatch parity for the download path. A non-media file
|
"""FC-2d-iii dispatch parity for the download path. A non-media file
|
||||||
(.pdf, .txt etc.) downloaded by gallery-dl must become a PostAttachment,
|
(.pdf, .txt etc.) downloaded by gallery-dl must become a PostAttachment,
|
||||||
|
|||||||
+30
-10
@@ -26,30 +26,50 @@ def _make_batch(session) -> int:
|
|||||||
return batch.id
|
return batch.id
|
||||||
|
|
||||||
|
|
||||||
def test_cleanup_orphaned_temp_files_removes_stale_only(tmp_path, monkeypatch):
|
def test_cleanup_orphaned_temp_files_removes_stale_only(db_sync, tmp_path, monkeypatch):
|
||||||
import os
|
import os
|
||||||
|
|
||||||
|
from backend.app.models import ImageRecord, PostAttachment
|
||||||
from backend.app.tasks import maintenance as m
|
from backend.app.tasks import maintenance as m
|
||||||
|
|
||||||
monkeypatch.setattr(m, "IMAGES_ROOT", tmp_path)
|
monkeypatch.setattr(m, "IMAGES_ROOT", tmp_path)
|
||||||
stale = tmp_path / "artist" / "img.jpg.part" # killed download → orphan
|
|
||||||
stale.parent.mkdir(parents=True)
|
|
||||||
stale.write_bytes(b"x")
|
|
||||||
old = datetime.now(UTC).timestamp() - 8 * 3600 # older than the 6h guard
|
old = datetime.now(UTC).timestamp() - 8 * 3600 # older than the 6h guard
|
||||||
os.utime(stale, (old, old))
|
|
||||||
fresh = tmp_path / "in_progress.jpg.partial" # active download → keep
|
def staged(rel: str, *, aged: bool = True):
|
||||||
fresh.write_bytes(b"x")
|
p = tmp_path / rel
|
||||||
|
p.parent.mkdir(parents=True, exist_ok=True)
|
||||||
|
p.write_bytes(b"x")
|
||||||
|
if aged:
|
||||||
|
os.utime(p, (old, old))
|
||||||
|
return p
|
||||||
|
|
||||||
|
stale = staged("artist/img.jpg.part") # killed download → orphan
|
||||||
|
mega = staged("artist/.megatmp.AbCd123") # abandoned megatools stage
|
||||||
|
fresh = staged("in_progress.jpg.partial", aged=False) # active download → keep
|
||||||
keep = tmp_path / "real.jpg" # a real image → keep
|
keep = tmp_path / "real.jpg" # a real image → keep
|
||||||
keep.write_bytes(b"x")
|
keep.write_bytes(b"x")
|
||||||
mega = tmp_path / "artist" / ".megatmp.AbCd123" # abandoned megatools stage
|
# Staging-named files a library row points at are library content.
|
||||||
mega.write_bytes(b"x")
|
attached = staged("attachments/ab/abc.part")
|
||||||
os.utime(mega, (old, old))
|
imaged = staged("artist/video.mp4.part")
|
||||||
|
db_sync.add(PostAttachment(
|
||||||
|
post_id=None, artist_id=None, sha256="tempsweep-att".ljust(64, "x"),
|
||||||
|
path=str(attached), original_filename="film.mp4.part", ext=".part",
|
||||||
|
size_bytes=1,
|
||||||
|
))
|
||||||
|
db_sync.add(ImageRecord(
|
||||||
|
path=str(imaged), sha256="tempsweep-img".ljust(64, "x"), size_bytes=1,
|
||||||
|
mime="video/mp4", width=1, height=1, origin="downloaded",
|
||||||
|
integrity_status="ok",
|
||||||
|
))
|
||||||
|
db_sync.commit()
|
||||||
|
|
||||||
assert m.cleanup_orphaned_temp_files() == 2
|
assert m.cleanup_orphaned_temp_files() == 2
|
||||||
assert not stale.exists()
|
assert not stale.exists()
|
||||||
assert not mega.exists()
|
assert not mega.exists()
|
||||||
assert fresh.exists()
|
assert fresh.exists()
|
||||||
assert keep.exists()
|
assert keep.exists()
|
||||||
|
assert attached.exists()
|
||||||
|
assert imaged.exists()
|
||||||
|
|
||||||
|
|
||||||
def test_recover_interrupted_only_old(db_sync, monkeypatch):
|
def test_recover_interrupted_only_old(db_sync, monkeypatch):
|
||||||
|
|||||||
Reference in New Issue
Block a user