diff --git a/graphify/ingest.py b/graphify/ingest.py index 20962250b..5c3920fc5 100644 --- a/graphify/ingest.py +++ b/graphify/ingest.py @@ -222,9 +222,21 @@ Source: {url} def _download_binary(url: str, suffix: str, target_dir: Path) -> Path: - """Download a binary file (PDF, image) directly.""" + """Save a sequential capture without replacing an existing directory entry. + + Selection and writing are separate; concurrent captures are not atomic. + """ filename = _safe_filename(url, suffix) + stem = Path(filename).stem out_path = target_dir / filename + counter = 1 + while True: + try: + out_path.lstat() + except FileNotFoundError: + break + out_path = target_dir / f"{stem}_{counter}{suffix}" + counter += 1 out_path.write_bytes(safe_fetch(url)) return out_path diff --git a/tests/test_ingest_url_type.py b/tests/test_ingest_url_type.py index 45d5f11c6..c5adfeeab 100644 --- a/tests/test_ingest_url_type.py +++ b/tests/test_ingest_url_type.py @@ -103,3 +103,129 @@ def test_tweet_oembed_rewrites_only_the_host(url, expected, monkeypatch): query = urllib.parse.parse_qs(urllib.parse.urlsplit(requested[0]).query) assert query["url"] == [expected] + + +@pytest.mark.parametrize( + ("first_url", "second_url"), + [ + ("https://example.org/report.pdf?version=1", "https://example.org/report.pdf?version=2"), + ("https://example.org/photo.png?version=1", "https://example.org/photo.png?version=2"), + ("https://example.org/report.pdf", "https://example.org/report.pdf"), + ("https://example.org/report+a.pdf", "https://example.org/report_a.pdf"), + ("https://example.org/" + "a" * 90 + "1.pdf", "https://example.org/" + "a" * 90 + "2.pdf"), + ], +) +def test_binary_captures_preserve_colliding_payloads(first_url, second_url, tmp_path, monkeypatch): + payloads = iter([b"first capture", b"second capture"]) + fetched = [] + + def fetch(url): + fetched.append(url) + return next(payloads) + + monkeypatch.setattr(ingest_mod, "validate_url", lambda url: url) + monkeypatch.setattr(ingest_mod, "safe_fetch", fetch) + + first = ingest_mod.ingest(first_url, tmp_path) + second = ingest_mod.ingest(second_url, tmp_path) + + assert first != second + assert second.name == f"{first.stem}_1{first.suffix}" + assert first.read_bytes() == b"first capture" + assert second.read_bytes() == b"second capture" + assert fetched == [first_url, second_url] + assert set(tmp_path.iterdir()) == {first, second} + + +@pytest.mark.parametrize( + ("entry_kind", "numbered"), + [("file", False), ("file", True), ("dangling_link", False), + ("dangling_link", True), ("live_link", True), ("directory", False), ("directory", True)], +) +def test_binary_captures_skip_existing_entries(entry_kind, numbered, tmp_path, monkeypatch): + corpus = tmp_path / "corpus" + corpus.mkdir() + preferred = corpus / "example_org_report_pdf.pdf" + occupied = preferred.with_name("example_org_report_pdf_1.pdf") if numbered else preferred + sentinel = tmp_path / "sentinel" + if numbered: + preferred.write_bytes(b"retained base") + if entry_kind == "file": + occupied.write_bytes(b"retained file") + elif entry_kind == "directory": + occupied.mkdir() + else: + if entry_kind == "live_link": + sentinel.write_bytes(b"outside sentinel") + try: + occupied.symlink_to(sentinel) + except (OSError, NotImplementedError) as exc: + pytest.skip(f"host cannot create symlinks: {exc}") + + monkeypatch.setattr(ingest_mod, "validate_url", lambda url: url) + monkeypatch.setattr(ingest_mod, "safe_fetch", lambda url: b"new capture") + result = ingest_mod.ingest("https://example.org/report.pdf", corpus) + + expected_number = 2 if numbered else 1 + assert result == corpus / f"example_org_report_pdf_{expected_number}.pdf" + assert result.read_bytes() == b"new capture" + if numbered: + assert preferred.read_bytes() == b"retained base" + if entry_kind == "file": + assert occupied.read_bytes() == b"retained file" + elif entry_kind == "directory": + assert occupied.is_dir() and not list(occupied.iterdir()) + else: + assert occupied.is_symlink() and occupied.readlink() == sentinel + if entry_kind == "live_link": + assert sentinel.read_bytes() == b"outside sentinel" + else: + assert not sentinel.exists() + + +@pytest.mark.parametrize("error_type", [PermissionError, OSError]) +def test_binary_capture_inspection_error_precedes_fetch(error_type, tmp_path, monkeypatch): + from pathlib import Path + + preferred = tmp_path / "example_org_report_pdf.pdf" + original_lstat = Path.lstat + inspected = [] + fetched = [] + + def lstat(path, *args, **kwargs): + if path == preferred: + inspected.append(path) + raise error_type("cannot inspect destination") + return original_lstat(path, *args, **kwargs) + + monkeypatch.setattr(Path, "lstat", lstat) + monkeypatch.setattr(ingest_mod, "safe_fetch", lambda url: fetched.append(url) or b"new capture") + with pytest.raises(error_type, match="cannot inspect destination"): + ingest_mod._download_binary("https://example.org/report.pdf", ".pdf", tmp_path) + assert inspected == [preferred] + assert fetched == [] + assert not list(tmp_path.iterdir()) + + +@pytest.mark.parametrize("suffix", [".pdf", ".png"]) +def test_binary_capture_free_name_is_unchanged(suffix, tmp_path, monkeypatch): + monkeypatch.setattr(ingest_mod, "validate_url", lambda url: url) + monkeypatch.setattr(ingest_mod, "safe_fetch", lambda url: b"capture") + result = ingest_mod.ingest(f"https://example.org/report{suffix}", tmp_path) + assert result == tmp_path / f"example_org_report_{suffix[1:]}{suffix}" + assert result.read_bytes() == b"capture" + + +def test_binary_capture_fetch_failure_preserves_existing_file(tmp_path, monkeypatch): + retained = tmp_path / "example_org_report_pdf.pdf" + retained.write_bytes(b"retained capture") + + def fail_fetch(url): + raise OSError("download failed") + + monkeypatch.setattr(ingest_mod, "validate_url", lambda url: url) + monkeypatch.setattr(ingest_mod, "safe_fetch", fail_fetch) + with pytest.raises(RuntimeError, match="download failed"): + ingest_mod.ingest("https://example.org/report.pdf", tmp_path) + assert retained.read_bytes() == b"retained capture" + assert list(tmp_path.iterdir()) == [retained]