Nora
abaf23c122
Followup #3 from the SP37 final-state tracker. The router's per-file try/except previously coerced every unexpected exception to SubmitOutcome.SFTP_FAILED — misleading because a RuntimeError from cycl_store.add has nothing to do with SFTP. Changes: * submission/result.py — add UNEXPECTED_ERROR = 'unexpected_error' enum value. Docstring distinguishes it from the typed SFTP_FAILED. * api_routers/submission.py — exception handler now returns UNEXPECTED_ERROR (was SFTP_FAILED). * test_api_submit_batch.py::test_submit_batch_unexpected_exception_recorded_in_results — updated assertion (was sftp_failed, now unexpected_error). * test_unexpected_error_outcome.py (new, 4 tests): - enum value exists - distinct from typed failure values - API surfaces 'unexpected_error' for uncaught exceptions - typed SFTP_FAILED path still surfaces 'sftp_failed' (regression lock) Tests: 40/40 pass in 1.41s (full SP37 chain + new file). Live curl: POST /api/submit-batch in stub mode → HTTP 409 (unchanged behavior, no regression on the config-level guards).
354 lines
12 KiB
Python
354 lines
12 KiB
Python
"""SP37 Task 6: POST /api/submit-batch endpoint.
|
|
|
|
Thin HTTP wrapper around ``cyclone.submission.submit_file``. Mirrors the
|
|
``cyclone submit-batch`` CLI walker exactly: each ``batch-*-claims/*.x12``
|
|
under ``ingest_dir`` is submitted in order, ``._*`` AppleDouble files are
|
|
skipped, ``limit`` truncates after collection.
|
|
|
|
Tests monkey-patch ``cyclone.api_routers.submission.submit_file`` so we
|
|
never touch the real paramiko factory or a live MFT.
|
|
"""
|
|
from __future__ import annotations
|
|
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
from fastapi.testclient import TestClient
|
|
|
|
from cyclone.submission.result import SubmitOutcome, SubmitResult
|
|
|
|
_FIXTURE = Path(__file__).parent / "fixtures" / "submit-batch" / "single-claim.x12"
|
|
|
|
|
|
@pytest.fixture
|
|
def client(tmp_path) -> TestClient:
|
|
"""Standard TestClient; conftest.py autouse handles DB + auth gate.
|
|
|
|
Does NOT touch env vars (per the plan-vs-reality correction in
|
|
Task 6's instructions). Seeds the clearhouse then flips
|
|
``sftp_block.stub`` to ``False`` so the file-walking tests reach
|
|
the walker (otherwise the default-seeded stub=True trips the 409
|
|
guard before any per-file work runs). The ``stub_mode`` test
|
|
re-seeds stub=True explicitly so its 409 still fires.
|
|
"""
|
|
from cyclone import db as db_mod
|
|
from cyclone.store import store as cycl_store
|
|
|
|
db_mod._reset_for_tests()
|
|
db_mod.init_db()
|
|
cycl_store.ensure_clearhouse_seeded()
|
|
ch = cycl_store.get_clearhouse()
|
|
# Flip stub=False so the walker actually runs (SFTP calls are
|
|
# intercepted via the submit_file monkey-patch in each test).
|
|
cycl_store.update_clearhouse(
|
|
ch.model_copy(update={"sftp_block": ch.sftp_block.model_copy(update={"stub": False})}),
|
|
)
|
|
|
|
from cyclone.api import app
|
|
|
|
return TestClient(app)
|
|
|
|
|
|
def _stage_batch(tmp_path: Path, n: int) -> Path:
|
|
"""Copy N copies of the single-claim fixture into ``batch-test-claims/``."""
|
|
batch_dir = tmp_path / "batch-test-claims"
|
|
batch_dir.mkdir()
|
|
payload = _FIXTURE.read_bytes()
|
|
for i in range(n):
|
|
(batch_dir / f"claim-{i}.x12").write_bytes(payload)
|
|
return batch_dir
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# Happy path + walker semantics
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_submit_batch_happy_path(client: TestClient, tmp_path, monkeypatch):
|
|
"""3 valid 837 files → 200 + body shape with submitted=3 (or skipped=3 on re-run).
|
|
|
|
Monkey-patches ``cyclone.api_routers.submission.submit_file`` to
|
|
return a synthetic SUBMITTED SubmitResult per file. Skipped stays
|
|
possible because SubmitResult's batch_id is what matters and the
|
|
fake returns it consistently — but a fresh DB should always land
|
|
in the SUBMITTED branch.
|
|
"""
|
|
_stage_batch(tmp_path, 3)
|
|
|
|
def _fake_submit(path, **_kw):
|
|
return SubmitResult(
|
|
file=path.name, outcome=SubmitOutcome.SUBMITTED, batch_id="b1",
|
|
)
|
|
|
|
monkeypatch.setattr(
|
|
"cyclone.api_routers.submission.submit_file", _fake_submit,
|
|
)
|
|
|
|
resp = client.post(
|
|
"/api/submit-batch",
|
|
json={"ingest_dir": str(tmp_path), "validate": False},
|
|
)
|
|
assert resp.status_code == 200, resp.text
|
|
body = resp.json()
|
|
# Tolerate either counter — counter totals are deterministic with a
|
|
# fresh DB, but the assertion below focuses on the structural shape
|
|
# the operator cares about: every file is accounted for.
|
|
assert body["submitted"] + body["skipped"] + body["failed"] == 3
|
|
assert isinstance(body["results"], list)
|
|
assert len(body["results"]) == 3
|
|
for row in body["results"]:
|
|
assert "file" in row
|
|
assert "outcome" in row
|
|
assert "batch_id" in row
|
|
assert "error" in row
|
|
|
|
|
|
def test_submit_batch_skips_apple_double_files(client: TestClient, tmp_path, monkeypatch):
|
|
"""``._foo.x12`` AppleDouble files must be skipped (mirrors the CLI walker)."""
|
|
batch_dir = tmp_path / "batch-test-claims"
|
|
batch_dir.mkdir()
|
|
(batch_dir / "real.x12").write_bytes(_FIXTURE.read_bytes())
|
|
(batch_dir / "._real.x12").write_bytes(b"\x00\x01\x02") # macOS AppleDouble noise
|
|
|
|
def _fake_submit(path, **_kw):
|
|
return SubmitResult(
|
|
file=path.name, outcome=SubmitOutcome.SUBMITTED, batch_id="b1",
|
|
)
|
|
|
|
monkeypatch.setattr(
|
|
"cyclone.api_routers.submission.submit_file", _fake_submit,
|
|
)
|
|
|
|
resp = client.post(
|
|
"/api/submit-batch",
|
|
json={"ingest_dir": str(tmp_path), "validate": False},
|
|
)
|
|
assert resp.status_code == 200, resp.text
|
|
body = resp.json()
|
|
# Only real.x12 was submitted (one call); AppleDouble was skipped.
|
|
assert len(body["results"]) == 1
|
|
assert body["results"][0]["file"] == "real.x12"
|
|
|
|
|
|
def test_submit_batch_limit_truncates(client: TestClient, tmp_path, monkeypatch):
|
|
"""``limit=1`` against 3 files → body has exactly 1 result."""
|
|
_stage_batch(tmp_path, 3)
|
|
|
|
submitted_files: list[str] = []
|
|
|
|
def _fake_submit(path, **_kw):
|
|
submitted_files.append(path.name)
|
|
return SubmitResult(
|
|
file=path.name, outcome=SubmitOutcome.SUBMITTED, batch_id="b1",
|
|
)
|
|
|
|
monkeypatch.setattr(
|
|
"cyclone.api_routers.submission.submit_file", _fake_submit,
|
|
)
|
|
|
|
resp = client.post(
|
|
"/api/submit-batch",
|
|
json={"ingest_dir": str(tmp_path), "validate": False, "limit": 1},
|
|
)
|
|
assert resp.status_code == 200, resp.text
|
|
body = resp.json()
|
|
assert len(body["results"]) == 1
|
|
assert body["submitted"] + body["skipped"] + body["failed"] == 1
|
|
assert len(submitted_files) == 1
|
|
|
|
|
|
def test_submit_batch_empty_ingest_dir_returns_422(client: TestClient, tmp_path):
|
|
"""No batch-*-claims dir under ingest_dir → 422 (matches the CLI's exit 2 path)."""
|
|
resp = client.post(
|
|
"/api/submit-batch",
|
|
json={"ingest_dir": str(tmp_path), "validate": False},
|
|
)
|
|
assert resp.status_code == 422, resp.text
|
|
|
|
|
|
def test_submit_batch_per_file_failure_keeps_status_200(
|
|
client: TestClient, tmp_path, monkeypatch,
|
|
):
|
|
"""A SUBMITTED + SFTP_FAILED mix → 200 with failed=1, submitted=1.
|
|
|
|
Per-file failures live in the JSON body, NOT in the status code.
|
|
The endpoint always returns 200 on a completed run.
|
|
"""
|
|
_stage_batch(tmp_path, 2)
|
|
|
|
outcomes = iter([
|
|
SubmitResult(file="claim-0.x12", outcome=SubmitOutcome.SUBMITTED, batch_id="b1"),
|
|
SubmitResult(
|
|
file="claim-1.x12", outcome=SubmitOutcome.SFTP_FAILED,
|
|
batch_id="b2", error="sftp down",
|
|
),
|
|
])
|
|
|
|
def _fake_submit(path, **_kw):
|
|
return next(outcomes)
|
|
|
|
monkeypatch.setattr(
|
|
"cyclone.api_routers.submission.submit_file", _fake_submit,
|
|
)
|
|
|
|
resp = client.post(
|
|
"/api/submit-batch",
|
|
json={"ingest_dir": str(tmp_path), "validate": False},
|
|
)
|
|
assert resp.status_code == 200, resp.text
|
|
body = resp.json()
|
|
assert body["submitted"] == 1
|
|
assert body["failed"] == 1
|
|
assert body["skipped"] == 0
|
|
assert len(body["results"]) == 2
|
|
# The failure row carries the error string so the operator can see
|
|
# what went wrong without a separate API call.
|
|
assert body["results"][1]["outcome"] == "sftp_failed"
|
|
assert body["results"][1]["error"] == "sftp down"
|
|
|
|
|
|
def test_submit_batch_unexpected_exception_recorded_in_results(
|
|
client: TestClient, tmp_path, monkeypatch,
|
|
):
|
|
"""If submit_file raises, the file lands in results[] as a failed row.
|
|
|
|
The router catches the exception, builds a SubmitResult with
|
|
``outcome=SubmitOutcome.UNEXPECTED_ERROR`` (followup #3 — distinct
|
|
from the typed SFTP_FAILED path so operators can tell "real bug"
|
|
from "SFTP server down"), and counts it as ``failed``. The full
|
|
error string (including exception class name) is in ``error``.
|
|
"""
|
|
_stage_batch(tmp_path, 1)
|
|
|
|
def _explode(path, **_kw):
|
|
raise RuntimeError("kaboom")
|
|
|
|
monkeypatch.setattr(
|
|
"cyclone.api_routers.submission.submit_file", _explode,
|
|
)
|
|
|
|
resp = client.post(
|
|
"/api/submit-batch",
|
|
json={"ingest_dir": str(tmp_path), "validate": False},
|
|
)
|
|
assert resp.status_code == 200, resp.text
|
|
body = resp.json()
|
|
assert body["failed"] == 1
|
|
# Followup #3: marker is UNEXPECTED_ERROR, NOT SFTP_FAILED — the
|
|
# exception class+message in ``error`` tells the operator this
|
|
# was a real exception (not a typed failure).
|
|
assert body["results"][0]["outcome"] == SubmitOutcome.UNEXPECTED_ERROR.value
|
|
assert "kaboom" in body["results"][0]["error"]
|
|
assert "RuntimeError" in body["results"][0]["error"]
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# Request validation
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_submit_batch_missing_ingest_dir_returns_422(client: TestClient):
|
|
"""Body without ``ingest_dir`` → 422 from Pydantic (NOT your manual check)."""
|
|
resp = client.post("/api/submit-batch", json={"validate": False})
|
|
assert resp.status_code == 422, resp.text
|
|
|
|
|
|
def test_submit_batch_ingest_dir_missing_on_disk_returns_422(
|
|
client: TestClient, tmp_path,
|
|
):
|
|
"""Body has a path that doesn't exist on disk → 422."""
|
|
resp = client.post(
|
|
"/api/submit-batch",
|
|
json={
|
|
"ingest_dir": str(tmp_path / "does-not-exist"),
|
|
"validate": False,
|
|
},
|
|
)
|
|
assert resp.status_code == 422, resp.text
|
|
assert "does not exist" in resp.text or "does-not-exist" in resp.text
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# Clearhouse posture (404 / 409)
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_submit_batch_no_clearhouse_returns_404(client: TestClient, tmp_path, monkeypatch):
|
|
"""Delete the clearhouse row → endpoint must refuse with 404, not 500.
|
|
|
|
404 per the Task 6 status code contract: a missing clearhouse is a
|
|
config-level "missing" (4xx), not an unexpected exception.
|
|
"""
|
|
from cyclone import db as db_mod
|
|
from cyclone.db import ClearhouseORM
|
|
|
|
# Wipe the seeded clearhouse row.
|
|
with db_mod.SessionLocal()() as s:
|
|
row = s.get(ClearhouseORM, 1)
|
|
if row is not None:
|
|
s.delete(row)
|
|
s.commit()
|
|
|
|
_stage_batch(tmp_path, 1)
|
|
|
|
resp = client.post(
|
|
"/api/submit-batch",
|
|
json={"ingest_dir": str(tmp_path), "validate": False},
|
|
)
|
|
assert resp.status_code == 404, resp.text
|
|
assert "clearhouse" in resp.text.lower()
|
|
|
|
|
|
def test_submit_batch_stub_mode_returns_409(client: TestClient, tmp_path, monkeypatch):
|
|
"""When the seeded clearhouse's ``sftp_block.stub`` is True → 409.
|
|
|
|
The default client fixture flips stub=False so file-walking tests
|
|
reach the walker. This test flips it back to True so the
|
|
stub-mode 409 guard fires before any per-file work runs.
|
|
"""
|
|
from cyclone.store import store as cycl_store
|
|
|
|
ch = cycl_store.get_clearhouse()
|
|
assert ch is not None
|
|
# Re-flip stub=True (the default client fixture set it to False).
|
|
cycl_store.update_clearhouse(
|
|
ch.model_copy(update={
|
|
"sftp_block": ch.sftp_block.model_copy(update={"stub": True}),
|
|
}),
|
|
)
|
|
|
|
_stage_batch(tmp_path, 1)
|
|
|
|
resp = client.post(
|
|
"/api/submit-batch",
|
|
json={"ingest_dir": str(tmp_path), "validate": False},
|
|
)
|
|
assert resp.status_code == 409, resp.text
|
|
assert "stub" in resp.text.lower()
|
|
|
|
|
|
# --------------------------------------------------------------------------- #
|
|
# Auth gate
|
|
# --------------------------------------------------------------------------- #
|
|
|
|
|
|
def test_submit_batch_auth_gate(client: TestClient, tmp_path, monkeypatch):
|
|
"""No cookie + AUTH_DISABLED=False → 401 from matrix_gate.
|
|
|
|
The conftest autouse fixture sets ``AUTH_DISABLED = True`` at
|
|
fixture setup and resets to False at teardown. Inside the test we
|
|
flip the module attribute so the gate fires — but to avoid leaking
|
|
the flip into a sibling test we use monkeypatch.setattr (which
|
|
restores at test teardown, before conftest's finally block runs).
|
|
"""
|
|
import cyclone.auth.deps as auth_deps
|
|
|
|
monkeypatch.setattr(auth_deps, "AUTH_DISABLED", False)
|
|
|
|
_stage_batch(tmp_path, 1)
|
|
|
|
resp = client.post(
|
|
"/api/submit-batch",
|
|
json={"ingest_dir": str(tmp_path), "validate": False},
|
|
)
|
|
assert resp.status_code == 401, resp.text |