From 7d8a506f4de7b5b386d02cf76f6e0a35b08db980 Mon Sep 17 00:00:00 2001 From: Andrey Epifancev Date: Fri, 9 Oct 2026 16:54:54 +0400 Subject: [PATCH] =?UTF-8?q?fix(hotfolder):=20=D0=BE=D1=88=D0=B8=D0=B1?= =?UTF-8?q?=D0=BA=D0=B0=20=D0=BE=D1=82=D0=BA=D1=80=D1=8B=D1=82=D0=B8=D1=8F?= =?UTF-8?q?=20=D0=B2=D1=85=D0=BE=D0=B4=D1=8F=D1=89=D0=B5=D0=B3=D0=BE=20?= =?UTF-8?q?=D1=84=D0=B0=D0=B9=D0=BB=D0=B0=20=D0=BD=D0=B5=20=D1=83=D0=B4?= =?UTF-8?q?=D0=B0=D0=BB=D1=8F=D0=B5=D1=82=20=D1=84=D0=B0=D0=B9=D0=BB=20?= =?UTF-8?q?=D0=BF=D1=80=D0=BE=D1=88=D0=BB=D0=BE=D0=B9=20=D0=B4=D0=BE=D1=81?= =?UTF-8?q?=D1=82=D0=B0=D0=B2=D0=BA=D0=B8=20(#26,=20I-1)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Открытие вынесено из блока снятия недописанного файла: при SCAN_CLEANUP=true удаляется только файл, сбойнувший после открытия. Решения R-5…R-8 — в design.md. Co-Authored-By: Claude Opus 5.5 --- api/src/gateway/hotfolder.py | 7 ++- api/src/gateway/smb.py | 6 ++- api/tests/unit/test_local_hotfolder.py | 46 +++++++++++++++---- api/tests/unit/test_smb_hotfolder.py | 45 ++++++++++++++++-- .../changes/hotfolder-direct-write/design.md | 4 ++ 5 files changed, 92 insertions(+), 16 deletions(-) diff --git a/api/src/gateway/hotfolder.py b/api/src/gateway/hotfolder.py index e1a2a01..cbc554b 100644 --- a/api/src/gateway/hotfolder.py +++ b/api/src/gateway/hotfolder.py @@ -204,12 +204,15 @@ class LocalHotFolder: def put(self, name: str, data: bytes) -> None: """Записать `input_dir/` сразу (`wb`, перезапись существующего, как в старом АРМ — решение владельца 09.10.2026). `Input` создаётся без родителей, корень — нет (нет корня — `FileNotFoundError`). При ошибке записи - недописанный файл снимается только при включённой очистке (R-3), ошибка снятия подавляется.""" + недописанный файл снимается только при включённой очистке (R-3), ошибка снятия подавляется. Ошибка самого + открытия ничего не удаляет: на месте может лежать целый файл прошлой доставки (I-1).""" validate_name(name) self.input_dir.mkdir(exist_ok=True) target = self.input_dir / name + handle = open(target, "wb") # noqa: SIM115 — открытие вне try: его ошибка не снимает чужой файл (I-1) try: - target.write_bytes(data) + with handle: + handle.write(data) except BaseException: self._drop_partial(target) raise diff --git a/api/src/gateway/smb.py b/api/src/gateway/smb.py index 5f2c62f..5699b3d 100644 --- a/api/src/gateway/smb.py +++ b/api/src/gateway/smb.py @@ -104,13 +104,15 @@ class SmbHotFolder: файла и rename нет, существующий файл перезаписывается. `smbclient.open_file` открывает файл монопольно — пока запись идёт, ИС его не прочитает (ASSUMPTION(OQ-79)). Имя проверяется до любого обращения к шаре. При ошибке записи недописанный файл снимается только при включённой очистке (`SCAN_CLEANUP`, R-3); ошибка снятия исходную - не маскирует. Нет корня — `SMBOSError` (ENOENT) от `mkdir`. + не маскирует. Ошибка самого открытия (файл занят ИС, нет прав) ничего не удаляет: недописанного файла нет, а на + месте может лежать целый файл прошлой доставки (I-1). Нет корня — `SMBOSError` (ENOENT) от `mkdir`. """ validate_name(name) self._ensure_dir(self.input_dir) target = ntpath.join(self.input_dir, name) + fd = smbclient.open_file(target, mode="wb", **self._credentials) try: - with smbclient.open_file(target, mode="wb", **self._credentials) as fd: + with fd: fd.write(data) except BaseException: self._drop_partial(target) diff --git a/api/tests/unit/test_local_hotfolder.py b/api/tests/unit/test_local_hotfolder.py index 910ede2..c85c8e4 100644 --- a/api/tests/unit/test_local_hotfolder.py +++ b/api/tests/unit/test_local_hotfolder.py @@ -12,6 +12,7 @@ from typing import Any import pytest +from src.gateway import hotfolder as hotfolder_module from src.gateway.hotfolder import AnswerFile, HotFolder, LocalHotFolder, SettleTracker, inbox_name @@ -41,11 +42,25 @@ def _settled(tmp_path: Path, settle: float = 2) -> tuple[LocalHotFolder, FakeClo return LocalHotFolder(tmp_path, settle_seconds=settle, clock=clock), clock -def _failing_write(self: Path, data: bytes) -> int: - """Недописанный файл: на диск попадают первые два байта, затем ошибка ввода-вывода.""" - with open(self, "wb") as handle: - handle.write(data[:2]) - raise OSError(errno.ENOSPC, "диск полон") +class _FailingFile: + """Файл, открытый на запись: `write` пишет первые два байта и падает (диск полон).""" + + def __init__(self, path: Any, mode: str) -> None: + self._handle = open(path, mode) # noqa: SIM115 — закрывается в __exit__ + + def __enter__(self) -> "_FailingFile": + return self + + def __exit__(self, *exc: object) -> None: + self._handle.close() + + def write(self, data: bytes) -> int: + self._handle.write(data[:2]) + raise OSError(errno.ENOSPC, "диск полон") + + +def _failing_open(path: Any, mode: str = "r", *args: Any, **kwargs: Any) -> _FailingFile: + return _FailingFile(path, mode) def _failing_unlink(self: Path, missing_ok: bool = False) -> None: @@ -186,7 +201,7 @@ class TestPut: self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: hf = LocalHotFolder(tmp_path, settle_seconds=0) - monkeypatch.setattr(Path, "write_bytes", _failing_write) + monkeypatch.setattr(hotfolder_module, "open", _failing_open, raising=False) with pytest.raises(OSError, match="диск полон"): hf.put("job.pdf", b"data") assert (hf.input_dir / "job.pdf").read_bytes() == b"da" @@ -195,18 +210,33 @@ class TestPut: self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: hf = LocalHotFolder(tmp_path, settle_seconds=0, cleanup=True) - monkeypatch.setattr(Path, "write_bytes", _failing_write) + monkeypatch.setattr(hotfolder_module, "open", _failing_open, raising=False) with pytest.raises(OSError, match="диск полон"): hf.put("job.pdf", b"data") assert not (hf.input_dir / "job.pdf").exists() def test_remove_failure_does_not_mask_write_error(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: hf = LocalHotFolder(tmp_path, settle_seconds=0, cleanup=True) - monkeypatch.setattr(Path, "write_bytes", _failing_write) + monkeypatch.setattr(hotfolder_module, "open", _failing_open, raising=False) monkeypatch.setattr(Path, "unlink", _failing_unlink) with pytest.raises(OSError, match="диск полон"): hf.put("job.pdf", b"data") + @pytest.mark.parametrize("cleanup", [False, True]) + def test_open_failure_never_removes_existing_file(self, tmp_path: Path, cleanup: bool) -> None: + """I-1: открыть не удалось — недописанного файла нет, целый файл прошлой доставки не трогаем.""" + hf = LocalHotFolder(tmp_path, settle_seconds=0, cleanup=cleanup) + hf.input_dir.mkdir() + delivered = hf.input_dir / "job.pdf" + delivered.write_bytes(b"delivered") + delivered.chmod(0o444) + try: + with pytest.raises(PermissionError): + hf.put("job.pdf", b"new") + assert delivered.read_bytes() == b"delivered" + finally: + delivered.chmod(0o644) + def test_existing_input_dir_is_fine(self, tmp_path: Path) -> None: (tmp_path / "Input").mkdir() LocalHotFolder(tmp_path).put("a.pdf", b"1") diff --git a/api/tests/unit/test_smb_hotfolder.py b/api/tests/unit/test_smb_hotfolder.py index 5962743..ab725e5 100644 --- a/api/tests/unit/test_smb_hotfolder.py +++ b/api/tests/unit/test_smb_hotfolder.py @@ -170,6 +170,26 @@ def _write_answer(share_root: Path, job_id: str, data: bytes, name: str | None = return path +def _fail_write(fake: FakeSmbClient) -> SMBOSError: + """`open_file` открывает файл, но `write` пишет два байта и падает (диск полон): недописанный файл остаётся.""" + error = SMBOSError(NtStatus.STATUS_DISK_FULL, INPUT) + real_open = fake.open_file + + def open_file(path: str, mode: str = "r", **kwargs: Any) -> IO[Any]: + handle = real_open(path, mode, **kwargs) + + def write(data: bytes) -> int: + handle.raw.write(data[:2]) if hasattr(handle, "raw") else None + handle.flush() + raise error + + handle.write = write # type: ignore[method-assign] + return handle + + fake.open_file = open_file # type: ignore[method-assign] + return error + + class TestPaths: def test_default_layout(self) -> None: hf = _hotfolder() @@ -309,28 +329,45 @@ class TestPut: def test_write_failure_keeps_partial_file_when_cleanup_disabled( self, fake: FakeSmbClient, share_root: Path ) -> None: - fake.fail["open_file"] = SMBOSError(NtStatus.STATUS_ACCESS_DENIED, INPUT) + _fail_write(fake) with pytest.raises(SMBOSError): _hotfolder().put("a.pdf", b"data") + assert (share_root / "Input" / "a.pdf").read_bytes() == b"da" assert "remove" not in fake.funcs() def test_write_failure_removes_partial_file_when_cleanup_enabled( self, fake: FakeSmbClient, share_root: Path ) -> None: - fake.fail["open_file"] = SMBOSError(NtStatus.STATUS_ACCESS_DENIED, INPUT) + _fail_write(fake) with pytest.raises(SMBOSError): _hotfolder(cleanup=True).put("a.pdf", b"data") assert fake.calls[-1] == ("remove", (f"{INPUT}\\a.pdf",), {}) + assert not (share_root / "Input" / "a.pdf").exists() def test_cleanup_error_does_not_mask_original(self, fake: FakeSmbClient, share_root: Path) -> None: - original = SMBOSError(NtStatus.STATUS_ACCESS_DENIED, INPUT) - fake.fail["open_file"] = original + original = _fail_write(fake) fake.fail["remove"] = SMBException("соединение потеряно") with pytest.raises(SMBOSError) as excinfo: _hotfolder(cleanup=True).put("a.pdf", b"data") assert excinfo.value is original assert fake.funcs() == ["mkdir", "open_file", "remove"] + @pytest.mark.parametrize("cleanup", [False, True]) + def test_open_failure_never_removes_existing_file( + self, fake: FakeSmbClient, share_root: Path, cleanup: bool + ) -> None: + """I-1: открыть не удалось (например, ИС держит файл повторённого задания) — недописанного файла нет, целый + файл прошлой доставки не трогаем даже при включённой очистке; ошибка открытия поднимается как есть.""" + (share_root / "Input").mkdir() + (share_root / "Input" / "a.pdf").write_bytes(b"delivered") + denied = SMBOSError(NtStatus.STATUS_SHARING_VIOLATION, INPUT + "\\a.pdf") + fake.fail["open_file"] = denied + with pytest.raises(SMBOSError) as excinfo: + _hotfolder(cleanup=cleanup).put("a.pdf", b"new") + assert excinfo.value is denied + assert "remove" not in fake.funcs() + assert (share_root / "Input" / "a.pdf").read_bytes() == b"delivered" + def test_passes_credentials_to_every_call(self, fake: FakeSmbClient, share_root: Path) -> None: _hotfolder(login="user", password="secret").put("a.pdf", b"1") assert [kwargs for _, _, kwargs in fake.calls] == [{"username": "user", "password": "secret"}] * 2 diff --git a/openspec/changes/hotfolder-direct-write/design.md b/openspec/changes/hotfolder-direct-write/design.md index 4d7e198..09eed69 100644 --- a/openspec/changes/hotfolder-direct-write/design.md +++ b/openspec/changes/hotfolder-direct-write/design.md @@ -56,3 +56,7 @@ | R-2 | 2026-10-09 | `Input` по-прежнему создаётся перед записью (D-2), хотя старый АРМ этого не делал | Отказ в правах на `mkdir` при существующем каталоге уже не ошибка; dev и тесты на этом держатся | Лишний вызов `mkdir` на шаре на каждое задание | | R-3 | 2026-10-09 | Ошибка записи: недописанный файл в `Input` удаляется только при `SCAN_CLEANUP=true` (D-5) | Требование «при `SCAN_CLEANUP=false` ничего не удалять» (OQ-144) сохраняется | Обрезанный файл остаётся в `Input`, ИС может его распознать с ошибкой; ядро получит `RECOGNITION_FAILED` от шлюза раньше | | R-4 | 2026-10-09 | `SCAN_TMP_DIR` удаляется из шлюза, у эмулятора остаётся (D-3) | Шлюзу каталог больше не нужен; эмулятору нужен для атомарной сборки ответа | Нет: `extra="ignore"` — старые `.env` стартуют | +| R-5 | 2026-10-09 | Правка вызова `SmbHotFolder` в фабрике (`gateway/__init__.py`) перенесена из задачи 2 плана в задачу 1 | Без неё mypy красный между задачами (лишний позиционный аргумент) | Нет | +| R-6 | 2026-10-09 | Сверх плана: `tests/unit/conftest.py` не передаёт `SCAN_TMP_DIR`; `test_worker.py::test_existing_input_waits_for_answer` ждёт перезаписи объектом S3; в `test_roundtrip.py` эмулятору передаётся литерал `"Tmp"` | Прямые следствия D-1 и D-3 | Нет | +| R-7 | 2026-10-09 | Финальное ревью I-1: ошибка **открытия** входящего файла ничего не удаляет (открытие вынесено из блока снятия недописанного файла); снимается только файл, сбойнувший после открытия | Иначе при `SCAN_CLEANUP=true` удалялся бы целый файл прошлой доставки, который, возможно, читает ИС | Нет | +| R-8 | 2026-10-09 | Финальное ревью I-1(б): если при повторе задания в другом процессе файл занят ИС (`STATUS_SHARING_VIOLATION` на открытии), шлюз публикует `RECOGNITION_FAILED`, а не считает файл доставленным — как старый АРМ (`wb` без особых случаев) | Решение владельца «как в старом АРМ»; повтор в другом процессе — только после потери ack (редко) | Ядро может получить `RECOGNITION_FAILED` раньше успешного ответа первого экземпляра (OCR `max_attempts` = 1) — аналитику придётся перезагрузить файл; вынесено вопросом владельцу |