diff --git a/api/src/core/worker.py b/api/src/core/worker.py index 51d370b..2e21ff9 100644 --- a/api/src/core/worker.py +++ b/api/src/core/worker.py @@ -626,7 +626,7 @@ class Worker: return try: removed = await asyncio.to_thread(self._hotfolder.cleanup, job_id, name) - except Exception as exc: # noqa: BLE001 — любая ошибка удаления не влияет на конверт + except Exception as exc: # любая ошибка удаления не влияет на конверт logger.warning( "Очистка горячей папки не удалась, задание продолжается. job_id={job_id}, error={error}", job_id=job_id, diff --git a/api/src/gateway/smb.py b/api/src/gateway/smb.py index a3ca8b6..91bcb76 100644 --- a/api/src/gateway/smb.py +++ b/api/src/gateway/smb.py @@ -102,10 +102,12 @@ class SmbHotFolder: return ntpath.join(self.answer_dir(job_id), f"{job_id}.json") def put(self, name: str, data: bytes) -> None: - """Записать `..part` в служебный каталог, затем `smbclient.rename` в `\\` без замены: - назначение есть — файл считается доставленным, `.part` остаётся (удаляется только при включённой очистке). - Имя проверяется до любого обращения к шаре. При сбое временный файл удаляется (ошибка удаления не маскирует - исходную). Нет корня — `SMBOSError` (ENOENT) от первого `mkdir`. + """Записать `..part` в служебный каталог, затем `smbclient.rename` в `\\` без замены. + Имя проверяется до любого обращения к шаре. Назначение уже есть (повтор задания) — это не ошибка: файл + считается доставленным и не заменяется. В этом случае, как и при любой другой ошибке записи или переименования, + `.part` снимается только при включённой очистке (`SCAN_CLEANUP`, D-2), иначе остаётся в `Tmp`; ошибка снятия + исходную не маскирует. При успехе `.part` забирает сам rename. Нет корня — `SMBOSError` (ENOENT) от первого + `mkdir`. """ validate_name(name) self._ensure_dir(self.tmp_dir) diff --git a/api/src/normalizer/__init__.py b/api/src/normalizer/__init__.py index 5cfcc13..6287fb1 100644 --- a/api/src/normalizer/__init__.py +++ b/api/src/normalizer/__init__.py @@ -72,7 +72,12 @@ class NormalizeOutcome(BaseModel): @runtime_checkable class Normalizer(Protocol): - """Превращает склеенный ответ ИС (D-4) в `NormalizeOutcome`; не бросает исключений на любом входе.""" + """Превращает склеенный ответ ИС (D-4) в `NormalizeOutcome`. + + Структурные ошибки входа (не объект, нет `files`) возвращаются как `status = error`, а не исключением; + `NullNormalizer` не бросает ни на каком входе. `OcrNormalizer` изолирует только ошибки разбора таблицы (одна + таблица — одно предупреждение `BACKEND_PARSING_ERROR`, файл и набор живут дальше); исключения вне разбора таблиц + поднимаются сознательно (D-4): воркер превращает их в `UNKNOWN_ERROR`.""" def normalize(self, raw: object) -> NormalizeOutcome: ... diff --git a/api/tests/unit/conftest.py b/api/tests/unit/conftest.py index 3de0c1c..83d676b 100644 --- a/api/tests/unit/conftest.py +++ b/api/tests/unit/conftest.py @@ -38,7 +38,7 @@ async def make_env(tmp_path: Path) -> AsyncIterator[EnvFactory]: fixtures = tmp_path / "fixtures" fixtures.mkdir(exist_ok=True) (fixtures / "ok_doc.json").write_text(json.dumps(SUCCESS_ANSWER, ensure_ascii=False), encoding="utf-8") - (fixtures / "ok_doc.pdf").write_bytes(PDF_BYTES) # пара ответа по sha256 содержимого (эмулятор, Task 14) + (fixtures / "ok_doc.pdf").write_bytes(PDF_BYTES) # пара ответа по sha256 содержимого (эмулятор) settings = get_settings().model_copy( update={ "SCAN_HOTFOLDER_PATH": str(root), diff --git a/api/tests/unit/test_smb_hotfolder.py b/api/tests/unit/test_smb_hotfolder.py index d8c3574..74df057 100644 --- a/api/tests/unit/test_smb_hotfolder.py +++ b/api/tests/unit/test_smb_hotfolder.py @@ -39,6 +39,7 @@ class FakeSmbClient: self.base = base self.calls: list[tuple[str, tuple[str, ...], dict[str, Any]]] = [] self.fail: dict[str, BaseException] = {} # имя функции -> исключение, которое она бросит + self.plain_collision = False # rename на существующее назначение: обычный FileExistsError вместо SMBOSError self.path = SimpleNamespace(isdir=self._isdir) # подмодуль smbclient.path def _isdir(self, path: str, **kwargs: Any) -> bool: @@ -84,7 +85,10 @@ class FakeSmbClient: self._record("rename", (src, dst), kwargs) target = self._local(dst) if target.exists(): - raise FileExistsError(dst) + # настоящий smbprotocol: SMBOSError(STATUS_OBJECT_NAME_COLLISION), errno = EEXIST, не FileExistsError + raise ( + FileExistsError(dst) if self.plain_collision else SMBOSError(NtStatus.STATUS_OBJECT_NAME_COLLISION, dst) + ) os.rename(self._local(src), target) def remove(self, path: str, **kwargs: Any) -> None: @@ -96,7 +100,12 @@ class FakeSmbClient: def rmdir(self, path: str, **kwargs: Any) -> None: self._record("rmdir", (path,), kwargs) - os.rmdir(self._local(path)) + local = self._local(path) + if not local.exists(): + raise SMBOSError(NtStatus.STATUS_OBJECT_NAME_NOT_FOUND, path) + if any(local.iterdir()): + raise SMBOSError(NtStatus.STATUS_DIRECTORY_NOT_EMPTY, path) + os.rmdir(local) def scandir(self, path: str, **kwargs: Any) -> list[SimpleNamespace]: """Как `smbclient.scandir`: элементы с `name`, `is_file()` и `smb_info.end_of_file` (размер из листинга).""" @@ -315,7 +324,9 @@ class TestPut: assert excinfo.value is denied assert fake.funcs() == ["mkdir", "isdir"] - def test_no_replace_on_existing_file(self, fake: FakeSmbClient, share_root: Path) -> None: + @pytest.mark.parametrize("plain", [False, True], ids=["SMBOSError", "FileExistsError"]) + def test_no_replace_on_existing_file(self, fake: FakeSmbClient, share_root: Path, plain: bool) -> None: + fake.plain_collision = plain hf = _hotfolder() hf.put("a.pdf", b"old") hf.put("a.pdf", b"new") @@ -390,13 +401,29 @@ class TestPutWithoutReplace: assert fake._local(INPUT + "\\job.pdf").read_bytes() == b"x" assert not fake._local(TMP + "\\.job.pdf.part").exists() - def test_existing_destination_kept_and_part_left(self, fake: FakeSmbClient, share_root: Path) -> None: + @pytest.mark.parametrize("plain", [False, True], ids=["SMBOSError", "FileExistsError"]) + def test_existing_destination_kept_and_part_left(self, fake: FakeSmbClient, share_root: Path, plain: bool) -> None: + fake.plain_collision = plain hf = _hotfolder() hf.put("job.pdf", b"first") hf.put("job.pdf", b"second") assert fake._local(INPUT + "\\job.pdf").read_bytes() == b"first" assert fake._local(TMP + "\\.job.pdf.part").read_bytes() == b"second" + @pytest.mark.parametrize("plain", [False, True], ids=["SMBOSError", "FileExistsError"]) + def test_existing_destination_drops_part_when_cleanup_enabled( + self, fake: FakeSmbClient, share_root: Path, plain: bool + ) -> None: + """Назначение есть (повтор задания) и очистка включена: файл не заменён, `.part` снят.""" + fake.plain_collision = plain + hf = _hotfolder(cleanup=True) + hf.put("job.pdf", b"first") + fake.calls.clear() + hf.put("job.pdf", b"second") # исключения нет: назначение есть — файл считается доставленным + assert fake._local(INPUT + "\\job.pdf").read_bytes() == b"first" + assert not fake._local(TMP + "\\.job.pdf.part").exists() + assert fake.calls[-1] == ("remove", (f"{TMP}\\.job.pdf.part",), {}) + class TestCleanup: def test_disabled_is_noop(self, fake: FakeSmbClient, share_root: Path) -> None: @@ -439,6 +466,98 @@ class TestCleanup: hf.cleanup("job", "job.pdf") +class TestCleanupDetails: + """Очистка (D-2): имена, сироты, ошибки шары; `cleanup=True` во всех тестах.""" + + @staticmethod + def _hf(fake: FakeSmbClient, **kwargs: Any) -> SmbHotFolder: + (fake.base / HOST / "share" / "scan").mkdir(parents=True, exist_ok=True) + return _hotfolder(cleanup=True, **kwargs) + + @pytest.mark.parametrize("name", ["../x", "..\\x", "sub/a.pdf", ".hidden", "", "a:b.pdf"]) + def test_rejects_unsafe_name_before_any_call(self, fake: FakeSmbClient, name: str) -> None: + hf = self._hf(fake) + with pytest.raises(ValueError, match="имя файла"): + hf.cleanup(_job_id(), name) + assert fake.calls == [] + + def test_orphan_part_and_done_marker_are_removed(self, fake: FakeSmbClient) -> None: + hf = self._hf(fake) + job_id = _job_id() + name = f"{job_id}.pdf" + root = fake.base / HOST / "share" / "scan" + for folder in ("Input", "Tmp", "Output"): + (root / folder).mkdir() + (root / "Input" / name).write_bytes(b"pdf") + (root / "Input" / f"{name}.done").write_bytes(b"") # маркер ИС + (root / "Tmp" / f".{name}.part").write_bytes(b"orphan") # `.part` осиротевшей записи + other = root / "Input" / f"{_job_id()}.pdf" + other.write_bytes(b"other job") # чужое задание не трогаем + assert hf.cleanup(job_id, name) is True + assert sorted(p.name for p in (root / "Input").iterdir()) == [other.name] + assert list((root / "Tmp").iterdir()) == [] + removed = [args[0] for func, args, _ in fake.calls if func == "remove"] + assert removed == [f"{INPUT}\\{name}", f"{INPUT}\\{name}.done", f"{TMP}\\.{name}.part"] + + def test_scandir_access_denied_propagates_and_nothing_else_is_touched(self, fake: FakeSmbClient) -> None: + hf = self._hf(fake) + job_id = _job_id() + hf.put(f"{job_id}.pdf", b"x") + denied = SMBOSError(NtStatus.STATUS_ACCESS_DENIED, hf.answer_dir(job_id)) + fake.fail["scandir"] = denied + fake.calls.clear() + with pytest.raises(SMBOSError) as excinfo: + hf.cleanup(job_id, f"{job_id}.pdf") + assert excinfo.value is denied # не принят за «каталога нет» + assert fake.funcs() == ["scandir"] + assert fake._local(INPUT + f"\\{job_id}.pdf").exists() + + def test_subdirectory_in_answer_dir_fails_rmdir_and_propagates(self, fake: FakeSmbClient) -> None: + hf = self._hf(fake) + job_id = _job_id() + answer = fake._local(OUTPUT + f"\\{job_id}") + (answer / "nested").mkdir(parents=True) + (answer / f"{job_id}.json").write_bytes(b"{}") + with pytest.raises(SMBOSError) as excinfo: + hf.cleanup(job_id, f"{job_id}.pdf") + assert excinfo.value.ntstatus == NtStatus.STATUS_DIRECTORY_NOT_EMPTY + assert not (answer / f"{job_id}.json").exists() # файлы сняты, каталог остался вместе с подкаталогом + assert (answer / "nested").is_dir() + assert fake.funcs()[-1] == "rmdir" # дальше (входящий файл, `.done`, `.part`) очистка не пошла + + def test_passes_credentials_to_every_call(self, fake: FakeSmbClient) -> None: + hf = self._hf(fake, login="user", password="secret") + job_id = _job_id() + name = f"{job_id}.pdf" + hf.put(name, b"x") + answer = fake._local(OUTPUT + f"\\{job_id}") + answer.mkdir(parents=True) + (answer / f"{job_id}.json").write_bytes(b"{}") + (answer / f"{job_id}_ofr.json").write_bytes(b"{}") + fake.calls.clear() + assert hf.cleanup(job_id, name) is True + assert fake.funcs() == ["scandir", "remove", "remove", "rmdir", "remove", "remove", "remove"] + assert all(kwargs == {"username": "user", "password": "secret"} for _, _, kwargs in fake.calls) + + def test_missing_answer_dir_and_files_are_fine(self, fake: FakeSmbClient) -> None: + hf = self._hf(fake) + assert hf.cleanup(_job_id(), "nothing.pdf") is True + assert fake.funcs() == ["scandir", "remove", "remove", "remove"] + + def test_answer_dir_is_a_file_is_fine(self, fake: FakeSmbClient) -> None: + hf = self._hf(fake) + job_id = _job_id() + (fake.base / HOST / "share" / "scan" / "Output").mkdir() + (fake.base / HOST / "share" / "scan" / "Output" / job_id).write_bytes(b"not a dir") # ENOTDIR + assert hf.cleanup(job_id, "nothing.pdf") is True + + def test_missing_file_error_other_than_enoent_propagates(self, fake: FakeSmbClient) -> None: + hf = self._hf(fake) + fake.fail["remove"] = SMBOSError(NtStatus.STATUS_SHARING_VIOLATION, INPUT + "\\job.pdf") + with pytest.raises(SMBOSError): + hf.cleanup(_job_id(), "job.pdf") + + class TestPoll: def test_none_until_answer_dir_has_json(self, fake: FakeSmbClient, share_root: Path) -> None: hf = _hotfolder() diff --git a/api/tests/unit/test_worker.py b/api/tests/unit/test_worker.py index 7c32076..2ae1dac 100644 --- a/api/tests/unit/test_worker.py +++ b/api/tests/unit/test_worker.py @@ -219,7 +219,7 @@ async def test_real_fixture_error_is_passed_through(make_env: EnvFactory, tmp_pa real.mkdir() for source in REAL_FIXTURES.glob("*.json"): shutil.copy(source, real / source.name) - (real / "aep_5.pdf").write_bytes(PDF_BYTES) # пара ответа `aep_5.json` по sha256 содержимого запроса (Task 14) + (real / "aep_5.pdf").write_bytes(PDF_BYTES) # пара ответа `aep_5.json` по sha256 содержимого запроса expected = json.loads((real / "aep_5.json").read_text(encoding="utf-8"))["error"] env = await make_env() request = env.request(original_name="aep_5.PDF") @@ -928,19 +928,29 @@ class TestSlice3a: assert ("cleanup", f"{body['job_id']}.pdf") not in env.events async def test_cleanup_error_does_not_change_result( - self, make_env: EnvFactory, monkeypatch: pytest.MonkeyPatch + self, make_env: EnvFactory, monkeypatch: pytest.MonkeyPatch, log_lines: list[str] ) -> None: env = await make_env(SCAN_CLEANUP=True) + cleanup_calls: list[tuple[str, str]] = [] def deny(job_id: str, name: str) -> bool: + cleanup_calls.append((job_id, name)) raise PermissionError("нет прав") monkeypatch.setattr(env.hotfolder.inner, "cleanup", deny) body = env.request() async with emulator(env): result = await env.run(body) + job_id = body["job_id"] + # очистка действительно вызывалась (а не пропущена), и её ошибка не изменила исход задания + assert cleanup_calls == [(job_id, f"{job_id}.pdf")] + assert ("cleanup", f"{job_id}.pdf") in env.events assert result["status"] == "error" and result["error"]["code"] == "NOT_IMPLEMENTED" - assert "metrics" not in result or "warnings" not in result.get("metrics", {}) + assert "warnings" not in result.get("metrics", {}) # ошибка очистки в конверт не попадает + # ошибка очистки — ровно одна строка лога уровня WARNING + warnings = [line for line in log_lines if line.startswith("WARNING|")] + assert len(warnings) == 1 + assert "Очистка горячей папки не удалась" in warnings[0] and job_id in warnings[0] and "нет прав" in warnings[0] async def test_existing_input_waits_for_answer(self, make_env: EnvFactory) -> None: """Review Focus 4: входящий файл уже лежит (повтор задания), ответа нет — TIMEOUT, файл не перезаписан."""