This is an automated email from the ASF dual-hosted git repository. imbajin pushed a commit to branch feat/oink-community-content in repository https://gitbox.apache.org/repos/asf/hugegraph-doc.git
commit c2138cd297fee331b1f09c604ac887a08ed2afa5 Author: dark <[email protected]> AuthorDate: Fri Sep 4 19:45:15 2026 +0800 fix(community): close refresh cleanup window - remove candidate files before mutating the checked-in bundle\n- install candidate avatar bytes through an injectable safe copy\n- roll back roster and avatar state after orphan cleanup failure\n- cover pre-commit cleanup failure with fault injection --- scripts/community_roster.py | 31 ++++++++++++++++++++++--------- scripts/test_community_roster.py | 32 +++++++++++++++++++++++--------- 2 files changed, 45 insertions(+), 18 deletions(-) diff --git a/scripts/community_roster.py b/scripts/community_roster.py index 78b82a4cb..a917b59dd 100644 --- a/scripts/community_roster.py +++ b/scripts/community_roster.py @@ -275,7 +275,7 @@ def validate_bundle(warn_after_days: int) -> list[str]: tail = entries[1:] if role == "pmc" else entries actual_order = [(p.get("name", "").casefold(), p.get("asf_id", "").casefold()) for p in tail] if actual_order != sorted(actual_order): - raise RosterError(f"roster.json: {role} must be sorted by public name casefold") + raise RosterError(f"roster.json: {role} must be sorted by public name and ASF ID casefold") for person in entries: if any(key not in person for key in ("asf_id", "name", "initials", "chair", "profile_url")): raise RosterError(f"roster.json: incomplete member {person!r}") @@ -364,7 +364,14 @@ def _unlink(path: pathlib.Path) -> None: path.unlink() -def _commit_bundle(candidate: dict, candidate_avatars: pathlib.Path) -> None: +def _copy_candidate(raw: bytes, destination: pathlib.Path) -> None: + with destination.open("xb") as stream: + stream.write(raw) + stream.flush() + os.fsync(stream.fileno()) + + +def _commit_bundle(candidate: dict, candidate_avatars: dict[str, bytes]) -> None: """Install one bundle or restore the exact prior roster/avatar state.""" old_roster = ROSTER_PATH.read_bytes() if ROSTER_PATH.exists() else None AVATAR_DIR.mkdir(parents=True, exist_ok=True) @@ -379,13 +386,13 @@ def _commit_bundle(candidate: dict, candidate_avatars: pathlib.Path) -> None: installed: list[pathlib.Path] = [] roster_replaced = False try: - for avatar in sorted(candidate_avatars.glob("*.webp")): - destination = AVATAR_DIR / avatar.name + for name, avatar in sorted(candidate_avatars.items()): + destination = AVATAR_DIR / name if destination.exists(): continue - staged = AVATAR_DIR / f".{avatar.name}.candidate" + staged = AVATAR_DIR / f".{name}.candidate" try: - shutil.copyfile(avatar, staged) + _copy_candidate(avatar, staged) os.replace(staged, destination) finally: if staged.exists(): @@ -418,10 +425,16 @@ def _commit_bundle(candidate: dict, candidate_avatars: pathlib.Path) -> None: def refresh() -> None: source_data = {key: _fetch_json(url) for key, url in SOURCES.items()} candidate = build_roster(source_data["committee"], source_data["projects"], source_data["people"], _read_json(MAP_PATH)) - with tempfile.TemporaryDirectory(prefix=".community-refresh-", dir=DATA_DIR) as work: - candidate_avatars = pathlib.Path(work) / "avatars" + work = pathlib.Path(tempfile.mkdtemp(prefix=".community-refresh-", dir=DATA_DIR)) + try: + candidate_avatars = work / "avatars" _install_avatars(candidate, candidate_avatars) - _commit_bundle(candidate, candidate_avatars) + avatar_bytes = {path.name: path.read_bytes() for path in candidate_avatars.glob("*.webp")} + finally: + # Candidate cleanup is deliberately completed before the checked-in + # bundle changes, so cleanup failure cannot publish a new roster. + shutil.rmtree(work) + _commit_bundle(candidate, avatar_bytes) def main() -> int: diff --git a/scripts/test_community_roster.py b/scripts/test_community_roster.py index 4e2f4fa7f..f6fc2062b 100644 --- a/scripts/test_community_roster.py +++ b/scripts/test_community_roster.py @@ -119,30 +119,44 @@ print(json.dumps([person["asf_id"] for person in result["roles"]["pmc"]])) def test_copy_failure_preserves_last_good_bundle(self): with tempfile.TemporaryDirectory(prefix="community-copy-test-") as directory: root = pathlib.Path(directory) - roster_path, avatar_dir, candidates = root / "roster.json", root / "avatars", root / "candidates" + roster_path, avatar_dir = root / "roster.json", root / "avatars" avatar_dir.mkdir() - candidates.mkdir() roster_path.write_bytes(b"last-good\n") (avatar_dir / "old.webp").write_bytes(b"old") - (candidates / "new.webp").write_bytes(b"new") candidate = {"roles": {"pmc": [{"avatar": "/img/community/avatars/new.webp"}], "committers": []}} with mock.patch.object(roster, "ROSTER_PATH", roster_path), \ mock.patch.object(roster, "AVATAR_DIR", avatar_dir), \ - mock.patch.object(roster.shutil, "copyfile", side_effect=OSError("copy failed")): + mock.patch.object(roster, "_copy_candidate", side_effect=OSError("copy failed")): with self.assertRaisesRegex(OSError, "copy failed"): - roster._commit_bundle(candidate, candidates) + roster._commit_bundle(candidate, {"new.webp": b"new"}) self.assertEqual(b"last-good\n", roster_path.read_bytes()) self.assertEqual(b"old", (avatar_dir / "old.webp").read_bytes()) + def test_candidate_cleanup_failure_does_not_publish_roster(self): + with tempfile.TemporaryDirectory(prefix="community-cleanup-test-") as directory: + root = pathlib.Path(directory) + roster_path, map_path = root / "roster.json", root / "github-map.json" + roster_path.write_bytes(b"last-good\n") + map_path.write_text('{"schema_version": 1, "mappings": {}}') + candidate = {"roles": {"pmc": [], "committers": []}} + with mock.patch.object(roster, "DATA_DIR", root), \ + mock.patch.object(roster, "ROSTER_PATH", roster_path), \ + mock.patch.object(roster, "MAP_PATH", map_path), \ + mock.patch.object(roster, "_fetch_json", return_value={}), \ + mock.patch.object(roster, "build_roster", return_value=candidate), \ + mock.patch.object(roster, "_install_avatars"), \ + mock.patch.object(roster.shutil, "rmtree", side_effect=OSError("cleanup failed")): + with self.assertRaisesRegex(OSError, "cleanup failed"): + roster.refresh() + self.assertEqual(b"last-good\n", roster_path.read_bytes()) + def test_unlink_failure_rolls_back_roster_and_avatars(self): with tempfile.TemporaryDirectory(prefix="community-unlink-test-") as directory: root = pathlib.Path(directory) - roster_path, avatar_dir, candidates = root / "roster.json", root / "avatars", root / "candidates" + roster_path, avatar_dir = root / "roster.json", root / "avatars" avatar_dir.mkdir() - candidates.mkdir() roster_path.write_bytes(b"last-good\n") (avatar_dir / "old.webp").write_bytes(b"old") - (candidates / "new.webp").write_bytes(b"new") candidate = {"roles": {"pmc": [{"avatar": "/img/community/avatars/new.webp"}], "committers": []}} real_unlink, failed = roster._unlink, False @@ -157,7 +171,7 @@ print(json.dumps([person["asf_id"] for person in result["roles"]["pmc"]])) mock.patch.object(roster, "AVATAR_DIR", avatar_dir), \ mock.patch.object(roster, "_unlink", side_effect=fail_once): with self.assertRaisesRegex(OSError, "unlink failed"): - roster._commit_bundle(candidate, candidates) + roster._commit_bundle(candidate, {"new.webp": b"new"}) self.assertEqual(b"last-good\n", roster_path.read_bytes()) self.assertEqual(b"old", (avatar_dir / "old.webp").read_bytes()) self.assertFalse((avatar_dir / "new.webp").exists())
