Harden canonical publication against races
This commit is contained in:
parent
8919e2af32
commit
a901c9705b
8 changed files with 1059 additions and 78 deletions
|
|
@ -3,13 +3,16 @@ from __future__ import annotations
|
|||
import hashlib
|
||||
import json
|
||||
import multiprocessing
|
||||
import os
|
||||
import shutil
|
||||
import stat
|
||||
import tempfile
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from unittest import mock
|
||||
|
||||
import docforge.application as application_module
|
||||
from docforge.application import CanonicalApplicationService, GenericCanonicalApplier
|
||||
from docforge.changesets import ChangesetStore
|
||||
from docforge.errors import DocForgeError
|
||||
|
|
@ -271,6 +274,291 @@ class DocForgeChangesetTests(unittest.TestCase):
|
|||
service.apply("apply-all", str(final["changeset_hash"]))
|
||||
self.assertEqual("changeset_closed", closed.exception.code)
|
||||
|
||||
def test_canonical_update_exchange_preserves_a_raced_external_edit(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as directory:
|
||||
root = self.copy_fixture(Path(directory))
|
||||
project = Project.open(root)
|
||||
store = ChangesetStore(project, "alpha-editor")
|
||||
proposal = store.register(
|
||||
"update-race",
|
||||
[
|
||||
{
|
||||
"operation": "update",
|
||||
"node_id": "guide.workflow",
|
||||
"metadata": {"summary": "Approved summary."},
|
||||
"rationale": "Exercise the atomic update boundary.",
|
||||
}
|
||||
],
|
||||
)
|
||||
proposal_path = root / ".docforge/changesets/update-race.json"
|
||||
proposal_bytes = proposal_path.read_bytes()
|
||||
target = root / "docs/content/workflow.md"
|
||||
exchange = application_module.rename_exchange_at
|
||||
raced = False
|
||||
|
||||
def race(directory_fd: int, first: str, second: str) -> None:
|
||||
nonlocal raced
|
||||
if second == target.name and not raced:
|
||||
raced = True
|
||||
target.write_bytes(target.read_bytes() + b"\nExternal edit at exchange.\n")
|
||||
exchange(directory_fd, first, second)
|
||||
|
||||
with (
|
||||
mock.patch(
|
||||
"docforge.application.rename_exchange_at",
|
||||
side_effect=race,
|
||||
),
|
||||
self.assertRaises(DocForgeError) as captured,
|
||||
):
|
||||
store.apply(
|
||||
changeset_id="update-race",
|
||||
expected_changeset_hash=str(proposal["changeset_hash"]),
|
||||
applier_id="alpha-editor",
|
||||
application=GenericCanonicalApplier(project).apply,
|
||||
)
|
||||
|
||||
self.assertEqual("base_conflict", captured.exception.code)
|
||||
self.assertIn("External edit at exchange.", target.read_text(encoding="utf-8"))
|
||||
self.assertEqual(proposal_bytes, proposal_path.read_bytes())
|
||||
self.assertFalse((root / ".docforge/changesets/.state/update-race.json").exists())
|
||||
self.assertFalse(tuple(target.parent.glob(".docforge-apply-*")))
|
||||
|
||||
def test_canonical_create_and_delete_races_preserve_foreign_targets(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as directory:
|
||||
parent = Path(directory)
|
||||
|
||||
create_root = self.copy_fixture(parent / "create")
|
||||
create_project = Project.open(create_root)
|
||||
create_store = ChangesetStore(create_project, "alpha-editor")
|
||||
create = create_store.register(
|
||||
"create-race",
|
||||
[
|
||||
{
|
||||
"operation": "create",
|
||||
"node_id": "guide.raced",
|
||||
"target_source": "docs/content/raced.md",
|
||||
"metadata": self.new_metadata(),
|
||||
"content": "Approved new content.",
|
||||
"rationale": "Exercise no-replace creation.",
|
||||
}
|
||||
],
|
||||
)
|
||||
create_target = create_root / "docs/content/raced.md"
|
||||
real_link = application_module.os.link
|
||||
appeared = False
|
||||
|
||||
def race_create(
|
||||
source: str,
|
||||
target: str,
|
||||
*,
|
||||
src_dir_fd: int,
|
||||
dst_dir_fd: int,
|
||||
follow_symlinks: bool,
|
||||
) -> None:
|
||||
nonlocal appeared
|
||||
if target == create_target.name and not appeared:
|
||||
appeared = True
|
||||
create_target.write_bytes(b"foreign create target\n")
|
||||
real_link(
|
||||
source,
|
||||
target,
|
||||
src_dir_fd=src_dir_fd,
|
||||
dst_dir_fd=dst_dir_fd,
|
||||
follow_symlinks=follow_symlinks,
|
||||
)
|
||||
|
||||
with (
|
||||
mock.patch("docforge.application.os.link", side_effect=race_create),
|
||||
self.assertRaises(DocForgeError) as create_error,
|
||||
):
|
||||
create_store.apply(
|
||||
changeset_id="create-race",
|
||||
expected_changeset_hash=str(create["changeset_hash"]),
|
||||
applier_id="alpha-editor",
|
||||
application=GenericCanonicalApplier(create_project).apply,
|
||||
)
|
||||
self.assertEqual("base_conflict", create_error.exception.code)
|
||||
self.assertEqual(b"foreign create target\n", create_target.read_bytes())
|
||||
self.assertFalse(tuple(create_target.parent.glob(".docforge-apply-*")))
|
||||
|
||||
delete_root = self.copy_fixture(parent / "delete")
|
||||
delete_project = Project.open(delete_root)
|
||||
delete_store = ChangesetStore(delete_project, "alpha-editor")
|
||||
delete = delete_store.register(
|
||||
"delete-race",
|
||||
[
|
||||
{
|
||||
"operation": "delete",
|
||||
"node_id": "proof.validation",
|
||||
"relationship_changes": [
|
||||
{
|
||||
"action": "remove",
|
||||
"source_id": "proof.validation",
|
||||
"relation": "proves",
|
||||
"target_id": "guide.workflow",
|
||||
}
|
||||
],
|
||||
"rationale": "Exercise atomic deletion.",
|
||||
}
|
||||
],
|
||||
)
|
||||
delete_target = delete_root / "docs/content/proof.toml"
|
||||
exchange = application_module.rename_exchange_at
|
||||
deleted_race = False
|
||||
|
||||
def race_delete(directory_fd: int, first: str, second: str) -> None:
|
||||
nonlocal deleted_race
|
||||
if second == delete_target.name and not deleted_race:
|
||||
deleted_race = True
|
||||
delete_target.write_bytes(
|
||||
delete_target.read_bytes() + b"\n# foreign delete edit\n"
|
||||
)
|
||||
exchange(directory_fd, first, second)
|
||||
|
||||
with (
|
||||
mock.patch(
|
||||
"docforge.application.rename_exchange_at",
|
||||
side_effect=race_delete,
|
||||
),
|
||||
self.assertRaises(DocForgeError) as delete_error,
|
||||
):
|
||||
delete_store.apply(
|
||||
changeset_id="delete-race",
|
||||
expected_changeset_hash=str(delete["changeset_hash"]),
|
||||
applier_id="alpha-editor",
|
||||
application=GenericCanonicalApplier(delete_project).apply,
|
||||
)
|
||||
self.assertEqual("base_conflict", delete_error.exception.code)
|
||||
self.assertIn("# foreign delete edit", delete_target.read_text(encoding="utf-8"))
|
||||
self.assertFalse(tuple(delete_target.parent.glob(".docforge-apply-*")))
|
||||
|
||||
def test_rollback_never_clobbers_a_foreign_edit_and_retains_original_bytes(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as directory:
|
||||
root = self.copy_fixture(Path(directory))
|
||||
project = Project.open(root)
|
||||
store = ChangesetStore(project, "alpha-editor")
|
||||
proposal = store.register(
|
||||
"rollback-race",
|
||||
[
|
||||
{
|
||||
"operation": "update",
|
||||
"node_id": "guide.foundation",
|
||||
"metadata": {"summary": "First approved update."},
|
||||
"rationale": "Publish before the synthetic failure.",
|
||||
},
|
||||
{
|
||||
"operation": "update",
|
||||
"node_id": "guide.workflow",
|
||||
"metadata": {"summary": "Second approved update."},
|
||||
"rationale": "Trigger rollback after the first publication.",
|
||||
},
|
||||
],
|
||||
)
|
||||
first_target = root / "docs/content/foundation.md"
|
||||
first_before = first_target.read_bytes()
|
||||
publish = GenericCanonicalApplier._publish
|
||||
calls = 0
|
||||
|
||||
def fail_after_foreign_edit(
|
||||
applier: GenericCanonicalApplier,
|
||||
publication: Any,
|
||||
) -> None:
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
if calls == 1:
|
||||
publish(applier, publication)
|
||||
first_target.write_bytes(
|
||||
first_target.read_bytes() + b"\nForeign edit after publication.\n"
|
||||
)
|
||||
return
|
||||
raise DocForgeError("application_failure", "Synthetic second-target failure")
|
||||
|
||||
with (
|
||||
mock.patch.object(
|
||||
GenericCanonicalApplier,
|
||||
"_publish",
|
||||
autospec=True,
|
||||
side_effect=fail_after_foreign_edit,
|
||||
),
|
||||
self.assertRaises(DocForgeError) as captured,
|
||||
):
|
||||
store.apply(
|
||||
changeset_id="rollback-race",
|
||||
expected_changeset_hash=str(proposal["changeset_hash"]),
|
||||
applier_id="alpha-editor",
|
||||
application=GenericCanonicalApplier(project).apply,
|
||||
)
|
||||
|
||||
self.assertEqual("application_recovery_required", captured.exception.code)
|
||||
conflicts = captured.exception.details["conflicts"]
|
||||
self.assertEqual("target_or_backup_changed", conflicts[0]["reason"])
|
||||
self.assertIn(
|
||||
"Foreign edit after publication.",
|
||||
first_target.read_text(encoding="utf-8"),
|
||||
)
|
||||
retained = root / conflicts[0]["retained"]
|
||||
self.assertTrue(retained.is_file())
|
||||
self.assertEqual(first_before, retained.read_bytes())
|
||||
self.assertFalse((root / ".docforge/changesets/.state/rollback-race.json").exists())
|
||||
|
||||
def test_canonical_update_preserves_existing_file_mode(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as directory:
|
||||
root = self.copy_fixture(Path(directory))
|
||||
target = root / "docs/content/workflow.md"
|
||||
target.chmod(0o640)
|
||||
project = Project.open(root)
|
||||
store = ChangesetStore(project, "alpha-editor")
|
||||
proposal = store.register(
|
||||
"mode",
|
||||
[
|
||||
{
|
||||
"operation": "update",
|
||||
"node_id": "guide.workflow",
|
||||
"metadata": {"summary": "Mode-preserving update."},
|
||||
"rationale": "Preserve canonical file permissions.",
|
||||
}
|
||||
],
|
||||
)
|
||||
|
||||
result = store.apply(
|
||||
changeset_id="mode",
|
||||
expected_changeset_hash=str(proposal["changeset_hash"]),
|
||||
applier_id="alpha-editor",
|
||||
application=GenericCanonicalApplier(project).apply,
|
||||
)
|
||||
|
||||
self.assertTrue(result["applied"])
|
||||
self.assertEqual(0o640, stat.S_IMODE(target.stat().st_mode))
|
||||
self.assertEqual([], result["retained_recovery_files"])
|
||||
|
||||
def test_changeset_rollback_fsyncs_the_parent_directory(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as directory:
|
||||
root = Path(directory)
|
||||
target = root / "proposal.json"
|
||||
real_fsync = os.fsync
|
||||
fsynced_modes: list[int] = []
|
||||
|
||||
def record_fsync(descriptor: int) -> None:
|
||||
fsynced_modes.append(os.fstat(descriptor).st_mode)
|
||||
real_fsync(descriptor)
|
||||
|
||||
for previous in (b"previous proposal\n", None):
|
||||
with self.subTest(previous=previous):
|
||||
target.write_bytes(b"replacement proposal\n")
|
||||
fsynced_modes.clear()
|
||||
|
||||
with mock.patch(
|
||||
"docforge.changesets.os.fsync",
|
||||
side_effect=record_fsync,
|
||||
):
|
||||
ChangesetStore._restore(target, previous, root)
|
||||
|
||||
self.assertTrue(any(stat.S_ISDIR(mode) for mode in fsynced_modes))
|
||||
if previous is None:
|
||||
self.assertFalse(target.exists())
|
||||
else:
|
||||
self.assertEqual(previous, target.read_bytes())
|
||||
|
||||
def test_abandoned_proposal_releases_overlap_and_stale_work_remains_active(self) -> None:
|
||||
with tempfile.TemporaryDirectory() as directory:
|
||||
root = self.copy_fixture(Path(directory))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue