Skip to content

Support Range requests for multi-file ROM downloads#3167

Open
tmgast wants to merge 6 commits intorommapp:masterfrom
tmgast:fix/mod-zip-range-requests
Open

Support Range requests for multi-file ROM downloads#3167
tmgast wants to merge 6 commits intorommapp:masterfrom
tmgast:fix/mod-zip-range-requests

Conversation

@tmgast
Copy link
Copy Markdown
Member

@tmgast tmgast commented Mar 23, 2026

Description

Adds HTTP Range request support for multi-file ROM downloads by caching assembled ZIPs to disk on demand. When a client sends a Range header, the server builds a static ZIP and serves it via X-Accel-Redirect, letting nginx handle 206 Partial Content natively. Non-Range requests continue to stream via mod_zip as before.

This also applies to the bulk /download endpoint for up to 100 ROMs.

How it works

  1. Client sends Range: bytes=0- on a multi-file ROM download
  2. Server computes a deterministic cache key from the ROM's file set and state
  3. If no cached ZIP exists, one is built (ZIP_STORED, no compression) and written to ROMM_BASE_PATH/cache/zips/{namespace}/{key}.zip
  4. Server returns FileRedirectResponse with X-Accel-Redirect pointing to the cached file
  5. nginx serves the static file with native Range/206 support
  6. Subsequent requests hit the cache; content changes produce a new key (old ZIP ages out)

Content change safety

nginx auto-generates ETags on static files. When content changes (re-scan), a new cache key produces a new file with a new ETag. Clients resuming with If-Range + the old ETag get 200 (full content) per RFC 7233, signaling they should restart. Old cache files are preserved for in-progress downloads and cleaned up by the scheduled task.

Tiered TTL and eviction

ZIP size TTL
Under 8 GB 48 hours
Over 8 GB 12 hours

When disk space is insufficient for a new cache entry, the oldest cached ZIPs (over 1 hour old) are evicted LRU-style until enough space is available or no evictable entries remain. If still insufficient, the request falls back to mod_zip streaming.

A 2x disk space buffer is required before building a new cached ZIP.

Endpoints affected

Endpoint Cache condition
GET /{id}/content/{name} Range header present
HEAD /{id}/content/{name} Returns Accept-Ranges: bytes + Content-Length when cache exists
GET /download Range header present + <= 100 ROMs

Known gaps

  • Dev mode (DEV_MODE=true) builds in-memory ZIPs and does not reach the Range/cache code path. Range support is production-only (requires nginx).
  • RomFile.updated_at tracks DB row modification, not file content change. Files replaced on disk without re-scanning may serve a stale cached ZIP until the next scan.
  • No total cache size cap. Cleanup is time-based (tiered TTL) and eviction is on-demand. Monitor disk usage for large libraries.

zipfile stdlib override

A third-party library in the import chain replaces zipfile._get_compressor with a version that has an incompatible call signature, breaking ZipFile.writestr() and ZipFile.write() on CPython 3.13. The cache builder works around this by reloading the zipfile module before use. This is a pre-existing issue that also affects the dev-mode in-memory ZIP builder.

New files

File Purpose
backend/utils/zip_cache.py Cache key generation, ZIP building, lookup, eviction, cleanup
backend/utils/m3u.py Shared M3U playlist generation (extracted from endpoint)
backend/tasks/scheduled/cleanup_zip_cache.py Daily stale cache cleanup task

AI Disclosure
Planning and review assisted by Claude Code

Checklist

  • I've tested the changes locally
  • I've updated relevant comments
  • I've assigned reviewers for this PR
  • I've added unit tests that cover the changes (45 tests across 8 classes)

tmgast added 5 commits March 24, 2026 05:56
Cache ZIPs to disk on Range requests so nginx can serve 206 natively.
Non-Range requests still stream via mod_zip as before.

Refs: rommapp#3160
- Use ZipFile.write() instead of reading entire files into memory
- Extract magic numbers to module constants
- Use hl() for log message highlighting
- Tests for zip_cache, m3u, and cleanup task using mocker fixture
- Bulk /download endpoint now caches ZIPs for Range requests (max 100 ROMs)
- LRU eviction frees space by removing oldest cached ZIPs (>1h old)
- Tiered TTL: 48h default, 12h for ZIPs over 8GB
- Generalize cache paths from rom_id to namespace (supports bulk-{ids})
A third-party library in the import chain replaces
zipfile._get_compressor with an incompatible signature. Reload
the zipfile module before building cached ZIPs to restore it.
Copilot AI review requested due to automatic review settings March 23, 2026 22:50
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds HTTP Range/resume support for multi-file ROM downloads (and bulk /download) by building deterministic, on-disk cached ZIPs and serving them via nginx X-Accel-Redirect, while keeping the existing mod_zip streaming path for non-Range requests.

Changes:

  • Add nginx internal route for cached ZIP delivery (/cache/${ROMM_BASE_PATH}/cache/).
  • Introduce ZIP cache utilities (keying, build-to-disk, eviction, TTL cleanup) and extract shared M3U playlist generation.
  • Add a scheduled cleanup task and register it for startup + tasks endpoint visibility.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
docker/nginx/templates/default.conf.template Adds internal /cache/ alias used by X-Accel-Redirect to serve cached ZIPs with native Range support.
backend/utils/zip_cache.py Implements cache keying, disk-space checks/eviction, ZIP building, redirect path, and TTL-based cleanup.
backend/utils/m3u.py Extracts shared M3U playlist generation logic for multi-file ROMs.
backend/tasks/scheduled/cleanup_zip_cache.py Adds daily scheduled task to delete stale cached ZIPs.
backend/startup.py Initializes the new scheduled ZIP cache cleanup task on startup.
backend/endpoints/tasks.py Exposes the ZIP cache cleanup task in the scheduled tasks list.
backend/endpoints/roms/init.py Adds Range-aware cache/redirect behavior for multi-file ROM downloads and bulk downloads; uses shared M3U generator.
backend/config/init.py Adds ZIP_CACHE_PATH configuration constant.
backend/tests/utils/test_zip_cache.py Adds unit tests for zip cache behavior (keying, build, TTL cleanup, eviction).
backend/tests/utils/test_m3u.py Adds unit tests for M3U content generation behavior.
backend/tests/tasks/test_cleanup_zip_cache.py Adds unit tests for the new scheduled cleanup task.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Use zipfile.ZipFile after reload instead of stale direct import
- Use ZipFile.write() for streaming (reload fixes CPython 3.13 bug)
- Wrap cache build in try/except, fall back to mod_zip on failure
- Guard empty entries list in get_cache_key
- Hash bulk namespace to avoid ENAMETOOLONG with 100 ROM IDs
- Use MagicMock for sync unschedule method in tests
- Mock _get_available_space in eviction tests to force eviction path
- Use sparse file (truncate) for large file TTL test
- Add tests for get_bulk_namespace and empty entries
@gantoine gantoine self-assigned this Mar 24, 2026
@gantoine gantoine self-requested a review March 24, 2026 14:54
@gantoine gantoine added the on-hold Pending further research or blocked by another issue label Mar 24, 2026
@gantoine gantoine removed their assignment Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

on-hold Pending further research or blocked by another issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants