diff --git a/backend/app/services/importer.py b/backend/app/services/importer.py index 88d1a76..c51a9fc 100644 --- a/backend/app/services/importer.py +++ b/backend/app/services/importer.py @@ -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, diff --git a/tests/test_importer_archive.py b/tests/test_importer_archive.py index 713678b..113d00f 100644 --- a/tests/test_importer_archive.py +++ b/tests/test_importer_archive.py @@ -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"