Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 51 additions & 17 deletions pipeline/fetch/src/drive_fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 <fid>.json. Compared case-insensitively for case-folding filesystems
(macOS APFS, Windows, Docker Desktop bind mounts), where <fid>.JSON == <fid>.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.
<fid>.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:
Expand All @@ -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/<fid><ext> (atomically) + a metadata sidecar. Returns the local name.
`.json` content is stored as <fid>.data.json — <fid>.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/<content-name> (atomically) + a metadata sidecar. Returns the local name.
The content name is disambiguated from the sidecar (<fid>.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]
Expand All @@ -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()

Expand Down Expand Up @@ -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():
Expand All @@ -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 <fid>.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=""),
Expand Down
31 changes: 31 additions & 0 deletions pipeline/fetch/tests/test_drive_fetch.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (<fid>.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"
Expand Down Expand Up @@ -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) <fid>.JSON and the sidecar <fid>.json are the same file, so an
uppercase-extension JSON must also be routed to <fid>.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."""
Expand Down
Loading