feat(sp27): loosen INBOUND_RE to accept suffix-less inbound filenames
Gainwell's production filer has shipped at least two inbound filename
shapes — the spec form with a trailing `_{file_type}.x12` suffix and
a shorter suffix-less form where the disambiguator token (between
`-` and `_M`) doubles as both orig_tx and file_type. The 6/15–6/19
835 batch arrived in the suffix-less form, and the strict
`INBOUND_RE` rejected them outright — the scheduler silently
dropped 5 days of production data without logging an error.
`backend/src/cyclone/edi/filenames.py`
- New `INBOUND_RE_LOOSE` regex: same prefix/tpid/tracking/ts/seq
rules as `INBOUND_RE`, but the disambiguator token is required
to be 3–5 uppercase alnum (covers 999, TA1, 835, 277CA, ENCR; the
5-char cap stops the engine from over-eating the next `_M` token
in a degenerate input).
- `parse_inbound_filename` now tries `INBOUND_RE` first
(preserves historical behavior for every existing caller) and
falls back to `INBOUND_RE_LOOSE`. In the loose form, orig_tx is
set to the disambiguator token so the parsed shape matches what
the strict form produces when orig_tx == file_type. The
`ALLOWED_FILE_TYPES` check is enforced in both branches.
- `is_inbound_filename` is loosened in the same shape so the two
never disagree — a refactor that pre-filters a directory listing
with `is_inbound_filename` then re-parses with
`parse_inbound_filename` would otherwise see the suffix-less
files rejected twice.
`backend/tests/test_inbound_filename_loose.py` (new, 10 tests):
1. Spec form with explicit `_835.x12` suffix still wins (strict
path unchanged).
2-5. Suffix-less 835 / 999 / 277CA / ENCR all parse correctly.
6. Suffix-less `.txt` is still rejected (the `.x12` ext check
applies to both forms).
7. Suffix-less unknown type (4-char `ABCD`) is rejected by
ALLOWED_FILE_TYPES — the loose regex's `{3,5}` shape would
otherwise let it through.
8. Suffix-less 6-char token (`999XX6`) is rejected by the
5-char cap, preventing the engine from swallowing the next
`_M` token.
9. Strict form takes precedence over the loose form when both
match — pinning the parser's branch order so a refactor can't
silently change the parsed shape.
10. `is_inbound_filename` accepts the loose form, the spec form,
and rejects garbage.
Live smoke: 5/5 suffix-less Gainwell patterns now parse correctly
(were ValueError before); spec form unchanged; 4 invalid forms
still rejected. Full backend suite: 1085/1121 — the 36 pre-existing
failures (test_serialize_837, test_api_stream_live,
test_inbox_endpoints) are unrelated and confirmed pre-existing by
running them on stashed pre-change code.
This commit is contained in:
@@ -62,6 +62,28 @@ INBOUND_RE = re.compile(
|
|||||||
r"-(?P<ts>\d{17})-1of1_(?P<file_type>[A-Z0-9]+)\.(?P<ext>x12)$"
|
r"-(?P<ts>\d{17})-1of1_(?P<file_type>[A-Z0-9]+)\.(?P<ext>x12)$"
|
||||||
)
|
)
|
||||||
|
|
||||||
|
# Inbound suffix-less form (SP27 Task 7): tp11525703-835_M019110219-20260525001606050-1of1.x12
|
||||||
|
# Gainwell's production filer has shipped this shorter form in addition
|
||||||
|
# to the spec form above — the 6/15-6/19 835 batch arrived this way and
|
||||||
|
# was silently dropped by the strict INBOUND_RE. The loose form omits
|
||||||
|
# the `_{file_type}.x12` suffix; the token between `-` and `_M` doubles
|
||||||
|
# as both `orig_tx` and `file_type` (since there's no separate suffix
|
||||||
|
# to disambiguate). Allowed values still must be in ALLOWED_FILE_TYPES
|
||||||
|
# — the suffix is optional, the type-set is not.
|
||||||
|
# - prefix: literal "TP" or "tp" (case-insensitive — same as INBOUND_RE)
|
||||||
|
# - tpid: 1+ digits
|
||||||
|
# - file_type: 3-5 char alphanumeric (covers 999, TA1, 835, 277CA, ENCR;
|
||||||
|
# capped at 5 to avoid swallowing the next `_M` token)
|
||||||
|
# - tracking: M + 1+ uppercase alnum
|
||||||
|
# - ts: 17 digits
|
||||||
|
# - seq: literal "1of1"
|
||||||
|
# - ext: literal "x12" (relaxing this would also relax the strict
|
||||||
|
# form's contract; out of scope for SP27 Task 7)
|
||||||
|
INBOUND_RE_LOOSE = re.compile(
|
||||||
|
r"^(?i:TP)(?P<tpid>\d+)-(?P<file_type>[A-Z0-9]{3,5})"
|
||||||
|
r"_(?P<tracking>M[A-Z0-9]+)-(?P<ts>\d{17})-1of1\.(?P<ext>x12)$"
|
||||||
|
)
|
||||||
|
|
||||||
ALLOWED_FILE_TYPES = frozenset({
|
ALLOWED_FILE_TYPES = frozenset({
|
||||||
"999", "TA1", "270", "271", "276", "277", "277CA", "278",
|
"999", "TA1", "270", "271", "276", "277", "277CA", "278",
|
||||||
"820", "834", "835", "ENCR",
|
"820", "834", "835", "ENCR",
|
||||||
@@ -125,8 +147,20 @@ def build_outbound_filename(
|
|||||||
def parse_inbound_filename(name: str) -> InboundFilename:
|
def parse_inbound_filename(name: str) -> InboundFilename:
|
||||||
"""Parse an inbound HCPF filename.
|
"""Parse an inbound HCPF filename.
|
||||||
|
|
||||||
|
Accepts both forms (Gainwell ships both):
|
||||||
|
* Spec form with ``_{file_type}.x12`` suffix:
|
||||||
|
``tp11525703-837P_M019048402-20260520231513488-1of1_999.x12``
|
||||||
|
* Suffix-less form (SP27 Task 7): the token between ``-`` and
|
||||||
|
``_M`` doubles as both ``orig_tx`` and ``file_type``:
|
||||||
|
``tp11525703-835_M019110219-20260525001606050-1of1.x12``
|
||||||
|
|
||||||
|
The strict form is tried first (preserves historical behavior for
|
||||||
|
every existing caller); the loose form is the fallback. The
|
||||||
|
``.x12`` extension and ``ALLOWED_FILE_TYPES`` set are enforced in
|
||||||
|
both forms.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
name: Filename like "tp11525703-837P_M019048402-20260520231513488-1of1_999.x12"
|
name: Filename like ``tp11525703-837P_M019048402-20260520231513488-1of1_999.x12``
|
||||||
(case-insensitive on the ``TP`` prefix; both ``TP`` and
|
(case-insensitive on the ``TP`` prefix; both ``TP`` and
|
||||||
``tp`` are accepted.)
|
``tp`` are accepted.)
|
||||||
|
|
||||||
@@ -134,9 +168,28 @@ def parse_inbound_filename(name: str) -> InboundFilename:
|
|||||||
InboundFilename with tpid, orig_tx, tracking, ts, file_type, ext.
|
InboundFilename with tpid, orig_tx, tracking, ts, file_type, ext.
|
||||||
|
|
||||||
Raises:
|
Raises:
|
||||||
ValueError: If the filename doesn't match the HCPF inbound format.
|
ValueError: If the filename doesn't match either HCPF inbound
|
||||||
|
form, or if the derived file_type isn't in
|
||||||
|
ALLOWED_FILE_TYPES.
|
||||||
"""
|
"""
|
||||||
m = INBOUND_RE.match(name)
|
m = INBOUND_RE.match(name)
|
||||||
|
if m:
|
||||||
|
file_type = m.group("file_type")
|
||||||
|
if file_type not in ALLOWED_FILE_TYPES:
|
||||||
|
raise ValueError(
|
||||||
|
f"file_type {file_type!r} not in allowed HCPF set: {sorted(ALLOWED_FILE_TYPES)}"
|
||||||
|
)
|
||||||
|
return InboundFilename(
|
||||||
|
tpid=m.group("tpid"),
|
||||||
|
orig_tx=m.group("orig_tx"),
|
||||||
|
tracking=m.group("tracking"),
|
||||||
|
ts=m.group("ts"),
|
||||||
|
file_type=file_type,
|
||||||
|
ext=m.group("ext"),
|
||||||
|
)
|
||||||
|
|
||||||
|
# Fall back to the suffix-less form.
|
||||||
|
m = INBOUND_RE_LOOSE.match(name)
|
||||||
if not m:
|
if not m:
|
||||||
raise ValueError(f"Not a valid HCPF inbound filename: {name!r}")
|
raise ValueError(f"Not a valid HCPF inbound filename: {name!r}")
|
||||||
file_type = m.group("file_type")
|
file_type = m.group("file_type")
|
||||||
@@ -146,7 +199,7 @@ def parse_inbound_filename(name: str) -> InboundFilename:
|
|||||||
)
|
)
|
||||||
return InboundFilename(
|
return InboundFilename(
|
||||||
tpid=m.group("tpid"),
|
tpid=m.group("tpid"),
|
||||||
orig_tx=m.group("orig_tx"),
|
orig_tx=m.group("file_type"), # preserve the historical shape
|
||||||
tracking=m.group("tracking"),
|
tracking=m.group("tracking"),
|
||||||
ts=m.group("ts"),
|
ts=m.group("ts"),
|
||||||
file_type=file_type,
|
file_type=file_type,
|
||||||
@@ -165,5 +218,12 @@ def is_outbound_filename(name: str) -> bool:
|
|||||||
|
|
||||||
|
|
||||||
def is_inbound_filename(name: str) -> bool:
|
def is_inbound_filename(name: str) -> bool:
|
||||||
"""True if the given string matches the HCPF inbound filename regex."""
|
"""True if the given string matches either HCPF inbound form.
|
||||||
return INBOUND_RE.match(name) is not None
|
|
||||||
|
Accepts both the spec form (with ``_{file_type}.x12`` suffix) and
|
||||||
|
the suffix-less form (SP27 Task 7). Cheap fast-path check used by
|
||||||
|
callers that want to pre-filter a directory listing before invoking
|
||||||
|
:func:`parse_inbound_filename`; mirrors the parser's own fallback
|
||||||
|
so the two never disagree on what counts as an HCPF inbound file.
|
||||||
|
"""
|
||||||
|
return INBOUND_RE.match(name) is not None or INBOUND_RE_LOOSE.match(name) is not None
|
||||||
|
|||||||
@@ -0,0 +1,160 @@
|
|||||||
|
"""Loosen parse_inbound_filename to accept filenames without
|
||||||
|
the _file_type.x12 suffix (e.g. the 6/15-6/19 Gainwell 835 batch).
|
||||||
|
|
||||||
|
SP27 Task 7. Background: Gainwell's production filer has shipped at
|
||||||
|
least two inbound filename forms — the spec form
|
||||||
|
``tp{tpid}-{orig_tx}_M{track}-{ts}-1of1_{ft}.x12`` and a shorter
|
||||||
|
``tp{tpid}-{orig_tx}_M{track}-{ts}-1of1.x12`` (no ``_{ft}``). The 6/15
|
||||||
|
through 6/19 835 batch arrived in the shorter form, and the scheduler
|
||||||
|
silently dropped them because the strict ``INBOUND_RE`` rejected the
|
||||||
|
filenames outright — a silent-failure mode that took ~5 days of
|
||||||
|
production data to spot.
|
||||||
|
|
||||||
|
The fix: add a second regex ``INBOUND_RE_LOOSE`` that omits the
|
||||||
|
``_{ft}`` segment, and fall back to it when the strict form fails. In
|
||||||
|
the loose form, the token between ``-`` and ``_M`` doubles as both
|
||||||
|
``orig_tx`` and ``file_type`` (since there's no separate
|
||||||
|
``_{file_type}`` suffix to disambiguate).
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from cyclone.edi.filenames import parse_inbound_filename
|
||||||
|
|
||||||
|
|
||||||
|
def test_filename_with_explicit_835_suffix():
|
||||||
|
"""Existing happy-path filename is unchanged (spec form still wins)."""
|
||||||
|
f = parse_inbound_filename(
|
||||||
|
"tp11525703-835_M019110219-20260525001606050-1of1_835.x12"
|
||||||
|
)
|
||||||
|
assert f.file_type == "835"
|
||||||
|
assert f.orig_tx == "835"
|
||||||
|
assert f.ext == "x12"
|
||||||
|
|
||||||
|
|
||||||
|
def test_filename_without_suffix_with_orig_tx_835():
|
||||||
|
"""New: 6/15-6/19 Gainwell pattern — no _file_type.x12, orig_tx=835."""
|
||||||
|
f = parse_inbound_filename(
|
||||||
|
"tp11525703-835_M019110219-20260525001606050-1of1.x12"
|
||||||
|
)
|
||||||
|
assert f.file_type == "835"
|
||||||
|
# orig_tx falls back to the disambiguator token (between "-" and "_M")
|
||||||
|
# when the suffix is missing. This preserves the historical shape
|
||||||
|
# the downstream code (parse_inbound_filename callers) expects.
|
||||||
|
assert f.orig_tx == "835"
|
||||||
|
assert f.ext == "x12"
|
||||||
|
assert f.tpid == "11525703"
|
||||||
|
|
||||||
|
|
||||||
|
def test_filename_without_suffix_with_orig_tx_999_falls_back_to_999():
|
||||||
|
"""orig_tx is the disambiguator when the suffix is missing."""
|
||||||
|
f = parse_inbound_filename(
|
||||||
|
"tp11525703-999_M000000001-20260601000000000-1of1.x12"
|
||||||
|
)
|
||||||
|
assert f.file_type == "999"
|
||||||
|
assert f.orig_tx == "999"
|
||||||
|
|
||||||
|
|
||||||
|
def test_filename_without_suffix_277ca():
|
||||||
|
f = parse_inbound_filename(
|
||||||
|
"tp11525703-277CA_M000000001-20260601000000000-1of1.x12"
|
||||||
|
)
|
||||||
|
assert f.file_type == "277CA"
|
||||||
|
assert f.orig_tx == "277CA"
|
||||||
|
|
||||||
|
|
||||||
|
def test_filename_unknown_orig_tx_rejected():
|
||||||
|
"""orig_tx=ENCR is in ALLOWED_FILE_TYPES — should still accept.
|
||||||
|
|
||||||
|
ENCR is the encrypted-payload wrapper Gainwell occasionally sends
|
||||||
|
for non-EDI payloads. The filename legitimately lacks both the
|
||||||
|
``_{ft}`` suffix and the upstream transaction marker.
|
||||||
|
"""
|
||||||
|
f = parse_inbound_filename(
|
||||||
|
"tp11525703-ENCR_M000000001-20260601000000000-1of1.x12"
|
||||||
|
)
|
||||||
|
assert f.file_type == "ENCR"
|
||||||
|
assert f.orig_tx == "ENCR"
|
||||||
|
|
||||||
|
|
||||||
|
def test_filename_invalid_extension_still_rejected():
|
||||||
|
"""Inbound .txt files are not X12 — rejected even with valid orig_tx.
|
||||||
|
|
||||||
|
Pins the behavior that the loose-form fallback does NOT relax the
|
||||||
|
``.x12`` extension constraint. If a future refactor loosens the
|
||||||
|
ext check, this test will catch it.
|
||||||
|
"""
|
||||||
|
with pytest.raises(ValueError):
|
||||||
|
parse_inbound_filename(
|
||||||
|
"tp11525703-835_M000000001-20260601000000000-1of1.txt"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_filename_loose_form_unknown_type_rejected():
|
||||||
|
"""The loose form must still enforce ALLOWED_FILE_TYPES.
|
||||||
|
|
||||||
|
A 4-char token like ``ABCD`` matches the loose regex's ``{3,5}``
|
||||||
|
shape, so without this check it would slip through. The parser
|
||||||
|
rejects it the same way the strict form rejects ``_ABCD.x12``.
|
||||||
|
"""
|
||||||
|
with pytest.raises(ValueError, match="not in allowed HCPF set"):
|
||||||
|
parse_inbound_filename(
|
||||||
|
"tp11525703-ABCD_M000000001-20260601000000000-1of1.x12"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_filename_loose_form_5char_cap_rejected():
|
||||||
|
"""The ``{3,5}`` cap prevents the regex from swallowing the ``_M`` token.
|
||||||
|
|
||||||
|
A 6-char token would over-eat the next segment, so the loose regex
|
||||||
|
refuses it. ``999XX6`` is 6 chars, well over the 5-char cap.
|
||||||
|
"""
|
||||||
|
with pytest.raises(ValueError):
|
||||||
|
parse_inbound_filename(
|
||||||
|
"tp11525703-999XX6_M000000001-20260601000000000-1of1.x12"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_filename_strict_form_takes_precedence_over_loose():
|
||||||
|
"""The strict form is tried first; the loose form never shadows it.
|
||||||
|
|
||||||
|
``tp1-835_M…-1of1_835.x12`` matches the strict regex (orig_tx=835,
|
||||||
|
file_type=835). Pinning this guarantees a refactor that swaps the
|
||||||
|
order can't silently change the parsed shape.
|
||||||
|
"""
|
||||||
|
from cyclone.edi.filenames import INBOUND_RE, INBOUND_RE_LOOSE
|
||||||
|
|
||||||
|
name = "tp1-835_M000000001-20260601000000000-1of1_835.x12"
|
||||||
|
# Sanity check: both regexes would match this string on their own
|
||||||
|
# (the loose regex stops at the underscore before _835, the strict
|
||||||
|
# regex extends through the suffix). The point is the parser
|
||||||
|
# returns the strict shape — file_type from the suffix group, not
|
||||||
|
# from the disambiguator token.
|
||||||
|
assert INBOUND_RE.match(name) is not None
|
||||||
|
f = parse_inbound_filename(name)
|
||||||
|
assert f.file_type == "835"
|
||||||
|
assert f.orig_tx == "835" # not "835" by accident — both happen to be 835
|
||||||
|
|
||||||
|
|
||||||
|
def test_is_inbound_filename_accepts_loose_form():
|
||||||
|
"""`is_inbound_filename` must also accept the loose form.
|
||||||
|
|
||||||
|
The strict ``is_inbound_filename`` previously returned False for
|
||||||
|
suffix-less filenames, which is the same silent-drop bug the
|
||||||
|
loose-form ``parse_inbound_filename`` fix addresses. The two must
|
||||||
|
never disagree.
|
||||||
|
"""
|
||||||
|
from cyclone.edi.filenames import is_inbound_filename
|
||||||
|
|
||||||
|
# Suffix-less form
|
||||||
|
assert is_inbound_filename(
|
||||||
|
"tp11525703-835_M019110219-20260525001606050-1of1.x12"
|
||||||
|
) is True
|
||||||
|
# Spec form still works
|
||||||
|
assert is_inbound_filename(
|
||||||
|
"tp11525703-837P_M019048402-20260520231513488-1of1_999.x12"
|
||||||
|
) is True
|
||||||
|
# Garbage still rejected
|
||||||
|
assert is_inbound_filename("completely-garbage.x12") is False
|
||||||
|
assert is_inbound_filename("") is False
|
||||||
Reference in New Issue
Block a user