diff --git a/CONVENTIONS.md b/CONVENTIONS.md index 4c768dca..1c809573 100644 --- a/CONVENTIONS.md +++ b/CONVENTIONS.md @@ -1773,7 +1773,21 @@ are asked, at most 10,000 rows. is a new download). An answer that reached the cache before the book was downloaded (while its files did, or before launch's `hydrate`) is taken as it becomes downloaded (`withCachedChapters`). The audio is never touched, and a queue already playing keeps its chapters until the book is - next started. + next started. The same files in another order (server v2.4.0 plays them in natural order, "2-" + before "10-") are taken **only by a start that loads the engine in that order** + (`adoptStartedChapters`, from `playBook` once its load landed and it still owns the engine; + saved at once; a start given no chapters plays the download's own files and chapters), never + from a background answer, and a start's answer in the manifest's own order is left to the + background refresh: JS sees only what it loaded, not a book the Android service or a Cast + receiver holds before `adoptLoaded` / the receiver adoption, which map the engine's track index + through the manifest. So an offline start, Android's no-JS car start + and `adoptLoaded` keep the manifest's order until an online start plays the new one. The + chapters carry the order (every queue over a download orders its files by them and finds each by + path); `manifest.files` itself never moves, since a file's index there is its name on disk + (`fileName`, rebuilt by `relocateEntry` at every launch and by a retry). A place saved on the old + timeline reads as moved once the new order is taken (as the server's re-index moves any + whole-book place: no remap). A downloaded book whose chapters change gets its Android car play + spec rebuilt (`car-controller.ts`). - **Offline companion** (`src/downloads/offline-meta.ts`): a downloaded book keeps its community metadata (asked by `metaEnabledFor`, so a book with content generated on its server too; the `/meta` envelope, plus the previous work(s) the envelope doesn't hold) as `meta.json` beside its audio (`engine.writeText`: the book's folder on native, its Cache API diff --git a/src/car/car-controller.test.ts b/src/car/car-controller.test.ts index d7f71889..913e4205 100644 --- a/src/car/car-controller.test.ts +++ b/src/car/car-controller.test.ts @@ -515,6 +515,27 @@ describe('startCarSync', () => { expect(e.play).toMatchObject({ startIndex: 1, positionInTrack: 50, rate: 1.25 }); }); + it("rebuilds a downloaded book's spec when its chapters change (its files in a new order)", async () => { + Object.defineProperty(Platform, 'OS', { value: 'android', configurable: true }); + connect(['c1']); + mockClients.c1 = fakeClient({}); + useDownloads.setState({ entries: { 'c1:2:D': downloaded('c1', 'D') } }); + stop = startCarSync(); + await settle(); + const played = () => tab(lastSnapshot(), 'downloads')!.items[0].play!.tracks.map((t) => t.url); + expect(played()).toEqual(['file:///d/1.mp3', 'file:///d/2.mp3']); + + // The server now plays them the other way round (`refreshedChapters`): same status. + const reordered = downloaded('c1', 'D'); + reordered.manifest.chapters = { + ...twoFileChapters, + files: twoFileChapters.files.map((f) => ({ ...f, seq: 1 - f.seq })), + }; + useDownloads.setState({ entries: { 'c1:2:D': reordered } }); + await settle(SETTLE_MS + MIN_GAP_MS); + expect(played()).toEqual(['file:///d/2.mp3', 'file:///d/1.mp3']); + }); + it('leaves Up next out where the default server keeps no queue', async () => { connect(['c1']); mockClients.c1 = fakeClient({}); diff --git a/src/car/car-controller.ts b/src/car/car-controller.ts index 045dd501..f205324f 100644 --- a/src/car/car-controller.ts +++ b/src/car/car-controller.ts @@ -40,7 +40,7 @@ import { startBookInPlace } from '@/components/player/start-book'; import { queueConnectionId } from '@/components/upnext/up-next-model'; import { statusSignature } from '@/downloads/downloads-view'; import { downloadedEntryOf, useDownloads } from '@/downloads/store'; -import type { DownloadManifest } from '@/downloads/types'; +import type { DownloadEntry, DownloadManifest } from '@/downloads/types'; import i18n from '@/i18n'; import { bootstrapPlayback } from '@/lib/bootstrap'; import { resolveConnectionId } from '@/lib/connection-id'; @@ -979,6 +979,24 @@ export function carSyncReady(): Promise { return ready; } +/** Whether a book downloaded in both registries has other chapters in `next` (a chapters + * refresh, `refreshedChapters`, or a start's, `adoptStartedChapters`, which can also give + * its files a new play order): its play spec is built from them (`playSpec`), and a + * refresh changes no download's status. */ +function chaptersChanged( + next: Record, + prev: Record, +): boolean { + return Object.entries(next).some(([k, e]) => { + const was = prev[k]; + return ( + e.status === 'downloaded' && + was?.status === 'downloaded' && + e.manifest.chapters !== was.manifest.chapters + ); + }); +} + /** * Start the car sync (idempotent: callers share one; each returned stop releases one, and the * last releases everything). Native only; a no-op on the web and on a binary without the car @@ -1016,7 +1034,7 @@ export function startCarSync(): () => void { useDownloads.subscribe((s, prev) => { if (s.entries === prev.entries) return; const sig = statusSignature(s.entries); - if (sig === downloadsSig) return; + if (sig === downloadsSig && !chaptersChanged(s.entries, prev.entries)) return; downloadsSig = sig; request(false); }), diff --git a/src/downloads/store.test.ts b/src/downloads/store.test.ts index 68cf6000..f30bd56f 100644 --- a/src/downloads/store.test.ts +++ b/src/downloads/store.test.ts @@ -132,10 +132,12 @@ jest.mock('@/api/hooks', () => ({ /* eslint-disable import/first */ import { resolveClient } from '@/api/connection-clients'; import { engine } from '@/downloads/engine'; +import { bookFileSpecs } from '@/playback/book-queue'; import { downloadedCountFor, downloadedEntryOf, downloadKey, + adoptStartedChapters, isDeclined, refreshedChapters, reviveEntry, @@ -544,6 +546,15 @@ describe("refreshing a downloaded book's chapters", () => { chapters_source: 'community', ...over, }); + /** The order a downloaded book's queue plays its files in: its chapters'. */ + const playOrder = (manifest: DownloadManifest) => + bookFileSpecs(manifest.book, manifest.chapters ?? undefined).map((s) => s.path); + /** The query cache holds `data` as A/Book's chapters answer. */ + const cacheHolds = (data: ChaptersResponse) => + mockGetQueryData.mockImplementation((k: unknown) => + Array.isArray(k) && k[0] === 'chapters' && k[3] === 'A/Book' ? data : undefined, + ); + afterEach(() => mockGetQueryData.mockReset()); describe('refreshedChapters', () => { it('takes the fresh chapters when the audio files are the same', () => { @@ -558,8 +569,46 @@ describe("refreshing a downloaded book's chapters", () => { it('leaves the manifest alone when the files differ (a re-download)', () => { const e = downloadedEntry(); expect(refreshedChapters(e.manifest, fresh(['A/Book/01.mp3']))).toBeNull(); - expect(refreshedChapters(e.manifest, fresh(['A/Book/02.mp3', 'A/Book/01.mp3']))).toBeNull(); expect(refreshedChapters(e.manifest, fresh(['A/Book/01.mp3', 'A/Book/03.mp3']))).toBeNull(); + expect(refreshedChapters(e.manifest, fresh(['A/Book/01.mp3', 'A/Book/01.mp3']))).toBeNull(); + expect( + refreshedChapters(e.manifest, fresh(['A/Book/01.mp3', 'A/Book/02.mp3', 'A/Book/03.mp3'])), + ).toBeNull(); + }); + + it('never takes the same files in another order (only a start that plays it does)', () => { + const e = downloadedEntry(); + e.manifest.chapters = fresh(undefined, { chapters_source: undefined }); + expect(refreshedChapters(e.manifest, fresh(['A/Book/02.mp3', 'A/Book/01.mp3']))).toBeNull(); + }); + + it('keeps each file on its own audio across a launch once its book took a new order', async () => { + // Two extensions: a file named for another one's index would be missing on disk, and + // the launch would drop the book and delete its folder. + const paths = ['A/Book/01.mp3', 'A/Book/02.m4a']; + const e = downloadedEntry(); + e.manifest.chapters = fresh(paths, { chapters_source: undefined }); + e.manifest.files = [ + { relPath: paths[0], localUri: 'live:c1:2:A/Book:0.mp3' }, + { relPath: paths[1], localUri: 'live:c1:2:A/Book:1.m4a' }, + ]; + const onDisk = new Set(e.manifest.files.map((f) => f.localUri)); + mockEngine.fileExists.mockImplementation(async (uri: string) => onDisk.has(uri)); + const key = downloadKey('c1', 2, 'A/Book'); + useDownloads.setState({ entries: { [key]: e } }); + adoptStartedChapters( + { connectionId: 'c1', libraryId: 2, path: 'A/Book' }, + fresh([paths[1], paths[0]]), + ); + await seed({ [key]: useDownloads.getState().entries[key] }); + + await useDownloads.getState().hydrate(); + + const after = useDownloads.getState().entries[key]; + expect(after?.status).toBe('downloaded'); + expect(mockEngine.removeBook).not.toHaveBeenCalled(); + expect(after.manifest.files).toEqual(e.manifest.files); + expect(playOrder(after.manifest)).toEqual([paths[1], paths[0]]); }); it('leaves the manifest alone when a file at the same path changed size (replaced in place)', () => { @@ -657,6 +706,57 @@ describe("refreshing a downloaded book's chapters", () => { expect(useDownloads.getState().entries[key]).toBe(queued); }); + describe('a new file order', () => { + const key = downloadKey('c1', 2, 'A/Book'); + const ref = { connectionId: 'c1', libraryId: 2, path: 'A/Book' }; + const reordered = () => fresh(['A/Book/02.mp3', 'A/Book/01.mp3']); + const order = () => playOrder(useDownloads.getState().entries[key].manifest); + beforeEach(() => { + // Downloaded from the server's old answer (its own chapters, in the old order). + const e = downloadedEntry(); + e.manifest.chapters = fresh(undefined, { chapters_source: undefined }); + useDownloads.setState({ entries: { [key]: e } }); + }); + + it('never comes from a background answer: JS cannot see what a native engine holds', async () => { + const e = useDownloads.getState().entries[key]; + answer(reordered()); + expect(useDownloads.getState().entries[key]).toBe(e); + // Nor from one the cache held when launch read the registry. + await seed({ [key]: e }); + cacheHolds(reordered()); + await useDownloads.getState().hydrate(); + expect(useDownloads.getState().entries[key].manifest.chapters).toEqual(e.manifest.chapters); + }); + + it('is taken by a start that played it, with its chapters, nothing downloaded again', async () => { + const e = useDownloads.getState().entries[key]; + adoptStartedChapters(ref, reordered()); + const now = useDownloads.getState().entries[key]; + expect(now.manifest.chapters).toEqual(reordered()); + expect(order()).toEqual(['A/Book/02.mp3', 'A/Book/01.mp3']); + // The local files stay as the download wrote them (a file's index is its name on disk). + expect(now.manifest.files).toBe(e.manifest.files); + // Saved at once: a JS restart right after the start must find the order it played. + expect((await readPersisted())[key]?.manifest.chapters).toEqual(reordered()); + }); + + it('is not taken from a start whose chapters name other files, or another size', () => { + const e = useDownloads.getState().entries[key]; + adoptStartedChapters(ref, fresh(['A/Book/02.mp3', 'A/Book/03.mp3'])); + adoptStartedChapters(ref, fresh(['A/Book/02.mp3'])); + const resized = reordered(); + resized.files = resized.files.map((f) => + f.rel_path === 'A/Book/01.mp3' ? { ...f, size: 2 } : f, + ); + adoptStartedChapters(ref, resized); + adoptStartedChapters({ ...ref, path: 'B/Book' }, reordered()); + // The same order is the background refresh's to take, not a start's (it can be older). + adoptStartedChapters(ref, fresh()); + expect(useDownloads.getState().entries[key]).toBe(e); + }); + }); + it('stops listening once unsubscribed', () => { const e = downloadedEntry(); const key = downloadKey('c1', 2, 'A/Book'); @@ -671,11 +771,6 @@ describe("refreshing a downloaded book's chapters", () => { // before (while the files did, or before launch read the registry) is taken from the cache. describe('an answer that landed before the book was downloaded', () => { const key = downloadKey('c1', 2, 'A/Book'); - const cacheHolds = (data: ChaptersResponse) => - mockGetQueryData.mockImplementation((k: unknown) => - Array.isArray(k) && k[0] === 'chapters' && k[3] === 'A/Book' ? data : undefined, - ); - afterEach(() => mockGetQueryData.mockReset()); it('is taken when the download finishes', async () => { mockResolveClient.mockReturnValue({ diff --git a/src/downloads/store.ts b/src/downloads/store.ts index f1e11009..300a4663 100644 --- a/src/downloads/store.ts +++ b/src/downloads/store.ts @@ -6,11 +6,12 @@ import { chaptersKeyParts, chaptersQuery, itemQuery, qk } from '@/api/hooks'; import { queryClient } from '@/api/provider'; import type { Book, ChaptersResponse } from '@/api/types'; import { identityMovePending } from '@/lib/connection-id'; -import { contentKey } from '@/lib/content-key'; +import { contentKey, contentKeyOf } from '@/lib/content-key'; import { mapLimit } from '@/lib/map-limit'; import { getItem, setItem } from '@/lib/storage'; import { DOWNLOADS_KEY } from '@/lib/storage-keys'; import { bookFileSpecs } from '@/playback/book-queue'; +import type { BookRef } from '@/playback/types'; import { webTranscodeFromCache } from '@/playback/transcode-capability'; import { onConnectionRemoved } from '@/stores/session'; @@ -117,7 +118,7 @@ export const useDownloads = create()((set, get) => ({ const key = downloadKey(e.connectionId, e.libraryId, e.path); if (revived?.status === 'downloaded') { // A chapters answer can reach the cache before this read the registry. - const manifest = withCachedChapters(e.connectionId, e.libraryId, e.path, revived.manifest); + const manifest = withCachedChapters(e, revived.manifest); cleaned[key] = manifest === revived.manifest ? revived : { ...revived, manifest }; seedQueryCache(e.connectionId, e.libraryId, e.path, manifest); } else if (revived) { @@ -373,31 +374,76 @@ export function reviveEntry(e: DownloadEntry, allPresent: boolean): DownloadEntr * it was downloaded (a rescan, a community chapter list fitted onto its audio, the admin * switching it back), and offline it plays and lists what the manifest holds. Only the * chapters move: when the fresh answer's audio files are not the ones on the device (the - * same paths in the same order, `bookFileSpecs` as the download read them, and the same + * same paths in the order the manifest plays them now, `bookFileSpecs`, and the same * sizes where both answers know them: a file replaced in place keeps its path), the local - * audio no longer matches and that is a new download, so nothing changes. Null too when - * nothing differs. + * audio no longer matches and that is a new download, so nothing changes. The same files + * in another order are taken only by a start that plays them that way + * (`adoptStartedChapters`), never from a background answer. Null too when nothing differs. */ export function refreshedChapters( manifest: DownloadManifest, fresh: ChaptersResponse, ): DownloadManifest | null { + const refresh = chaptersRefresh(manifest, fresh); + return refresh && !refresh.reordered ? refresh.manifest : null; +} + +/** The manifest with `fresh` chapters when they name the device's own files (the same set + * of paths, the same sizes where both answers know them), and whether they play those + * files in another order than the manifest's chapters do (`reordered`); else null. */ +function chaptersRefresh( + manifest: DownloadManifest, + fresh: ChaptersResponse, +): { manifest: DownloadManifest; reordered: boolean } | null { // An unchanged refetch hands back the same object (structural sharing): skip the compare. if (fresh === manifest.chapters) return null; const want = bookFileSpecs(manifest.book, fresh); - // The files as the download read them (sizes 0 when only the chapters named them). + if (want.length !== manifest.files.length) return null; + // The files in the order the manifest plays them now, with the sizes the download read + // (0 when only the chapters named the files). const read = bookFileSpecs(manifest.book, manifest.chapters ?? undefined); - const have = manifest.files; - const same = - want.length === have.length && - want.every((s, i) => { - if (s.path !== have[i].relPath) return false; - const was = read[i]; - return was?.path !== s.path || !(was.size > 0 && s.size > 0) || was.size === s.size; - }); - if (!same) return null; - if (JSON.stringify(fresh) === JSON.stringify(manifest.chapters)) return null; - return { ...manifest, chapters: fresh }; + const readSize = new Map(read.map((s) => [s.path, s.size])); + // The device's files, each one named exactly once, in any order. + const onDisk = manifest.files.map((f) => f.relPath); + const onDiskSet = new Set(onDisk); + const onDevice = new Set(onDiskSet); + for (const s of want) { + if (!onDevice.delete(s.path)) return null; + const was = readSize.get(s.path) ?? 0; + if (was > 0 && s.size > 0 && was !== s.size) return null; + } + // The order the manifest plays them in now: its chapters', else (saved chapters that name + // no file list of their own) the order the download wrote them. + const playing = + read.length === onDisk.length && read.every((s) => onDiskSet.has(s.path)) + ? read.map((s) => s.path) + : onDisk; + const reordered = want.some((s, i) => s.path !== playing[i]); + if (!reordered && JSON.stringify(fresh) === JSON.stringify(manifest.chapters)) return null; + return { manifest: { ...manifest, chapters: fresh }, reordered }; +} + +/** + * A start of a downloaded book loaded the engine from `chapters` (`playBook`, once its + * load landed): the manifest takes them, so a later start, offline or from Android's + * service (`adoptLoaded`, which gets only a track index), plays the order the engine was + * given. This is the only way the same files take a new order (server v2.4.0 plays them + * in natural order, "2-" before "10-"): the chapters carry the order every queue over the + * download plays them in (`bookFileSpecs`, each file found by its path: + * `localFromManifest`), and `files` itself never moves, since a file's index there is its + * name on disk (`fileName`, which `relocateEntry` and a retry rebuild it from). A + * background answer never does it (`refreshedChapters`): JS cannot see a book the Android + * service or a Cast receiver holds before it is adopted. Chapters naming other files + * change nothing. + */ +export function adoptStartedChapters(ref: BookRef, chapters: ChaptersResponse): void { + const entry = downloadedEntryOf(ref); + const refresh = entry && chaptersRefresh(entry.manifest, chapters); + // The same order is the background refresh's to take (`startChapterRefresh`): a start's + // answer can be older (cached) than one the manifest took while it loaded. + if (!refresh?.reordered) return; + patchEntry(contentKeyOf(ref), { manifest: refresh.manifest }); + void persist(); } /** @@ -407,14 +453,9 @@ export function refreshedChapters( * the launch's `hydrate` read the registry. That answer is the one the download was given * or a newer one (the callers read it from the same cache entry). */ -function withCachedChapters( - connectionId: string, - libraryId: number, - path: string, - manifest: DownloadManifest, -): DownloadManifest { +function withCachedChapters(ref: BookRef, manifest: DownloadManifest): DownloadManifest { const cached = queryClient.getQueryData( - qk.chapters(connectionId, libraryId, path), + qk.chapters(ref.connectionId, ref.libraryId, ref.path), ); return (cached && refreshedChapters(manifest, cached)) || manifest; } @@ -433,12 +474,11 @@ export function startChapterRefresh(): () => void { const book = chaptersKeyParts(event.query.queryKey); const fresh = event.action.data as ChaptersResponse | undefined; if (!book || !fresh) return; - const key = downloadKey(book.cid, book.libraryId, book.path); - const entry = useDownloads.getState().entries[key]; - if (entry?.status !== 'downloaded') return; - const manifest = refreshedChapters(entry.manifest, fresh); + const ref = { connectionId: book.cid, libraryId: book.libraryId, path: book.path }; + const entry = downloadedEntryOf(ref); + const manifest = entry && refreshedChapters(entry.manifest, fresh); if (!manifest) return; - patchEntry(key, { manifest }); + patchEntry(contentKeyOf(ref), { manifest }); // A burst of answers (a reconnect's refetches, a search fan-out) saves the registry // once, not once per book. persistSoon(); @@ -786,7 +826,7 @@ async function runOne(key: string) { } // With the server's chapters answer when one landed while the files did. - const manifest = withCachedChapters(connectionId, libraryId, path, { + const manifest = withCachedChapters(entry, { ...entry.manifest, files, coverUri, diff --git a/src/downloads/types.ts b/src/downloads/types.ts index 7d712a44..6c60d915 100644 --- a/src/downloads/types.ts +++ b/src/downloads/types.ts @@ -43,7 +43,10 @@ export type DownloadOutcome = /** * Offline source of truth for a downloaded book - everything the playback layer * needs to build the queue and render the player with no network: the book + - * chapters metadata, the local audio files (in play order), and a local cover. + * chapters metadata, the local audio files, and a local cover. `files` is in the order + * the download wrote them, each saved under its index there (`fileName` in store.ts), so + * it never moves; the chapters give the play order (`bookFileSpecs`), which a later + * start in the server's new order can change (`adoptStartedChapters`). */ export type DownloadManifest = { book: Book; diff --git a/src/playback/book-queue.ts b/src/playback/book-queue.ts index e9a8ca3b..1d20ad6c 100644 --- a/src/playback/book-queue.ts +++ b/src/playback/book-queue.ts @@ -33,9 +33,10 @@ export type FileSpec = { path: string; duration: number; size: number }; /** * The distinct audio files that make up a book, in play order - the single * source of truth for both playback (`buildBookQueue`) and offline downloads, so - * download order ≡ play order. Prefers an explicit file list; falls back to the - * files referenced by the chapters; finally the book path itself. This must never - * resolve to a folder path. + * a download writes the files in play order (a later start can play the same files + * in another; the chapters say which: `adoptStartedChapters` in the downloads store). + * Prefers an explicit file list; falls back to the files referenced by the chapters; + * finally the book path itself. This must never resolve to a folder path. */ export function bookFileSpecs(book: Book, chapterData?: ChaptersResponse): FileSpec[] { const files = chapterData?.files?.length ? chapterData.files : (book.files ?? []); diff --git a/src/playback/store-adopt.test.ts b/src/playback/store-adopt.test.ts index 7e013ab7..9f748931 100644 --- a/src/playback/store-adopt.test.ts +++ b/src/playback/store-adopt.test.ts @@ -54,8 +54,16 @@ jest.mock('./progress-sync', () => ({ })); const mockFetchQuery = jest.fn(async (..._a: unknown[]): Promise => ({})); +// The query cache's listeners (`startChapterRefresh` subscribes one). +const mockCacheListeners = new Set<(event: unknown) => void>(); jest.mock('@/api/provider', () => ({ queryClient: { + getQueryCache: () => ({ + subscribe: (l: (event: unknown) => void) => { + mockCacheListeners.add(l); + return () => mockCacheListeners.delete(l); + }, + }), invalidateQueries: jest.fn(), setQueryData: jest.fn(), setQueryDefaults: jest.fn(), @@ -79,7 +87,8 @@ jest.mock('@/api/connection-clients', () => ({ })); /* eslint-disable import/first */ -import { useDownloads } from '@/downloads/store'; +import { qk } from '@/api/hooks'; +import { startChapterRefresh, useDownloads } from '@/downloads/store'; import type { DownloadEntry } from '@/downloads/types'; import { setConnectionAliases } from '@/lib/connection-id'; import { resetProfilesForTests, switchProfile, useProfiles } from '@/stores/profiles'; @@ -323,6 +332,39 @@ describe('adoptLoaded', () => { ); }); + // The service plays B in its download's order after a JS restart; the server has since + // re-ordered B's files (v2.4.0's natural order). Its answer landing before the adoption must + // not move the order the adoption maps the engine's track index through. + it('maps the engine onto the download order a re-ordered server answer left alone', async () => { + useDownloads.setState({ entries: { 'c1:2:B': downloadedB() } }); + const stop = startChapterRefresh(); + const reordered: ChaptersResponse = { + ...twoFileChapters, + files: [...twoFileChapters.files].reverse().map((f, seq) => ({ ...f, seq })), + }; + for (const l of mockCacheListeners) { + l({ + type: 'updated', + query: { queryKey: qk.chapters('c1', 2, 'B') }, + action: { type: 'success', data: reordered }, + }); + } + stop(); + expect(useDownloads.getState().entries['c1:2:B'].manifest.chapters).toBe(twoFileChapters); + + await usePlayer.getState().adoptLoaded(loadedB({ trackIndex: 1, position: 30 })); + const { nowPlaying } = usePlayer.getState(); + expect(nowPlaying!.queue.tracks.map((t) => t.url)).toEqual([ + 'file:///d/1.mp3', + 'file:///d/2.mp3', + ]); + pushSnapshot(snap('playing', 1, 45)); + jest.advanceTimersByTime(15_000); // one save tick + await flushMicrotasks(); + // Track 1 is B/2.mp3, the second 100 s of the download's timeline. + expect(mockSaveProgress.mock.calls[0][1]).toMatchObject({ path: 'B', position: 145 }); + }); + it("saves the next time under the adopted book's path, at the engine's place", async () => { // Another book was loaded and playing in the app. await usePlayer.getState().playBook('c1', 2, makeBook(), undefined, 10); diff --git a/src/playback/store.test.ts b/src/playback/store.test.ts index 06ac0898..914bb9e8 100644 --- a/src/playback/store.test.ts +++ b/src/playback/store.test.ts @@ -1,4 +1,4 @@ -import type { Book, Progress } from '@/api/types'; +import type { Book, ChaptersResponse, Progress } from '@/api/types'; import type { ResumeLookup } from './progress-sync'; import type { PlaybackService, PlaybackSnapshot, PlaybackState } from './types'; @@ -134,6 +134,7 @@ import type { DownloadEntry, DownloadManifest } from '@/downloads/types'; import { resetProfilesForTests, switchProfile, useProfiles } from '@/stores/profiles'; import { useSettings } from '@/stores/settings'; +import { bookFileSpecs } from './book-queue'; import { flushConnection } from './progress-sync'; import { AutoplayBlockedError } from './types'; import { @@ -1345,6 +1346,127 @@ describe('the download hot-swap', () => { }); }); +// --- a downloaded book whose server now plays its files in another order ----------- +// (server v2.4.0's natural order: "2-" before "10-"). The download takes the server's order +// and chapters only from a start that loaded the engine in it. +describe('a downloaded book re-ordered on its server', () => { + const folder = 'A/Folder'; + const paths = ['A/Folder/10.mp3', 'A/Folder/2.mp3']; + const serverOrder = ['A/Folder/2.mp3', 'A/Folder/10.mp3']; + const chaptersIn = (order: string[]): ChaptersResponse => ({ + library_id: 2, + path: folder, + duration: 100, + is_folder: true, + files: order.map((rel_path, seq) => ({ rel_path, seq, duration: 50, format: 'mp3', size: 1 })), + chapters: order.map((file_path, index) => ({ + index, + title: `Chapter ${index + 1}`, + file_index: index, + file_path, + start: 0, + end: 50, + book_offset: index * 50, + })), + }); + const book = makeBook({ rel_path: folder, is_folder: true, format: 'mp3' }); + const key = `c1:2:${folder}`; + /** The order the download plays its files in (its chapters'; `files` never moves). */ + const order = () => { + const { manifest } = useDownloads.getState().entries[key]; + return bookFileSpecs(manifest.book, manifest.chapters ?? undefined).map((s) => s.path); + }; + + beforeEach(() => { + const manifest: DownloadManifest = { + book, + chapters: chaptersIn(paths), + files: paths.map((relPath, i) => ({ relPath, localUri: `file:///dl/${i}.mp3` })), + coverUri: null, + savedAt: '2026-01-01T00:00:00Z', + }; + useDownloads.setState({ + entries: { + [key]: { + connectionId: 'c1', + libraryId: 2, + path: folder, + title: book.title, + status: 'downloaded', + progress: 1, + bytes: 0, + totalBytes: 0, + manifest, + }, + }, + }); + // The server's answer, in its new order, sits in the query cache. + const chaptersKey = JSON.stringify(qk.chapters('c1', 2, folder)); + (queryClient.getQueryData as jest.Mock).mockImplementation((k: unknown) => + JSON.stringify(k) === chaptersKey ? chaptersIn(serverOrder) : undefined, + ); + }); + afterEach(() => (queryClient.getQueryData as jest.Mock).mockImplementation(() => undefined)); + + it("takes the server's order once a start plays it", async () => { + await usePlayer.getState().playBook('c1', 2, book, chaptersIn(serverOrder), 0); + const tracks = usePlayer.getState().nowPlaying?.queue.tracks.map((t) => t.url); + expect(tracks).toEqual(['file:///dl/1.mp3', 'file:///dl/0.mp3']); + expect(order()).toEqual(serverOrder); + // The files stay where the download wrote them. + expect(useDownloads.getState().entries[key].manifest.files.map((f) => f.relPath)).toEqual( + paths, + ); + }); + + it('keeps the old order for an offline start, and after it, whatever the cache holds', async () => { + // Started from the download's own copy (offline, or the fresh read timed out). + await usePlayer.getState().playBook('c1', 2, book, chaptersIn(paths), 0); + const tracks = usePlayer.getState().nowPlaying?.queue.tracks.map((t) => t.url); + expect(tracks).toEqual(['file:///dl/0.mp3', 'file:///dl/1.mp3']); + expect(order()).toEqual(paths); + // The book unloads: the server's answer in the cache is still not taken. + usePlayer.setState({ nowPlaying: null }); + expect(order()).toEqual(paths); + }); + + it("plays the download's order when a start has no chapters for it", async () => { + // The server's item lists the files in its new order; its chapters read failed. + const fresh = { ...book, files: chaptersIn(serverOrder).files }; + await usePlayer.getState().playBook('c1', 2, fresh, undefined, 0); + const tracks = usePlayer.getState().nowPlaying?.queue.tracks.map((t) => t.url); + expect(tracks).toEqual(['file:///dl/0.mp3', 'file:///dl/1.mp3']); + expect(order()).toEqual(paths); + }); + + it("plays the download's file list when it saved no chapters and a start has none", async () => { + const e = useDownloads.getState().entries[key]; + const saved = { ...book, files: chaptersIn(paths).files }; + useDownloads.setState({ + entries: { [key]: { ...e, manifest: { ...e.manifest, book: saved, chapters: null } } }, + }); + const fresh = { ...book, files: chaptersIn(serverOrder).files }; + await usePlayer.getState().playBook('c1', 2, fresh, undefined, 0); + const tracks = usePlayer.getState().nowPlaying?.queue.tracks.map((t) => t.url); + expect(tracks).toEqual(['file:///dl/0.mp3', 'file:///dl/1.mp3']); + expect(order()).toEqual(paths); + }); + + it('takes nothing from a start overtaken by another before its load landed', async () => { + let loaded: () => void = () => {}; + (mockSvc.load as jest.Mock).mockImplementationOnce( + () => new Promise((resolve) => (loaded = resolve)), + ); + const start = usePlayer.getState().playBook('c1', 2, book, chaptersIn(serverOrder), 0); + await flushMicrotasks(); + // Another book starts while this load is on its way: it owns the engine. + await startBook(makeBook(), 0); + loaded(); + await start; + expect(order()).toEqual(paths); + }); +}); + // --- offline playback of a downloaded book whose connection is gone ---------------- // A connection whose secure-store token fails to hydrate is dropped from the session, so // resolveClient() returns null - but downloads.hydrate keeps its entry. A fully-local diff --git a/src/playback/store.ts b/src/playback/store.ts index f4bb5c68..254e8773 100644 --- a/src/playback/store.ts +++ b/src/playback/store.ts @@ -6,7 +6,12 @@ import { qk } from '@/api/hooks'; import { queryClient } from '@/api/provider'; import { isReachable, noteError } from '@/api/reachability'; import type { Book, Chapter, ChaptersResponse, Progress } from '@/api/types'; -import { downloadedEntryOf, downloadKey, useDownloads } from '@/downloads/store'; +import { + adoptStartedChapters, + downloadedEntryOf, + downloadKey, + useDownloads, +} from '@/downloads/store'; import { resolveConnectionId } from '@/lib/connection-id'; import { contentKey } from '@/lib/content-key'; import { canAutoDownload } from '@/lib/network'; @@ -814,11 +819,16 @@ export const usePlayer = create()((set, get) => ({ !local && mayNeedWebTranscode(book, chapterData) && (await resolveWebTranscode(connectionId, api, book, chapterData)); + // A downloaded book given no chapters (its chapters read failed) plays its download's + // files and chapters, the order its manifest holds, so the engine never holds an order + // the manifest does not (`adoptStartedChapters`). + const own = dl && !chapterData ? dl.manifest : undefined; + const queueChapters = chapterData ?? own?.chapters ?? undefined; const queue = buildBookQueue( api, libraryId, - book, - chapterData, + own ? { ...book, files: own.book.files, chapters: own.book.chapters } : book, + queueChapters, local, useSettings.getState().virtualChapterInterval, transcode, @@ -911,6 +921,9 @@ export const usePlayer = create()((set, get) => ({ // Another book was started while this one loaded (a second tap, Up next): that start // owns the engine now, so this one neither sets its speed nor plays. if (selectBookKey(get()) !== key) return; + // The engine holds the queue built from these chapters now: a download takes them, + // with a new file order when its server re-ordered the files (`adoptStartedChapters`). + if (dl && queueChapters) adoptStartedChapters(ref, queueChapters); await svc.setRate(speed); if (!current()) return; await startEngine(svc);