feat(import): recurse nested archives + precise "no images" reason (#718)
Root cause (operator-confirmed via event metadata + lsar): a "High Resolution files" pack often wraps a per-chapter .rar/.zip INSIDE one outer archive (incase). _import_archive only extracted one level — a nested-archive member failed is_supported and was skipped, so the real pages were silently dropped and the post showed "archive but no images". The disk scan found this pattern recurring across the attachment store. - Recurse into nested archives via _collect_archive_members: a member that is itself an archive is bomb-probed and extracted too, depth-capped at _ARCHIVE_MAX_DEPTH=3. Nested members attribute to the OUTER archive's sidecar so they link to the right Post. Each level is wrapped so one bad nested archive can't abort the import. The shared path means external (mega/gdrive) archives recurse too. - Replace the catch-all "held no supported members" string with a per-outcome tally (media/deduped/unsupported/failed/nested/nested_rejected). The all-deduped case is now recognised as BENIGN — images already in the library, re-linked to this post via enrich-on-duplicate — and returns attached WITHOUT error, so it no longer false-flags in event metadata.unextracted_archives. Genuine failures carry the precise breakdown. Tests: nested zip-in-cbz imports both inner images + links them to the outer post; all-deduped archive returns attached with error=None and links images to both posts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -79,6 +79,12 @@ IMAGE_EXTS = {".png", ".jpg", ".jpeg", ".gif", ".webp", ".bmp", ".tif", ".tiff"}
|
||||
VIDEO_EXTS = {".mp4", ".mov", ".avi", ".mkv", ".webm", ".m4v", ".wmv", ".flv"}
|
||||
ALL_EXTS = IMAGE_EXTS | VIDEO_EXTS
|
||||
|
||||
# A "high-resolution files" pack often wraps per-chapter .rar/.zip inside one
|
||||
# outer archive (incase #718) — recurse into nested archives so their images
|
||||
# land instead of being silently dropped. Cap the depth so a maliciously
|
||||
# deep-nested archive can't recurse forever; the bomb-probe re-runs per level.
|
||||
_ARCHIVE_MAX_DEPTH = 3
|
||||
|
||||
|
||||
def is_supported(path: Path) -> bool:
|
||||
return path.suffix.lower() in ALL_EXTS
|
||||
@@ -491,17 +497,17 @@ class Importer:
|
||||
artist_use = artist if artist is not None else self._resolve_artist(source)
|
||||
post = self._post_for_sidecar(source, artist_use)
|
||||
member_ids: list[int] = []
|
||||
member_total = 0
|
||||
with extract_archive(source) as members:
|
||||
for _name, member_path in members:
|
||||
member_total += 1
|
||||
if not is_supported(member_path):
|
||||
continue # non-media preserved via the stored archive
|
||||
res = self._import_media(
|
||||
member_path, source, explicit_source=source_row,
|
||||
)
|
||||
if res.status in ("imported", "superseded") and res.image_id:
|
||||
member_ids.append(res.image_id)
|
||||
# Per-outcome tally so the "no images" reason names the ACTUAL cause
|
||||
# (#718): nested-archive packs, all-deduped (benign), unsupported formats,
|
||||
# or failed/corrupt members — instead of one catch-all string.
|
||||
counts = {
|
||||
"media": 0, "deduped": 0, "unsupported": 0,
|
||||
"failed": 0, "nested": 0, "nested_rejected": 0,
|
||||
}
|
||||
self._collect_archive_members(
|
||||
source, attribution=source, source_row=source_row,
|
||||
depth=0, member_ids=member_ids, counts=counts,
|
||||
)
|
||||
# Preserve the archive itself (links to the same Post/Artist).
|
||||
self._capture_attachment(
|
||||
source, post=post, artist=artist_use, resolved=True
|
||||
@@ -511,20 +517,93 @@ class Importer:
|
||||
status="imported", image_id=member_ids[0],
|
||||
member_image_ids=member_ids,
|
||||
)
|
||||
# No images landed — surface WHY so a post showing "no images" beside an
|
||||
# archive is diagnosable instead of silent. Zero members usually means
|
||||
# the extractor backend is missing/failed (unar for rar, py7zr for 7z)
|
||||
# or the file is corrupt; non-zero-but-no-images means it held only
|
||||
# non-media files.
|
||||
reason = (
|
||||
"archive extracted but held no supported image/video members"
|
||||
if member_total
|
||||
else "archive yielded no members (unsupported/corrupt, or the "
|
||||
"extractor backend failed)"
|
||||
)
|
||||
# No NEW images landed. Name WHY precisely so a post showing "no images"
|
||||
# beside an archive is diagnosable — and DON'T flag the benign all-deduped
|
||||
# case as a problem: those images already exist in the library and were
|
||||
# re-linked to this post (enrich-on-duplicate), so the post DOES show them.
|
||||
if counts["media"] == 0 and counts["nested"] == 0:
|
||||
reason = ("archive yielded no members (unsupported/corrupt, or the "
|
||||
"extractor backend failed)")
|
||||
elif counts["deduped"] and not (
|
||||
counts["unsupported"] or counts["failed"] or counts["nested_rejected"]
|
||||
):
|
||||
log.info(
|
||||
"%s: all %d image member(s) already in library (deduped + linked "
|
||||
"to this post) — archive preserved, no new import",
|
||||
source.name, counts["deduped"],
|
||||
)
|
||||
return ImportResult(status="attached")
|
||||
else:
|
||||
reason = (
|
||||
f"no new images — media={counts['media']} deduped={counts['deduped']} "
|
||||
f"unsupported/non-media={counts['unsupported']} failed={counts['failed']} "
|
||||
f"nested={counts['nested']} nested-rejected={counts['nested_rejected']}"
|
||||
)
|
||||
log.warning("%s: %s", source.name, reason)
|
||||
return ImportResult(status="attached", error=reason)
|
||||
|
||||
def _collect_archive_members(
|
||||
self, archive_path: Path, *, attribution: Path,
|
||||
source_row: Source | None, depth: int,
|
||||
member_ids: list[int], counts: dict,
|
||||
) -> None:
|
||||
"""Extract `archive_path` and import its image/video members, RECURSING
|
||||
into nested archives (#718). Members attribute to `attribution` — the
|
||||
OUTER archive's path, so its sidecar resolves them to the right Post even
|
||||
when they came from a nested archive. Depth-capped + bomb-probed per
|
||||
nested level. Mutates `member_ids` and `counts` in place.
|
||||
|
||||
extract_archive is fail-soft, but each level is wrapped so one bad nested
|
||||
archive can't abort the whole import."""
|
||||
try:
|
||||
with extract_archive(archive_path) as members:
|
||||
for _name, member_path in members:
|
||||
if not member_path.is_file():
|
||||
continue # directory entries etc. — not media
|
||||
if is_archive(member_path):
|
||||
counts["nested"] += 1
|
||||
if depth + 1 > _ARCHIVE_MAX_DEPTH:
|
||||
counts["nested_rejected"] += 1
|
||||
log.warning(
|
||||
"nested archive past depth cap %d, skipped: %s",
|
||||
_ARCHIVE_MAX_DEPTH, member_path.name,
|
||||
)
|
||||
continue
|
||||
probe = safe_probe.probe_archive(member_path)
|
||||
if not probe.ok:
|
||||
counts["nested_rejected"] += 1
|
||||
log.warning(
|
||||
"nested archive rejected (%s): %s",
|
||||
probe.reason, member_path.name,
|
||||
)
|
||||
continue
|
||||
self._collect_archive_members(
|
||||
member_path, attribution=attribution,
|
||||
source_row=source_row, depth=depth + 1,
|
||||
member_ids=member_ids, counts=counts,
|
||||
)
|
||||
continue
|
||||
counts["media"] += 1
|
||||
if not is_supported(member_path):
|
||||
counts["unsupported"] += 1
|
||||
continue # non-media preserved via the stored archive
|
||||
res = self._import_media(
|
||||
member_path, attribution, explicit_source=source_row,
|
||||
)
|
||||
if res.status in ("imported", "superseded") and res.image_id:
|
||||
member_ids.append(res.image_id)
|
||||
elif res.status == "skipped" and res.skip_reason in (
|
||||
SkipReason.duplicate_hash, SkipReason.duplicate_phash
|
||||
):
|
||||
counts["deduped"] += 1
|
||||
else:
|
||||
counts["failed"] += 1
|
||||
except Exception as exc: # noqa: BLE001 — defensive per level; keep going
|
||||
log.warning(
|
||||
"archive extraction failed for %s (depth %d): %s",
|
||||
archive_path.name, depth, exc,
|
||||
)
|
||||
|
||||
def _import_media(
|
||||
self, source: Path, attribution_path: Path,
|
||||
*, explicit_source: Source | None = None,
|
||||
|
||||
@@ -93,6 +93,77 @@ def test_archive_imports_members_and_stores_archive(importer, import_layout):
|
||||
assert att.post_id == post.id
|
||||
|
||||
|
||||
def test_nested_archive_members_imported(importer, import_layout):
|
||||
"""#718: a 'high-res' pack that wraps a nested archive must still import the
|
||||
nested images and link them to the post — not silently drop them (incase's
|
||||
per-chapter .rar inside an outer 'High Resolution files' pack)."""
|
||||
import_root, _ = import_layout
|
||||
importer.settings.phash_threshold = 0 # v/h splits are distinct only at 0
|
||||
inner = io.BytesIO()
|
||||
with zipfile.ZipFile(inner, "w") as zf:
|
||||
zf.writestr("p1.jpg", _split_bytes("v"))
|
||||
zf.writestr("p2.jpg", _split_bytes("h"))
|
||||
arc = import_root / "Nessie" / "hr.cbz"
|
||||
arc.parent.mkdir(parents=True, exist_ok=True)
|
||||
with zipfile.ZipFile(arc, "w") as zf:
|
||||
zf.writestr("chapter.zip", inner.getvalue()) # nested archive, no direct media
|
||||
arc.with_suffix(".cbz.json").write_text(json.dumps(
|
||||
{"category": "patreon", "id": "900", "title": "HR Pack"}))
|
||||
|
||||
r = importer.import_one(arc)
|
||||
assert r.status == "imported"
|
||||
assert len(r.member_image_ids) == 2 # both nested images surfaced
|
||||
|
||||
assert importer.session.execute(
|
||||
select(func.count()).select_from(ImageRecord)
|
||||
).scalar_one() == 2
|
||||
post = importer.session.execute(select(Post)).scalar_one()
|
||||
provs = importer.session.execute(
|
||||
select(func.count()).select_from(ImageProvenance)
|
||||
).scalar_one()
|
||||
assert provs == 2 # both nested members linked to the one outer post
|
||||
# The outer pack is still preserved as an attachment.
|
||||
att = importer.session.execute(select(PostAttachment)).scalar_one()
|
||||
assert att.original_filename == "hr.cbz"
|
||||
assert att.post_id == post.id
|
||||
|
||||
|
||||
def test_archive_all_deduped_is_benign_not_flagged(importer, import_layout):
|
||||
"""#718 reason-string fix: when every image in an archive already exists
|
||||
(cross-posted), the images re-link to the new post and the archive is NOT
|
||||
flagged as an unextracted-archive problem (no error set)."""
|
||||
import_root, _ = import_layout
|
||||
importer.settings.phash_threshold = 0
|
||||
pv, ph = _split_bytes("v"), _split_bytes("h")
|
||||
first = import_root / "Dup" / "a.cbz"
|
||||
first.parent.mkdir(parents=True, exist_ok=True)
|
||||
with zipfile.ZipFile(first, "w") as zf:
|
||||
zf.writestr("a.jpg", pv)
|
||||
zf.writestr("b.jpg", ph)
|
||||
first.with_suffix(".cbz.json").write_text(json.dumps(
|
||||
{"category": "patreon", "id": "111", "title": "First"}))
|
||||
assert importer.import_one(first).status == "imported"
|
||||
|
||||
second = import_root / "Dup" / "b.cbz" # different post, same two images
|
||||
with zipfile.ZipFile(second, "w") as zf:
|
||||
zf.writestr("a.jpg", pv)
|
||||
zf.writestr("b.jpg", ph)
|
||||
second.with_suffix(".cbz.json").write_text(json.dumps(
|
||||
{"category": "patreon", "id": "222", "title": "Second"}))
|
||||
|
||||
r = importer.import_one(second)
|
||||
assert r.status == "attached"
|
||||
assert r.error is None # benign all-deduped — NOT a flagged problem
|
||||
|
||||
assert importer.session.execute(
|
||||
select(func.count()).select_from(ImageRecord)
|
||||
).scalar_one() == 2 # no new records
|
||||
provs = importer.session.execute(
|
||||
select(func.count()).select_from(ImageProvenance)
|
||||
).scalar_one()
|
||||
assert provs == 4 # 2 images × 2 posts (enrich-on-duplicate)
|
||||
|
||||
|
||||
def test_corrupt_archive_still_stored(importer, import_layout):
|
||||
import_root, _ = import_layout
|
||||
arc = import_root / "Bob" / "broken.zip"
|
||||
|
||||
Reference in New Issue
Block a user