diff --git a/Dockerfile b/Dockerfile index dd39913..0038f85 100644 --- a/Dockerfile +++ b/Dockerfile @@ -152,9 +152,14 @@ RUN mkdir -p \ EXPOSE ${FLASK_PORT} # Add healthcheck for container status -# Uses /api/health which doesn't require authentication -HEALTHCHECK --interval=60s --timeout=60s --start-period=60s --retries=3 \ - CMD curl -s http://localhost:${FLASK_PORT}/api/health > /dev/null || exit 1 +# Uses /api/health which doesn't require authentication. +# curl needs -f so an HTTP error status fails the probe instead of passing it: +# plain `curl -s` exits 0 on a 500, which reported a broken app as healthy. +# timeout stays well under interval so a hung probe cannot occupy a whole cycle. +# --start-interval matches the daemon default (5s), made explicit so startup +# probing does not depend on that default staying put. +HEALTHCHECK --interval=30s --timeout=10s --start-period=90s --start-interval=5s --retries=3 \ + CMD curl -fsS http://localhost:${FLASK_PORT}/api/health > /dev/null || exit 1 # Use dumb-init as the entrypoint to handle signals properly ENTRYPOINT ["/usr/bin/dumb-init", "--"] diff --git a/docs/environment-variables.md b/docs/environment-variables.md index 6e395f2..d77ba45 100644 --- a/docs/environment-variables.md +++ b/docs/environment-variables.md @@ -704,7 +704,7 @@ Choose how downloaded audiobook files are named and organized. - **Type:** string (choice) - **Default:** `rename` -- **Options:** `none` (None), `rename` (Rename Only), `organize` (Rename and Organize) +- **Options:** `none` (None), `rename` (Rename Only), `organize` (Rename and Organize), `rename_and_group` (Rename and Group) #### `TEMPLATE_AUDIOBOOK_RENAME` diff --git a/shelfmark/config/settings.py b/shelfmark/config/settings.py index b510d17..1d39b49 100644 --- a/shelfmark/config/settings.py +++ b/shelfmark/config/settings.py @@ -1300,7 +1300,7 @@ def download_settings() -> list[SettingsField]: { "value": "rename_and_group", "label": "Rename and Group", - "description": "Rename single-file downloads; keep multi-file downloads grouped in their source folder.", + "description": "Rename single-file downloads; keep multi-file downloads grouped in their source folder. Do not use with ingest folders.", }, ], default="rename", diff --git a/shelfmark/download/postprocess/transfer.py b/shelfmark/download/postprocess/transfer.py index d4cd2c6..94118ca 100644 --- a/shelfmark/download/postprocess/transfer.py +++ b/shelfmark/download/postprocess/transfer.py @@ -17,6 +17,7 @@ from shelfmark.core.naming import ( sanitize_filename, ) from shelfmark.core.utils import is_audiobook as check_audiobook +from shelfmark.download.archive import is_archive from shelfmark.download.fs import ( atomic_copy, atomic_hardlink, @@ -160,6 +161,24 @@ def _transfer_single_file( return atomic_move(source_path, dest_path, max_attempts=max_attempts), "move" +def _group_folder_name(source_root: Path | None) -> str: + """Name the folder a grouped multi-file audiobook is transferred into. + + A directory names the group directly. A file cannot hold several book files + on its own, so a non-directory source that produced more than one means + `collect_staged_files` extracted an archive: the stem is the release name and + the suffix is packaging, which is why `Book.zip` groups into `Book/` rather + than `Book.zip/` or, worse, not at all. + """ + if source_root is None: + return "" + if run_blocking_io(source_root.is_dir): + return sanitize_filename(source_root.name) + if is_archive(source_root): + return sanitize_filename(source_root.stem) + return "" + + def transfer_book_files( book_files: list[Path], destination: Path, @@ -240,14 +259,8 @@ def transfer_book_files( return final_paths, None, op_counts transfer_destination = destination - if ( - is_audiobook - and len(book_files) > 1 - and organization_mode == "rename_and_group" - and source_root is not None - and run_blocking_io(source_root.is_dir) - ): - source_folder = sanitize_filename(source_root.name) + if is_audiobook and len(book_files) > 1 and organization_mode == "rename_and_group": + source_folder = _group_folder_name(source_root) if source_folder: transfer_destination = destination / source_folder run_blocking_io(transfer_destination.mkdir, parents=True, exist_ok=True) diff --git a/tests/core/test_hardlink.py b/tests/core/test_hardlink.py index ca6b139..82289a6 100644 --- a/tests/core/test_hardlink.py +++ b/tests/core/test_hardlink.py @@ -14,6 +14,7 @@ Two approaches to preserve torrent files for seeding: import os import shutil import tempfile +import zipfile from pathlib import Path from threading import Event from unittest.mock import MagicMock, patch @@ -1230,6 +1231,61 @@ class TestTorrentSourceCleanupProtection: assert Path(result) == library / expected_name assert torrent_file.exists() + def test_torrent_audiobook_archive_groups_under_the_release_name(self, tmp_path): + """Torrent: a single archive of chapters groups under the release name. + + Simulates: a torrent whose only file is "Project Hail Mary.zip" holding + two chapters. Extraction only runs with hardlinking off, and the archive + itself must stay put for seeding. + """ + from shelfmark.core.models import DownloadTask, SearchMode + from shelfmark.download.postprocess.router import ( + post_process_download as _post_process_download, + ) + + torrent_file = tmp_path / "downloads" / "Project Hail Mary.zip" + torrent_file.parent.mkdir(parents=True) + with zipfile.ZipFile(torrent_file, "w") as zf: + zf.writestr("Part 01.mp3", "audio 1") + zf.writestr("Part 02.mp3", "audio 2") + + library = tmp_path / "library" + library.mkdir() + task = DownloadTask( + task_id="torrent_archive_audiobook", + source="prowlarr", + title="Project Hail Mary", + author="Andy Weir", + format="mp3", + content_type="audiobook", + search_mode=SearchMode.UNIVERSAL, + original_download_path=str(torrent_file), + ) + + with ( + patch("shelfmark.core.config.config") as mock_orch, + patch("shelfmark.config.env.TMP_DIR", tmp_path / "staging"), + ): + mock_orch.get = self._make_config_mock( + str(library), + hardlink=False, + organization_mode="rename_and_group", + ) + mock_orch.CUSTOM_SCRIPT = None + result = _post_process_download(torrent_file, task, Event(), MagicMock()) + + grouped_dir = library / "Project Hail Mary" + assert result is not None + assert Path(result).parent == grouped_dir + assert sorted(path.name for path in grouped_dir.glob("*.mp3")) == [ + "Part 01.mp3", + "Part 02.mp3", + ] + assert not list(library.glob("*.mp3")) + # The archive stays behind for seeding, and never names the folder. + assert torrent_file.exists() + assert not (library / "Project Hail Mary.zip").exists() + def test_torrent_audiobook_multifile_hardlink(self, tmp_path): """Torrent: Multi-file audiobook - all source files preserved for seeding. diff --git a/tests/core/test_processing_integration.py b/tests/core/test_processing_integration.py index 8cea26e..a247db8 100644 --- a/tests/core/test_processing_integration.py +++ b/tests/core/test_processing_integration.py @@ -748,7 +748,14 @@ def test_archive_extraction_organize_multifile_assigns_part_numbers(tmp_path): assert files[1].name == "Archive Audio - 02.mp3" -def test_archive_extraction_grouping_does_not_use_archive_as_folder(tmp_path): +@pytest.mark.parametrize( + ("organization_mode", "grouped"), + [("rename_and_group", True), ("rename", False)], +) +def test_archive_extraction_groups_chapters_under_the_archive_stem( + tmp_path, organization_mode, grouped +): + """An extracted multi-chapter archive groups into `Book/`, never `Book.zip/`.""" from shelfmark.download.postprocess.router import ( post_process_download as _post_process_download, ) @@ -764,7 +771,7 @@ def test_archive_extraction_grouping_does_not_use_archive_as_folder(tmp_path): zf.writestr("Part 2.mp3", "audio2") task = DownloadTask( - task_id="direct-archive-audio-grouped", + task_id=f"direct-archive-audio-{organization_mode}", source="direct_download", title="Archive Audio", author="Tester", @@ -777,14 +784,18 @@ def test_archive_extraction_grouping_does_not_use_archive_as_folder(tmp_path): patch("shelfmark.core.config.config") as mock_config, patch("shelfmark.config.env.TMP_DIR", staging), ): - mock_config.get = _build_config(ingest, organization="rename_and_group") + mock_config.get = _build_config(ingest, organization=organization_mode) mock_config.CUSTOM_SCRIPT = None _sync_config(mock_config, mock_config) result = _post_process_download(archive_path, task, Event(), lambda *_args: None) + transfer_dir = ingest / "Book" if grouped else ingest assert result is not None - assert sorted(path.name for path in ingest.glob("*.mp3")) == ["Part 1.mp3", "Part 2.mp3"] + assert Path(result).parent == transfer_dir + assert sorted(path.name for path in transfer_dir.glob("*.mp3")) == ["Part 1.mp3", "Part 2.mp3"] + assert bool(list(ingest.glob("*.mp3"))) is not grouped + # The suffix is packaging, not part of the release name. assert not (ingest / "Book.zip").exists()