Browse Source

fix: publish exports without clobbering

master
Codex 2 months ago
parent
commit
7538e739b9
  1. 34
      finance/exports.py
  2. 104
      tests/finance/test_exports.py
  3. 61
      tests/test_export_command.py

34
finance/exports.py

@ -194,28 +194,12 @@ def write_export(name: str | None = None) -> ExportResult:
destination = root / name
if destination.is_symlink():
raise ExportError("invalid_name")
if destination.exists():
raise ExportError("existing_destination")
reservation_fd = None
owns_reservation = False
temporary_path = None
published = False
phase = "reservation"
phase = "snapshot"
try:
try:
reservation_fd = os.open(
destination,
os.O_WRONLY | os.O_CREAT | os.O_EXCL,
0o600,
)
owns_reservation = True
except FileExistsError:
raise ExportError("target_exists") from None
finally:
if reservation_fd is not None:
os.close(reservation_fd)
reservation_fd = None
phase = "snapshot"
artifact = build_export()
phase = "publication"
temporary_fd, temporary_name = tempfile.mkstemp(
@ -226,9 +210,12 @@ def write_export(name: str | None = None) -> ExportResult:
stream.write(artifact.content)
stream.flush()
os.fsync(stream.fileno())
os.replace(temporary_path, destination)
try:
os.link(temporary_path, destination)
except FileExistsError:
raise ExportError("existing_destination") from None
temporary_path.unlink()
temporary_path = None
published = True
return ExportResult(
name=name,
path=destination,
@ -246,11 +233,6 @@ def write_export(name: str | None = None) -> ExportResult:
temporary_path.unlink(missing_ok=True)
except OSError:
pass
if owns_reservation and not published:
try:
destination.unlink(missing_ok=True)
except OSError:
pass
def _csv_content(model, fields):

104
tests/finance/test_exports.py

@ -1,5 +1,6 @@
import csv
import json
import threading
from datetime import date, datetime, timezone as datetime_timezone
from decimal import Decimal
from hashlib import sha256
@ -8,7 +9,9 @@ from uuid import UUID
from zipfile import ZipFile
import pytest
from django.db import connections, transaction
import finance.exports as exports
from finance.exports import build_export
from finance.models import (
Account,
@ -35,11 +38,26 @@ EXPECTED_FILES = [
"transfers.csv",
"source_records.csv",
]
EXPECTED_TRANSACTION_HEADER = (
"id,account_id,amount,posting_date,occurrence_date,description,kind,payee_id,"
"category_id,statement_id,refund_of_id,installment_plan_id,installment_number,"
"created_at,updated_at"
)
EXPECTED_HEADERS = {
"accounts.csv": "id,name,kind,currency,archived,created_at,updated_at",
"payees.csv": "id,name,archived,created_at,updated_at",
"categories.csv": "id,name,kind,archived,created_at,updated_at",
"statements.csv": (
"id,account_id,period_start,period_end,due_date,statement_balance,created_at,updated_at"
),
"installment_plans.csv": (
"id,account_id,description,total_installments,created_at,updated_at"
),
"transactions.csv": (
"id,account_id,amount,posting_date,occurrence_date,description,kind,payee_id,"
"category_id,statement_id,refund_of_id,installment_plan_id,installment_number,"
"created_at,updated_at"
),
"transfers.csv": "id,outgoing_id,incoming_id,created_at,updated_at",
"source_records.csv": (
"id,transaction_id,document_hash,source_format,parser_version,item_locator,created_at"
),
}
def rows(package, name):
@ -156,9 +174,9 @@ def test_export_has_versioned_manifest_and_stable_csv_contract():
assert manifest["version"] == 1
assert manifest["application_version"] == "0.1.0"
assert list(manifest["files"]) == EXPECTED_FILES
assert package.read("transactions.csv").decode("utf-8").splitlines()[0] == EXPECTED_TRANSACTION_HEADER
for name in EXPECTED_FILES:
content = package.read(name)
assert content.decode("utf-8").splitlines()[0] == EXPECTED_HEADERS[name]
assert manifest["files"][name]["sha256"] == sha256(content).hexdigest()
assert manifest["files"][name]["rows"] == len(rows(package, name))
@ -218,3 +236,77 @@ def test_export_is_byte_stable_for_the_same_snapshot(monkeypatch):
assert first.filename == second.filename == "gizmoduck-export-20260907T123000Z.zip"
assert first.content == second.content
def test_export_uses_one_repeatable_read_snapshot_across_related_files(monkeypatch):
seed_complete_export()
accounts_read = threading.Event()
writer_finished = threading.Event()
writer_errors = []
new_account_id = UUID("10000000-0000-4000-8000-000000000003")
new_payee_id = UUID("20000000-0000-4000-8000-000000000002")
new_transaction_id = UUID("60000000-0000-4000-8000-000000000005")
new_source_id = UUID("80000000-0000-4000-8000-000000000002")
original_csv_content = exports._csv_content
def pause_after_accounts(model, fields):
content = original_csv_content(model, fields)
if model is Account:
accounts_read.set()
assert writer_finished.wait(10), "separate writer did not finish"
return content
def commit_related_rows():
try:
assert accounts_read.wait(10), "export did not read accounts"
with transaction.atomic():
account = Account.objects.create(
id=new_account_id,
name="Synthetic Concurrent Account",
kind=Account.Kind.BANK,
currency="BRL",
)
payee = Payee.objects.create(
id=new_payee_id,
name="Synthetic Concurrent Payee",
)
row = Transaction.objects.create(
id=new_transaction_id,
account=account,
amount=Decimal("-7.25"),
posting_date=date(2026, 3, 1),
description="Synthetic concurrent transaction",
kind=Transaction.Kind.EXPENSE,
payee=payee,
)
SourceRecord.objects.create(
id=new_source_id,
transaction=row,
document_hash="b" * 64,
source_format=SourceRecord.Format.BANK_CSV,
parser_version="synthetic-v2",
item_locator="line:3",
)
except BaseException as exc:
writer_errors.append(exc)
finally:
connections.close_all()
writer_finished.set()
monkeypatch.setattr(exports, "_csv_content", pause_after_accounts)
writer = threading.Thread(target=commit_related_rows)
writer.start()
artifact = build_export()
writer.join(10)
assert not writer.is_alive()
assert writer_errors == []
with ZipFile(BytesIO(artifact.content)) as package:
assert str(new_account_id) not in {row["id"] for row in rows(package, "accounts.csv")}
assert str(new_payee_id) not in {row["id"] for row in rows(package, "payees.csv")}
assert str(new_transaction_id) not in {
row["id"] for row in rows(package, "transactions.csv")
}
assert str(new_source_id) not in {
row["id"] for row in rows(package, "source_records.csv")
}

61
tests/test_export_command.py

@ -1,4 +1,5 @@
import json
import threading
from datetime import datetime, timezone as datetime_timezone
from io import StringIO
@ -67,12 +68,66 @@ def test_write_export_rejects_existing_target_before_snapshot(export_root, monke
monkeypatch.setattr("finance.exports.build_export", unexpected_snapshot)
with pytest.raises(ExportError, match="target_exists"):
with pytest.raises(ExportError, match="existing_destination"):
write_export(existing.name)
assert existing.read_bytes() == b"original"
def test_write_export_does_not_expose_final_name_while_snapshot_is_built(
export_root, monkeypatch
):
destination = export_root / "private-until-complete.zip"
def inspect_during_snapshot():
assert not destination.exists()
return synthetic_artifact()
monkeypatch.setattr("finance.exports.build_export", inspect_during_snapshot)
write_export(destination.name)
assert destination.read_bytes() == b"synthetic zip bytes"
def test_write_export_preserves_final_file_created_concurrently(
export_root, monkeypatch
):
destination = export_root / "concurrent.zip"
snapshot_started = threading.Event()
competing_file_created = threading.Event()
writer_errors = []
def concurrent_snapshot():
snapshot_started.set()
assert competing_file_created.wait(5), "competing writer did not finish"
return synthetic_artifact()
def publish_competing_file():
try:
assert snapshot_started.wait(5), "export did not start its snapshot"
destination.write_bytes(b"concurrent writer")
except BaseException as exc:
writer_errors.append(exc)
finally:
competing_file_created.set()
monkeypatch.setattr("finance.exports.build_export", concurrent_snapshot)
writer = threading.Thread(target=publish_competing_file)
writer.start()
try:
with pytest.raises(ExportError, match="existing_destination"):
write_export(destination.name)
finally:
writer.join(5)
assert not writer.is_alive()
assert writer_errors == []
assert destination.read_bytes() == b"concurrent writer"
assert list(export_root.iterdir()) == [destination]
def test_write_export_rejects_destination_symlink(export_root, monkeypatch):
outside = export_root.parent / "outside-export.zip"
outside.write_bytes(b"outside original")
@ -108,10 +163,10 @@ def test_write_export_removes_reservation_and_temporary_file_on_snapshot_failure
def test_write_export_cleans_up_if_atomic_publication_fails(export_root, monkeypatch):
monkeypatch.setattr("finance.exports.build_export", synthetic_artifact)
def failed_replace(_source, _destination):
def failed_link(_source, _destination):
raise OSError("SYNTHETIC PRIVATE FILESYSTEM DETAIL")
monkeypatch.setattr("finance.exports.os.replace", failed_replace)
monkeypatch.setattr("finance.exports.os.link", failed_link)
with pytest.raises(ExportError, match="publication_failed") as caught:
write_export("failed.zip")

Loading…
Cancel
Save