diff --git a/shelfmark/download/fs.py b/shelfmark/download/fs.py index 7e3b987e..02701ea8 100644 --- a/shelfmark/download/fs.py +++ b/shelfmark/download/fs.py @@ -587,7 +587,7 @@ def atomic_hardlink(source_path: Path, dest_path: Path, max_attempts: int = 100) error=e, ) if permission_error or _hardlink_not_supported(e): - logger.debug( + logger.warning( "Hardlink failed (%s), falling back to copy: %s -> %s", e, source_path, diff --git a/shelfmark/download/postprocess/transfer.py b/shelfmark/download/postprocess/transfer.py index c78fd9ac..a0f5b7f5 100644 --- a/shelfmark/download/postprocess/transfer.py +++ b/shelfmark/download/postprocess/transfer.py @@ -13,7 +13,6 @@ from shelfmark.core.naming import ( build_library_path, derive_primary_title, parse_naming_template, - same_filesystem, sanitize_filename, ) from shelfmark.core.utils import is_audiobook as check_audiobook @@ -39,10 +38,7 @@ _TRANSFER_PROCESS_ERRORS = (AttributeError, KeyError, OSError, RuntimeError, Typ def should_hardlink(task: DownloadTask) -> bool: - """Check if hardlinking is enabled for this task (Prowlarr torrents only).""" - if task.source != "prowlarr": - return False - + """Check if hardlinking is enabled for this torrent-backed task.""" if not task.original_download_path: return False @@ -96,21 +92,21 @@ def resolve_hardlink_source( if hardlink_enabled and task.original_download_path: hardlink_source = Path(task.original_download_path) hardlink_source_exists = run_blocking_io(hardlink_source.exists) - if ( - destination - and hardlink_source_exists - and run_blocking_io(same_filesystem, hardlink_source, destination) - ): + if hardlink_source_exists: use_hardlink = True source_path = hardlink_source - elif hardlink_source_exists: - logger.warning( - "Cannot hardlink: %s and %s are on different filesystems. Falling back to copy. To fix: ensure torrent client downloads to same filesystem as destination.", + logger.info( + "Hardlink enabled for task %s; attempting link from %s to %s", + task.task_id, hardlink_source, destination, ) - if status_callback: - status_callback("resolving", "Cannot hardlink (different filesystems), using copy") + else: + logger.warning( + "Hardlink enabled for task %s, but source path does not exist: %s", + task.task_id, + hardlink_source, + ) return TransferPlan( source_path=source_path, diff --git a/tests/core/test_hardlink.py b/tests/core/test_hardlink.py index 8dc0f614..b52e398e 100644 --- a/tests/core/test_hardlink.py +++ b/tests/core/test_hardlink.py @@ -28,7 +28,6 @@ def _run_organize_post_process( task, library: Path, hardlink_enabled: bool = True, - same_fs: bool = True, ): from shelfmark.download.postprocess.router import ( post_process_download as _post_process_download, @@ -37,10 +36,7 @@ def _run_organize_post_process( status_cb = MagicMock() cancel_flag = Event() - with ( - patch("shelfmark.core.config.config") as mock_config, - patch("shelfmark.download.postprocess.transfer.same_filesystem", return_value=same_fs), - ): + with patch("shelfmark.core.config.config") as mock_config: mock_config.CUSTOM_SCRIPT = None mock_config.get = MagicMock( side_effect=lambda key, default=None, **_kwargs: { @@ -722,7 +718,6 @@ class TestHardlinkDecisionLogic: task=sample_task, library=library, hardlink_enabled=True, - same_fs=True, ) assert result is not None @@ -776,6 +771,29 @@ class TestHardlinkDecisionLogic: assert result is not None assert not staged.exists() + def test_non_prowlarr_torrent_with_original_path_can_hardlink(self, tmp_path, sample_task): + """Torrent-backed sources such as AudiobookBay can hardlink client files.""" + library = tmp_path / "library" + library.mkdir() + source = tmp_path / "downloads" / "book.m4b" + source.parent.mkdir() + source.write_bytes(b"content") + + sample_task.source = "audiobookbay" + sample_task.content_type = "audiobook" + sample_task.format = "m4b" + sample_task.original_download_path = str(source) + + result, _ = _run_organize_post_process( + temp_file=source, + task=sample_task, + library=library, + hardlink_enabled=True, + ) + + assert result is not None + assert Path(result).stat().st_ino == source.stat().st_ino + class TestHardlinkInodeVerification: """Tests that verify hardlinks share the same inode.""" @@ -1347,7 +1365,7 @@ class TestTorrentSourceCleanupProtection: from shelfmark.download.postprocess.pipeline import transfer_file_to_library # Simulate by directly calling transfer_file_to_library with use_hardlink=False - # (this is what happens after same_filesystem check fails) + # (this is what happens when hardlinking is disabled before transfer) downloads = tmp_path / "downloads" downloads.mkdir() torrent_file = downloads / "book.epub" diff --git a/tests/core/test_processing_integration.py b/tests/core/test_processing_integration.py index dc8c28f8..0aad4a67 100644 --- a/tests/core/test_processing_integration.py +++ b/tests/core/test_processing_integration.py @@ -1,7 +1,9 @@ """Integration tests for real filesystem processing flows.""" +import errno import os import zipfile +from contextlib import nullcontext from pathlib import Path from threading import Event from unittest.mock import MagicMock, patch @@ -46,6 +48,15 @@ def _sync_config(mock_config, mock_core): mock_core.CUSTOM_SCRIPT = mock_config.CUSTOM_SCRIPT +def _hardlink_support_patch(supported: bool): + if supported: + return nullcontext() + return patch( + "shelfmark.download.fs.os.link", + side_effect=OSError(errno.EXDEV, "Invalid cross-device link"), + ) + + def test_direct_download_rename_moves_file(tmp_path): from shelfmark.download.postprocess.router import ( post_process_download as _post_process_download, @@ -366,7 +377,7 @@ def test_torrent_hardlink_enabled_copy_fallback_does_not_extract_archives(tmp_pa with ( patch("shelfmark.core.config.config") as mock_config, patch("shelfmark.config.env.TMP_DIR", staging), - patch("shelfmark.download.postprocess.transfer.same_filesystem", return_value=False), + _hardlink_support_patch(False), ): mock_config.get = _build_config(ingest, organization="none", hardlink=True) mock_config.CUSTOM_SCRIPT = None @@ -385,7 +396,7 @@ def test_torrent_hardlink_enabled_copy_fallback_does_not_extract_archives(tmp_pa # Most importantly: hardlink-setting-enabled fallback to copy should NOT extract. assert list(ingest.glob("*.epub")) == [] - assert any(msg.startswith("Copying") for _, msg in statuses) + assert any(msg.startswith("Hardlinking") for _, msg in statuses) def test_torrent_hardlink_enabled_copy_fallback_directory_archive_kept_when_zip_supported(tmp_path): @@ -422,7 +433,7 @@ def test_torrent_hardlink_enabled_copy_fallback_directory_archive_kept_when_zip_ with ( patch("shelfmark.core.config.config") as mock_config, patch("shelfmark.config.env.TMP_DIR", staging), - patch("shelfmark.download.postprocess.transfer.same_filesystem", return_value=False), + _hardlink_support_patch(False), ): mock_config.get = _build_config( ingest, @@ -1028,20 +1039,20 @@ def test_postprocess_folder_blackbox_matrix( @pytest.mark.parametrize("content_kind", ["book", "audiobook"]) @pytest.mark.parametrize("organization", ["none", "organize"]) @pytest.mark.parametrize("hardlink_enabled", [False, True]) -@pytest.mark.parametrize("same_filesystem", [True, False]) +@pytest.mark.parametrize("hardlink_supported", [True, False]) def test_postprocess_torrent_blackbox_matrix( tmp_path, input_kind: str, content_kind: str, organization: str, hardlink_enabled: bool, - same_filesystem: bool, + hardlink_supported: bool, ): """Torrent-like (original_download_path set) black-box test matrix. This exercises: - hardlink enabled/disabled - - same-filesystem hardlink vs copy fallback + - successful hardlink vs copy fallback - content type differences (book vs audiobook) Assertions focus on invariants: @@ -1087,7 +1098,7 @@ def test_postprocess_torrent_blackbox_matrix( source_file.write_text("content") task = DownloadTask( - task_id=f"torrent-matrix-{input_kind}-{content_kind}-{organization}-{hardlink_enabled}-{same_filesystem}", + task_id=f"torrent-matrix-{input_kind}-{content_kind}-{organization}-{hardlink_enabled}-{hardlink_supported}", source="prowlarr", title=title, author=author, @@ -1100,9 +1111,7 @@ def test_postprocess_torrent_blackbox_matrix( with ( patch("shelfmark.core.config.config") as mock_config, patch("shelfmark.config.env.TMP_DIR", staging), - patch( - "shelfmark.download.postprocess.transfer.same_filesystem", return_value=same_filesystem - ), + _hardlink_support_patch(hardlink_supported), ): mock_config.get = _build_config( ingest, @@ -1131,8 +1140,8 @@ def test_postprocess_torrent_blackbox_matrix( assert result_path.parent == ingest assert result_path.name == f"random.{extension}" - # Hardlink only when enabled and same filesystem. - if hardlink_enabled and same_filesystem: + # Hardlink only when enabled and supported by the filesystem. + if hardlink_enabled and hardlink_supported: assert os.stat(source_file).st_ino == os.stat(result_path).st_ino else: assert os.stat(source_file).st_ino != os.stat(result_path).st_ino