mirror of
https://github.com/calibrain/shelfmark.git
synced 2026-09-24 21:00:31 +01:00
fix: group multi-file audiobooks that arrive as an archive (#1254)
Follow-up to #1237. \`rename_and_group\` only grouped when the source root was a directory, so a multi-file audiobook delivered as a single archive fell through to the flat path: a \`Book.zip\` of twelve chapters landed loose in the destination root with its original chapter names — the layout #1181 is about. The \`is_dir()\` guard was there to keep \`Book.zip/\` from becoming the folder name, but skipping the file case gives up the grouping instead of naming it. A non-directory source can only produce several book files by having been extracted (\`collect_staged_files\` returns a single-element list for every other file shape), so the archive stem is the release name and the suffix is packaging: group under \`Book/\`. Also regenerates the env docs for the new option and gives it the same \"do not use with ingest folders\" caveat Rename and Organize carries, since both now create directories in the destination. Tested: reverting only the source fix makes both new tests fail and the \`rename\` control case pass, so grouping stays opt-in. Full non-e2e suite green (2653 passed).
This commit is contained in:
+8
-3
@@ -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", "--"]
|
||||
|
||||
@@ -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`
|
||||
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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()
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user