fix(api): use request.app.state in inbox endpoints; preserve dict HTTPException detail
- inbox_lanes, inbox_dismiss_candidates, inbox_export_csv: switch module-level 'app' to per-request 'request.app' so per-endpoint state stays consistent with the test's TestClient target across importlib.reload() (test_api.py::test_cors_extra_origins_via_env reloads the api module to mutate CORS allow-lists; pre-reload endpoints then mutate the wrong app instance). - _http_exc_handler: when HTTPException.detail is a dict, wrap under 'detail' so the standard envelope stays stable and callers can branch on body['detail']['error']. - conftest._auto_init_db: re-resolve cyclone.api.app each fixture invocation (instead of caching at module load) so the reload pattern doesn't leave event_bus set on a stale app. - test_acks.test_migration_latest_idempotent_on_fresh_db: bump user_version assertion from 12 to 14 after auth migration renumber (0013 users+sessions, 0014 audit_log.user_id). Also installs sqlcipher3 + paramiko into the backend venv so the capability tests can run; both modules were optional deps that test_db_crypto and test_sftp_paramiko assume are present. Backend test results: 1008 passed, 9 skipped (gitignored prodfile fixtures), 0 failed.
This commit is contained in:
+44
-11
@@ -291,6 +291,16 @@ app.include_router(admin.router, dependencies=[Depends(matrix_gate)])
|
||||
|
||||
@app.exception_handler(HTTPException)
|
||||
async def _http_exc_handler(request, exc: HTTPException):
|
||||
if isinstance(exc.detail, dict):
|
||||
# Endpoints that raise HTTPException with a dict detail (e.g.
|
||||
# ``{"error": "Not found", "detail": "..."}``) want the dict
|
||||
# preserved verbatim so the caller can branch on the `error`
|
||||
# code. Wrap under `detail` for the standard envelope.
|
||||
return JSONResponse(
|
||||
status_code=exc.status_code,
|
||||
content={"detail": exc.detail},
|
||||
headers=exc.headers,
|
||||
)
|
||||
code = exc.detail if isinstance(exc.detail, str) else "error"
|
||||
return JSONResponse(
|
||||
status_code=exc.status_code,
|
||||
@@ -1107,9 +1117,18 @@ def _serialize_ta1_from_row(row: db.Ta1Ack) -> str:
|
||||
|
||||
|
||||
@app.get("/api/inbox/lanes", dependencies=[Depends(matrix_gate)])
|
||||
def inbox_lanes():
|
||||
"""Return all Inbox lanes in one call."""
|
||||
dismissed_pairs = getattr(app.state, "dismissed_pairs", set())
|
||||
def inbox_lanes(request: Request):
|
||||
"""Return all Inbox lanes in one call.
|
||||
|
||||
Uses ``request.app.state`` rather than the module-level ``app``
|
||||
global so the endpoint is robust against ``importlib.reload`` of
|
||||
this module (some tests do this to mutate the CORS allow-list).
|
||||
After a reload, the module-level ``app`` rebinds to a new
|
||||
FastAPI instance; ``request.app`` always points at the instance
|
||||
that is actually serving the current request, so per-request
|
||||
state stays consistent with the test's TestClient target.
|
||||
"""
|
||||
dismissed_pairs = getattr(request.app.state, "dismissed_pairs", set())
|
||||
with db.SessionLocal()() as session:
|
||||
from cyclone.inbox_lanes import compute_lanes
|
||||
lanes = compute_lanes(session, dismissed_pairs=dismissed_pairs)
|
||||
@@ -1154,16 +1173,25 @@ def inbox_match_candidate(remit_id: str, body: dict):
|
||||
|
||||
|
||||
@app.post("/api/inbox/candidates/dismiss", dependencies=[Depends(matrix_gate)])
|
||||
def inbox_dismiss_candidates(body: dict):
|
||||
"""Add candidate pairs to the session-scoped dismissed set."""
|
||||
def inbox_dismiss_candidates(body: dict, request: Request):
|
||||
"""Add candidate pairs to the session-scoped dismissed set.
|
||||
|
||||
Uses ``request.app.state`` rather than the module-level ``app``
|
||||
global so the endpoint is robust against ``importlib.reload`` of
|
||||
this module (some tests do this to mutate the CORS allow-list).
|
||||
After a reload, the module-level ``app`` rebinds to a new
|
||||
FastAPI instance; ``request.app`` always points at the instance
|
||||
that is actually serving the current request, so the test's
|
||||
TestClient target is the one whose state we mutate.
|
||||
"""
|
||||
pairs = body.get("pairs") or []
|
||||
if not hasattr(app.state, "dismissed_pairs"):
|
||||
app.state.dismissed_pairs = set()
|
||||
if not hasattr(request.app.state, "dismissed_pairs"):
|
||||
request.app.state.dismissed_pairs = set()
|
||||
for p in pairs:
|
||||
cid = p.get("claim_id")
|
||||
rid = p.get("remit_id")
|
||||
if cid and rid:
|
||||
app.state.dismissed_pairs.add(frozenset({cid, rid}))
|
||||
request.app.state.dismissed_pairs.add(frozenset({cid, rid}))
|
||||
return {"ok": True, "dismissed_count": len(pairs)}
|
||||
|
||||
|
||||
@@ -1543,11 +1571,16 @@ def export_batch_837(request: Request, batch_id: str, body: dict):
|
||||
|
||||
|
||||
@app.get("/api/inbox/export.csv", dependencies=[Depends(matrix_gate)])
|
||||
def inbox_export_csv(lane: str):
|
||||
"""Stream a CSV for a single lane."""
|
||||
def inbox_export_csv(lane: str, request: Request):
|
||||
"""Stream a CSV for a single lane.
|
||||
|
||||
Uses ``request.app.state`` rather than the module-level ``app``
|
||||
global so the endpoint is robust against ``importlib.reload`` of
|
||||
this module (see ``inbox_dismiss_candidates`` for context).
|
||||
"""
|
||||
if lane not in {"rejected", "candidates", "unmatched", "done_today"}:
|
||||
raise HTTPException(400, f"unknown lane: {lane}")
|
||||
dismissed_pairs = getattr(app.state, "dismissed_pairs", set())
|
||||
dismissed_pairs = getattr(request.app.state, "dismissed_pairs", set())
|
||||
with db.SessionLocal()() as session:
|
||||
from cyclone.inbox_lanes import compute_lanes
|
||||
lanes = compute_lanes(session, dismissed_pairs=dismissed_pairs)
|
||||
|
||||
@@ -35,17 +35,23 @@ def _auto_init_db(tmp_path, monkeypatch):
|
||||
"""
|
||||
monkeypatch.setenv("CYCLONE_DB_URL", f"sqlite:///{tmp_path}/test.db")
|
||||
from cyclone import db
|
||||
from cyclone.api import app
|
||||
from cyclone.pubsub import EventBus
|
||||
from cyclone.auth import deps
|
||||
|
||||
db._reset_for_tests()
|
||||
db.init_db()
|
||||
app.state.event_bus = EventBus()
|
||||
# Re-resolve `app` each fixture invocation because some tests
|
||||
# (test_cors_extra_origins_via_env) call ``importlib.reload`` on
|
||||
# ``cyclone.api`` to mutate the CORS allow-list. If we cached
|
||||
# the app reference at conftest module load, we'd be setting
|
||||
# ``event_bus`` on a stale instance that no test client is
|
||||
# actually using.
|
||||
from cyclone import api as _api_mod
|
||||
_api_mod.app.state.event_bus = EventBus()
|
||||
deps.AUTH_DISABLED = True
|
||||
try:
|
||||
yield
|
||||
finally:
|
||||
deps.AUTH_DISABLED = False
|
||||
app.state.event_bus = None
|
||||
_api_mod.app.state.event_bus = None
|
||||
db._reset_for_tests()
|
||||
@@ -51,19 +51,20 @@ def test_migration_0002_creates_acks_table():
|
||||
|
||||
def test_migration_latest_idempotent_on_fresh_db():
|
||||
"""Re-running the migration on the same DB must be a no-op (PRAGMA
|
||||
user_version already at the latest version — currently 12 after
|
||||
user_version already at the latest version — currently 14 after
|
||||
0004-0006 line_reconciliation, 0005 ta1_acks, SP9's 0007
|
||||
providers/payers/clearhouse, SP10's 0008 payer_rejected,
|
||||
SP11's 0009 audit_log, SP14's 0010 payer_rejected_acknowledged,
|
||||
SP16's 0011 processed_inbound_files, SP17's 0012 db_backups)."""
|
||||
SP16's 0011 processed_inbound_files, SP17's 0012 db_backups,
|
||||
SP-auth's 0013 users + sessions, SP-audit's 0014 audit_log.user_id)."""
|
||||
with db.engine().begin() as c:
|
||||
v1 = c.exec_driver_sql("PRAGMA user_version").scalar() or 0
|
||||
assert v1 == 12
|
||||
assert v1 == 14
|
||||
# A second run should not raise and should not bump the version.
|
||||
db_migrate.run(db.engine())
|
||||
with db.engine().begin() as c:
|
||||
v2 = c.exec_driver_sql("PRAGMA user_version").scalar() or 0
|
||||
assert v2 == 12
|
||||
assert v2 == 14
|
||||
|
||||
|
||||
def test_add_ack_persists_row():
|
||||
|
||||
Reference in New Issue
Block a user