diff --git a/pipeline/fetch/src/drive_fetch.py b/pipeline/fetch/src/drive_fetch.py index b1b0dbb..6ca3506 100644 --- a/pipeline/fetch/src/drive_fetch.py +++ b/pipeline/fetch/src/drive_fetch.py @@ -240,9 +240,45 @@ def ext_for(cfg: Config, d: dict, mime: str) -> tuple[str | None, str]: return None, (os.path.splitext(name)[1] or ".bin") +def sidecar_name(fid: str) -> str: + """The metadata sidecar's filename — the on-disk contract clean reads. Single-sourced.""" + return f"{fid}.json" + + +def content_name(cfg: Config, d: dict, mime: str, fid: str) -> tuple[str | None, str]: + """(export_format|None, on-disk content filename), disambiguated from the sidecar so content + can never overwrite .json. Compared case-insensitively for case-folding filesystems + (macOS APFS, Windows, Docker Desktop bind mounts), where .JSON == .json.""" + fmt, ext = ext_for(cfg, d, mime) + name = f"{fid}{ext}" + if name.lower() == sidecar_name(fid).lower(): + name = f"{fid}.data{ext}" + return fmt, name + + +def _clobbered_by_sidecar(cfg: Config, fid: str, prev_local: str | None) -> bool: + """True if the manifest's content path IS the sidecar file — its bytes were overwritten by the + sidecar write (the pre-.data.json bug), so the entry must re-download to heal. Decided by file + identity, not name: an exact sidecar-name localPath is always the clobber; a case-variant (e.g. + .JSON) is the clobber only on a case-folding filesystem, where it resolves to the same file + as the sidecar — on a case-sensitive one it is a distinct, valid content file that must be kept + (else it would be needlessly re-downloaded and orphaned).""" + if not prev_local: + return False + sidecar = sidecar_name(fid) + if prev_local == sidecar: + return True + if prev_local.lower() != sidecar.lower(): + return False + try: + return (cfg.raw_dir / prev_local).samefile(cfg.raw_dir / sidecar) + except OSError: # content or sidecar missing -> not a live clobber; let the exists() skip re-fetch + return False + + def merge_sidecar_lineage(cfg: Config, fid: str, lineage: dict) -> None: """Adds lineage paths to an existing sidecar without touching the rest of its metadata.""" - sidecar = cfg.raw_dir / f"{fid}.json" + sidecar = cfg.raw_dir / sidecar_name(fid) meta: object = {} if sidecar.exists(): try: @@ -256,13 +292,11 @@ def merge_sidecar_lineage(cfg: Config, fid: str, lineage: dict) -> None: def download_file(cfg: Config, d: dict, fid: str, mime: str, lineage: dict) -> str: - """Downloads to raw/ (atomically) + a metadata sidecar. Returns the local name. - `.json` content is stored as .data.json — .json IS the sidecar.""" - fmt, ext = ext_for(cfg, d, mime) - if ext == ".json": - ext = ".data.json" - out = cfg.raw_dir / f"{fid}{ext}" - tmp = cfg.raw_dir / f".tmp-{fid}{ext}" + """Downloads to raw/ (atomically) + a metadata sidecar. Returns the local name. + The content name is disambiguated from the sidecar (.json) so the two never collide.""" + fmt, name = content_name(cfg, d, mime, fid) + out = cfg.raw_dir / name + tmp = cfg.raw_dir / f".tmp-{name}" args = ["drive", "download", fid, "--out", str(tmp)] if fmt: args += ["--format", fmt] @@ -274,12 +308,12 @@ def download_file(cfg: Config, d: dict, fid: str, mime: str, lineage: dict) -> s meta = meta["file"] if isinstance(meta, dict): meta.update(lineage) - write_atomic(cfg.raw_dir / f"{fid}.json", json.dumps(meta, indent=2, ensure_ascii=False).encode()) + write_atomic(cfg.raw_dir / sidecar_name(fid), json.dumps(meta, indent=2, ensure_ascii=False).encode()) return out.name def remove_local(cfg: Config, entry: dict, fid: str) -> None: - for p in (entry.get("localPath"), f"{fid}.json"): + for p in (entry.get("localPath"), sidecar_name(fid)): if p and (cfg.raw_dir / p).exists(): (cfg.raw_dir / p).unlink() @@ -349,10 +383,12 @@ def sync_once(cfg: Config, folder_id: str) -> dict: "parentPath": f"/{cfg.folder or folder_id}", "parentIds": parents(d), }) - # A localPath equal to the sidecar means the content was clobbered by the sidecar write - # (pre-.data.json state): never treat it as a valid mirror — re-download to heal. - if (prev and prev.get("fingerprint") == fp and prev_local - and prev_local != f"{fid}.json" and (cfg.raw_dir / prev_local).exists()): + # A localPath that is really the sidecar file is a pre-.data.json entry whose content the + # sidecar write clobbered: treat it as absent so the file re-downloads and heals, and so the + # rename-cleanup below never unlinks the freshly written sidecar. + if _clobbered_by_sidecar(cfg, fid, prev_local): + prev_local = None + if prev and prev.get("fingerprint") == fp and prev_local and (cfg.raw_dir / prev_local).exists(): # Backfill/refresh path metadata without redownloading unchanged files. touched = False for k, v in lineage.items(): @@ -375,9 +411,7 @@ def sync_once(cfg: Config, folder_id: str) -> dict: continue # a rename or export-format change alters the local name: drop the previous file so its # stale content isn't orphaned in the mirror (deletion later only removes the current name). - # Never unlink .json — that is the sidecar (a healing entry's prev_local points at it). - if (prev_local and prev_local != local and prev_local != f"{fid}.json" - and (cfg.raw_dir / prev_local).exists()): + if prev_local and prev_local != local and (cfg.raw_dir / prev_local).exists(): (cfg.raw_dir / prev_local).unlink() manifest[fid] = { "name": _field(d, "name", "title", default=""), diff --git a/pipeline/fetch/tests/test_drive_fetch.py b/pipeline/fetch/tests/test_drive_fetch.py index 1359888..84b3b45 100644 --- a/pipeline/fetch/tests/test_drive_fetch.py +++ b/pipeline/fetch/tests/test_drive_fetch.py @@ -106,6 +106,23 @@ def test_write_atomic_and_sidecar_merge(tmp_path): assert json.loads((tmp_path / "F.json").read_text()) == {"kept": True, "drivePath": "/X/a.pdf"} +def test_clobbered_by_sidecar_uses_file_identity_not_name(tmp_path, monkeypatch): + """The clobber is decided by file identity, not name: an exact sidecar-name localPath is always + the clobber, but a case-variant (.JSON) is the clobber ONLY on a case-folding filesystem + (where it resolves to the sidecar). On a case-sensitive filesystem it is a distinct valid file + that must be kept, else it is needlessly re-downloaded and orphaned.""" + cfg = _cfg(tmp_path) + (tmp_path / "C.json").write_text("{}") # the sidecar + assert df._clobbered_by_sidecar(cfg, "C", None) is False + assert df._clobbered_by_sidecar(cfg, "C", "C.json") is True # exact name -> always clobber + assert df._clobbered_by_sidecar(cfg, "C", "C.pdf") is False # unrelated content -> never + # case-variant name: outcome follows the filesystem, probed via samefile + monkeypatch.setattr(df.Path, "samefile", lambda self, other: False) # case-sensitive fs + assert df._clobbered_by_sidecar(cfg, "C", "C.JSON") is False # distinct file -> keep it + monkeypatch.setattr(df.Path, "samefile", lambda self, other: True) # case-folding fs + assert df._clobbered_by_sidecar(cfg, "C", "C.JSON") is True # same file -> heal + + # ── folder resolution ──────────────────────────────────────────────────────── def test_resolve_folder_id_explicit_wins(tmp_path): assert df.resolve_folder_id(_cfg(tmp_path, folder_id="explicit-id")) == "explicit-id" @@ -262,6 +279,20 @@ def test_sync_once_json_content_does_not_collide_with_sidecar(sync_env): assert manifest["C"]["localPath"] == "C.data.json" +def test_sync_once_uppercase_json_content_does_not_collide_with_sidecar(sync_env): + """The collision is case-insensitive: on a case-folding filesystem (macOS APFS, Windows, + Docker Desktop bind mounts) .JSON and the sidecar .json are the same file, so an + uppercase-extension JSON must also be routed to .data.JSON.""" + cfg, tmp_path, fake = sync_env + fake.items.append({"id": "C", "name": "CONFIG.JSON", "mimeType": "application/json", + "modifiedTime": "t1", "parents": ["folder1"]}) + df.sync_once(cfg, "root") + assert (tmp_path / "C.data.JSON").read_text() == "content-C" + assert json.loads((tmp_path / "C.json").read_text())["id"] == "C" # sidecar not clobbered + manifest = json.loads((tmp_path / "_state.json").read_text())["files"] + assert manifest["C"]["localPath"] == "C.data.JSON" + + def test_sync_once_heals_json_entry_clobbered_by_sidecar(sync_env): """A pre-fix manifest whose localPath points at the sidecar must re-download the content (even with an unchanged fingerprint) and must not delete the sidecar while healing."""