From 404e8cc5c52190d36e52fffab6bf2710aa07829d Mon Sep 17 00:00:00 2001 From: CaliBrain Date: Mon, 6 Jul 2026 01:51:58 -0400 Subject: [PATCH] path timeouts (#1103) - Add configurable completed-path wait for external clients : - Add a configurable Advanced setting, DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT, for how long Shelfmark waits after a torrent or usenet client reports completion before treating the completed path as missing. - Keep the default at the existing 60-second grace period, with a maximum of 3600 seconds - Add e2e testing - Add e2e testing Should fix #861 --- .github/workflows/e2e-platform.yml | 2 +- docs/configuration.md | 4 + docs/environment-variables.md | 11 ++ shelfmark/config/settings.py | 49 ++++++ shelfmark/download/clients/base_handler.py | 115 ++++++++++++-- tests/config/test_environment.py | 9 ++ tests/e2e/platform/docker-compose.e2e.yml | 50 ++++++- .../env/client-qbittorrent-delayed.env | 21 +++ tests/e2e/platform/run-e2e.sh | 6 + tests/e2e/platform/run-matrix.sh | 3 +- .../platform/suite/test_cluster_clients.py | 32 +++- tests/prowlarr/test_remote_path_mappings.py | 141 ++++++++++++++++++ 12 files changed, 422 insertions(+), 21 deletions(-) create mode 100644 tests/e2e/platform/env/client-qbittorrent-delayed.env diff --git a/.github/workflows/e2e-platform.yml b/.github/workflows/e2e-platform.yml index 97cda72b..8fda89fa 100644 --- a/.github/workflows/e2e-platform.yml +++ b/.github/workflows/e2e-platform.yml @@ -54,7 +54,7 @@ jobs: if [ "${{ github.event_name }}" = "pull_request" ]; then echo 'profiles=["baseline","bypasser-external","dns-blocked"]' >> "$GITHUB_OUTPUT" else - echo 'profiles=["baseline","bypasser-external","bypasser-disabled","dns-manual","dns-blocked","dns-doh","proxy-http","proxy-socks","tor","client-transmission","client-deluge"]' >> "$GITHUB_OUTPUT" + echo 'profiles=["baseline","bypasser-external","bypasser-disabled","dns-manual","dns-blocked","dns-doh","proxy-http","proxy-socks","tor","client-transmission","client-deluge","client-qbittorrent-delayed"]' >> "$GITHUB_OUTPUT" fi e2e: diff --git a/docs/configuration.md b/docs/configuration.md index 5b1632f8..190c281e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -91,6 +91,10 @@ Example: - Shelfmark can see the same files at `/downloads/books/...` - Add a mapping from Remote Path `/data/torrents` to Local Path `/downloads` +If the files are copied or synced into Shelfmark on a delay, increase **Completed Path Wait (seconds)** +in Settings -> Advanced. The default is 60 seconds; seedbox or remote-sync setups may need a value +longer than the sync interval. + ## File Processing Options ### Transfer Method (Torrent / Usenet Only) diff --git a/docs/environment-variables.md b/docs/environment-variables.md index c4666390..0c414eb7 100644 --- a/docs/environment-variables.md +++ b/docs/environment-variables.md @@ -1051,6 +1051,7 @@ Comma-separated hosts to bypass proxy (e.g., localhost,127.0.0.1,10.*,*.local) | `CUSTOM_SCRIPT` | Path to a script to run after each successful download. Must be executable. | string | _none_ | | `CUSTOM_SCRIPT_PATH_MODE` | Pass the path to the custom script as an absolute path or relative to the destination folder. | string (choice) | `absolute` | | `CUSTOM_SCRIPT_JSON_PAYLOAD` | Send a JSON payload to the script via stdin. Useful for multi-file imports (audiobooks) or richer metadata without relying on path parsing. | boolean | `false` | +| `DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT` | How long to wait after a torrent or usenet client reports completion for the completed file path to become visible to Shelfmark. Increase this for seedbox or remote-sync workflows. | number | `60` | | `COVERS_CACHE_ENABLED` | Cache book covers on the server for faster loading. | boolean | `true` | | `COVERS_CACHE_TTL` | How long to keep cached covers. Set to 0 to keep forever (recommended for static artwork). | number | `0` | | `COVERS_CACHE_MAX_SIZE_MB` | Maximum disk space for cached covers. Oldest images are removed when limit is reached. | number | `500` | @@ -1131,6 +1132,16 @@ Send a JSON payload to the script via stdin. Useful for multi-file imports (audi - **Type:** boolean - **Default:** `false` +#### `DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT` + +**Completed Path Wait (seconds)** + +How long to wait after a torrent or usenet client reports completion for the completed file path to become visible to Shelfmark. Increase this for seedbox or remote-sync workflows. + +- **Type:** number +- **Default:** `60` +- **Constraints:** min: 0, max: 3600 + #### `COVERS_CACHE_ENABLED` **Enable Cover Cache** diff --git a/shelfmark/config/settings.py b/shelfmark/config/settings.py index 0e5076e7..e0f3727b 100644 --- a/shelfmark/config/settings.py +++ b/shelfmark/config/settings.py @@ -36,6 +36,9 @@ from shelfmark.core.settings_registry import ( register_settings, ) +_DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT_DEFAULT = 60 +_DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT_MAX = 3600 + def _on_save_advanced(values: dict[str, Any]) -> dict[str, Any]: """Validate advanced settings before persisting.""" @@ -43,6 +46,40 @@ def _on_save_advanced(values: dict[str, Any]) -> dict[str, Any]: logger = setup_logger(__name__) + timeout_key = "DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT" + if timeout_key in values: + raw_timeout = values.get(timeout_key) + if isinstance(raw_timeout, bool): + return { + "error": True, + "message": "Completed Path Wait must be a number of seconds", + "values": values, + } + if raw_timeout is None: + return { + "error": True, + "message": "Completed Path Wait must be a number of seconds", + "values": values, + } + try: + timeout_seconds = int(raw_timeout) + except TypeError, ValueError: + return { + "error": True, + "message": "Completed Path Wait must be a number of seconds", + "values": values, + } + if timeout_seconds < 0 or timeout_seconds > _DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT_MAX: + return { + "error": True, + "message": ( + "Completed Path Wait must be between 0 and " + f"{_DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT_MAX} seconds" + ), + "values": values, + } + values[timeout_key] = timeout_seconds + mappings = values.get("PROWLARR_REMOTE_PATH_MAPPINGS") if mappings is None: return {"error": False, "values": values} @@ -1780,6 +1817,18 @@ def advanced_settings() -> list[SettingsField]: title="Remote Path Mappings", description="Map download client paths to paths inside Shelfmark. Needed when volume mounts differ between containers.", ), + NumberField( + key="DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT", + label="Completed Path Wait (seconds)", + description=( + "How long to wait after a torrent or usenet client reports completion " + "for the completed file path to become visible to Shelfmark. Increase " + "this for seedbox or remote-sync workflows." + ), + default=_DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT_DEFAULT, + min_value=0, + max_value=_DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT_MAX, + ), TableField( key="PROWLARR_REMOTE_PATH_MAPPINGS", label="Path Mappings", diff --git a/shelfmark/download/clients/base_handler.py b/shelfmark/download/clients/base_handler.py index 8571f79d..7a1d69a5 100644 --- a/shelfmark/download/clients/base_handler.py +++ b/shelfmark/download/clients/base_handler.py @@ -3,6 +3,7 @@ from __future__ import annotations import errno +import math import shutil import time from abc import ABC, abstractmethod @@ -55,6 +56,19 @@ SECONDS_PER_HOUR = 3600 # How long to wait for completed files to appear (seconds) COMPLETED_PATH_RETRY_INTERVAL = 5 COMPLETED_PATH_MAX_ATTEMPTS = 12 # 12 attempts * 5s = 60s grace period +COMPLETED_PATH_TIMEOUT_SETTING = "DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT" +COMPLETED_PATH_TIMEOUT_MAX_SECONDS = 3600 +_RETRYABLE_COMPLETED_PATH_ERRNOS = frozenset( + code + for code in ( + errno.ENOENT, + getattr(errno, "ESTALE", None), + getattr(errno, "EAGAIN", None), + getattr(errno, "EBUSY", None), + getattr(errno, "ETIMEDOUT", None), + ) + if code is not None +) @dataclass(frozen=True) @@ -69,6 +83,39 @@ class DownloadRequest: ratio_limit: float | None = None +@dataclass(frozen=True) +class _CompletedPathResolution: + path: Path | None + error: str | None + retryable: bool + + +def _coerce_completed_path_timeout_seconds(value: object, default: float) -> float: + if isinstance(value, bool) or value is None: + return default + if isinstance(value, (int, float)): + parsed = float(value) + elif isinstance(value, str): + try: + parsed = float(value.strip()) + except ValueError: + return default + else: + return default + + if not math.isfinite(parsed) or parsed < 0: + return default + return min(parsed, float(COMPLETED_PATH_TIMEOUT_MAX_SECONDS)) + + +def _is_retryable_completed_path_probe(error: OSError | None) -> bool: + return error is not None and error.errno in _RETRYABLE_COMPLETED_PATH_ERRNOS + + +def _path_needs_mapping(path: str) -> bool: + return (len(path) >= WINDOWS_DRIVE_PREFIX_LENGTH and path[1] == ":") or "\\" in path + + def _diagnose_path_issue(path: str) -> str: """Analyze a path and return diagnostic hints for common issues. @@ -167,6 +214,12 @@ class ExternalClientHandler(DownloadHandler, ABC): """Maximum attempts when waiting for completed files.""" return COMPLETED_PATH_MAX_ATTEMPTS + def _completed_path_timeout_seconds(self) -> float: + """Total time to wait for completed files to appear on disk.""" + fallback = self._completed_path_retry_interval() * self._completed_path_max_attempts() + configured = config.get(COMPLETED_PATH_TIMEOUT_SETTING, fallback) + return _coerce_completed_path_timeout_seconds(configured, fallback) + def _get_category_for_task(self, client: DownloadClient, task: DownloadTask) -> str | None: """Get audiobook category if configured and applicable, else None for default.""" if not is_audiobook(task.content_type): @@ -387,6 +440,21 @@ class ExternalClientHandler(DownloadHandler, ABC): log_details: bool, ) -> tuple[Path | None, str | None]: """Resolve and validate the completed download path once.""" + result = self._resolve_download_path_once_detailed( + client, + download_id, + log_details=log_details, + ) + return result.path, result.error + + def _resolve_download_path_once_detailed( + self, + client: DownloadClient, + download_id: str, + *, + log_details: bool, + ) -> _CompletedPathResolution: + """Resolve and validate a completed path, including retryability.""" try: raw_path = client.get_download_path(download_id) except Exception as e: @@ -402,7 +470,7 @@ class ExternalClientHandler(DownloadHandler, ABC): logger.debug( "Failed to resolve download path for %s %s: %s", client.name, download_id, e ) - return None, message + return _CompletedPathResolution(None, message, retryable=False) if not raw_path: message = ( @@ -417,7 +485,7 @@ class ExternalClientHandler(DownloadHandler, ABC): logger.debug( "Download client returned empty path for %s %s", client.name, download_id ) - return None, message + return _CompletedPathResolution(None, message, retryable=False) from shelfmark.core.path_mappings import ( get_client_host_identifier, @@ -457,7 +525,7 @@ class ExternalClientHandler(DownloadHandler, ABC): logger.error(failure_log, *failure_args) else: logger.debug(failure_log, *failure_args) - return None, message + return _CompletedPathResolution(None, message, retryable=False) remapped_exists, remapped_error = _probe_completed_path(remapped) @@ -480,7 +548,7 @@ class ExternalClientHandler(DownloadHandler, ABC): source_path_obj, remapped, ) - return remapped, None + return _CompletedPathResolution(remapped, None, retryable=False) message = ( f"Remapped path '{remapped}' does not exist. " @@ -499,7 +567,11 @@ class ExternalClientHandler(DownloadHandler, ABC): logger.error(failure_log, *failure_args) else: logger.debug(failure_log, *failure_args) - return None, message + return _CompletedPathResolution( + None, + message, + retryable=_is_retryable_completed_path_probe(remapped_error), + ) source_exists, source_error = _probe_completed_path(source_path_obj) @@ -522,7 +594,7 @@ class ExternalClientHandler(DownloadHandler, ABC): download_id, source_path_obj, ) - return source_path_obj, None + return _CompletedPathResolution(source_path_obj, None, retryable=False) hint = _diagnose_path_issue(raw_path) if mappings: @@ -555,7 +627,12 @@ class ExternalClientHandler(DownloadHandler, ABC): logger.error(failure_log, *failure_args) else: logger.debug(failure_log, *failure_args) - return None, message + return _CompletedPathResolution( + None, + message, + retryable=not _path_needs_mapping(raw_path) + and _is_retryable_completed_path_probe(source_error), + ) def _wait_for_completed_path( self, @@ -567,23 +644,37 @@ class ExternalClientHandler(DownloadHandler, ABC): ) -> tuple[Path | None, str | None]: """Wait briefly for completed files to appear on disk.""" last_error: str | None = None - max_attempts = self._completed_path_max_attempts() retry_interval = self._completed_path_retry_interval() + timeout_seconds = self._completed_path_timeout_seconds() + if retry_interval <= 0 or timeout_seconds <= 0: + max_attempts = 1 + else: + max_attempts = int(math.ceil(timeout_seconds / retry_interval)) + 1 for attempt in range(1, max_attempts + 1): if cancel_flag and cancel_flag.is_set(): return None, last_error log_details = attempt == max_attempts - resolved_path, error = self._resolve_download_path_once( + result = self._resolve_download_path_once_detailed( client, download_id, log_details=log_details, ) - if resolved_path: - return resolved_path, None + if result.path: + return result.path, None - last_error = error + last_error = result.error + + if not result.retryable: + if not log_details: + logger.error( + "Completed path resolution is not retryable for %s (%s): %s", + client.name, + download_id, + last_error, + ) + return None, last_error if attempt < max_attempts: status_callback("locating", "Waiting for completed files...") diff --git a/tests/config/test_environment.py b/tests/config/test_environment.py index e093a73c..f36a9485 100644 --- a/tests/config/test_environment.py +++ b/tests/config/test_environment.py @@ -507,6 +507,15 @@ class TestConcurrencyConfiguration: assert interval >= 1 assert interval <= 10 + def test_completed_path_timeout_default(self): + """Completed external-client path wait should default to the legacy grace period.""" + from shelfmark.core.config import config + + config.refresh() + + timeout = config.get("DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT", 60) + assert timeout == 60 + # ============================================================================= # Cache Configuration Tests diff --git a/tests/e2e/platform/docker-compose.e2e.yml b/tests/e2e/platform/docker-compose.e2e.yml index ca036c24..1c1f8482 100644 --- a/tests/e2e/platform/docker-compose.e2e.yml +++ b/tests/e2e/platform/docker-compose.e2e.yml @@ -62,6 +62,7 @@ services: QBITTORRENT_USERNAME: "${SM_QBITTORRENT_USERNAME:-}" QBITTORRENT_PASSWORD: "${SM_QBITTORRENT_PASSWORD:-}" QBITTORRENT_CATEGORY: "${SM_QBITTORRENT_CATEGORY:-}" + DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT: "${SM_DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT:-60}" # transmission / deluge / rtorrent (client-* profiles) TRANSMISSION_URL: "${SM_TRANSMISSION_URL:-}" TRANSMISSION_USERNAME: "${SM_TRANSMISSION_USERNAME:-}" @@ -90,7 +91,7 @@ services: volumes: - ./.state/config:/config - ./.state/books:/books - - ./.state/downloads:/downloads + - ${SM_DOWNLOADS_HOST_DIR:-./.state/downloads}:/downloads - ./.state/tmp:/tmp/shelfmark networks: - e2e @@ -211,7 +212,7 @@ services: mock-prowlarr: <<: *mock-build container_name: e2e-mock-prowlarr - profiles: ["full", "client-transmission", "client-deluge"] + profiles: ["full", "client-transmission", "client-deluge", "client-qbittorrent-delayed"] environment: MOCK_ROLE: prowlarr AA_INTERNAL_URL: http://mock-aa @@ -221,13 +222,13 @@ services: aliases: - prowlarr.mock.test - # ----- Real qBittorrent download client (profile: full) ----------------- # + # ----- Real qBittorrent download client (profiles: full, delayed path) --- # # Auth is bypassed for the e2e subnet (qBittorrent.conf) so shelfmark connects # without juggling the image's random temp password. qbittorrent: image: lscr.io/linuxserver/qbittorrent:latest container_name: e2e-qbittorrent - profiles: ["full"] + profiles: ["full", "client-qbittorrent-delayed"] environment: PUID: "1000" PGID: "1000" @@ -235,8 +236,45 @@ services: WEBUI_PORT: "8080" volumes: - ./qbittorrent/qBittorrent.conf:/config/qBittorrent/qBittorrent.conf - # Shared with shelfmark so completed files are visible for the move step. - - ./.state/downloads:/downloads + # Normally shared with Shelfmark. In the delayed profile this points at a + # client-only directory and the sync sidecar makes it visible later. + - ${SM_QBITTORRENT_DOWNLOADS_HOST_DIR:-./.state/downloads}:/downloads + networks: + - e2e + + # ----- Delayed file visibility sidecar (profile: client-qbittorrent-delayed) # + # Simulates a seedbox/remote-sync workflow: qBittorrent writes into one host + # directory while Shelfmark sees a separate /downloads directory that is synced + # only after a delay. This exercises DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT. + delayed-download-sync: + image: alpine:3.20 + container_name: e2e-delayed-download-sync + profiles: ["client-qbittorrent-delayed"] + environment: + SYNC_DELAY_SECONDS: "${SM_DELAYED_DOWNLOAD_SYNC_SECONDS:-20}" + command: + - /bin/sh + - -c + - | + mkdir -p /synced + while true; do + found=0 + for path in /remote/*; do + [ -e "$${path}" ] || continue + found=1 + before="$$(du -sk /remote 2>/dev/null | awk '{print $$1}')" + sleep "$${SYNC_DELAY_SECONDS}" + after="$$(du -sk /remote 2>/dev/null | awk '{print $$1}')" + if [ -n "$${before}" ] && [ "$${before}" = "$${after}" ]; then + cp -a /remote/. /synced/ 2>/dev/null || true + fi + break + done + [ "$${found}" = "1" ] || sleep 1 + done + volumes: + - ${SM_QBITTORRENT_DOWNLOADS_HOST_DIR:-./.state/downloads}:/remote:ro + - ${SM_DOWNLOADS_HOST_DIR:-./.state/downloads}:/synced networks: - e2e diff --git a/tests/e2e/platform/env/client-qbittorrent-delayed.env b/tests/e2e/platform/env/client-qbittorrent-delayed.env new file mode 100644 index 00000000..9505f15d --- /dev/null +++ b/tests/e2e/platform/env/client-qbittorrent-delayed.env @@ -0,0 +1,21 @@ +# Profile: client-qbittorrent-delayed (cluster 5 — delayed completed-path visibility) +# Same mock Prowlarr + webseed torrent as `full`, but qBittorrent writes to a +# directory Shelfmark cannot see until the delayed sync sidecar copies it over. +# This models seedbox / remote-sync setups where the client reports completion +# before the completed path exists inside Shelfmark. +COMPOSE_PROFILES=client-qbittorrent-delayed +SM_PROWLARR_ENABLED=true +SM_PROWLARR_URL=http://mock-prowlarr +SM_PROWLARR_API_KEY=e2e-test-key +SM_PROWLARR_TORRENT_CLIENT=qbittorrent +SM_QBITTORRENT_URL=http://qbittorrent:8080 +SM_QBITTORRENT_USERNAME=admin +SM_QBITTORRENT_PASSWORD=adminadmin +SM_QBITTORRENT_CATEGORY=books + +# The sync delay must be longer than the default client path polling interval +# so Shelfmark actually enters the completed-path wait loop. +SM_DELAYED_DOWNLOAD_SYNC_SECONDS=20 +SM_DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT=90 +E2E_DOWNLOAD_TIMEOUT=180 +E2E_PROFILE=client-qbittorrent-delayed diff --git a/tests/e2e/platform/run-e2e.sh b/tests/e2e/platform/run-e2e.sh index b71f4111..c12e83ff 100755 --- a/tests/e2e/platform/run-e2e.sh +++ b/tests/e2e/platform/run-e2e.sh @@ -25,6 +25,12 @@ fi # shellcheck disable=SC1090 set -a; source "$ENV_FILE"; set +a # export SM_*, COMPOSE_PROFILES, E2E_PROFILE PROFILE="${E2E_PROFILE:-baseline}" +if [[ "$PROFILE" == "client-qbittorrent-delayed" ]]; then + E2E_RUN_ID="${E2E_RUN_ID:-$(date +%s)-$$}" + export SM_DOWNLOADS_HOST_DIR="${SM_DOWNLOADS_HOST_DIR:-./.state/$PROFILE/$E2E_RUN_ID/shelfmark-downloads}" + export SM_QBITTORRENT_DOWNLOADS_HOST_DIR="${SM_QBITTORRENT_DOWNLOADS_HOST_DIR:-./.state/$PROFILE/$E2E_RUN_ID/client-downloads}" + mkdir -p "$SM_DOWNLOADS_HOST_DIR" "$SM_QBITTORRENT_DOWNLOADS_HOST_DIR" +fi COMPOSE=(docker compose --env-file "$ENV_FILE" -f docker-compose.e2e.yml) STATE_DIR="$PLATFORM_DIR/.state" diff --git a/tests/e2e/platform/run-matrix.sh b/tests/e2e/platform/run-matrix.sh index 06733796..9a198aec 100755 --- a/tests/e2e/platform/run-matrix.sh +++ b/tests/e2e/platform/run-matrix.sh @@ -12,7 +12,8 @@ if [[ $# -gt 0 ]]; then PROFILES=("$@") else PROFILES=(baseline bypasser-external bypasser-disabled dns-manual dns-blocked dns-doh \ - proxy-http proxy-socks tor client-transmission client-deluge) + proxy-http proxy-socks tor client-transmission client-deluge \ + client-qbittorrent-delayed) fi # Build the (heavy) images once, then reuse them across every profile so the diff --git a/tests/e2e/platform/suite/test_cluster_clients.py b/tests/e2e/platform/suite/test_cluster_clients.py index efb2558a..441984c7 100644 --- a/tests/e2e/platform/suite/test_cluster_clients.py +++ b/tests/e2e/platform/suite/test_cluster_clients.py @@ -14,12 +14,19 @@ client matrix. from __future__ import annotations import os +import shutil +import subprocess import time from pathlib import Path import pytest -pytestmark = pytest.mark.profiles("full", "client-transmission", "client-deluge") +pytestmark = pytest.mark.profiles( + "full", + "client-transmission", + "client-deluge", + "client-qbittorrent-delayed", +) BOOK = "Moby Dick" @@ -85,3 +92,26 @@ def test_prowlarr_to_real_torrent_client_download(client, active_profile) -> Non time.sleep(2) assert new_files, f"[{active_profile}] client completed but no file landed in /books" assert all(Path(n).suffix for n in new_files), f"file without extension: {new_files}" + + if active_profile == "client-qbittorrent-delayed": + _assert_completed_path_wait_engaged() + + +def _assert_completed_path_wait_engaged() -> None: + """Confirm the delayed-sync profile actually exercised the path wait loop.""" + if shutil.which("docker") is None: + return + result = subprocess.run( + ["docker", "logs", "e2e-shelfmark"], + capture_output=True, + text=True, + timeout=20, + check=False, + ) + blob = (result.stdout + result.stderr).lower() + if not blob.strip(): + return + assert "completed files not available yet" in blob, ( + "delayed qBittorrent profile completed, but Shelfmark logs did not show " + "the completed-path wait loop; the test may not have exercised #861" + ) diff --git a/tests/prowlarr/test_remote_path_mappings.py b/tests/prowlarr/test_remote_path_mappings.py index 8a7da4a2..cdcb5587 100644 --- a/tests/prowlarr/test_remote_path_mappings.py +++ b/tests/prowlarr/test_remote_path_mappings.py @@ -3,6 +3,7 @@ This focuses on integration of mapping logic into the Prowlarr handler. """ +import errno import tempfile from pathlib import Path from threading import Event @@ -209,6 +210,8 @@ def test_remap_fails_when_mapping_exists_but_path_missing(): "localPath": str(local_dir), } ] + if key == "DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT": + return 0 return default with ( @@ -254,6 +257,144 @@ def test_remap_fails_when_mapping_exists_but_path_missing(): assert not local_file.exists() +def test_wait_for_completed_path_allows_delayed_remapped_file(monkeypatch, tmp_path): + import shelfmark.download.clients.base_handler as base_handler + + local_file = tmp_path / "local" / "downloads" / "book.epub" + remote_path = "/remote/downloads/book.epub" + + mock_client = MagicMock() + mock_client.name = "qbittorrent" + mock_client.get_download_path.return_value = remote_path + + def config_get(key: str, default=""): + if key == "PROWLARR_REMOTE_PATH_MAPPINGS": + return [ + { + "host": "qbittorrent", + "remotePath": "/remote/downloads", + "localPath": str(local_file.parent), + } + ] + if key == "DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT": + return 1 + return default + + probe_calls = 0 + original_probe = base_handler._probe_completed_path + + def delayed_probe(path: Path): + nonlocal probe_calls + probe_calls += 1 + if path == local_file and probe_calls == 2: + local_file.parent.mkdir(parents=True) + local_file.write_text("synced") + return original_probe(path) + + handler = ProwlarrHandler() + recorder = ProgressRecorder() + monkeypatch.setattr(base_handler.config, "get", config_get) + monkeypatch.setattr(base_handler, "_probe_completed_path", delayed_probe) + monkeypatch.setattr(handler, "_completed_path_retry_interval", lambda: 0.01) + + resolved_path, error = handler._wait_for_completed_path( + mock_client, + "download_id", + cancel_flag=Event(), + status_callback=recorder.status_callback, + ) + + assert resolved_path == local_file + assert error is None + assert ("locating", "Waiting for completed files...") in recorder.status_updates + + +def test_wait_for_completed_path_fails_fast_for_unsafe_mapping(monkeypatch, tmp_path): + import shelfmark.download.clients.base_handler as base_handler + + remote_dir = tmp_path / "remote" / "downloads" + local_dir = tmp_path / "local" / "downloads" + raw_path = f"{remote_dir}/../escape/book.epub" + + mock_client = MagicMock() + mock_client.name = "qbittorrent" + mock_client.get_download_path.return_value = raw_path + + def config_get(key: str, default=""): + if key == "PROWLARR_REMOTE_PATH_MAPPINGS": + return [ + { + "host": "qbittorrent", + "remotePath": str(remote_dir), + "localPath": str(local_dir), + } + ] + if key == "DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT": + return 900 + return default + + handler = ProwlarrHandler() + recorder = ProgressRecorder() + monkeypatch.setattr(base_handler.config, "get", config_get) + monkeypatch.setattr(handler, "_completed_path_retry_interval", lambda: 999) + + resolved_path, error = handler._wait_for_completed_path( + mock_client, + "download_id", + cancel_flag=Event(), + status_callback=recorder.status_callback, + ) + + assert resolved_path is None + assert error is not None + assert "rejected unsafe path" in error + assert mock_client.get_download_path.call_count == 1 + assert recorder.status_updates == [] + + +def test_wait_for_completed_path_fails_fast_for_permission_error(monkeypatch): + import shelfmark.download.clients.base_handler as base_handler + + raw_path = "/downloads/book.epub" + + mock_client = MagicMock() + mock_client.name = "qbittorrent" + mock_client.get_download_path.return_value = raw_path + + def config_get(key: str, default=""): + if key == "PROWLARR_REMOTE_PATH_MAPPINGS": + return [] + if key == "DOWNLOAD_CLIENT_COMPLETED_PATH_TIMEOUT": + return 900 + return default + + probe_calls = 0 + + def permission_denied_probe(_path: Path): + nonlocal probe_calls + probe_calls += 1 + return False, PermissionError(errno.EACCES, "Permission denied", raw_path) + + handler = ProwlarrHandler() + recorder = ProgressRecorder() + monkeypatch.setattr(base_handler.config, "get", config_get) + monkeypatch.setattr(base_handler, "_probe_completed_path", permission_denied_probe) + monkeypatch.setattr(handler, "_completed_path_retry_interval", lambda: 999) + + resolved_path, error = handler._wait_for_completed_path( + mock_client, + "download_id", + cancel_flag=Event(), + status_callback=recorder.status_callback, + ) + + assert resolved_path is None + assert error is not None + assert "not accessible" in error + assert probe_calls == 1 + assert recorder.status_updates == [] + + def test_remaps_windows_path_to_linux(): """Test that Windows paths from external download clients are correctly remapped.""" with tempfile.TemporaryDirectory() as tmp_dir: