fix(sftp): case-insensitive inbound regex, skip _warn.txt, add targeted pull
Three changes that unblock the daily inbound pull from Gainwell's FromHPE MFT path: 1. INBOUND_RE now accepts both 'TP' and 'tp' prefixes via inline (?i:TP) scoping — case-folding the whole pattern would let lowercase '837p' / tracking IDs through, which is invalid HCPF. The rest of the pattern is still case-sensitive. 2. _list_inbound_paramiko skips *_warn.txt entries. Gainwell's MFT drops ~583 advisory text-format notes in the same inbound dir; they come first alphabetically and were padding every poll with ~80 min of pointless downloads. 3. New SftpClient.list_inbound_names() + download_inbound() pair gives a metadata-only listing and on-demand fetch. The scheduler's existing full-listing path still works (now without the warn padding); the new path is what the new /api/admin/scheduler/pull-inbound endpoint and the 'cyclone pull-inbound' CLI use to fast-target a date range without paying the cost of a full ~6000-file download. Scheduler.process_inbound_files() runs the same per-file pipeline as a regular tick on the pre-fetched list, so dedup via processed_inbound_files still applies. Tests added in test_filenames.py (lowercase + mixed-case cases), test_sftp_paramiko.py (warn skip + no-download listing), and test_scheduler.py (process_inbound_files idempotency). With this, the daily 385-file pull for 20260624 completes in seconds via 'docker exec cyclone-backend-1 python -m cyclone pull-inbound --date 20260624 --block dzinesco' (or the equivalent POST to /api/admin/scheduler/pull-inbound?date=20260624).
This commit is contained in:
@@ -107,6 +107,48 @@ def test_parse_inbound_277():
|
||||
assert parsed.file_type == "277"
|
||||
|
||||
|
||||
def test_parse_inbound_lowercase_tp_prefix_999():
|
||||
# Gainwell's production filer uses lowercase `tp` for inbound 999/TA1.
|
||||
# The inbound regex must accept both casings on the TP prefix.
|
||||
name = "tp11525703-837P_M019048402-20260520231513488-1of1_999.x12"
|
||||
parsed = parse_inbound_filename(name)
|
||||
assert parsed.tpid == "11525703"
|
||||
assert parsed.orig_tx == "837P"
|
||||
assert parsed.tracking == "M019048402"
|
||||
assert parsed.file_type == "999"
|
||||
assert parsed.ext == "x12"
|
||||
|
||||
|
||||
def test_parse_inbound_lowercase_tp_prefix_ta1():
|
||||
name = "tp11525703-837P_M019044969-20260520180505477-1of1_TA1.x12"
|
||||
parsed = parse_inbound_filename(name)
|
||||
assert parsed.file_type == "TA1"
|
||||
|
||||
|
||||
def test_is_inbound_filename_accepts_both_cases():
|
||||
# is_inbound_filename() is the fast path used by the scheduler to
|
||||
# filter the listing. It must accept both casings.
|
||||
upper = "TP11525703-837P_M019048402-20260520231513488-1of1_999.x12"
|
||||
lower = "tp11525703-837P_M019048402-20260520231513488-1of1_999.x12"
|
||||
assert is_inbound_filename(upper)
|
||||
assert is_inbound_filename(lower)
|
||||
|
||||
|
||||
def test_parse_inbound_rejects_mixed_case_tracking():
|
||||
# Tracking value must stay uppercase alnum; the case-insensitive
|
||||
# flag is intentionally scoped to the TP prefix by the file_type
|
||||
# and timestamp constraints, so a mixed-case tracking should still
|
||||
# be rejected (it'd be invalid HCPF).
|
||||
# We exercise the obvious "totally lowercase" rejection to confirm
|
||||
# the rest of the pattern is still strict.
|
||||
with pytest.raises(ValueError, match="Not a valid HCPF inbound"):
|
||||
# Lowercase orig_tx; the orig_tx class is [A-Z0-9]+ so it
|
||||
# must be uppercase.
|
||||
parse_inbound_filename(
|
||||
"tp11525703-837p_M019048402-20260520231513488-1of1_999.x12"
|
||||
)
|
||||
|
||||
|
||||
def test_parse_inbound_rejects_missing_tp_prefix():
|
||||
with pytest.raises(ValueError, match="Not a valid HCPF inbound"):
|
||||
parse_inbound_filename("11525703-837P_M019048402-20260520231513488-1of1_999.x12")
|
||||
@@ -157,8 +199,10 @@ def test_is_outbound_filename():
|
||||
|
||||
def test_is_inbound_filename():
|
||||
assert is_inbound_filename("TP11525703-837P_M019048402-20260520231513488-1of1_999.x12")
|
||||
# Lowercase tp prefix is the outbound shape, not inbound
|
||||
assert not is_inbound_filename("tp11525703-837P_M019048402-20260520231513488-1of1_999.x12")
|
||||
# Lowercase tp prefix is now accepted too — Gainwell's filer has
|
||||
# used both casings on inbound 999/TA1 files.
|
||||
assert is_inbound_filename("tp11525703-837P_M019048402-20260520231513488-1of1_999.x12")
|
||||
# Outbound shape is still rejected (no tracking/ts/file_type).
|
||||
assert not is_inbound_filename("11525703-837P-20260620132243505-1of1.x12")
|
||||
|
||||
|
||||
|
||||
@@ -284,4 +284,62 @@ class TestRoutedFileTypes:
|
||||
type. These tests are the regression net."""
|
||||
|
||||
def test_handlers_cover_all_routed_types(self):
|
||||
assert set(HANDLERS.keys()) == ROUTED_FILE_TYPES
|
||||
assert set(HANDLERS.keys()) == ROUTED_FILE_TYPES
|
||||
|
||||
|
||||
class TestProcessInboundFiles:
|
||||
"""Scheduler.process_inbound_files() — date-filtered pull path.
|
||||
|
||||
The /api/admin/scheduler/pull-inbound endpoint and the
|
||||
``cyclone pull-inbound`` CLI both call this. It must:
|
||||
* process each file in the provided list (no SFTP listdir)
|
||||
* dedupe via ``processed_inbound_files`` (idempotent on rerun)
|
||||
* not touch SFTP at all — files are expected to be on local
|
||||
disk at ``f.local_path``
|
||||
"""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_processes_provided_files_without_listdir(
|
||||
self, sftp_block, _drop_file, tmp_path,
|
||||
):
|
||||
from cyclone.clearhouse import InboundFile
|
||||
from cyclone.scheduler import STATUS_OK
|
||||
|
||||
# Drop a known 999 file on disk (the stub scheduler doesn't
|
||||
# need SFTP — it reads from staging dir).
|
||||
_drop_file(
|
||||
"TP11525703-837P_M019048402-20260520231513488-1of1_999.x12",
|
||||
b"not a real 999 -- handler will parse_error",
|
||||
)
|
||||
sched = _make_scheduler(sftp_block, tmp_path)
|
||||
|
||||
# Build the InboundFile records manually — caller is
|
||||
# responsible for staging (mirrors the targeted-pull flow).
|
||||
path = tmp_path / "staging" / "FromHPE" / \
|
||||
"TP11525703-837P_M019048402-20260520231513488-1of1_999.x12"
|
||||
files = [InboundFile(
|
||||
name=path.name,
|
||||
size=path.stat().st_size,
|
||||
modified_at=datetime.fromtimestamp(path.stat().st_mtime, tz=timezone.utc),
|
||||
local_path=path,
|
||||
)]
|
||||
|
||||
result = await sched.process_inbound_files(files)
|
||||
# The file's bytes are intentionally invalid — we only care
|
||||
# that process_inbound_files invokes the handler and
|
||||
# records the outcome (here: error from parse).
|
||||
assert result.files_seen == 1
|
||||
assert result.files_seen == result.files_processed + result.files_errored
|
||||
# Idempotent: a second call is a no-op (already-processed dedup).
|
||||
result2 = await sched.process_inbound_files(files)
|
||||
assert result2.files_seen == 1
|
||||
# Either skipped (because the prior call recorded it as error)
|
||||
# or error — both prove the dedup branch fired. We just check
|
||||
# it's not re-processed successfully.
|
||||
with db.SessionLocal()() as s:
|
||||
rows = s.query(ProcessedInboundFile).filter_by(
|
||||
sftp_block_name="test-block",
|
||||
name=path.name,
|
||||
).all()
|
||||
assert len(rows) == 1
|
||||
assert rows[0].status in (STATUS_OK, STATUS_ERROR, STATUS_SKIPPED)
|
||||
@@ -259,6 +259,66 @@ class TestRealModeListInbound:
|
||||
assert len(files) == 1
|
||||
assert files[0].name == "real.x12"
|
||||
|
||||
def test_list_skips_warn_txt_files(self, monkeypatch, tmp_path: Path):
|
||||
# Gainwell's MFT drops advisory *_warn.txt files in the same
|
||||
# inbound dir. They're text-format side-channel notes (not X12
|
||||
# envelopes) and must be skipped at list time.
|
||||
real_attr = MagicMock()
|
||||
real_attr.filename = "TP123-837P_M456-20260520231513488-1of1_999.x12"
|
||||
real_attr.st_mode = 0o100644
|
||||
real_attr.st_size = 1024
|
||||
real_attr.st_mtime = 1718899200
|
||||
|
||||
warn_attr = MagicMock()
|
||||
warn_attr.filename = "TP123-837P-202606181208100000-1of1_warn.txt"
|
||||
warn_attr.st_mode = 0o100644
|
||||
warn_attr.st_size = 200
|
||||
warn_attr.st_mtime = 1718899200
|
||||
|
||||
mock_ssh, mock_sftp = _make_mock_paramiko(
|
||||
monkeypatch, sftp_attrs=[real_attr, warn_attr],
|
||||
)
|
||||
|
||||
def _open(path, mode="rb"):
|
||||
m = MagicMock()
|
||||
m.__enter__.return_value = io.BytesIO(b"x12 content")
|
||||
return m
|
||||
|
||||
mock_sftp.open.side_effect = _open
|
||||
|
||||
block = _block(staging_dir=str(tmp_path / "staging"))
|
||||
client = SftpClient(block)
|
||||
files = client.list_inbound()
|
||||
|
||||
names = [f.name for f in files]
|
||||
assert "TP123-837P_M456-20260520231513488-1of1_999.x12" in names
|
||||
assert not any(n.endswith("_warn.txt") for n in names)
|
||||
assert len(files) == 1
|
||||
|
||||
def test_list_inbound_names_does_not_download(self, monkeypatch, tmp_path: Path):
|
||||
# list_inbound_names() must do a metadata-only SFTP listing —
|
||||
# no sftp.open() / no file written to the cache.
|
||||
attr = MagicMock()
|
||||
attr.filename = "TP123-837P_M456-20260520231513488-1of1_999.x12"
|
||||
attr.st_mode = 0o100644
|
||||
attr.st_size = 1024
|
||||
attr.st_mtime = 1718899200
|
||||
|
||||
mock_ssh, mock_sftp = _make_mock_paramiko(
|
||||
monkeypatch, sftp_attrs=[attr],
|
||||
)
|
||||
|
||||
block = _block(staging_dir=str(tmp_path / "staging"))
|
||||
client = SftpClient(block)
|
||||
files = client.list_inbound_names()
|
||||
|
||||
# sftp.open() must NOT have been called (no download).
|
||||
mock_sftp.open.assert_not_called()
|
||||
assert len(files) == 1
|
||||
# The InboundFile.local_path is set to the planned cache path
|
||||
# but the file itself doesn't exist yet.
|
||||
assert not files[0].local_path.exists()
|
||||
|
||||
|
||||
class TestRealModeReadFile:
|
||||
def test_read_returns_bytes(self, monkeypatch):
|
||||
|
||||
Reference in New Issue
Block a user