fix(manual-ledger): harden schema init race and sanitize error responses
Co-authored-by: genichin <4282227+genichin@users.noreply.github.com>
This commit is contained in:
co-authored by
genichin
parent
78e347703e
commit
a36b451a3d
@@ -736,6 +736,22 @@ def _extract_duplicate_dividend_entry_id(message: str) -> int | None:
|
|||||||
return None
|
return None
|
||||||
|
|
||||||
|
|
||||||
|
def _manual_error_message(code: str) -> str:
|
||||||
|
if code == "LEDGER_ENTRY_NOT_FOUND":
|
||||||
|
return "Target ledger entry not found"
|
||||||
|
if code == "POTENTIAL_DUPLICATE_DIVIDEND":
|
||||||
|
return "Potential duplicate dividend entry detected"
|
||||||
|
if code == "INSUFFICIENT_CASH":
|
||||||
|
return "Insufficient cash balance"
|
||||||
|
if code == "INSUFFICIENT_QUANTITY":
|
||||||
|
return "Insufficient asset quantity"
|
||||||
|
if code == "INSUFFICIENT_BUFFER":
|
||||||
|
return "Insufficient profit buffer"
|
||||||
|
if code == "LEDGER_ENTRY_ALREADY_REVERSED":
|
||||||
|
return "Ledger entry is already reversed"
|
||||||
|
return "Manual ledger request could not be processed"
|
||||||
|
|
||||||
|
|
||||||
def _encode_ledger_cursor(*, portfolio_id: str, source: str | None, recorded_at: datetime, ledger_entry_id: int) -> str:
|
def _encode_ledger_cursor(*, portfolio_id: str, source: str | None, recorded_at: datetime, ledger_entry_id: int) -> str:
|
||||||
payload = {
|
payload = {
|
||||||
"portfolio_id": portfolio_id,
|
"portfolio_id": portfolio_id,
|
||||||
@@ -855,17 +871,17 @@ def record_manual_ledger_v1(
|
|||||||
code = _manual_error_code(message)
|
code = _manual_error_code(message)
|
||||||
if code == "LEDGER_ENTRY_NOT_FOUND":
|
if code == "LEDGER_ENTRY_NOT_FOUND":
|
||||||
status_code = status.HTTP_404_NOT_FOUND
|
status_code = status.HTTP_404_NOT_FOUND
|
||||||
detail: str | dict[str, object] = {"code": code, "message": message}
|
detail: str | dict[str, object] = {"code": code, "message": _manual_error_message(code)}
|
||||||
elif code == "POTENTIAL_DUPLICATE_DIVIDEND":
|
elif code == "POTENTIAL_DUPLICATE_DIVIDEND":
|
||||||
status_code = status.HTTP_409_CONFLICT
|
status_code = status.HTTP_409_CONFLICT
|
||||||
detail = {
|
detail = {
|
||||||
"code": code,
|
"code": code,
|
||||||
"message": "Potential duplicate dividend entry detected",
|
"message": _manual_error_message(code),
|
||||||
"matched_ledger_entry_id": _extract_duplicate_dividend_entry_id(message),
|
"matched_ledger_entry_id": _extract_duplicate_dividend_entry_id(message),
|
||||||
}
|
}
|
||||||
else:
|
else:
|
||||||
status_code = status.HTTP_409_CONFLICT if code != "MANUAL_LEDGER_CONFLICT" else status.HTTP_422_UNPROCESSABLE_ENTITY
|
status_code = status.HTTP_409_CONFLICT if code != "MANUAL_LEDGER_CONFLICT" else status.HTTP_422_UNPROCESSABLE_ENTITY
|
||||||
detail = {"code": code, "message": message}
|
detail = {"code": code, "message": _manual_error_message(code)}
|
||||||
raise HTTPException(status_code=status_code, detail=detail) from exc
|
raise HTTPException(status_code=status_code, detail=detail) from exc
|
||||||
finally:
|
finally:
|
||||||
_close_repo(repo)
|
_close_repo(repo)
|
||||||
|
|||||||
@@ -206,6 +206,7 @@ class PostgresPortfolioRepository:
|
|||||||
def ensure_schema(self) -> None:
|
def ensure_schema(self) -> None:
|
||||||
conn = self._connect()
|
conn = self._connect()
|
||||||
with conn.cursor() as cur:
|
with conn.cursor() as cur:
|
||||||
|
cur.execute("SELECT pg_advisory_lock(hashtext('portfoliohub.ensure_schema.v1'))")
|
||||||
cur.execute(
|
cur.execute(
|
||||||
"""
|
"""
|
||||||
CREATE TABLE IF NOT EXISTS users (
|
CREATE TABLE IF NOT EXISTS users (
|
||||||
@@ -507,6 +508,7 @@ class PostgresPortfolioRepository:
|
|||||||
SELECT create_hypertable('portfolio_ledger', 'recorded_at', if_not_exists => TRUE)
|
SELECT create_hypertable('portfolio_ledger', 'recorded_at', if_not_exists => TRUE)
|
||||||
"""
|
"""
|
||||||
)
|
)
|
||||||
|
cur.execute("SELECT pg_advisory_unlock(hashtext('portfoliohub.ensure_schema.v1'))")
|
||||||
|
|
||||||
@staticmethod
|
@staticmethod
|
||||||
def _row_to_user(row: Any) -> User:
|
def _row_to_user(row: Any) -> User:
|
||||||
|
|||||||
@@ -588,4 +588,4 @@ API가 수용한 최대 8자리 소수의 `amount`와 `tax`는 PostgreSQL 저장
|
|||||||
|
|
||||||
- 상태: `approved`
|
- 상태: `approved`
|
||||||
- 승인 주체: `genichin`
|
- 승인 주체: `genichin`
|
||||||
- 승인 근거: 이 대화에서 `REQ-001` 승인과 OQ-001 선택지 A를 명시적으로 확정함
|
- 승인 근거: 이 대화에서 `REQ-001` revision `1.0` 승인과 비-`DIVIDEND` `confirm_duplicate=false` 허용/`true` HTTP 422 결정을 명시적으로 확정함 (`#31` OQ-001 결정 B와 정합)
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ or production application database.
|
|||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import os
|
import os
|
||||||
|
import threading
|
||||||
from datetime import UTC, datetime
|
from datetime import UTC, datetime
|
||||||
from decimal import Decimal
|
from decimal import Decimal
|
||||||
from urllib.parse import unquote, urlparse
|
from urllib.parse import unquote, urlparse
|
||||||
@@ -407,3 +408,58 @@ def test_large_invest_and_large_dividend_tax_metadata_are_accepted_when_snapshot
|
|||||||
)
|
)
|
||||||
assert dividend.tax_amount == 50000000.0
|
assert dividend.tax_amount == 50000000.0
|
||||||
repo.close()
|
repo.close()
|
||||||
|
|
||||||
|
|
||||||
|
def test_cold_repositories_concurrent_dividend_requests_follow_duplicate_contract(
|
||||||
|
isolated_settings: Settings,
|
||||||
|
) -> None:
|
||||||
|
"""Cold concurrent writes should not fail on ensure_schema DDL race."""
|
||||||
|
upgrade_to_head(isolated_settings)
|
||||||
|
seed_repo = _repo(isolated_settings)
|
||||||
|
portfolio_id = _seed_portfolio(seed_repo, "cold-concurrent-dividend")
|
||||||
|
seed_repo.close()
|
||||||
|
barrier = threading.Barrier(2)
|
||||||
|
results: list[tuple[str, object]] = []
|
||||||
|
|
||||||
|
def _worker(note: str) -> None:
|
||||||
|
repo = _repo(isolated_settings)
|
||||||
|
try:
|
||||||
|
barrier.wait()
|
||||||
|
entry = repo.record_manual_ledger(
|
||||||
|
portfolio_id,
|
||||||
|
action="DIVIDEND",
|
||||||
|
amount=Decimal("1.00000000"),
|
||||||
|
executed_at=datetime(2026, 9, 1, 0, 0, 0, tzinfo=UTC),
|
||||||
|
note=note,
|
||||||
|
)
|
||||||
|
results.append(("ok", entry))
|
||||||
|
except ValueError as exc:
|
||||||
|
results.append(("error", str(exc)))
|
||||||
|
finally:
|
||||||
|
repo.close()
|
||||||
|
|
||||||
|
t1 = threading.Thread(target=_worker, args=("same-note",))
|
||||||
|
t2 = threading.Thread(target=_worker, args=("same-note",))
|
||||||
|
t1.start()
|
||||||
|
t2.start()
|
||||||
|
t1.join()
|
||||||
|
t2.join()
|
||||||
|
|
||||||
|
assert len(results) == 2
|
||||||
|
successes = [value for kind, value in results if kind == "ok"]
|
||||||
|
errors = [value for kind, value in results if kind == "error"]
|
||||||
|
assert len(successes) == 1
|
||||||
|
assert len(errors) == 1
|
||||||
|
assert "POTENTIAL_DUPLICATE_DIVIDEND:" in str(errors[0])
|
||||||
|
assert "duplicate key value" not in str(errors[0]).lower()
|
||||||
|
|
||||||
|
verify_repo = _repo(isolated_settings)
|
||||||
|
with verify_repo._connect().cursor() as cur:
|
||||||
|
cur.execute(
|
||||||
|
"SELECT COUNT(*) FROM portfolio_ledger WHERE portfolio_id = %s AND side = 'dividend'",
|
||||||
|
(portfolio_id,),
|
||||||
|
)
|
||||||
|
assert cur.fetchone()[0] == 1
|
||||||
|
cur.execute("SELECT profit_buffer::text FROM portfolios WHERE portfolio_id = %s", (portfolio_id,))
|
||||||
|
assert cur.fetchone()[0] == "1"
|
||||||
|
verify_repo.close()
|
||||||
|
|||||||
@@ -386,6 +386,31 @@ def test_insufficient_cash_rejected_as_409() -> None:
|
|||||||
|
|
||||||
assert resp.status_code == 409
|
assert resp.status_code == 409
|
||||||
assert resp.json()["detail"]["code"] == "INSUFFICIENT_CASH"
|
assert resp.json()["detail"]["code"] == "INSUFFICIENT_CASH"
|
||||||
|
assert resp.json()["detail"]["message"] == "Insufficient cash balance"
|
||||||
|
|
||||||
|
|
||||||
|
def test_manual_ledger_conflict_message_is_sanitized() -> None:
|
||||||
|
repo = _FakeRepo(_portfolio())
|
||||||
|
client = _make_client(repo)
|
||||||
|
|
||||||
|
secret = "SENTINEL_SECRET_TOKEN_12345"
|
||||||
|
|
||||||
|
def _raise_secret(*args: Any, **kwargs: Any) -> LedgerEntry:
|
||||||
|
raise ValueError(f"synthetic backend failure: {secret}")
|
||||||
|
|
||||||
|
repo.record_manual_ledger = _raise_secret # type: ignore[method-assign]
|
||||||
|
|
||||||
|
resp = client.post(
|
||||||
|
f"/v1/portfolios/{_PORTFOLIO_ID}/ledger/manual",
|
||||||
|
json={"action": "DIVIDEND", "amount": 1, "executed_at": "2026-09-01"},
|
||||||
|
headers=_auth(),
|
||||||
|
)
|
||||||
|
|
||||||
|
assert resp.status_code == 422
|
||||||
|
detail = resp.json()["detail"]
|
||||||
|
assert detail["code"] == "MANUAL_LEDGER_CONFLICT"
|
||||||
|
assert detail["message"] == "Manual ledger request could not be processed"
|
||||||
|
assert secret not in resp.text
|
||||||
|
|
||||||
|
|
||||||
def test_manual_history_returns_total_value_after() -> None:
|
def test_manual_history_returns_total_value_after() -> None:
|
||||||
|
|||||||
Reference in New Issue
Block a user