diff --git a/backend/src/cyclone/submission/core.py b/backend/src/cyclone/submission/core.py index 5c394b6..a0fef45 100644 --- a/backend/src/cyclone/submission/core.py +++ b/backend/src/cyclone/submission/core.py @@ -29,6 +29,7 @@ from cyclone.parsers.parse_837 import parse as parse_837_text from cyclone.parsers.payer import PayerConfig from cyclone.providers import SftpBlock from cyclone.store import store as cycl_store +from cyclone.store.exceptions import DuplicateClaimError from cyclone.store.records import BatchRecord837 from cyclone.store.submission_dedup import check_duplicate @@ -157,8 +158,24 @@ def submit_file( # the exception carries ``claim_id`` + ``original_submission_at`` # so the caller can surface structured detail to the operator # without re-querying. Approach A per the Task 9 spec. - for claim in parsed.claims: - check_duplicate(claim.claim_id) + # + # Symmetry with the sibling failure paths (parse / DB / SFTP / + # audit-event — each logs ``submit_file %s: : %s`` before + # returning or re-raising). The router's special-case handler + # also logs this exception at the api_routers boundary, but + # the helper has its own log line so the operator tracing a + # failure through stdout sees the dedup trip even when + # submit_file is invoked directly (e.g. from the CLI or a + # future call site that does not go through the router). + try: + for claim in parsed.claims: + check_duplicate(claim.claim_id) + except DuplicateClaimError as exc: + log.warning( + "submit_file %s: duplicate claim %s (originally submitted %s)", + file_label, exc.claim_id, exc.original_submission_at.isoformat(), + ) + raise # 2. DB write — DB-first, upload-second invariant. # ``parsed`` is required to construct a BatchRecord837 (it embeds the diff --git a/backend/tests/test_submission_dedup.py b/backend/tests/test_submission_dedup.py index dacde85..6fb3b13 100644 --- a/backend/tests/test_submission_dedup.py +++ b/backend/tests/test_submission_dedup.py @@ -118,3 +118,129 @@ def test_submit_file_raises_on_duplicate_claim_id(tmp_path): "dedup guard must block the BatchRecord837 DB write; " f"found {batch_count} Batch row(s)" ) + + +# --------------------------------------------------------------------------- # +# SP41 Task 9 — router-level coverage: api_routers/submission.submit_batch +# catches DuplicateClaimError BEFORE the generic Exception handler and +# surfaces it as a per-file ``outcome="unexpected_error"`` row, with the +# duplicated claim_id baked into ``error`` and ``batch_id=None`` (no DB +# row written). The HTTP status code stays 200 — per-file failures live +# in the JSON body, not in the status (see the top-of-module contract +# in api_routers/submission.py). +# --------------------------------------------------------------------------- # + + +def test_submit_batch_per_file_duplicate_claim_classified_as_unexpected( + tmp_path, monkeypatch, +): + """Pre-record CLM001 (the fixture's only CLM01) then POST a batch + containing one copy of the fixture. Asserts the router: + + - keeps the HTTP status at 200 (per-file failures don't change status) + - classifies the row as ``outcome == "unexpected_error"`` (the + special-case handler in api_routers/submission.py) + - includes the duplicated ``claim_id`` ("CLM001") in ``error`` + - sets ``batch_id is None`` (no BatchRecord837 row written) + + Does NOT monkeypatch ``submit_file`` — the test exercises the real + pre-flight raise path so the router's special-case catch is what + produces the unexpected_error classification. submit_file raises + DuplicateClaimError BEFORE the SFTP factory is invoked, so the + paramiko factory is never reached even though the client fixture + flips ``sftp_block.stub`` to ``False`` (no live MFT needed). + """ + from fastapi.testclient import TestClient + + from cyclone import db as db_mod + from cyclone.api import app + from cyclone.store import store as cycl_store + + # 1. Per-test DB + clearhouse (mirror test_api_submit_batch.py's + # ``client`` fixture inline so this test is self-contained). + # The autouse conftest already wired CYCLONE_DB_URL + init_db; + # we just need to seed the clearhouse row and flip stub=False. + db_mod._reset_for_tests() + db_mod.init_db() + cycl_store.ensure_clearhouse_seeded() + ch = cycl_store.get_clearhouse() + cycl_store.update_clearhouse( + ch.model_copy(update={ + "sftp_block": ch.sftp_block.model_copy(update={"stub": False}), + }), + ) + + # 2. Pre-record the duplicate. CLM001 is the fixture's only CLM01; + # ``record_submission`` is the same helper the dedup unit test + # uses, so the window + exception shape match. + record_submission( + _FIXTURE_CLAIM_ID, + submitted_at=datetime.now(timezone.utc), + db_url=f"sqlite:///{tmp_path}/test.db", + ) + + # 3. Stage a single batch-* dir containing one copy of the fixture. + # Mirrors _stage_batch from test_api_submit_batch.py. + batch_dir = tmp_path / "batch-dedup-claims" + batch_dir.mkdir() + (batch_dir / "dup-claim.x12").write_bytes(_FIXTURE.read_bytes()) + + # 4. Sentinel on submit_file's sftp_client_factory — submit_file + # raises BEFORE the factory is invoked, so this AssertionError + # only fires if the dedup guard is bypassed end-to-end. We + # don't actually call submit_file directly here; the sentinel + # is wired via monkeypatch on the function the router uses, so + # if anything in the chain accidentally reaches the factory + # call we'd see it. In practice submit_file raises and we never + # get there — this is a regression tripwire only. + def _sentinel_factory(_block): # pragma: no cover - regression only + raise AssertionError( + "SFTP factory must not be called: dedup blocks submit_file " + "before the factory runs" + ) + + monkeypatch.setattr( + "cyclone.submission.core._default_sftp_factory", _sentinel_factory, + ) + + # 5. Hit the endpoint. Per-file failures don't change the status + # code → 200 even though one file was rejected. + client = TestClient(app) + resp = client.post( + "/api/submit-batch", + json={"ingest_dir": str(tmp_path), "validate": True}, + ) + assert resp.status_code == 200, resp.text + body = resp.json() + + # 6. Counts: 1 file total, classified as failed. (UNEXPECTED_ERROR + # rolls into the ``failed`` counter in api_routers/submission.py + # — see the counter branch at the bottom of the per-file loop.) + assert body["submitted"] == 0 + assert body["skipped"] == 0 + assert body["failed"] == 1 + assert len(body["results"]) == 1 + + # 7. The single result row carries the special-case classification + # (NOT a generic "runtime error: kaboom" — that's a different + # test in test_api_submit_batch.py). The router's + # DuplicateClaimError handler sets UNEXPECTED_ERROR + bakes the + # claim_id into ``error``. + row = body["results"][0] + assert row["outcome"] == "unexpected_error" + assert _FIXTURE_CLAIM_ID in row["error"], ( + f"expected claim_id {_FIXTURE_CLAIM_ID!r} in error string; " + f"got {row['error']!r}" + ) + # The router's handler does NOT construct a batch_id — the dedup + # guard fires BEFORE the DB write, so no BatchRecord837 row exists. + assert row["batch_id"] is None + + # 8. DB invariant (defense in depth): even though we trust the + # router-level assertion above, pin that no Batch row leaked. + with db_mod.SessionLocal()() as session: + batch_count = session.query(db_mod.Batch).count() + assert batch_count == 0, ( + "dedup guard must block the BatchRecord837 DB write end-to-end; " + f"found {batch_count} Batch row(s)" + )