diff --git a/shelfmark/download/orchestrator.py b/shelfmark/download/orchestrator.py index dc1e55cd..841cdcaa 100644 --- a/shelfmark/download/orchestrator.py +++ b/shelfmark/download/orchestrator.py @@ -367,8 +367,38 @@ def queue_status(user_id: int | None = None) -> dict[str, dict[str, Any]]: } +def get_book_path(task_id: str) -> tuple[str | None, DownloadTask | None]: + """Path of a task's downloaded file, without reading it into memory. + + Serving a completed book used to go through :func:`get_book_data`, which read the + whole file so the route could wrap it in a BytesIO: two copies resident for a + transfer that the web server can stream straight off disk. On a large audiobook + that is enough to reach a container's memory limit. + """ + task = None + try: + task = book_queue.get_task(task_id) + if not task: + return None, None + + path = task.download_path + if not path or not Path(path).is_file(): + return None, task + except OSError as e: + logger.error_trace(f"Error resolving book path: {e}") + if task: + task.download_path = None + return None, task + else: + return path, task + + def get_book_data(task_id: str) -> tuple[bytes | None, DownloadTask | None]: - """Get downloaded file data for a specific task.""" + """Get downloaded file data for a specific task. + + Prefer :func:`get_book_path` when the bytes are only going to be written straight + back out; this reads the entire file into memory. + """ task = None try: task = book_queue.get_task(task_id) diff --git a/shelfmark/main.py b/shelfmark/main.py index 4c6b640a..e1ad0960 100644 --- a/shelfmark/main.py +++ b/shelfmark/main.py @@ -1,7 +1,6 @@ """Flask app - routes, WebSocket handlers, and middleware.""" import binascii -import io import logging import os import re @@ -1698,8 +1697,10 @@ def api_local_download() -> Response | tuple[Response, int]: return jsonify({"error": "No book ID provided"}), 400 try: - file_data, book_info = backend.get_book_data(book_id) - if file_data is None: + # The path, not the bytes: send_file streams it off disk, where reading it + # first cost two resident copies of the whole book. + file_path, book_info = backend.get_book_path(book_id) + if file_path is None: # Fallback for dismissed/history entries where queue task may no longer exist. if download_history_service is not None: is_admin, db_user_id, can_access_status = _resolve_status_scope() @@ -1732,9 +1733,7 @@ def api_local_download() -> Response | tuple[Response, int]: return jsonify({"error": "File not found"}), 404 file_name = book_info.get_filename() if book_info is not None else Path(book_id).name - # Prepare the file for sending to the client - data = io.BytesIO(file_data) - return send_file(data, download_name=file_name, as_attachment=True) + return send_file(file_path, download_name=file_name, as_attachment=True) except _OPERATIONAL_ERRORS as e: logger.error_trace(f"Local download error: {e}") diff --git a/tests/download/test_orchestrator_retry.py b/tests/download/test_orchestrator_retry.py index fc822f98..d938c3bb 100644 --- a/tests/download/test_orchestrator_retry.py +++ b/tests/download/test_orchestrator_retry.py @@ -285,3 +285,50 @@ def test_get_book_data_clears_download_path_when_file_read_fails(monkeypatch, tm assert file_data is None assert returned_task is task assert task.download_path is None + + +def test_get_book_path_returns_the_path_without_reading_the_file(monkeypatch, tmp_path): + import shelfmark.download.orchestrator as orchestrator + + book = tmp_path / "book.epub" + book.write_bytes(b"x" * 4096) + task = DownloadTask( + task_id="task-book-path-1", + source="direct_download", + title="Streamed Book", + download_path=str(book), + ) + + mock_queue = MagicMock() + mock_queue.get_task.return_value = task + monkeypatch.setattr(orchestrator, "book_queue", mock_queue) + + def _no_reads(*_args, **_kwargs): + raise AssertionError("get_book_path must not open the file") + + monkeypatch.setattr(Path, "open", _no_reads) + + path, returned_task = orchestrator.get_book_path(task.task_id) + + assert path == str(book) + assert returned_task is task + + +def test_get_book_path_reports_a_missing_file_rather_than_a_dead_path(monkeypatch, tmp_path): + import shelfmark.download.orchestrator as orchestrator + + task = DownloadTask( + task_id="task-book-path-2", + source="direct_download", + title="Gone", + download_path=str(tmp_path / "gone.epub"), + ) + + mock_queue = MagicMock() + mock_queue.get_task.return_value = task + monkeypatch.setattr(orchestrator, "book_queue", mock_queue) + + path, returned_task = orchestrator.get_book_path(task.task_id) + + assert path is None + assert returned_task is task