refactor(maps): migrate to zip.js with ranged reading - #77
Merged
Conversation
unzipper is Node-only, so the client can't share it, and its API forces the whole archive to be resident. zip.js reads the same archives through an isomorphic, seekable Reader interface, which is the prerequisite for streaming the archive out of S3 instead of downloading it whole. Entries stay valid after the ZipReader call returns (close() is a no-op), so album art entries still cross the module boundary to the S3 handlers as before, rather than being buffered eagerly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ding them Validation used to pull the whole archive out of blob storage as a Buffer, so a 500MB upload was 500MB resident before any check ran. S3Handler now hands back a zip.js Reader over ranged GETs plus the object size from HEAD, and validation reads only the central directory and the rlrr files - a bounded few hundred KB whatever the archive size. The dev disk fake reads ranges off the filesystem for the same reason; the in-memory test fake reads from the buffer it already holds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- drop s3Get, which lost its only caller when validation stopped downloading whole archives - validateMap takes the reader it actually uses rather than a MapArchive, so it no longer depends on the storage layer's types - retry short reads in the dev disk fake, which would otherwise leave the tail of the buffer zeroed and silently corrupt what the zip parser sees Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t8Arrays A zip's central directory is laid out immediately before the end-of-central- directory record that points at it, so the fixed-size tail window zip.js scans for that record almost always contains the directory too. Caching the tail collapses those two fetches into one, and falls back to a second fetch when the directory is too large to have been covered. Both readers were also handing back Buffers, which zip.js mis-parses: it pulls records out with slice(), relying on it copying, whereas Buffer.slice() returns a view over a shared pool and drops the byte offset. Nothing caught this because the existing tests read through zip.js' own Uint8ArrayReader; the new fake returns offset Buffer views like the real readers do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Only opening the zip was guarded. Reading an entry decompresses it and, on a remote archive, fetches more of it, so a storage blip mid-validation threw straight past completeMapUpload's rollback: the map stayed stuck in VALIDATING with no way back, and a reupload lost its previously valid state. The album art upload had the same hole, and was discarding failed writes on top of that. Also covers the two real ranged readers, which nothing exercised before: S3RangeReader now takes the client it sends through, so a fake can stand in and parse a whole archive over ranges, and the dev disk reader is driven through a temp directory. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…g dep Buffer bought us nothing along this path and its slice() semantics are a trap for anything handing bytes to zip.js. The only genuine Buffer feature in use was decoding a UTF-16LE rlrr, which TextDecoder does natively - so the `encoding` package (and its iconv dependency) goes with it. UTF-16LE rlrr files had no fixture, so nothing covered that decode either way; the generator can now write one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace unzipper with zip.js to support lazy, ranged archive reads. Archives are now read on-demand rather than fully buffered, making validation efficient even for large maps.
Changes:
Benefits: