Skip to content

Commit 39a55bc

Browse files
committed
fix(dump): prune stale output pages for deleted records
`python -m app.dump` only ever wrote/updated pages for records that currently exist; it never compared the output tree against the live data, so when a source record was deleted or renamed its per-record page directory (`<category>/<slug>/index.json` and the `score/` subdir) was left on disk forever. This surfaced during today's Atom CPU dedup, where 18 removed duplicate CPU records left orphaned pages that had to be deleted by hand. Add `_prune_orphaned_pages`, invoked after each collection is written, to remove any immediate child directory of the per-category output dir whose slug is not backed by a current record (including its nested `score/` folder). Only per-slug page directories the dump owns are touched: the collection's own `index.json` list file, the top-level manifest, and `openapi.json` are non-directory entries and are never at risk. Pruning runs on every dump so the tree stays a deterministic, accurate mirror of the data — a no-change re-run remains byte-identical. Add tests covering a real record deletion (seed, dump, delete, re-dump, assert the page directory is gone while survivors and the list file remain) plus the prune helper's file/valid-slug safety guarantees. Refs #100
1 parent 0e9ee25 commit 39a55bc

2 files changed

Lines changed: 106 additions & 1 deletion

File tree

‎app/dump.py‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import argparse
1414
import json
15+
import shutil
1516
from pathlib import Path
1617
from typing import Any
1718

@@ -56,6 +57,33 @@ def _write_json(path: Path, data: object) -> None:
5657
path.write_text(text, encoding="utf-8")
5758

5859

60+
def _prune_orphaned_pages(collection_dir: Path, valid_slugs: set[str]) -> list[str]:
61+
"""Remove per-record page directories that no longer back a current record.
62+
63+
The dump writes each record to ``<collection_dir>/<slug>/index.json`` (and,
64+
for scored collections, ``<slug>/score/index.json``). When a source record
65+
is deleted or renamed, its old ``<slug>/`` directory is otherwise left on
66+
disk forever. This deletes any immediate child *directory* of
67+
``collection_dir`` whose name is not in ``valid_slugs``.
68+
69+
Only per-slug subdirectories the dump itself owns are touched. The
70+
collection's own ``index.json`` list file (and any other non-directory
71+
entry) is left alone, so top-level manifests, ``openapi.json``, etc. are
72+
never at risk — this only ever runs inside a per-category directory.
73+
"""
74+
if not collection_dir.is_dir():
75+
return []
76+
pruned: list[str] = []
77+
for child in collection_dir.iterdir():
78+
if not child.is_dir():
79+
continue
80+
if child.name in valid_slugs:
81+
continue
82+
shutil.rmtree(child)
83+
pruned.append(child.name)
84+
return pruned
85+
86+
5987
def _fetch_all(client: TestClient, resource: str) -> tuple[int, list[dict[str, Any]]]:
6088
"""Follow pagination to collect every list item for a resource."""
6189
items: list[dict[str, Any]] = []
@@ -95,6 +123,10 @@ def generate(
95123
_write_json(output_dir / "v1" / resource / slug / "score" / "index.json", score)
96124
if score.get("overall") is not None:
97125
scored += 1
126+
# Self-heal: drop any per-record page directory whose source record no
127+
# longer exists, so the dump stays a deterministic mirror of the data.
128+
valid_slugs = {item["slug"] for item in items}
129+
_prune_orphaned_pages(output_dir / "v1" / resource, valid_slugs)
98130
counts[resource] = len(items)
99131
manifest_collections = manifest["collections"]
100132
assert isinstance(manifest_collections, dict)

‎tests/integration/test_dump.py‎

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
import pytest
99
from fastapi.testclient import TestClient
1010

11-
from app.dump import COLLECTIONS, generate, resolve_collections
11+
from app.dump import COLLECTIONS, _prune_orphaned_pages, generate, resolve_collections
1212
from tests.integration.mobile_device_fixtures import ensure_mobile_device_fixtures
1313

1414

@@ -64,3 +64,76 @@ def test_resolve_collections_drops_excluded_and_keeps_order() -> None:
6464
def test_resolve_collections_rejects_unknown_names() -> None:
6565
with pytest.raises(ValueError, match="unknown collection"):
6666
resolve_collections(["gmaes"])
67+
68+
69+
def test_dump_prunes_output_pages_for_deleted_records(
70+
client: TestClient, tmp_path: Path
71+
) -> None:
72+
"""A record whose source is gone must lose its output page on the next run.
73+
74+
Reproduces the real-world bug found during the Atom CPU dedup: a record is
75+
removed, but re-running the dump used to leave its ``<slug>/`` page tree on
76+
disk forever. Here we seed fixtures, dump, delete one record from the
77+
database, re-dump, and assert the deleted record's output directory is gone
78+
while a surviving record's page (and the collection list file) remain.
79+
"""
80+
from sqlmodel import Session, select
81+
82+
from app.database import engine
83+
from app.models.mobile_device import Tablet
84+
85+
ensure_mobile_device_fixtures()
86+
collections = ["tablets"]
87+
88+
generate(client, output_dir=tmp_path, collections=collections)
89+
tablets_dir = tmp_path / "v1" / "tablets"
90+
deleted_slug = "ipad-pro-11-m4-wifi-8gb-256gb"
91+
deleted_page = tablets_dir / deleted_slug / "index.json"
92+
assert deleted_page.exists()
93+
94+
# Delete the record from the database, mimicking a removed source record.
95+
with Session(engine) as session:
96+
tablet = session.exec(select(Tablet).where(Tablet.slug == deleted_slug)).one()
97+
session.delete(tablet)
98+
session.commit()
99+
try:
100+
# Confirm the record is truly gone from the live API before re-dumping.
101+
assert client.get(f"/v1/tablets/{deleted_slug}").status_code == 404
102+
surviving = [
103+
item["slug"]
104+
for item in client.get("/v1/tablets?limit=100").json()["results"]
105+
]
106+
assert deleted_slug not in surviving
107+
108+
generate(client, output_dir=tmp_path, collections=collections)
109+
110+
# The deleted record's whole page directory is pruned...
111+
assert not (tablets_dir / deleted_slug).exists()
112+
# ...while the collection list file and any surviving pages remain.
113+
assert (tablets_dir / "index.json").exists()
114+
for slug in surviving:
115+
assert (tablets_dir / slug / "index.json").exists()
116+
finally:
117+
# Restore the fixture so later tests relying on it still find the record.
118+
ensure_mobile_device_fixtures()
119+
120+
121+
def test_prune_orphaned_pages_leaves_files_and_valid_slugs(tmp_path: Path) -> None:
122+
collection_dir = tmp_path / "v1" / "cpus"
123+
(collection_dir / "keep-me").mkdir(parents=True)
124+
(collection_dir / "keep-me" / "index.json").write_text("{}\n", encoding="utf-8")
125+
(collection_dir / "drop-me").mkdir()
126+
(collection_dir / "drop-me" / "index.json").write_text("{}\n", encoding="utf-8")
127+
# The collection's own list file must never be touched (it is not a dir).
128+
(collection_dir / "index.json").write_text('{"count": 0}\n', encoding="utf-8")
129+
130+
pruned = _prune_orphaned_pages(collection_dir, valid_slugs={"keep-me"})
131+
132+
assert pruned == ["drop-me"]
133+
assert (collection_dir / "keep-me").is_dir()
134+
assert not (collection_dir / "drop-me").exists()
135+
assert (collection_dir / "index.json").read_text() == '{"count": 0}\n'
136+
137+
138+
def test_prune_orphaned_pages_noop_when_dir_missing(tmp_path: Path) -> None:
139+
assert _prune_orphaned_pages(tmp_path / "does-not-exist", valid_slugs=set()) == []

0 commit comments

Comments
 (0)