Merge pull request #3167 from tmgast/fix/mod-zip-range-requests

feat: Support Range requests for multi-file ROM downloads

Author: gantoine

backend/config/__init__.py

@@ -36,6 +36,7 @@ def _get_env(var: str, fallback: str | None = None) -> str | None:
 LIBRARY_BASE_PATH: Final[str] = f"{ROMM_BASE_PATH}/library"
 RESOURCES_BASE_PATH: Final[str] = f"{ROMM_BASE_PATH}/resources"
 ASSETS_BASE_PATH: Final[str] = f"{ROMM_BASE_PATH}/assets"
+ZIP_CACHE_PATH: Final[str] = f"{ROMM_BASE_PATH}/cache/zips"
 FRONTEND_RESOURCES_PATH: Final[str] = "/assets/romm/resources"
 
 # SEVEN ZIP

backend/endpoints/roms/__init__.py

@@ -77,10 +77,19 @@
 from utils.database import safe_int, safe_str_to_bool
 from utils.filesystem import sanitize_filename
 from utils.hashing import crc32_to_hex
+from utils.m3u import generate_m3u_content
 from utils.nginx import FileRedirectResponse, ZipContentLine, ZipResponse
 from utils.router import APIRouter
 from utils.screenshots import continue_playing_screenshot
 from utils.validation import ValidationError
+from utils.zip_cache import (
+    BULK_CACHE_MAX_ROMS,
+    ZipFileEntry,
+    get_bulk_namespace,
+    get_cache_key,
+    get_cached_zip,
+    resolve_cached_zip,
+)
 
 from .files import router as files_router
 from .manual import router as manual_router
@@ -939,26 +948,49 @@ async def download_roms(
         f"User {hl(current_username, color=BLUE)} is downloading {len(rom_objects)} ROMs as zip"
     )
 
-    content_lines = []
+    all_entries = []
     for rom in rom_objects:
         rom_files = sorted(rom.files, key=lambda x: x.file_name)
         for file in rom_files:
-            content_lines.append(
-                ZipContentLine(
-                    crc32=None,  # The CRC hash stored for compressed files is for the uncompressed content
-                    size_bytes=file.file_size_bytes,
-                    encoded_location=quote(f"/library/{file.full_path}"),
-                    filename=file.full_path,
+            all_entries.append(
+                ZipFileEntry(
+                    download_name=file.full_path,
+                    full_path=file.full_path,
+                    file_size_bytes=file.file_size_bytes,
+                    updated_at_epoch=file.updated_at.timestamp(),
                 )
             )
 
     if filename:
         file_name = sanitize_filename(filename)
     else:
-        base64_content = b64encode(
-            ("\n".join([str(line) for line in content_lines])).encode()
+        content_summary = "\n".join(
+            f"{e.download_name}:{e.file_size_bytes}" for e in all_entries
+        ).encode()
+        file_name = f"{len(rom_objects)} ROMs ({crc32_to_hex(binascii.crc32(content_summary))}).zip"
+
+    range_header = request.headers.get("range")
+    if range_header and len(rom_objects) <= BULK_CACHE_MAX_ROMS:
+        redirect_path = await resolve_cached_zip(
+            get_bulk_namespace([r.id for r in rom_objects]),
+            all_entries,
+            log_label=f"bulk download ({len(rom_objects)} ROMs)",
+        )
+        if redirect_path:
+            return FileRedirectResponse(
+                download_path=redirect_path,
+                filename=file_name,
+            )
+
+    content_lines = [
+        ZipContentLine(
+            crc32=None,
+            size_bytes=e.file_size_bytes,
+            encoded_location=quote(f"/library/{e.full_path}"),
+            filename=e.download_name,
         )
-        file_name = f"{len(rom_objects)} ROMs ({crc32_to_hex(binascii.crc32(base64_content))}).zip"
+        for e in all_entries
+    ]
 
     return ZipResponse(
         content_lines=content_lines,
@@ -1201,6 +1233,21 @@ async def head_rom_content(
             download_path=Path(f"/library/{files[0].full_path}"),
         )
 
+    hidden_folder = safe_str_to_bool(request.query_params.get("hidden_folder", ""))
+    entries = [ZipFileEntry.from_rom_file(f, hidden_folder) for f in files]
+    namespace = str(rom.id)
+    cache_key = get_cache_key(namespace, entries, hidden_folder)
+    zip_path = get_cached_zip(namespace, cache_key)
+    if zip_path:
+        return Response(
+            headers={
+                "Content-Type": "application/zip",
+                "Content-Length": str(zip_path.stat().st_size),
+                "Accept-Ranges": "bytes",
+                "Content-Disposition": f"attachment; filename*=UTF-8''{quote(file_name)}.zip; filename=\"{quote(file_name)}.zip\"",
+            },
+        )
+
     return Response(
         media_type="application/zip",
         headers={
@@ -1347,6 +1394,25 @@ async def build_zip_in_memory() -> bytes:
             download_path=Path(f"/library/{files[0].full_path}"),
         )
 
+    # Multi-file path: serve cached ZIP for Range requests (resumable),
+    # fall through to mod_zip streaming for non-Range requests.
+    range_header = request.headers.get("range")
+    if range_header:
+        has_m3u = rom.has_m3u_file()
+        redirect_path = await resolve_cached_zip(
+            str(rom.id),
+            [ZipFileEntry.from_rom_file(f, hidden_folder) for f in files],
+            hidden_folder=hidden_folder,
+            m3u_content=None if has_m3u else generate_m3u_content(files, hidden_folder),
+            m3u_filename=None if has_m3u else f"{file_name}.m3u",
+            log_label=f"ROM {rom.id}",
+        )
+        if redirect_path:
+            return FileRedirectResponse(
+                download_path=redirect_path,
+                filename=f"{file_name}.zip",
+            )
+
     content_lines = [
         ZipContentLine(
             crc32=None,  # The CRC hash stored for compressed files is for the uncompressed content
@@ -1358,9 +1424,7 @@ async def build_zip_in_memory() -> bytes:
     ]
 
     if not rom.has_m3u_file():
-        m3u_encoded_content = "\n".join(
-            [f.file_name_for_download(hidden_folder) for f in m3u_files]
-        ).encode()
+        m3u_encoded_content = generate_m3u_content(files, hidden_folder)
         m3u_base64_content = b64encode(m3u_encoded_content).decode()
         m3u_line = ZipContentLine(
             crc32=crc32_to_hex(binascii.crc32(m3u_encoded_content)),

backend/endpoints/tasks.py

@@ -40,6 +40,7 @@
     recompute_save_content_hashes_task,
 )
 from tasks.manual.sync_folder_scan import sync_folder_scan_task
+from tasks.scheduled.cleanup_zip_cache import cleanup_zip_cache_task
 from tasks.scheduled.convert_images_to_webp import convert_images_to_webp_task
 from tasks.scheduled.scan_library import scan_library_task
 from tasks.scheduled.update_launchbox_metadata import update_launchbox_metadata_task
@@ -95,6 +96,13 @@ class ManualTask(ScheduledTask):
             "task": convert_images_to_webp_task,
         }
     ),
+    ScheduledTask(
+        {
+            "name": "cleanup_zip_cache",
+            "type": TaskType.CLEANUP,
+            "task": cleanup_zip_cache_task,
+        }
+    ),
 ]
 
 manual_tasks: list[ManualTask] = [

backend/startup.py

@@ -34,6 +34,7 @@
 )
 from tasks.scheduled.cleanup_netplay import cleanup_netplay_task
 from tasks.scheduled.cleanup_upload_tmp import cleanup_upload_tmp_task
+from tasks.scheduled.cleanup_zip_cache import cleanup_zip_cache_task
 from tasks.scheduled.convert_images_to_webp import convert_images_to_webp_task
 from tasks.scheduled.scan_library import scan_library_task
 from tasks.scheduled.sync_retroachievements_progress import (
@@ -140,6 +141,7 @@ async def main() -> None:
 
         # Initialize scheduled tasks
         cleanup_netplay_task.init()
+        cleanup_zip_cache_task.init()
         cleanup_upload_tmp_task.init()
 
         if ENABLE_SCHEDULED_RESCAN:

backend/tasks/scheduled/cleanup_zip_cache.py

@@ -0,0 +1,28 @@
+from logger.logger import log
+from tasks.tasks import PeriodicTask, TaskType
+from utils.zip_cache import cleanup_stale_zips
+
+
+class CleanupZipCacheTask(PeriodicTask):
+    def __init__(self):
+        super().__init__(
+            title="Scheduled ZIP cache cleanup",
+            description="Removes stale cached ZIP files based on tiered TTL",
+            task_type=TaskType.CLEANUP,
+            enabled=True,
+            manual_run=False,
+            cron_string="0 4 * * *",
+            func="tasks.scheduled.cleanup_zip_cache.cleanup_zip_cache_task.run",
+        )
+
+    async def run(self) -> None:
+        if not self.enabled:
+            self.unschedule()
+            return
+
+        deleted = cleanup_stale_zips()
+        if deleted:
+            log.info(f"Cleaned up {deleted} stale cached ZIP files")
+
+
+cleanup_zip_cache_task = CleanupZipCacheTask()

backend/tests/tasks/test_cleanup_zip_cache.py

@@ -0,0 +1,30 @@
+from unittest.mock import MagicMock
+
+from tasks.scheduled.cleanup_zip_cache import CleanupZipCacheTask
+
+
+class TestCleanupZipCacheTask:
+    def test_configuration(self):
+        task = CleanupZipCacheTask()
+        assert task.enabled is True
+        assert task.cron_string == "0 4 * * *"
+        assert "cleanup_zip_cache" in task.func
+
+    async def test_run_calls_cleanup(self, mocker):
+        task = CleanupZipCacheTask()
+        mock_cleanup = mocker.patch(
+            "tasks.scheduled.cleanup_zip_cache.cleanup_stale_zips",
+            return_value=3,
+        )
+        await task.run()
+        mock_cleanup.assert_called_once_with()
+
+    async def test_run_disabled_unschedules(self, mocker):
+        task = CleanupZipCacheTask()
+        task.enabled = False
+        mocker.patch.object(task, "unschedule", MagicMock())
+        mock_cleanup = mocker.patch(
+            "tasks.scheduled.cleanup_zip_cache.cleanup_stale_zips",
+        )
+        await task.run()
+        mock_cleanup.assert_not_called()

backend/tests/utils/test_m3u.py

@@ -0,0 +1,59 @@
+from unittest.mock import MagicMock
+
+from utils.m3u import generate_m3u_content
+
+
+def _make_file(name: str, extension: str, download_name: str | None = None):
+    f = MagicMock()
+    f.file_extension = extension
+    f.file_name_for_download.return_value = download_name or name
+    return f
+
+
+class TestGenerateM3uContent:
+    def test_single_file(self):
+        files = [_make_file("game.bin", "bin")]
+        result = generate_m3u_content(files, hidden_folder=False)
+        assert result == b"game.bin"
+        files[0].file_name_for_download.assert_called_once_with(False)
+
+    def test_multiple_files(self):
+        files = [
+            _make_file("disc1.chd", "chd"),
+            _make_file("disc2.chd", "chd"),
+            _make_file("disc3.chd", "chd"),
+        ]
+        result = generate_m3u_content(files, hidden_folder=False)
+        assert result == b"disc1.chd\ndisc2.chd\ndisc3.chd"
+
+    def test_cue_files_preferred_over_bin(self):
+        files = [
+            _make_file("track01.bin", "bin"),
+            _make_file("track02.bin", "bin"),
+            _make_file("game.cue", "cue"),
+        ]
+        result = generate_m3u_content(files, hidden_folder=False)
+        assert result == b"game.cue"
+
+    def test_cue_case_insensitive(self):
+        files = [
+            _make_file("track.bin", "bin"),
+            _make_file("game.CUE", "CUE", download_name="game.CUE"),
+        ]
+        result = generate_m3u_content(files, hidden_folder=False)
+        assert result == b"game.CUE"
+
+    def test_hidden_folder_passed_through(self):
+        files = [_make_file("game.chd", "chd", download_name=".hidden/game.chd")]
+        result = generate_m3u_content(files, hidden_folder=True)
+        assert result == b".hidden/game.chd"
+        files[0].file_name_for_download.assert_called_once_with(True)
+
+    def test_no_cue_files_lists_all(self):
+        files = [
+            _make_file("disc1.chd", "chd"),
+            _make_file("disc2.chd", "chd"),
+        ]
+        result = generate_m3u_content(files, hidden_folder=False)
+        lines = result.decode().split("\n")
+        assert len(lines) == 2