fix: normalize EPUB progression semantics#665
Merged
Conversation
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.
Problem
The offline EPUB progression path was mixing protocol details, response quirks, and local persistence semantics into the same control flow.
That showed up as a chain of brittle behaviors:
Acceptheader, causing406 Not Acceptable204,404, empty body, andlocator:{}all meant "no usable remote progression", but were handled inconsistentlyepubProgressionRaw == nilto mean both "unknown" and "checked but missing"The result was a startup sync path that kept accreting special cases and could continue retrying EPUB progression imports even after the app had already learned there was no usable remote progression to persist.
Approach
Refactor the flow around explicit remote and local progression semantics.
Remote progression semantics
Centralize remote EPUB progression interpretation in
BookServiceand normalize the endpoint into four outcomes:available(R2Progression)missingretryableFailureinvalidPayloadThis layer now also sends the correct header:
Accept: application/vnd.readium.progression+jsonAnd it treats these cases consistently as
missing:204404locator: {}Only transient failures remain retryable (
network,offline,429,5xx). Non-retryable protocol/payload problems are surfaced asinvalidPayload.Local progression semantics
Keep using
epubProgressionRaw, but store an explicit record instead of relying on implicit sentinel meanings.That preserves three local states without a schema migration:
unknown→epubProgressionRaw == nilmissing→ explicit stored missing recordavailable→ explicit stored progression recordLegacy values are normalized on read as well, including older empty sentinels, raw
R2Progressionpayloads, and empty-locator payloads.Consumers
Update the startup/offline import flow to consume those explicit semantics directly:
availablemissinginvalidPayloadas handled-missing so it does not retry foreverretryableFailureeligible for future retriesThis keeps protocol interpretation in
BookService, whileOfflineManagerand the reader-side import path only apply business policy.Scope
AcceptheaderBookService204/404/ empty body / empty locator cases as missing progressionValidation
make formatmake build-macos-ci