feat: persist authored action idempotency (#201)
This commit is contained in:
1 parent
4cc00d2e48
commit
a5f63a6fa6
7 files changed
+417
-80
No files matched your search
@@ -4,13 +4,16 @@ import httpx
|
||||
import pytest
|
||||
|
||||
from src import main
|
||||
from src.idempotency import IdempotencyLedger
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def clear_authored_operations():
|
||||
main._authored_action_operations.clear()
|
||||
main._idempotency_ledger.clear()
|
||||
yield
|
||||
main._authored_action_operations.clear()
|
||||
main._idempotency_ledger.clear()
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
@@ -140,3 +143,88 @@ async def test_review_replays_same_key_and_rejects_changed_head(monkeypatch):
|
||||
|
||||
assert [created.status_code, replayed.status_code, conflict.status_code] == [201, 201, 409]
|
||||
assert calls == [("abc", "approve", "Good")]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_completed_comment_replays_after_ledger_reconstruction(monkeypatch, tmp_path):
|
||||
calls = 0
|
||||
|
||||
async def assigned(_repository, _number):
|
||||
return True
|
||||
|
||||
async def comment(_repository, _number, body):
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
return {"id": 201, "body": body}
|
||||
|
||||
database = tmp_path / "actions.sqlite3"
|
||||
monkeypatch.setattr(
|
||||
main,
|
||||
"_idempotency_ledger",
|
||||
IdempotencyLedger(database, ttl_seconds=600, max_entries=256),
|
||||
raising=False,
|
||||
)
|
||||
monkeypatch.setattr(main.gitea_proxy, "is_assigned_issue", assigned)
|
||||
monkeypatch.setattr(main.gitea_proxy, "comment_on_issue", comment)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
headers = {"Idempotency-Key": "restart-comment-201"}
|
||||
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
created = await client.post(
|
||||
"/api/v1/repos/stackchain/api/issues/7/comments",
|
||||
json={"body": "Persist me"},
|
||||
headers=headers,
|
||||
)
|
||||
main._authored_action_operations.clear()
|
||||
monkeypatch.setattr(
|
||||
main,
|
||||
"_idempotency_ledger",
|
||||
IdempotencyLedger(database, ttl_seconds=600, max_entries=256),
|
||||
)
|
||||
replayed = await client.post(
|
||||
"/api/v1/repos/stackchain/api/issues/7/comments",
|
||||
json={"body": "Persist me"},
|
||||
headers=headers,
|
||||
)
|
||||
|
||||
assert [created.status_code, replayed.status_code] == [201, 201]
|
||||
assert replayed.json() == {"id": 201, "body": "Persist me"}
|
||||
assert calls == 1
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_orphaned_pending_comment_fails_closed_without_upstream_retry(
|
||||
monkeypatch, tmp_path
|
||||
):
|
||||
calls = 0
|
||||
|
||||
async def assigned(_repository, _number):
|
||||
return True
|
||||
|
||||
async def comment(_repository, _number, body):
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
return {"id": 202, "body": body}
|
||||
|
||||
ledger = IdempotencyLedger(
|
||||
tmp_path / "orphan.sqlite3", ttl_seconds=600, max_entries=256
|
||||
)
|
||||
fingerprint = ("issue-comment", "stackchain/api", 7, "Verify first")
|
||||
assert ledger.reserve("orphan-comment-201", fingerprint).state == "reserved"
|
||||
monkeypatch.setattr(main, "_idempotency_ledger", ledger)
|
||||
monkeypatch.setattr(main.gitea_proxy, "is_assigned_issue", assigned)
|
||||
monkeypatch.setattr(main.gitea_proxy, "comment_on_issue", comment)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
response = await client.post(
|
||||
"/api/v1/repos/stackchain/api/issues/7/comments",
|
||||
json={"body": "Verify first"},
|
||||
headers={"Idempotency-Key": "orphan-comment-201"},
|
||||
)
|
||||
|
||||
assert response.status_code == 503
|
||||
assert response.json()["detail"] == (
|
||||
"This action may still be completing; verify its result before retrying"
|
||||
)
|
||||
assert calls == 0
|
||||
@@ -0,0 +1,73 @@
|
||||
import sqlite3
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
from threading import Barrier
|
||||
|
||||
from src.idempotency import IdempotencyLedger
|
||||
|
||||
|
||||
def test_completed_result_replays_after_ledger_reconstruction(tmp_path):
|
||||
database = tmp_path / "idempotency.sqlite3"
|
||||
first = IdempotencyLedger(database, ttl_seconds=600, max_entries=256)
|
||||
|
||||
reservation = first.reserve("create-201", ("issue-create", "stackchain/api", "Ship it"))
|
||||
assert reservation.state == "reserved"
|
||||
first.complete("create-201", {"number": 42, "title": "Ship it"})
|
||||
|
||||
reconstructed = IdempotencyLedger(database, ttl_seconds=600, max_entries=256)
|
||||
replay = reconstructed.reserve(
|
||||
"create-201", ("issue-create", "stackchain/api", "Ship it")
|
||||
)
|
||||
|
||||
assert replay.state == "completed"
|
||||
assert replay.response == {"number": 42, "title": "Ship it"}
|
||||
assert sqlite3.connect(database).execute(
|
||||
"SELECT status FROM idempotency_operations WHERE key = ?", ("create-201",)
|
||||
).fetchone() == ("completed",)
|
||||
|
||||
|
||||
def test_reservation_expires_completed_rows_and_enforces_capacity(tmp_path):
|
||||
now = [1_000.0]
|
||||
ledger = IdempotencyLedger(
|
||||
tmp_path / "bounded.sqlite3",
|
||||
ttl_seconds=10,
|
||||
max_entries=2,
|
||||
clock=lambda: now[0],
|
||||
)
|
||||
assert ledger.reserve("old", ("comment", "old")).state == "reserved"
|
||||
ledger.complete("old", {"id": 1})
|
||||
now[0] += 11
|
||||
|
||||
assert ledger.reserve("new-1", ("comment", "new-1")).state == "reserved"
|
||||
ledger.complete("new-1", {"id": 2})
|
||||
assert ledger.reserve("new-2", ("comment", "new-2")).state == "reserved"
|
||||
ledger.complete("new-2", {"id": 3})
|
||||
replacement = ledger.reserve("new-3", ("comment", "new-3"))
|
||||
|
||||
assert replacement.state == "reserved"
|
||||
with sqlite3.connect(ledger.path) as connection:
|
||||
keys = connection.execute(
|
||||
"SELECT key FROM idempotency_operations ORDER BY key"
|
||||
).fetchall()
|
||||
indexes = connection.execute(
|
||||
"PRAGMA index_list(idempotency_operations)"
|
||||
).fetchall()
|
||||
assert keys == [("new-2",), ("new-3",)]
|
||||
assert any(index[1] == "idempotency_completed_at_idx" for index in indexes)
|
||||
|
||||
|
||||
def test_independent_connections_atomically_reserve_one_operation(tmp_path):
|
||||
database = tmp_path / "workers.sqlite3"
|
||||
ledgers = [
|
||||
IdempotencyLedger(database, ttl_seconds=600, max_entries=256)
|
||||
for _ in range(2)
|
||||
]
|
||||
barrier = Barrier(2)
|
||||
|
||||
def reserve(ledger):
|
||||
barrier.wait()
|
||||
return ledger.reserve("worker-key-201", ("review", "stackchain/api", 7)).state
|
||||
|
||||
with ThreadPoolExecutor(max_workers=2) as executor:
|
||||
states = list(executor.map(reserve, ledgers))
|
||||
|
||||
assert sorted(states) == ["pending", "reserved"]
|
||||
+72
-6
@@ -5,6 +5,7 @@ import httpx
|
||||
import pytest
|
||||
|
||||
from src import gitea_proxy, main
|
||||
from src.idempotency import IdempotencyLedger
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
@@ -226,9 +227,11 @@ async def test_edit_assigned_issue_reports_revision_conflict_without_mutation(mo
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def clear_issue_creation_operations():
|
||||
main._issue_creation_operations.clear()
|
||||
main._authored_action_operations.clear()
|
||||
main._idempotency_ledger.clear()
|
||||
yield
|
||||
main._issue_creation_operations.clear()
|
||||
main._authored_action_operations.clear()
|
||||
main._idempotency_ledger.clear()
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
@@ -331,6 +334,61 @@ async def test_create_issue_replays_one_upstream_result_for_concurrent_idempoten
|
||||
assert calls == 1
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_completed_issue_creation_replays_after_ledger_reconstruction(monkeypatch, tmp_path):
|
||||
calls = 0
|
||||
|
||||
async def user():
|
||||
return {"login": "timmy"}
|
||||
|
||||
async def available_repos():
|
||||
return [{"full_name": "stackchain/api"}]
|
||||
|
||||
async def create(repository, title, body, assignee, label_ids):
|
||||
nonlocal calls
|
||||
calls += 1
|
||||
return {
|
||||
"id": 201, "number": 201, "title": title, "state": "open",
|
||||
"repository": repository, "labels": [], "assignees": [assignee],
|
||||
"updated_at": "2026-08-07T15:00:00Z",
|
||||
"url": "https://forge.example/stackchain/api/issues/201",
|
||||
}
|
||||
|
||||
database = tmp_path / "issues.sqlite3"
|
||||
monkeypatch.setattr(
|
||||
main,
|
||||
"_idempotency_ledger",
|
||||
IdempotencyLedger(database, ttl_seconds=600, max_entries=256),
|
||||
)
|
||||
monkeypatch.setattr(main.gitea_proxy, "current_user", user)
|
||||
monkeypatch.setattr(main.gitea_proxy, "repos", available_repos)
|
||||
monkeypatch.setattr(main.gitea_proxy, "create_issue", create)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
headers = {"Idempotency-Key": "restart-create-201"}
|
||||
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
created = await client.post(
|
||||
"/api/v1/repos/stackchain/api/issues",
|
||||
json={"title": "Durable capture"},
|
||||
headers=headers,
|
||||
)
|
||||
main._authored_action_operations.clear()
|
||||
monkeypatch.setattr(
|
||||
main,
|
||||
"_idempotency_ledger",
|
||||
IdempotencyLedger(database, ttl_seconds=600, max_entries=256),
|
||||
)
|
||||
replayed = await client.post(
|
||||
"/api/v1/repos/stackchain/api/issues",
|
||||
json={"title": "Durable capture"},
|
||||
headers=headers,
|
||||
)
|
||||
|
||||
assert [created.status_code, replayed.status_code] == [201, 201]
|
||||
assert replayed.json()["number"] == 201
|
||||
assert calls == 1
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_create_issue_rejects_changed_payload_for_an_existing_idempotency_key(monkeypatch):
|
||||
calls = []
|
||||
@@ -412,7 +470,9 @@ async def test_create_issue_retry_recovers_result_after_the_first_request_times_
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_create_issue_idempotency_registry_evicts_oldest_entry_at_size_limit(monkeypatch):
|
||||
async def test_create_issue_durable_ledger_evicts_completed_entry_at_size_limit(
|
||||
monkeypatch, tmp_path
|
||||
):
|
||||
async def user():
|
||||
return {"login": "timmy"}
|
||||
|
||||
@@ -427,7 +487,10 @@ async def test_create_issue_idempotency_registry_evicts_oldest_entry_at_size_lim
|
||||
"url": "https://forge.example/stackchain/api/issues/17",
|
||||
}
|
||||
|
||||
monkeypatch.setattr(main, "ISSUE_CREATION_IDEMPOTENCY_MAX_ENTRIES", 2)
|
||||
ledger = IdempotencyLedger(
|
||||
tmp_path / "bounded-issues.sqlite3", ttl_seconds=600, max_entries=2
|
||||
)
|
||||
monkeypatch.setattr(main, "_idempotency_ledger", ledger)
|
||||
monkeypatch.setattr(main.gitea_proxy, "current_user", user)
|
||||
monkeypatch.setattr(main.gitea_proxy, "repos", available_repos)
|
||||
monkeypatch.setattr(main.gitea_proxy, "create_issue", create)
|
||||
@@ -441,8 +504,11 @@ async def test_create_issue_idempotency_registry_evicts_oldest_entry_at_size_lim
|
||||
)
|
||||
assert response.status_code == 201
|
||||
|
||||
assert len(main._issue_creation_operations) == 2
|
||||
assert "capture-177-bounded-0" not in main._issue_creation_operations
|
||||
replay = ledger.reserve(
|
||||
"capture-177-bounded-0",
|
||||
("issue-create", "stackchain/api", "Capture 0", "", ()),
|
||||
)
|
||||
assert replay.state == "reserved"
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
|
||||
Reference in new issue
Block a user