From 0d82d475f5e7df5b35d4e3aef6511fe4ccefdeb7 Mon Sep 17 00:00:00 2001 From: Gianni Carlo Date: Fri, 25 Sep 2026 09:13:52 -0500 Subject: [PATCH 1/4] Restore Deep Archive books on demand when a Pro user plays them A lapsed Pro subscriber's library is archived to DEEP_ARCHIVE by a per-user lifecycle rule, and nothing ever brings it back: after they resubscribe, presigned URLs for archived objects answer InvalidObjectState (403) forever. When GET /v1/library names one item (a non-root path with no trailing slash resolving to a single non-folder row) for a PRO caller on the presigned branch, HEAD the object first. If it sits in DEEP_ARCHIVE with no restore in flight, request a Standard-tier restore (Days=30), record it in the new glacier_restore_requests table (one row per user+key, re-opened on a later freeze) and mark the item storageState: "restoring". Listings never touch S3; a warm item costs one HEAD; the hook never fails the request. Only on the tap that issued the restore do the item's artwork and, for a bound book, its other chapters and the container's cover follow in the background. A bound book resolved by uuid probes up to three of its files (synced first) before walking. An in-process guard collapses concurrent taps on the same book into one walk. Legacy rows with a NULL type count as files, since they get a URL like any book. The thawed copy is temporary; the Lambda finalizer that copies restored objects back to INTELLIGENT_TIERING lands in the next PR, which is why the restore window is 30 days rather than 7. The task role already has s3:RestoreObject. --- CLAUDE.md | 2 +- .../services/GlacierRestoreHook.test.ts | 552 ++++++++++++++++++ .../services/S3ServiceRestore.test.ts | 68 +++ ...20260925120000_glacier_restore_requests.ts | 43 ++ src/services/GlacierRestoreService.ts | 292 +++++++++ src/services/LibraryService.ts | 31 +- src/services/S3Service.ts | 81 +++ src/services/StorageService.ts | 46 +- src/services/db/GlacierRestoreDB.ts | 92 +++ src/types/user.ts | 8 + 10 files changed, 1212 insertions(+), 3 deletions(-) create mode 100644 src/__tests__/services/GlacierRestoreHook.test.ts create mode 100644 src/__tests__/services/S3ServiceRestore.test.ts create mode 100644 src/database/migrations/20260925120000_glacier_restore_requests.ts create mode 100644 src/services/GlacierRestoreService.ts create mode 100644 src/services/db/GlacierRestoreDB.ts diff --git a/CLAUDE.md b/CLAUDE.md index 00a44ac..b6d5177 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -371,7 +371,7 @@ All routes require auth + an active subscription (`checkSubscription`); most als | Method | Path | Purpose | |--------|------|---------| -| GET | `/` | List / resolve items. **`uuid` names the item; a trailing `/` on `relativePath` asks for its contents.** A valid uuid is authoritative (not found → `[]`, no path fallback); a folder or bound book resolved by uuid with a trailing slash lists its children by the *server-side* key. No uuid → path lookup. `sign=true` presigns URLs (PRO only). A failed DB read is a 500, never an empty library. | +| GET | `/` | List / resolve items. **`uuid` names the item; a trailing `/` on `relativePath` asks for its contents.** A valid uuid is authoritative (not found → `[]`, no path fallback); a folder or bound book resolved by uuid with a trailing slash lists its children by the *server-side* key. No uuid → path lookup. `sign=true` presigns URLs (PRO only). A failed DB read is a 500, never an empty library. When the request names ONE item (a non-root path with no trailing slash resolving to a single non-folder row) for a PRO user and the file sits in Deep Archive, a Standard-tier restore is requested on the spot and the item carries `storageState: "restoring"` (`GlacierRestoreService`; a bound book resolved directly has no object of its own, so it carries no state and its files are checked in the background); only on that first frozen tap do the book's artwork and, for a bound book, its other chapters and cover follow in the background — a warm item costs one HEAD. | | POST / PUT / DELETE | `/` | Update metadata / upload metadata / soft-delete an item and its true children (bounded, escaped key match). POST and PUT bodies are validated with zod (`src/validation/libraryItem.ts`): every metadata field optional, unknown keys stripped, so `source_path` — and `synced` on PUT — can't be written by a client. Both apps retry a failed sync job forever, so keep those schemas matching what they send (the fixtures in `src/__tests__/validation/libraryItem.test.ts`) | | GET | `/last_played` | Resume item, or `null` when nothing has been played; 500 on a failed read | | PUT / DELETE | `/external` | Link / unlink an external resource (Jellyfin, Audiobookshelf, …) | diff --git a/src/__tests__/services/GlacierRestoreHook.test.ts b/src/__tests__/services/GlacierRestoreHook.test.ts new file mode 100644 index 0000000..90194df --- /dev/null +++ b/src/__tests__/services/GlacierRestoreHook.test.ts @@ -0,0 +1,552 @@ +import { describe, it, expect, beforeEach, jest } from '@jest/globals'; +import { LibraryService } from '../../services/LibraryService'; +import { RESTORE_DAYS, RESTORE_TIER } from '../../services/GlacierRestoreService'; +import { LibraryItem, SubscriptionTierEnum } from '../../types/user'; +import { ObjectHead } from '../../services/S3Service'; +import { + getTestTransaction, + mockLoggerService, + createTestUser, + createTestLibraryItem, +} from '../setup'; + +const APP_VERSION = '2022-12-12'; // the presigned branch every shipped app uses +const PREFIX = 'pfx'; +const ARCHIVED_COLD: ObjectHead = { storageClass: 'DEEP_ARCHIVE', restore: 'none', contentLength: 100 }; +const ARCHIVED_THAWING: ObjectHead = { storageClass: 'DEEP_ARCHIVE', restore: 'ongoing', contentLength: 100 }; +const ARCHIVED_READY: ObjectHead = { storageClass: 'DEEP_ARCHIVE', restore: 'ready', contentLength: 100 }; +const WARM: ObjectHead = { storageClass: 'INTELLIGENT_TIERING', restore: 'none', contentLength: 100 }; + +// createTestLibraryItem stores source_path as root/test_. +const objectKey = (key: string) => `${PREFIX}/root/test_${key.split('/').pop()}`; + +/** + * On-demand thaw: when a Pro client asks for the signed URL of ONE item, a book + * sitting in Deep Archive gets a restore requested and recorded; listings never + * touch S3; nothing here can fail the request. + */ +describe('LibraryService — on-demand Glacier restore hook', () => { + let service: LibraryService; + let heads: Record; + let headObject: jest.Mock; + let restoreObject: jest.Mock; + + beforeEach(() => { + service = new LibraryService(); + const trx = getTestTransaction(); + (service as any).db = trx; + (service as any)._libraryDB.db = trx; + (service as any)._libraryDB._logger = mockLoggerService; + (service as any)._logger = mockLoggerService; + (service as any)._prefix = { getPrefix: jest.fn(async () => PREFIX) }; + (service as any)._storage = { + getPresignedUrl: jest.fn(async ({ key }: { key: string }) => ({ url: `https://signed/${key}`, expires_in: 1 })), + }; + const glacier = (service as any)._glacier; + glacier._logger = mockLoggerService; + glacier._libraryDB.db = trx; + glacier._libraryDB._logger = mockLoggerService; + glacier._restoreDB.db = trx; + glacier._restoreDB._logger = mockLoggerService; + heads = {}; + headObject = jest.fn(async ({ key }: { key: string }) => (key in heads ? heads[key] : WARM)); + // Behaves like S3: once a restore is requested the object reads as thawing, + // so a later HEAD (e.g. from a background walk) no longer sees it cold. + restoreObject = jest.fn(async ({ key }: { key: string }) => { + const current = heads[key]; + if (current && current !== 'missing' && current.restore === 'none') { + heads[key] = { ...current, restore: 'ongoing' }; + } + return true; + }); + glacier._storage = { headObject, restoreObject }; + mockLoggerService.log.mockClear(); + }); + + const drain = () => (service as any)._glacier.drain(); + const rows = (userId: number) => (service as any)._glacier._restoreDB.getByUser(userId); + + function get( + user: { id_user: number; email: string }, + relativePath: string, + uuid?: string, + tier: string = SubscriptionTierEnum.PRO, + ): Promise { + return service.getLibrary( + { ...user, subscriptions: [tier] } as any, + `${user.email}/${relativePath}`, + { appVersion: APP_VERSION, withPresign: true }, + uuid, + ); + } + + // Solo.m4b Folder/a.m4b Series/ (bound): 01.mp3 (with artwork), 02.mp3, 03.mp3 + async function seed(userId: number) { + const trx = getTestTransaction(); + const solo = await createTestLibraryItem(trx, { user_id: userId, key: 'Solo.m4b' }); + await createTestLibraryItem(trx, { user_id: userId, key: 'Folder', type: 0 }); + await createTestLibraryItem(trx, { user_id: userId, key: 'Folder/a.m4b' }); + const series = await createTestLibraryItem(trx, { user_id: userId, key: 'Series', type: 1 }); + const ch1 = await createTestLibraryItem(trx, { user_id: userId, key: 'Series/01.mp3' }); + await trx('library_items').where({ id_library_item: ch1.id_library_item }).update({ thumbnail: 'cover.jpg' }); + const ch2 = await createTestLibraryItem(trx, { user_id: userId, key: 'Series/02.mp3' }); + const ch3 = await createTestLibraryItem(trx, { user_id: userId, key: 'Series/03.mp3' }); + return { solo, series, ch1, ch2, ch3 }; + } + + it('a folder listing never touches S3', async () => { + const user = await createTestUser(getTestTransaction()); + await seed(user.id_user); + + const items = await get(user, 'Folder/'); + await drain(); + + expect(items.map((i) => i.relativePath)).toEqual(['Folder/a.m4b']); + expect(headObject).not.toHaveBeenCalled(); + expect(items[0].storageState).toBeUndefined(); + }); + + it('the root listing never touches S3 either', async () => { + const user = await createTestUser(getTestTransaction()); + await seed(user.id_user); + + await get(user, ''); + await drain(); + + expect(headObject).not.toHaveBeenCalled(); + }); + + it('a frozen book requested by path gets a Standard restore for RESTORE_DAYS and a request row', async () => { + const user = await createTestUser(getTestTransaction()); + const { solo } = await seed(user.id_user); + heads[objectKey('Solo.m4b')] = ARCHIVED_COLD; + + const [item] = await get(user, 'Solo.m4b'); + await drain(); + + expect(item.storageState).toBe('restoring'); + expect(item.url).toContain('Solo.m4b'); // the URL still goes out as before + expect(restoreObject).toHaveBeenCalledWith({ key: objectKey('Solo.m4b'), days: RESTORE_DAYS, tier: RESTORE_TIER }); + expect(RESTORE_TIER).toBe('Standard'); + const saved = await rows(user.id_user); + expect(saved).toHaveLength(1); + expect(saved[0]).toMatchObject({ + key: objectKey('Solo.m4b'), + kind: 'object', + state: 'requested', + attempts: 1, + tier: 'Standard', + days: RESTORE_DAYS, + library_item_id: solo.id_library_item, + }); + }); + + it('the same by uuid', async () => { + const user = await createTestUser(getTestTransaction()); + const { solo } = await seed(user.id_user); + heads[objectKey('Solo.m4b')] = ARCHIVED_COLD; + + const [item] = await get(user, 'stale/path.m4b', solo.uuid); + await drain(); + + expect(item.storageState).toBe('restoring'); + expect(restoreObject).toHaveBeenCalledTimes(1); + }); + + it('a warm book is reported available and nothing is requested or recorded', async () => { + const user = await createTestUser(getTestTransaction()); + await seed(user.id_user); + + const [item] = await get(user, 'Solo.m4b'); + await drain(); + + expect(item.storageState).toBe('available'); + expect(restoreObject).not.toHaveBeenCalled(); + expect(await rows(user.id_user)).toHaveLength(0); + }); + + it('a book already thawing is recorded (so a hand-run restore gets finalized) but not re-requested', async () => { + const user = await createTestUser(getTestTransaction()); + await seed(user.id_user); + heads[objectKey('Solo.m4b')] = ARCHIVED_THAWING; + + const [item] = await get(user, 'Solo.m4b'); + await drain(); + + expect(item.storageState).toBe('restoring'); + expect(restoreObject).not.toHaveBeenCalled(); + expect(await rows(user.id_user)).toHaveLength(1); + }); + + it('a thawed copy that is ready reads as available and is recorded so the finalizer makes it permanent', async () => { + const user = await createTestUser(getTestTransaction()); + await seed(user.id_user); + heads[objectKey('Solo.m4b')] = ARCHIVED_READY; + + const [item] = await get(user, 'Solo.m4b'); + await drain(); + + expect(item.storageState).toBe('available'); + expect(restoreObject).not.toHaveBeenCalled(); + // A hand-run restore that finished, or one whose row was lost: without a + // row the copy would expire after RESTORE_DAYS and the book would refreeze. + expect(await rows(user.id_user)).toHaveLength(1); + }); + + it('a root listing with exactly one top-level item is still a listing: no HEAD', async () => { + const trx = getTestTransaction(); + const user = await createTestUser(trx); + await createTestLibraryItem(trx, { user_id: user.id_user, key: 'Only.m4b' }); + heads[objectKey('Only.m4b')] = ARCHIVED_COLD; + + const items = await get(user, ''); + await drain(); + + expect(items.map((i) => i.relativePath)).toEqual(['Only.m4b']); + expect(headObject).not.toHaveBeenCalled(); + expect(items[0].storageState).toBeUndefined(); + }); + + it('a warm chapter of a bound book costs exactly one HEAD: no sibling walk', async () => { + const user = await createTestUser(getTestTransaction()); + await seed(user.id_user); + + const [item] = await get(user, 'Series/02.mp3'); + await drain(); + + expect(item.storageState).toBe('available'); + expect(headObject).toHaveBeenCalledTimes(1); + expect(restoreObject).not.toHaveBeenCalled(); + }); + + it('a chapter already thawing is recorded but does not re-walk the book', async () => { + const user = await createTestUser(getTestTransaction()); + await seed(user.id_user); + heads[objectKey('Series/02.mp3')] = ARCHIVED_THAWING; + heads[objectKey('Series/01.mp3')] = ARCHIVED_COLD; + heads[objectKey('Series/03.mp3')] = ARCHIVED_COLD; + + const [item] = await get(user, 'Series/02.mp3'); + await drain(); + + expect(item.storageState).toBe('restoring'); + expect(headObject).toHaveBeenCalledTimes(1); // the first tap already walked the book + expect(restoreObject).not.toHaveBeenCalled(); + expect(await rows(user.id_user)).toHaveLength(1); + }); + + it('a warm bound book resolved as a container costs one probe HEAD', async () => { + const user = await createTestUser(getTestTransaction()); + const { series } = await seed(user.id_user); + + await get(user, 'Series', series.uuid); + await drain(); + + expect(headObject).toHaveBeenCalledTimes(1); + expect(restoreObject).not.toHaveBeenCalled(); + }); + + it("a frozen bound book's own cover is thawed along with its chapters", async () => { + const trx = getTestTransaction(); + const user = await createTestUser(trx); + const { series } = await seed(user.id_user); + await trx('library_items').where({ id_library_item: series.id_library_item }).update({ thumbnail: 'series.jpg' }); + heads[objectKey('Series/02.mp3')] = ARCHIVED_COLD; + heads[`${PREFIX}_thumbnail/series.jpg`] = ARCHIVED_COLD; + + await get(user, 'Series/02.mp3'); + await drain(); + + const requestedKeys = restoreObject.mock.calls.map((c: any[]) => c[0].key); + expect(requestedKeys).toContain(`${PREFIX}_thumbnail/series.jpg`); + const saved = await rows(user.id_user); + expect(saved.find((r: any) => r.kind === 'thumbnail')?.library_item_id).toBe(series.id_library_item); + }); + + it('two chapters of the same frozen book tapped at once walk the book once', async () => { + const user = await createTestUser(getTestTransaction()); + await seed(user.id_user); + heads[objectKey('Series/01.mp3')] = ARCHIVED_COLD; + heads[objectKey('Series/02.mp3')] = ARCHIVED_COLD; + heads[objectKey('Series/03.mp3')] = ARCHIVED_COLD; + + await Promise.all([get(user, 'Series/01.mp3'), get(user, 'Series/02.mp3')]); + await drain(); + + // Each chapter restored once: the two tapped ones in the foreground, the + // third by whichever walk ran; the second walk was skipped. + const restoredKeys = restoreObject.mock.calls.map((c: any[]) => c[0].key).sort(); + expect(restoredKeys).toEqual( + [objectKey('Series/01.mp3'), objectKey('Series/02.mp3'), objectKey('Series/03.mp3')].sort(), + ); + }); + + it("another user's identically named bound book is never touched", async () => { + const trx = getTestTransaction(); + const user = await createTestUser(trx); + const other = await createTestUser(trx, { email: `other-${Date.now()}@example.com` }); + await seed(user.id_user); + const otherSeries = await createTestLibraryItem(trx, { user_id: other.id_user, key: 'Series', type: 1 }); + const otherCh = await createTestLibraryItem(trx, { user_id: other.id_user, key: 'Series/02.mp3' }); + heads[objectKey('Series/01.mp3')] = ARCHIVED_COLD; + heads[objectKey('Series/02.mp3')] = ARCHIVED_COLD; + heads[objectKey('Series/03.mp3')] = ARCHIVED_COLD; + + await get(user, 'Series/01.mp3'); + await drain(); + + const saved = await rows(user.id_user); + expect(saved.every((r: any) => r.user_id === user.id_user)).toBe(true); + expect(saved.map((r: any) => r.library_item_id)).not.toContain(otherCh.id_library_item); + expect(saved.map((r: any) => r.library_item_id)).not.toContain(otherSeries.id_library_item); + expect(await rows(other.id_user)).toHaveLength(0); + }); + + it('a failed or 404 HEAD changes nothing: no state, no restore, the URL still returned', async () => { + const user = await createTestUser(getTestTransaction()); + await seed(user.id_user); + + heads[objectKey('Solo.m4b')] = null; + let [item] = await get(user, 'Solo.m4b'); + expect(item.storageState).toBeUndefined(); + expect(item.url).toContain('Solo.m4b'); + + heads[objectKey('Solo.m4b')] = 'missing'; + [item] = await get(user, 'Solo.m4b'); + await drain(); + expect(item.storageState).toBeUndefined(); + expect(restoreObject).not.toHaveBeenCalled(); + expect(await rows(user.id_user)).toHaveLength(0); + }); + + it('a restore request S3 refused is not recorded and not reported as restoring', async () => { + const user = await createTestUser(getTestTransaction()); + await seed(user.id_user); + heads[objectKey('Solo.m4b')] = ARCHIVED_COLD; + restoreObject.mockImplementation(async () => null); + + const [item] = await get(user, 'Solo.m4b'); + await drain(); + + expect(item.storageState).toBeUndefined(); + expect(item.url).toContain('Solo.m4b'); + expect(await rows(user.id_user)).toHaveLength(0); + }); + + it('repeated taps while thawing keep one row; a finalized row is re-opened on the next freeze', async () => { + const trx = getTestTransaction(); + const user = await createTestUser(trx); + await seed(user.id_user); + heads[objectKey('Solo.m4b')] = ARCHIVED_COLD; + + await get(user, 'Solo.m4b'); + await get(user, 'Solo.m4b'); + await drain(); + let [row] = await rows(user.id_user); + expect(row.attempts).toBe(1); + + await trx('glacier_restore_requests') + .where({ id_glacier_restore_request: row.id_glacier_restore_request }) + .update({ state: 'finalized', finalized_at: trx.fn.now() }); + await get(user, 'Solo.m4b'); + await drain(); + [row] = await rows(user.id_user); + expect(row).toMatchObject({ state: 'requested', attempts: 2, finalized_at: null }); + }); + + it('tapping one chapter of a bound book restores its siblings and artwork in the background', async () => { + const user = await createTestUser(getTestTransaction()); + const { ch1, ch2, ch3 } = await seed(user.id_user); + heads[objectKey('Series/01.mp3')] = ARCHIVED_COLD; + heads[objectKey('Series/02.mp3')] = ARCHIVED_COLD; + heads[objectKey('Series/03.mp3')] = ARCHIVED_THAWING; // already thawing: recorded, not re-requested + heads[`${PREFIX}_thumbnail/cover.jpg`] = ARCHIVED_COLD; + + const [item] = await get(user, 'Series/01.mp3'); + expect(item.storageState).toBe('restoring'); + await drain(); + + const requestedKeys = restoreObject.mock.calls.map((c: any[]) => c[0].key).sort(); + expect(requestedKeys).toEqual( + [objectKey('Series/01.mp3'), objectKey('Series/02.mp3'), `${PREFIX}_thumbnail/cover.jpg`].sort(), + ); + const saved = await rows(user.id_user); + expect(saved.map((r: any) => [r.key, r.kind, r.library_item_id]).sort()).toEqual( + [ + [objectKey('Series/01.mp3'), 'object', ch1.id_library_item], + [objectKey('Series/02.mp3'), 'object', ch2.id_library_item], + [objectKey('Series/03.mp3'), 'object', ch3.id_library_item], + [`${PREFIX}_thumbnail/cover.jpg`, 'thumbnail', ch1.id_library_item], + ].sort(), + ); + // Solo and Folder/a were never looked at. + expect(headObject.mock.calls.some((c: any[]) => c[0].key === objectKey('Solo.m4b'))).toBe(false); + }); + + it('a bound book resolved by uuid (no slash) restores its files', async () => { + const user = await createTestUser(getTestTransaction()); + const { series } = await seed(user.id_user); + heads[objectKey('Series/01.mp3')] = ARCHIVED_COLD; + heads[objectKey('Series/02.mp3')] = ARCHIVED_COLD; + heads[objectKey('Series/03.mp3')] = ARCHIVED_COLD; + + const [item] = await get(user, 'Series', series.uuid); + await drain(); + + expect(item.relativePath).toBe('Series'); + expect(item.storageState).toBeUndefined(); // a folder has no object of its own + expect(restoreObject).toHaveBeenCalledTimes(3); + }); + + it('a container tap whose first file cannot be checked probes the next one, and each file is HEADed once', async () => { + const user = await createTestUser(getTestTransaction()); + const { series } = await seed(user.id_user); + heads[objectKey('Series/01.mp3')] = 'missing'; // says nothing about the book + heads[objectKey('Series/02.mp3')] = ARCHIVED_COLD; + heads[objectKey('Series/03.mp3')] = ARCHIVED_COLD; + + await get(user, 'Series', series.uuid); + await drain(); + + const requestedKeys = restoreObject.mock.calls.map((c: any[]) => c[0].key).sort(); + expect(requestedKeys).toEqual([objectKey('Series/02.mp3'), objectKey('Series/03.mp3')]); + const headed = headObject.mock.calls.map((c: any[]) => c[0].key); + expect(headed.filter((k: string) => k === objectKey('Series/01.mp3'))).toHaveLength(1); + }); + + it('a container tap probes synced files before unsynced ones', async () => { + const trx = getTestTransaction(); + const user = await createTestUser(trx); + const { series, ch1 } = await seed(user.id_user); + await trx('library_items').where({ id_library_item: ch1.id_library_item }).update({ synced: false }); + heads[objectKey('Series/01.mp3')] = 'missing'; + heads[objectKey('Series/02.mp3')] = ARCHIVED_COLD; + heads[objectKey('Series/03.mp3')] = ARCHIVED_COLD; + + await get(user, 'Series', series.uuid); + await drain(); + + expect((headObject.mock.calls[0][0] as any).key).toBe(objectKey('Series/02.mp3')); + expect(restoreObject).toHaveBeenCalledTimes(2); + }); + + it('a container tap stops at the first definitive answer: a warm file means the book was not frozen', async () => { + const user = await createTestUser(getTestTransaction()); + const { series } = await seed(user.id_user); + heads[objectKey('Series/01.mp3')] = 'missing'; + heads[objectKey('Series/02.mp3')] = WARM; + heads[objectKey('Series/03.mp3')] = ARCHIVED_COLD; // never reached + + await get(user, 'Series', series.uuid); + await drain(); + + expect(headObject).toHaveBeenCalledTimes(2); + expect(restoreObject).not.toHaveBeenCalled(); + }); + + it('a container tap gives up after a few unanswerable probes', async () => { + const trx = getTestTransaction(); + const user = await createTestUser(trx); + const { series } = await seed(user.id_user); + await createTestLibraryItem(trx, { user_id: user.id_user, key: 'Series/04.mp3' }); + heads[objectKey('Series/01.mp3')] = 'missing'; + heads[objectKey('Series/02.mp3')] = 'missing'; + heads[objectKey('Series/03.mp3')] = null; // HEAD failed + heads[objectKey('Series/04.mp3')] = ARCHIVED_COLD; + + await get(user, 'Series', series.uuid); + await drain(); + + expect(headObject).toHaveBeenCalledTimes(3); + expect(restoreObject).not.toHaveBeenCalled(); + }); + + it("a frozen bound book tapped as a container also thaws the book's own cover", async () => { + const trx = getTestTransaction(); + const user = await createTestUser(trx); + const { series } = await seed(user.id_user); + await trx('library_items').where({ id_library_item: series.id_library_item }).update({ thumbnail: 'series.jpg' }); + heads[objectKey('Series/01.mp3')] = ARCHIVED_COLD; + heads[`${PREFIX}_thumbnail/series.jpg`] = ARCHIVED_COLD; + + await get(user, 'Series', series.uuid); + await drain(); + + expect(restoreObject.mock.calls.map((c: any[]) => c[0].key)).toContain(`${PREFIX}_thumbnail/series.jpg`); + }); + + it('a one-chapter bound book still gets its cover thawed when that chapter is tapped', async () => { + const trx = getTestTransaction(); + const user = await createTestUser(trx); + await seed(user.id_user); + const single = await createTestLibraryItem(trx, { user_id: user.id_user, key: 'Single', type: 1 }); + await trx('library_items').where({ id_library_item: single.id_library_item }).update({ thumbnail: 'single.jpg' }); + await createTestLibraryItem(trx, { user_id: user.id_user, key: 'Single/01.mp3' }); + heads[objectKey('Single/01.mp3')] = ARCHIVED_COLD; + heads[`${PREFIX}_thumbnail/single.jpg`] = ARCHIVED_COLD; + + const [item] = await get(user, 'Single/01.mp3'); + expect(item.storageState).toBe('restoring'); + await drain(); + + expect(restoreObject.mock.calls.map((c: any[]) => c[0].key).sort()).toEqual( + [objectKey('Single/01.mp3'), `${PREFIX}_thumbnail/single.jpg`].sort(), + ); + }); + + it('a legacy row with a NULL type is a file: a frozen one gets a restore', async () => { + const trx = getTestTransaction(); + const user = await createTestUser(trx); + const { solo } = await seed(user.id_user); + await trx('library_items').where({ id_library_item: solo.id_library_item }).update({ type: null }); + heads[objectKey('Solo.m4b')] = ARCHIVED_COLD; + + const [item] = await get(user, 'Solo.m4b'); + await drain(); + + expect(item.storageState).toBe('restoring'); + expect(restoreObject).toHaveBeenCalledTimes(1); + }); + + it('a plain folder resolved by uuid is never checked', async () => { + const trx = getTestTransaction(); + const user = await createTestUser(trx); + await seed(user.id_user); + const folder = (await trx('library_items').where({ user_id: user.id_user, key: 'Folder' }).first()) as any; + heads[objectKey('Folder/a.m4b')] = ARCHIVED_COLD; + + await get(user, 'Folder', folder.uuid); + await drain(); + + expect(headObject).not.toHaveBeenCalled(); + }); + + it('the resume item riding along with the root sync is never checked', async () => { + const trx = getTestTransaction(); + const user = await createTestUser(trx); + const { solo } = await seed(user.id_user); + await trx('library_items').where({ id_library_item: solo.id_library_item }).update({ last_play_date: 1700000000 }); + heads[objectKey('Solo.m4b')] = ARCHIVED_COLD; + + const item = await service.getLastItemPlayed( + { ...user, subscriptions: [SubscriptionTierEnum.PRO] } as any, + { appVersion: APP_VERSION, withPresign: true }, + ); + await drain(); + + expect(item?.url).toContain('Solo.m4b'); + expect(item?.storageState).toBeUndefined(); + expect(headObject).not.toHaveBeenCalled(); + }); + + it('a non-PRO caller never triggers a HEAD', async () => { + const user = await createTestUser(getTestTransaction()); + await seed(user.id_user); + heads[objectKey('Solo.m4b')] = ARCHIVED_COLD; + + await get(user, 'Solo.m4b', undefined, SubscriptionTierEnum.LITE); + await drain(); + + expect(headObject).not.toHaveBeenCalled(); + }); +}); diff --git a/src/__tests__/services/S3ServiceRestore.test.ts b/src/__tests__/services/S3ServiceRestore.test.ts new file mode 100644 index 0000000..bb867e1 --- /dev/null +++ b/src/__tests__/services/S3ServiceRestore.test.ts @@ -0,0 +1,68 @@ +import { describe, it, expect, beforeEach, jest } from '@jest/globals'; +import { S3Service } from '../../services/S3Service'; +import { mockLoggerService } from '../setup'; + +function s3Error(status: number, name = 'Error', message = 'boom') { + return Object.assign(new Error(message), { name, $metadata: { httpStatusCode: status } }); +} + +/** The two S3 calls the on-demand thaw relies on, at the wire: what is sent, how answers are classified. */ +describe('S3Service — headObject / restoreObject', () => { + let service: S3Service; + let client: { headObject: jest.Mock; restoreObject: jest.Mock }; + + beforeEach(() => { + service = new S3Service(); + client = { headObject: jest.fn(), restoreObject: jest.fn() }; + (service as any).client = client; + (service as any)._logger = mockLoggerService; + mockLoggerService.log.mockClear(); + }); + + it('headObject maps the Restore header to none / ongoing / ready', async () => { + client.headObject.mockResolvedValueOnce({ StorageClass: 'DEEP_ARCHIVE', ContentLength: 5, $metadata: { httpStatusCode: 200 } }); + expect(await service.headObject('k')).toEqual({ storageClass: 'DEEP_ARCHIVE', restore: 'none', contentLength: 5 }); + + client.headObject.mockResolvedValueOnce({ StorageClass: 'DEEP_ARCHIVE', Restore: 'ongoing-request="true"', $metadata: { httpStatusCode: 200 } }); + expect((await service.headObject('k') as any).restore).toBe('ongoing'); + + client.headObject.mockResolvedValueOnce({ + StorageClass: 'DEEP_ARCHIVE', + Restore: 'ongoing-request="false", expiry-date="Fri, 24 Oct 2026 00:00:00 GMT"', + $metadata: { httpStatusCode: 200 }, + }); + expect((await service.headObject('k') as any).restore).toBe('ready'); + + // STANDARD objects carry no StorageClass header at all. + client.headObject.mockResolvedValueOnce({ ContentLength: 7, $metadata: { httpStatusCode: 200 } }); + expect(await service.headObject('k')).toEqual({ storageClass: undefined, restore: 'none', contentLength: 7 }); + }); + + it('headObject: 404 is "missing", anything else is indeterminate (null) and logged at warn', async () => { + client.headObject.mockRejectedValueOnce(s3Error(404, 'NotFound')); + expect(await service.headObject('k')).toBe('missing'); + + client.headObject.mockRejectedValueOnce(s3Error(403, 'AccessDenied')); + expect(await service.headObject('k')).toBeNull(); + expect(mockLoggerService.log).toHaveBeenCalledWith(expect.objectContaining({ origin: 'S3Service.headObject' }), 'warn'); + }); + + it('restoreObject sends Days and the Glacier tier, and treats an in-flight restore as success', async () => { + client.restoreObject.mockResolvedValueOnce({ $metadata: { httpStatusCode: 202 } }); + expect(await service.restoreObject('pfx/root/a.m4b', { days: 30, tier: 'Standard' })).toBe(true); + expect(client.restoreObject).toHaveBeenCalledWith({ + Bucket: process.env.S3_BUCKET, + Key: 'pfx/root/a.m4b', + RestoreRequest: { Days: 30, GlacierJobParameters: { Tier: 'Standard' } }, + }); + + client.restoreObject.mockRejectedValueOnce(s3Error(409, 'RestoreAlreadyInProgress')); + expect(await service.restoreObject('k', { days: 30, tier: 'Standard' })).toBe(true); + }); + + it('restoreObject: any other failure is null and logged at warn', async () => { + client.restoreObject.mockRejectedValueOnce(s3Error(403, 'InvalidObjectState')); + expect(await service.restoreObject('k', { days: 30, tier: 'Standard' })).toBeNull(); + expect(mockLoggerService.log).toHaveBeenCalledWith(expect.objectContaining({ origin: 'S3Service.restoreObject' }), 'warn'); + }); +}); diff --git a/src/database/migrations/20260925120000_glacier_restore_requests.ts b/src/database/migrations/20260925120000_glacier_restore_requests.ts new file mode 100644 index 0000000..67e7700 --- /dev/null +++ b/src/database/migrations/20260925120000_glacier_restore_requests.ts @@ -0,0 +1,43 @@ +import { Knex } from 'knex'; + +// One row per (user, S3 key) whose restore out of Deep Archive was requested. +// Written by the on-demand hook in LibraryService when a Pro user plays or +// downloads a frozen book; read by the glacier-cleanup Lambda, which makes +// each READY restore permanent (self-copy to Intelligent-Tiering) and marks +// the row finalized. A book that freezes again later re-opens its own row. +// +// `library_item_id` lets a listing join open requests per item without a +// single S3 call; `kind` separates a book's file from its artwork, which live +// under different keys. `state = 'archived'` is reserved for the Lambda to +// record frozen objects it sees without restoring them. +export async function up(knex: Knex): Promise { + await knex.schema.createTable('glacier_restore_requests', (table) => { + table.increments('id_glacier_restore_request'); + table.integer('user_id').unsigned().notNullable(); + table.foreign('user_id').references('id_user').inTable('users'); + table.integer('library_item_id').unsigned().nullable(); + table + .foreign('library_item_id') + .references('id_library_item') + .inTable('library_items') + .onDelete('SET NULL'); + table.string('kind', 16).notNullable().defaultTo('object'); // object | thumbnail + table.string('key', 1024).notNullable(); // full S3 key, prefix included + table.string('tier', 16).notNullable().defaultTo('Standard'); + table.smallint('days').notNullable().defaultTo(30); + table.string('state', 16).notNullable().defaultTo('requested'); // requested | finalized | failed | archived + table.smallint('attempts').notNullable().defaultTo(1); + table.timestamp('requested_at', { useTz: true }).notNullable().defaultTo(knex.fn.now()); + table.timestamp('finalized_at', { useTz: true }).nullable(); + table.text('last_error').nullable(); + table.timestamps(true, true); + + table.unique(['user_id', 'key']); + table.index(['state', 'requested_at']); + table.index(['library_item_id', 'state']); + }); +} + +export async function down(knex: Knex): Promise { + await knex.schema.dropTableIfExists('glacier_restore_requests'); +} diff --git a/src/services/GlacierRestoreService.ts b/src/services/GlacierRestoreService.ts new file mode 100644 index 0000000..1964ad2 --- /dev/null +++ b/src/services/GlacierRestoreService.ts @@ -0,0 +1,292 @@ +import { logger } from './LoggerService'; +import { StorageService } from './StorageService'; +import { LibraryDB } from './db/LibraryDB'; +import { GlacierRestoreDB, RestoreRequestKind } from './db/GlacierRestoreDB'; +import { LibraryItemDB, LibraryItemType, StorageState, User } from '../types/user'; + +/** + * On-demand thaw of a Pro user's files out of Deep Archive. + * + * A lapsed subscription archives a user's whole prefix (GlacierMigrationService); + * nothing un-archives it when they come back. Instead of restoring libraries + * eagerly — most of a returner's books are never played again — this runs at the + * one moment intent is known: when a client asks for the signed URL of ONE item it + * is about to play or download. The file is checked with a HEAD; if it is in + * Deep Archive with no thaw in flight, a Standard-tier restore (~12 h) is + * requested and recorded in `glacier_restore_requests`, which the + * glacier-cleanup Lambda later turns into a permanent Intelligent-Tiering copy. + * + * Rules: + * - Never fails a request. Every S3 or DB problem is logged and swallowed; the + * presigned URL is returned exactly as it would have been. + * - Only the tapped item's own HEAD happens before the response. Its artwork and, + * for a bound book, every sibling file are restored in the background: one + * failed chapter brings the whole book back, without adding latency. + * - The restore window is long (RESTORE_DAYS) on purpose: until the Lambda makes + * the copy permanent, the thawed copy is what keeps the book playable, and the + * window costs nothing once the copy exists. + */ +export const RESTORE_TIER = 'Standard' as const; +export const RESTORE_DAYS = 30; +// Bound books are folders of files; CD rips run to a thousand tracks. A HEAD per +// file is a fraction of a cent, so no product cap — only a sanity ceiling. +const SIBLING_CEILING = 5000; +const CONCURRENCY = 10; + +const ARCHIVED_CLASSES = new Set(['DEEP_ARCHIVE', 'GLACIER']); + +export class GlacierRestoreService { + private readonly _logger = logger; + /** Background work in flight — awaited by tests, never by request handlers. */ + private _background = new Set>(); + /** + * Bound-book walks in flight, keyed `${user_id}:${parentKey}`. Two chapters of + * the same frozen book tapped within the same second would both see + * `restore: 'none'` (S3 has not reflected the first request yet) and both + * start a walk; the second is pure duplicate HEADs, so it is skipped. + */ + private _walking = new Set(); + + constructor( + private _storage: StorageService = new StorageService(), + private _libraryDB: LibraryDB = new LibraryDB(), + private _restoreDB: GlacierRestoreDB = new GlacierRestoreDB(), + ) {} + + /** + * Make the one item a client is about to open retrievable. Returns the state + * to report on that item, or undefined when the object could not be checked + * (missing, or the probe failed) — in which case nothing is reported and the + * URL goes out unchanged. + * + * Background work — the item's artwork and, for a bound book, its sibling + * files — starts only when THIS call found the object frozen and requested + * its thaw. A warm book costs exactly one HEAD, and the taps that follow + * during a thaw (the object is already `ongoing`) do not re-walk the book. + */ + async ensureRetrievable( + user: User, + item: LibraryItemDB, + storagePrefix: string, + ): Promise { + try { + const type = parseInt(`${item.type}`); + if (type === parseInt(LibraryItemType.BOUND)) { + // The container has no object of its own. Probe one of its files; if + // that one is frozen, the whole book is (the lifecycle rule archived the + // prefix), so restore the rest. + this.inBackground(this.expandBoundBook(user, item, storagePrefix, null)); + return undefined; + } + // Anything that is not a container is a file — including legacy rows whose + // `type` is NULL, which get a URL like any book and would 403 forever if + // frozen. Same positive classification LibraryService.getLibrary uses. + if (type === parseInt(LibraryItemType.FOLDER)) return undefined; + + const outcome = await this.ensureObject( + user, + item, + `${storagePrefix}/${item.source_path || item.key}`, + 'object', + ); + if (outcome.issued) { + if (item.thumbnail) { + this.inBackground( + this.ensureObject(user, item, `${storagePrefix}_thumbnail/${item.thumbnail}`, 'thumbnail'), + ); + } + this.inBackground(this.expandBoundBook(user, item, storagePrefix, item)); + } + return outcome.state; + } catch (err) { + // The hook must never fail a request: the URL goes out as it always has. + this._logger.log( + { + origin: 'GlacierRestoreService.ensureRetrievable', + message: err?.message ?? String(err), + data: { user_id: user.id_user, id_library_item: item.id_library_item }, + }, + 'warn', + ); + return undefined; + } + } + + /** Awaits every background restore started so far. For tests. */ + async drain(): Promise { + while (this._background.size) { + await Promise.all([...this._background]); + } + } + + private inBackground(work: Promise): void { + const tracked: Promise = work + .then(() => undefined) + .catch((err) => { + this._logger.log( + { origin: 'GlacierRestoreService.background', message: err?.message ?? String(err) }, + 'warn', + ); + }) + .finally(() => { + this._background.delete(tracked); + }); + this._background.add(tracked); + } + + /** + * HEAD one object; request its thaw if it is archived and nothing is thawing + * it. Every archived object that is thawing or already thawed is recorded, so + * the Lambda finalizes restores started by hand too — a READY copy that is not + * recorded would silently refreeze when its window closes. + * + * `issued` is true only when this call sent the RestoreObject — the signal + * that this is the first tap on a frozen object. + */ + private async ensureObject( + user: User, + item: LibraryItemDB, + key: string, + kind: RestoreRequestKind, + ): Promise<{ state: StorageState | undefined; issued: boolean }> { + const head = await this._storage.headObject({ key }); + if (head === null || head === 'missing') return { state: undefined, issued: false }; + if (!ARCHIVED_CLASSES.has(head.storageClass ?? '')) return { state: 'available', issued: false }; + + let issued = false; + if (head.restore === 'none') { + const requested = await this._storage.restoreObject({ + key, + days: RESTORE_DAYS, + tier: RESTORE_TIER, + }); + if (requested === null) return { state: undefined, issued: false }; + issued = true; + } + await this._restoreDB.upsertRequested({ + user_id: user.id_user, + library_item_id: item.id_library_item ?? null, + kind, + key, + tier: RESTORE_TIER, + days: RESTORE_DAYS, + }); + return { state: head.restore === 'ready' ? 'available' : 'restoring', issued }; + } + + /** + * Restore every other file of the bound book `item` belongs to (or is, when + * `tapped` is null and `item` is the container). A bound book streams chapter + * by chapter, so restoring only the tapped file would cost the user a 12 h + * wait per chapter. For a container, a file is probed first and the walk + * stops there unless it is frozen: a warm bound book costs one HEAD. + */ + private async expandBoundBook( + user: User, + item: LibraryItemDB, + storagePrefix: string, + tapped: LibraryItemDB | null, + ): Promise { + const type = parseInt(`${item.type}`); + let parent: LibraryItemDB | null = null; + if (type === parseInt(LibraryItemType.BOUND)) { + parent = item; + } else if (type !== parseInt(LibraryItemType.FOLDER)) { + const cut = item.key.lastIndexOf('/'); + if (cut > 0) { + const candidate = ( + await this._libraryDB.getLibrary(user.id_user, item.key.slice(0, cut), { exactly: true }) + )?.[0]; + if (candidate && parseInt(`${candidate.type}`) === parseInt(LibraryItemType.BOUND)) { + parent = candidate; + } + } + } + if (!parent) return; + const walkKey = `${user.id_user}:${parent.key}`; + if (this._walking.has(walkKey)) return; + this._walking.add(walkKey); + try { + await this.walkBoundBook(user, parent, storagePrefix, tapped); + } finally { + this._walking.delete(walkKey); + } + } + + /** How many files a container tap will HEAD looking for a definitive answer before giving up. */ + private static readonly PROBE_ATTEMPTS = 3; + + private async walkBoundBook( + user: User, + parent: LibraryItemDB, + storagePrefix: string, + tapped: LibraryItemDB | null, + ): Promise { + const children = (await this._libraryDB.getLibrary(user.id_user, `${parent.key}/`)) ?? []; + const isContainer = (row: LibraryItemDB) => { + const t = parseInt(`${row.type}`); + return t === parseInt(LibraryItemType.FOLDER) || t === parseInt(LibraryItemType.BOUND); + }; + const files = children.filter( + (child) => !isContainer(child) && child.id_library_item !== tapped?.id_library_item, + ); + const keyOf = (file: LibraryItemDB) => `${storagePrefix}/${file.source_path || file.key}`; + + let queue = files; + if (!tapped) { + // Container tapped: probe before walking. Files whose object cannot be + // checked (unsynced, 404, failed HEAD) say nothing about the book, so try + // the next one — synced files first — and stop on any definitive answer. + const candidates = [...files].sort((a, b) => Number(b.synced) - Number(a.synced)); + let issued = false; + const probed = new Set(); + for (const probe of candidates.slice(0, GlacierRestoreService.PROBE_ATTEMPTS)) { + probed.add(probe.id_library_item); + const outcome = await this.ensureObject(user, probe, keyOf(probe), 'object'); + if (outcome.issued) { + issued = true; + break; + } + if (outcome.state !== undefined) return; // warm, thawing or ready: not a fresh freeze + } + if (!issued) return; + queue = files.filter((file) => !probed.has(file.id_library_item)); + } + + // The book's own cover lives on the container row, not on any chapter — + // before the empty check, so a one-chapter book gets its cover too. + if (parent.thumbnail) { + await this.ensureObject(user, parent, `${storagePrefix}_thumbnail/${parent.thumbnail}`, 'thumbnail'); + } + if (!queue.length) return; + + if (queue.length > SIBLING_CEILING) { + this._logger.log( + { + origin: 'GlacierRestoreService.walkBoundBook', + message: `Bound book has ${queue.length} files; restoring the first ${SIBLING_CEILING}`, + data: { user_id: user.id_user, id_library_item: parent.id_library_item }, + }, + 'warn', + ); + queue = queue.slice(0, SIBLING_CEILING); + } + const workers = Array.from({ length: Math.min(CONCURRENCY, queue.length) }, async () => { + for (let next = queue.shift(); next; next = queue.shift()) { + try { + await this.ensureObject(user, next, keyOf(next), 'object'); + } catch (err) { + this._logger.log( + { + origin: 'GlacierRestoreService.walkBoundBook', + message: err?.message ?? String(err), + data: { user_id: user.id_user, id_library_item: next.id_library_item }, + }, + 'warn', + ); + } + } + }); + await Promise.all(workers); + } +} diff --git a/src/services/LibraryService.ts b/src/services/LibraryService.ts index 2af2b6b..1a0d004 100644 --- a/src/services/LibraryService.ts +++ b/src/services/LibraryService.ts @@ -26,6 +26,7 @@ import { } from '../utils'; import { LibraryDB, externalResourceRowToApi } from './db/LibraryDB'; import { StoragePrefixService } from './StoragePrefixService'; +import { GlacierRestoreService } from './GlacierRestoreService'; import { ApiError, ApiErrorCode } from '../types/apiError'; /** @@ -46,6 +47,7 @@ export const ITEM_DELETED = Symbol('ITEM_DELETED'); export class LibraryService { private readonly _logger = logger; private db = database; + private _glacier = new GlacierRestoreService(); constructor( private _storage: StorageService = new StorageService(), @@ -216,6 +218,28 @@ export class LibraryService { const storagePrefix = options.withPresign ? await this._prefix.getPrefix(user) : null; + // On-demand thaw (GlacierRestoreService): only when this request names + // ONE item the client is about to play or download — a path without a + // trailing slash that is not the root (`''` is the root listing, which has + // no slash either) resolving to a single row that is not a plain folder + // (a book, a bound book, or a legacy row with a NULL type — those get a + // URL too) — on the presigned branch every shipped app uses, for a PRO + // user (the only tier with S3 files). Listings never HEAD, even a root + // with one item. + const single = objectDB.length === 1 ? objectDB[0] : null; + const tapped = + single && + cleanPath !== '' && + !wantsContents && + parseInt(`${single.type}`) !== parseInt(LibraryItemType.FOLDER) && + storagePrefix && + user.subscriptions?.includes(SubscriptionTierEnum.PRO) && + !['2023-10-29', 'latest'].includes(options.appVersion) + ? single + : null; + const storageState = tapped + ? await this._glacier.ensureRetrievable(user, tapped, storagePrefix) + : undefined; for (let index = 0; index < objectDB.length; index++) { const itemDb = objectDB[index]; let fileUrl: string = null; @@ -273,7 +297,8 @@ export class LibraryService { url: fileUrl, thumbnail, synced: itemDb.synced, - externalResources: (externalsMp[itemDb.id_library_item] ?? []).map(externalResourceRowToApi) + externalResources: (externalsMp[itemDb.id_library_item] ?? []).map(externalResourceRowToApi), + storageState: itemDb === tapped ? storageState : undefined, }; library.push(libObj); } @@ -1115,6 +1140,10 @@ export class LibraryService { }); item.url = url; item.expires_in = expires_in; + // No thaw hook here on purpose: this rides along with every root + // sync (the apps' hottest request). A frozen resume item fails its + // first play with a 403, and the player's URL refresh — a + // single-item request — is where GlacierRestoreService runs. if (itemDb.thumbnail) { const { url } = await this._storage.getPresignedUrl({ diff --git a/src/services/S3Service.ts b/src/services/S3Service.ts index cf5a580..e3c03c1 100644 --- a/src/services/S3Service.ts +++ b/src/services/S3Service.ts @@ -11,6 +11,7 @@ import { PutBucketLifecycleConfigurationCommand, LifecycleRule, StorageClass, + Tier, TransitionStorageClass, CreateMultipartUploadCommand, UploadPartCommand, @@ -79,6 +80,15 @@ const isNoSuchUpload = (error: { name?: string }) => error?.name === 'NoSuchUplo // order, or a non-final part under the 5 MiB minimum. const INVALID_PART_ERRORS = new Set(['InvalidPart', 'InvalidPartOrder', 'EntityTooSmall']); +/** What a HEAD says about where an object's bytes are right now. */ +export type ObjectHead = { + /** undefined means STANDARD — S3 omits the header for it. */ + storageClass: string | undefined; + /** Deep Archive / Glacier objects: is a temporary thawed copy being made, ready, or absent. */ + restore: 'none' | 'ongoing' | 'ready'; + contentLength: number | undefined; +}; + export class S3Service { private readonly _logger = logger; private client = new S3({ region: process.env.S3_REGION }); @@ -147,6 +157,77 @@ export class S3Service { } } + /** + * Storage class and restore status of one object. Same tri-state discipline + * as fileExists: 'missing' for a 404, null when the probe failed (403, 5xx, + * SDK), and callers that act on either must check for them explicitly. + */ + async headObject(key: string): Promise { + try { + const data = await this.client.headObject({ + Bucket: process.env.S3_BUCKET, + Key: key, + }); + // e.g. `ongoing-request="true"` while thawing, or + // `ongoing-request="false", expiry-date="..."` once the copy is readable. + const header = data.Restore ?? ''; + const restore = header.includes('ongoing-request="true"') + ? 'ongoing' + : header.includes('ongoing-request="false"') + ? 'ready' + : 'none'; + return { storageClass: data.StorageClass, restore, contentLength: data.ContentLength }; + } catch (error) { + if (error.$metadata?.httpStatusCode === 404) { + return 'missing'; + } + this._logger.log( + { + origin: 'S3Service.headObject', + message: error.message, + data: { key: stripStoragePrefix(key), errorName: error.name }, + }, + 'warn', + ); + return null; + } + } + + /** + * Asks S3 to thaw a temporary copy of an archived object for `days` days. + * A restore already in flight counts as success. Deep Archive rejects the + * Expedited tier, so only Standard (~12 h) and Bulk (~48 h) are offered. + */ + async restoreObject( + key: string, + params: { days: number; tier: 'Standard' | 'Bulk' }, + ): Promise { + try { + await this.client.restoreObject({ + Bucket: process.env.S3_BUCKET, + Key: key, + RestoreRequest: { + Days: params.days, + GlacierJobParameters: { Tier: params.tier as Tier }, + }, + }); + return true; + } catch (error) { + if (error.name === 'RestoreAlreadyInProgress' || error.$metadata?.httpStatusCode === 409) { + return true; + } + this._logger.log( + { + origin: 'S3Service.restoreObject', + message: error.message, + data: { key: stripStoragePrefix(key), errorName: error.name, tier: params.tier }, + }, + 'warn', + ); + return null; + } + } + async getDirectoryContent( path: string, isFolder = true, diff --git a/src/services/StorageService.ts b/src/services/StorageService.ts index adb5870..a3258ad 100644 --- a/src/services/StorageService.ts +++ b/src/services/StorageService.ts @@ -5,7 +5,7 @@ import { StorageOrigin, } from '../types/user'; import { logger } from './LoggerService'; -import { S3Service } from './S3Service'; +import { S3Service, ObjectHead } from './S3Service'; import { Readable } from 'stream'; import { stripStoragePrefix } from '../utils'; import { @@ -53,6 +53,50 @@ export class StorageService { } } + async headObject(params: { + key: string; + origin?: StorageOrigin; + }): Promise { + try { + const { key, origin } = params; + switch (origin || StorageOrigin.S3) { + case StorageOrigin.S3: + return await this._s3Service.headObject(key); + default: + return null; + } + } catch (error) { + this._logger.log( + { origin: 'StorageService.headObject', message: error.message, data: { key: stripStoragePrefix(params.key) } }, + 'warn', + ); + return null; + } + } + + async restoreObject(params: { + key: string; + days: number; + tier: 'Standard' | 'Bulk'; + origin?: StorageOrigin; + }): Promise { + try { + const { key, days, tier, origin } = params; + switch (origin || StorageOrigin.S3) { + case StorageOrigin.S3: + return await this._s3Service.restoreObject(key, { days, tier }); + default: + return null; + } + } catch (error) { + this._logger.log( + { origin: 'StorageService.restoreObject', message: error.message, data: { key: stripStoragePrefix(params.key) } }, + 'warn', + ); + return null; + } + } + async getDirectoryContent(params: { path: string; isFolder: boolean; diff --git a/src/services/db/GlacierRestoreDB.ts b/src/services/db/GlacierRestoreDB.ts new file mode 100644 index 0000000..b239619 --- /dev/null +++ b/src/services/db/GlacierRestoreDB.ts @@ -0,0 +1,92 @@ +import { Knex } from 'knex'; +import database from '../../database'; +import { logger } from '../LoggerService'; + +export type RestoreRequestKind = 'object' | 'thumbnail'; +export type RestoreRequestState = 'requested' | 'finalized' | 'failed' | 'archived'; + +export interface GlacierRestoreRequestDB { + id_glacier_restore_request: number; + user_id: number; + library_item_id: number | null; + kind: RestoreRequestKind; + key: string; + tier: string; + days: number; + state: RestoreRequestState; + attempts: number; + requested_at: Date; + finalized_at: Date | null; + last_error: string | null; +} + +/** Owns `glacier_restore_requests`: what the on-demand hook asked S3 to thaw, for the Lambda to finalize. */ +export class GlacierRestoreDB { + private readonly _logger = logger; + private db = database; + + /** + * Records that a restore was requested for `key`. One row per (user, key): + * a finalized or failed row is re-opened instead of duplicated, and a row + * already `requested` is left alone, so repeated taps while a book thaws are + * idempotent. `null` means the write failed (logged), not "already there". + */ + async upsertRequested( + params: { + user_id: number; + library_item_id: number | null; + kind: RestoreRequestKind; + key: string; + tier: string; + days: number; + }, + trx?: Knex.Transaction, + ): Promise { + try { + const db = trx || this.db; + await db.raw( + ` + insert into glacier_restore_requests + (user_id, library_item_id, kind, key, tier, days, state, attempts, requested_at, created_at, updated_at) + values (?, ?, ?, ?, ?, ?, 'requested', 1, now(), now(), now()) + on conflict (user_id, key) do update + set state = 'requested', + requested_at = now(), + attempts = glacier_restore_requests.attempts + 1, + library_item_id = coalesce(excluded.library_item_id, glacier_restore_requests.library_item_id), + tier = excluded.tier, + days = excluded.days, + finalized_at = null, + last_error = null, + updated_at = now() + where glacier_restore_requests.state <> 'requested' + `, + [params.user_id, params.library_item_id, params.kind, params.key, params.tier, params.days], + ); + return true; + } catch (err) { + this._logger.log( + { + origin: 'GlacierRestoreDB.upsertRequested', + message: err.message, + data: { user_id: params.user_id, kind: params.kind }, + }, + 'warn', + ); + return null; + } + } + + async getByUser(user_id: number, trx?: Knex.Transaction): Promise { + try { + const db = trx || this.db; + return await db('glacier_restore_requests').where({ user_id }).orderBy('key'); + } catch (err) { + this._logger.log( + { origin: 'GlacierRestoreDB.getByUser', message: err.message, data: { user_id } }, + 'warn', + ); + return null; + } + } +} diff --git a/src/types/user.ts b/src/types/user.ts index 9b70dbb..4058b52 100644 --- a/src/types/user.ts +++ b/src/types/user.ts @@ -174,8 +174,16 @@ export interface LibraryItem { source_path?: string; uuid?: string; externalResources?: ExternalResource[] | null | undefined; + /** + * Only on the one item a client is about to play or download: whether its + * bytes are readable now, or being thawed out of Deep Archive (~12 h). Absent + * on listings and when the object could not be checked. + */ + storageState?: StorageState; } +export type StorageState = 'available' | 'restoring'; + // Providers that are servers a book's file comes from. The other kind — a // metadata service like Hardcover — links a book but never has a file, and its // sync_status is the client's own marker, never a file state for us to set. From c4157870d453257af766fbd977777f23a064e7de Mon Sep 17 00:00:00 2001 From: Gianni Carlo Date: Fri, 25 Sep 2026 09:21:02 -0500 Subject: [PATCH 2/4] fix: address review feedback (round 1) Bound the Glacier check on the URL path: the request waits at most HOOK_BUDGET_MS (2 s) for the HEAD/RestoreObject/insert and then answers without a state, while the check finishes in the background so the restore is still issued and recorded. Before this hook the path signed locally, so a degraded S3 must not turn a URL refresh into a hang. Bump requested_at/attempts whenever a RestoreObject was actually sent, even on a row still `requested`: a copy whose window closed before the finalizer ran now reads as a fresh request. Repeated taps on an already-thawing object still leave the row alone. Log at error when a thaw could not be recorded (the copy would silently refreeze), and inject GlacierRestoreService through the constructor, sharing LibraryService's StorageService and LibraryDB, per the DI convention. --- .../services/GlacierRestoreHook.test.ts | 56 +++++++++++++++++ src/services/GlacierRestoreService.ts | 63 +++++++++++++++++-- src/services/LibraryService.ts | 2 +- src/services/db/GlacierRestoreDB.ts | 16 +++-- 4 files changed, 127 insertions(+), 10 deletions(-) diff --git a/src/__tests__/services/GlacierRestoreHook.test.ts b/src/__tests__/services/GlacierRestoreHook.test.ts index 90194df..b3366ae 100644 --- a/src/__tests__/services/GlacierRestoreHook.test.ts +++ b/src/__tests__/services/GlacierRestoreHook.test.ts @@ -354,6 +354,62 @@ describe('LibraryService — on-demand Glacier restore hook', () => { expect(row).toMatchObject({ state: 'requested', attempts: 2, finalized_at: null }); }); + it('a restore issued for a row still open (the thawed copy expired unfinalized) reads as a fresh request', async () => { + const user = await createTestUser(getTestTransaction()); + await seed(user.id_user); + heads[objectKey('Solo.m4b')] = ARCHIVED_COLD; + + await get(user, 'Solo.m4b'); + await drain(); + const [first] = await rows(user.id_user); + expect(first.attempts).toBe(1); + + heads[objectKey('Solo.m4b')] = ARCHIVED_COLD; // window closed: S3 reads cold again + await get(user, 'Solo.m4b'); + await drain(); + const [second] = await rows(user.id_user); + expect(restoreObject).toHaveBeenCalledTimes(2); + expect(second.attempts).toBe(2); + expect(new Date(second.requested_at).getTime()).toBeGreaterThanOrEqual(new Date(first.requested_at).getTime()); + }); + + it('a restore that could not be recorded is still reported as restoring, and logged at error', async () => { + const user = await createTestUser(getTestTransaction()); + await seed(user.id_user); + heads[objectKey('Solo.m4b')] = ARCHIVED_COLD; + (service as any)._glacier._restoreDB.upsertRequested = jest.fn(async (): Promise => null); + + const [item] = await get(user, 'Solo.m4b'); + await drain(); + + expect(item.storageState).toBe('restoring'); + expect(restoreObject).toHaveBeenCalledTimes(1); + const errors = (mockLoggerService.log.mock.calls as any[][]).filter((c) => c[1] === 'error'); + expect(errors).toHaveLength(1); + expect(errors[0][0].origin).toBe('GlacierRestoreService.ensureObject'); + expect(JSON.stringify(errors[0][0])).not.toContain(PREFIX); + }); + + it('a check that outruns its budget answers without a state, and still finishes in the background', async () => { + const user = await createTestUser(getTestTransaction()); + await seed(user.id_user); + heads[objectKey('Solo.m4b')] = ARCHIVED_COLD; + (service as any)._glacier._budgetMs = 20; + headObject.mockImplementation( + ({ key }: { key: string }): Promise => + new Promise((resolve) => setTimeout(() => resolve(key in heads ? heads[key] : WARM), 80)), + ); + + const [item] = await get(user, 'Solo.m4b'); + expect(item.url).toContain('Solo.m4b'); + expect(item.storageState).toBeUndefined(); + expect(restoreObject).not.toHaveBeenCalled(); + + await drain(); + expect(restoreObject).toHaveBeenCalledTimes(1); + expect(await rows(user.id_user)).toHaveLength(1); + }); + it('tapping one chapter of a bound book restores its siblings and artwork in the background', async () => { const user = await createTestUser(getTestTransaction()); const { ch1, ch2, ch3 } = await seed(user.id_user); diff --git a/src/services/GlacierRestoreService.ts b/src/services/GlacierRestoreService.ts index 1964ad2..4f38b1d 100644 --- a/src/services/GlacierRestoreService.ts +++ b/src/services/GlacierRestoreService.ts @@ -28,6 +28,14 @@ import { LibraryItemDB, LibraryItemType, StorageState, User } from '../types/use */ export const RESTORE_TIER = 'Standard' as const; export const RESTORE_DAYS = 30; +/** + * How long a request waits for the tapped item's check (HEAD, RestoreObject, + * one insert — tens of milliseconds in-region) before answering without a + * state. The check keeps running in the background, so a slow S3 still gets + * the restore issued and recorded; only the response stops waiting for it. + */ +export const HOOK_BUDGET_MS = 2000; +const TIMED_OUT = Symbol('TIMED_OUT'); // Bound books are folders of files; CD rips run to a thousand tracks. A HEAD per // file is a fraction of a cent, so no product cap — only a sanity ceiling. const SIBLING_CEILING = 5000; @@ -46,6 +54,7 @@ export class GlacierRestoreService { * start a walk; the second is pure duplicate HEADs, so it is skipped. */ private _walking = new Set(); + private _budgetMs = HOOK_BUDGET_MS; constructor( private _storage: StorageService = new StorageService(), @@ -56,8 +65,8 @@ export class GlacierRestoreService { /** * Make the one item a client is about to open retrievable. Returns the state * to report on that item, or undefined when the object could not be checked - * (missing, or the probe failed) — in which case nothing is reported and the - * URL goes out unchanged. + * (missing, the probe failed, or the check outran HOOK_BUDGET_MS) — in which + * case nothing is reported and the URL goes out unchanged. * * Background work — the item's artwork and, for a bound book, its sibling * files — starts only when THIS call found the object frozen and requested @@ -68,6 +77,36 @@ export class GlacierRestoreService { user: User, item: LibraryItemDB, storagePrefix: string, + ): Promise { + const work = this.checkTapped(user, item, storagePrefix); + let timer: ReturnType | undefined; + const budget = new Promise((resolve) => { + timer = setTimeout(() => resolve(TIMED_OUT), this._budgetMs); + }); + try { + const result = await Promise.race([work, budget]); + if (result !== TIMED_OUT) return result; + // The URL path was local-only before this hook; a degraded S3 or DB must + // not turn a URL refresh into a hang. Let the check finish on its own. + this._logger.log( + { + origin: 'GlacierRestoreService.ensureRetrievable', + message: `Glacier check exceeded ${this._budgetMs} ms; answering without a state`, + data: { user_id: user.id_user, id_library_item: item.id_library_item }, + }, + 'warn', + ); + this.inBackground(work); + return undefined; + } finally { + clearTimeout(timer); + } + } + + private async checkTapped( + user: User, + item: LibraryItemDB, + storagePrefix: string, ): Promise { try { const type = parseInt(`${item.type}`); @@ -102,7 +141,7 @@ export class GlacierRestoreService { // The hook must never fail a request: the URL goes out as it always has. this._logger.log( { - origin: 'GlacierRestoreService.ensureRetrievable', + origin: 'GlacierRestoreService.checkTapped', message: err?.message ?? String(err), data: { user_id: user.id_user, id_library_item: item.id_library_item }, }, @@ -163,14 +202,30 @@ export class GlacierRestoreService { if (requested === null) return { state: undefined, issued: false }; issued = true; } - await this._restoreDB.upsertRequested({ + const recorded = await this._restoreDB.upsertRequested({ user_id: user.id_user, library_item_id: item.id_library_item ?? null, kind, key, tier: RESTORE_TIER, days: RESTORE_DAYS, + issued, }); + if (recorded === null) { + // The thaw is real but the Lambda will never hear of it: the copy would + // silently refreeze when its window closes. Loud, so the row can be + // recreated by hand (or the object re-tapped) before then. + this._logger.log( + { + origin: 'GlacierRestoreService.ensureObject', + message: issued + ? 'Restore requested but not recorded; it will not be finalized' + : 'Thawing object seen but not recorded; it will not be finalized', + data: { user_id: user.id_user, id_library_item: item.id_library_item, kind }, + }, + 'error', + ); + } return { state: head.restore === 'ready' ? 'available' : 'restoring', issued }; } diff --git a/src/services/LibraryService.ts b/src/services/LibraryService.ts index 1a0d004..2d2043c 100644 --- a/src/services/LibraryService.ts +++ b/src/services/LibraryService.ts @@ -47,12 +47,12 @@ export const ITEM_DELETED = Symbol('ITEM_DELETED'); export class LibraryService { private readonly _logger = logger; private db = database; - private _glacier = new GlacierRestoreService(); constructor( private _storage: StorageService = new StorageService(), private _libraryDB: LibraryDB = new LibraryDB(), private _prefix: StoragePrefixService = new StoragePrefixService(), + private _glacier: GlacierRestoreService = new GlacierRestoreService(_storage, _libraryDB), ) {} async parseLibraryItemDb( diff --git a/src/services/db/GlacierRestoreDB.ts b/src/services/db/GlacierRestoreDB.ts index b239619..5d7c74c 100644 --- a/src/services/db/GlacierRestoreDB.ts +++ b/src/services/db/GlacierRestoreDB.ts @@ -26,10 +26,14 @@ export class GlacierRestoreDB { private db = database; /** - * Records that a restore was requested for `key`. One row per (user, key): - * a finalized or failed row is re-opened instead of duplicated, and a row - * already `requested` is left alone, so repeated taps while a book thaws are - * idempotent. `null` means the write failed (logged), not "already there". + * Records that `key` is thawing. One row per (user, key): a finalized or + * failed row is re-opened instead of duplicated. When `issued` is true this + * call sent the RestoreObject, so `requested_at` and `attempts` move even on + * a row still `requested` — a copy whose window closed before it was + * finalized reads as a fresh request, not the stale one. When false (the + * object was already thawing) a `requested` row is left alone, so repeated + * taps while a book thaws are idempotent. `null` means the write failed + * (logged), not "already there". */ async upsertRequested( params: { @@ -39,11 +43,13 @@ export class GlacierRestoreDB { key: string; tier: string; days: number; + issued: boolean; }, trx?: Knex.Transaction, ): Promise { try { const db = trx || this.db; + const guard = params.issued ? '' : `where glacier_restore_requests.state <> 'requested'`; await db.raw( ` insert into glacier_restore_requests @@ -59,7 +65,7 @@ export class GlacierRestoreDB { finalized_at = null, last_error = null, updated_at = now() - where glacier_restore_requests.state <> 'requested' + ${guard} `, [params.user_id, params.library_item_id, params.kind, params.key, params.tier, params.days], ); From bab7c6b3252afe80ea2b1fc1a5230ae310b20503 Mon Sep 17 00:00:00 2001 From: Gianni Carlo Date: Fri, 25 Sep 2026 10:09:07 -0500 Subject: [PATCH 3/4] Make headObject the one HEAD request in S3Service MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fileExists and the delete path's size check each sent their own HEAD and threw away most of the answer; the on-demand thaw added a third for the storage class and restore header. Keep that one as the base request — it carries the tri-state rule (404 is 'missing', a 403 or any other failure is null, never "absent") and the 403 note that used to live on fileExists — and turn the other two into mappings over it. One wire call, one place where a HEAD answer is classified. --- .../services/S3ServiceFileExists.test.ts | 11 +++ src/services/S3Service.ts | 89 ++++++------------- 2 files changed, 39 insertions(+), 61 deletions(-) diff --git a/src/__tests__/services/S3ServiceFileExists.test.ts b/src/__tests__/services/S3ServiceFileExists.test.ts index 97364e7..cfb3a4c 100644 --- a/src/__tests__/services/S3ServiceFileExists.test.ts +++ b/src/__tests__/services/S3ServiceFileExists.test.ts @@ -55,6 +55,17 @@ describe('S3Service.fileExists — tri-state', () => { await expect(service.fileExists('prefix/root/a.m4b')).resolves.toBeNull(); }); + it('is a mapping over headObject, the one HEAD request', async () => { + const head = jest.spyOn(service, 'headObject'); + head.mockResolvedValueOnce('missing'); + await expect(service.fileExists('k')).resolves.toBe(false); + head.mockResolvedValueOnce(null); + await expect(service.fileExists('k')).resolves.toBeNull(); + head.mockResolvedValueOnce({ storageClass: 'DEEP_ARCHIVE', restore: 'none', contentLength: 1 }); + await expect(service.fileExists('k')).resolves.toBe(true); + expect(headObjectMock).not.toHaveBeenCalled(); + }); + it('does not log the storage prefix, which can be the account email', async () => { headObjectMock.mockImplementation(async () => { throw Object.assign(new Error('Forbidden'), { diff --git a/src/services/S3Service.ts b/src/services/S3Service.ts index e3c03c1..1a0b409 100644 --- a/src/services/S3Service.ts +++ b/src/services/S3Service.ts @@ -105,62 +105,32 @@ export class S3Service { }); /** - * Tri-state on purpose: true/false are definitive, null means the probe - * could not determine it and the caller must not read that as "absent". - * - * A 403 is indeterminate, not absent. S3 masks a missing key as 403 only - * when the caller lacks s3:ListBucket, and this role holds it (see - * getDirectoryContent / calculateFolderSize, which call ListObjectsV2), so - * a 403 here means a permission or KMS problem rather than a missing key. - * - * Callers that act on "missing" must check `=== false`, never `!exists`. + * Whether `key` is there: true, false on a 404, null when the probe could not + * tell (403, 5xx, SDK) — see headObject for why a 403 is not "absent". + * LibraryService.processMovedFiles depends on the distinction: only a + * definitive `false` from both the source and target probe lets it conclude + * a moved item's bytes are nowhere. Callers that act on "missing" must check + * `=== false`, never `!exists`. */ async fileExists(key: string): Promise { - try { - const data = await this.client.headObject({ - Bucket: process.env.S3_BUCKET, - Key: key, - }); - - return data.$metadata.httpStatusCode === 200; - } catch (error) { - if (error.$metadata?.httpStatusCode === 404) { - return false; - } else if (error.$metadata?.httpStatusCode === 403) { - // Indeterminate, not absent — see the tri-state note above. Returning - // false here would let a permission failure read as "the object is - // nowhere", which is how a caller ends up recording that nothing - // exists when in fact it could not look. - this._logger.log( - { - origin: 'S3Service.fileExists', - message: 'Existence probe denied (403); treating as indeterminate', - data: { key: stripStoragePrefix(key) }, - }, - 'warn', - ); - return null; - } else { - // Same level as the 403 branch: this is the wider indeterminate class - // (5xx, timeouts, SDK failures) and it drives the same caller - // decision, so it has to clear the production LOG_LEVEL of 'warn' too. - this._logger.log( - { - origin: 'S3Service.fileExists', - message: error.message, - data: { key: stripStoragePrefix(key), errorName: error.name }, - }, - 'warn', - ); - return null; - } - } + const head = await this.headObject(key); + if (head === 'missing') return false; + return head === null ? null : true; } /** - * Storage class and restore status of one object. Same tri-state discipline - * as fileExists: 'missing' for a 404, null when the probe failed (403, 5xx, - * SDK), and callers that act on either must check for them explicitly. + * The one HEAD request. Storage class, restore status and size of an object, + * or 'missing' on a 404, or null when the probe could not determine anything + * (403, 5xx, SDK failure). Tri-state on purpose: true/false-style answers are + * definitive, null is not, and callers that act on either must check for it + * explicitly, never `!head`. + * + * A 403 is indeterminate, not absent. S3 masks a missing key as 403 only + * when the caller lacks s3:ListBucket, and this role holds it (see + * getDirectoryContent / calculateFolderSize, which call ListObjectsV2), so + * a 403 here means a permission or KMS problem rather than a missing key. + * Both failure branches log at warn: they drive the same caller decision + * and have to clear the production LOG_LEVEL. */ async headObject(key: string): Promise { try { @@ -184,7 +154,10 @@ export class S3Service { this._logger.log( { origin: 'S3Service.headObject', - message: error.message, + message: + error.$metadata?.httpStatusCode === 403 + ? 'Probe denied (403); treating as indeterminate' + : error.message, data: { key: stripStoragePrefix(key), errorName: error.name }, }, 'warn', @@ -593,15 +566,9 @@ export class S3Service { } /** Only a definite answer counts: a failed HEAD keeps the delete's old behaviour. */ private async exceedsSingleCopyLimit(key: string): Promise { - try { - const head = await this.client.headObject({ - Bucket: process.env.S3_BUCKET, - Key: key, - }); - return (head.ContentLength ?? 0) > MAX_SINGLE_COPY_SIZE; - } catch { - return false; - } + const head = await this.headObject(key); + if (head === null || head === 'missing') return false; + return (head.contentLength ?? 0) > MAX_SINGLE_COPY_SIZE; } async calculateFolderSize(folderKey: string): Promise { From 2ce917066b335cb16d691ff6e246a698ee1b5d4d Mon Sep 17 00:00:00 2001 From: Gianni Carlo Date: Fri, 25 Sep 2026 10:14:09 -0500 Subject: [PATCH 4/4] fix: address review feedback (round 3) Thaw each sibling chapter's own artwork during a bound-book walk, and the probed file's when a container tap issues the restore. Chapters carry their own thumbnails, archived under the sibling `_thumbnail/` prefix, and the app shows them in the book's chapter list; only the tapped chapter's artwork was being restored, so the rest would have stayed frozen until the Lambda's thumbnails job exists. --- .../services/GlacierRestoreHook.test.ts | 27 +++++++++++++++---- src/services/GlacierRestoreService.ts | 9 +++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/src/__tests__/services/GlacierRestoreHook.test.ts b/src/__tests__/services/GlacierRestoreHook.test.ts index b3366ae..32d2106 100644 --- a/src/__tests__/services/GlacierRestoreHook.test.ts +++ b/src/__tests__/services/GlacierRestoreHook.test.ts @@ -410,13 +410,16 @@ describe('LibraryService — on-demand Glacier restore hook', () => { expect(await rows(user.id_user)).toHaveLength(1); }); - it('tapping one chapter of a bound book restores its siblings and artwork in the background', async () => { - const user = await createTestUser(getTestTransaction()); + it("tapping one chapter of a bound book restores its siblings and every chapter's artwork in the background", async () => { + const trx = getTestTransaction(); + const user = await createTestUser(trx); const { ch1, ch2, ch3 } = await seed(user.id_user); + await trx('library_items').where({ id_library_item: ch2.id_library_item }).update({ thumbnail: 'ch2.jpg' }); heads[objectKey('Series/01.mp3')] = ARCHIVED_COLD; heads[objectKey('Series/02.mp3')] = ARCHIVED_COLD; heads[objectKey('Series/03.mp3')] = ARCHIVED_THAWING; // already thawing: recorded, not re-requested heads[`${PREFIX}_thumbnail/cover.jpg`] = ARCHIVED_COLD; + heads[`${PREFIX}_thumbnail/ch2.jpg`] = ARCHIVED_COLD; // a sibling's own artwork const [item] = await get(user, 'Series/01.mp3'); expect(item.storageState).toBe('restoring'); @@ -424,7 +427,12 @@ describe('LibraryService — on-demand Glacier restore hook', () => { const requestedKeys = restoreObject.mock.calls.map((c: any[]) => c[0].key).sort(); expect(requestedKeys).toEqual( - [objectKey('Series/01.mp3'), objectKey('Series/02.mp3'), `${PREFIX}_thumbnail/cover.jpg`].sort(), + [ + objectKey('Series/01.mp3'), + objectKey('Series/02.mp3'), + `${PREFIX}_thumbnail/cover.jpg`, + `${PREFIX}_thumbnail/ch2.jpg`, + ].sort(), ); const saved = await rows(user.id_user); expect(saved.map((r: any) => [r.key, r.kind, r.library_item_id]).sort()).toEqual( @@ -433,25 +441,34 @@ describe('LibraryService — on-demand Glacier restore hook', () => { [objectKey('Series/02.mp3'), 'object', ch2.id_library_item], [objectKey('Series/03.mp3'), 'object', ch3.id_library_item], [`${PREFIX}_thumbnail/cover.jpg`, 'thumbnail', ch1.id_library_item], + [`${PREFIX}_thumbnail/ch2.jpg`, 'thumbnail', ch2.id_library_item], ].sort(), ); // Solo and Folder/a were never looked at. expect(headObject.mock.calls.some((c: any[]) => c[0].key === objectKey('Solo.m4b'))).toBe(false); }); - it('a bound book resolved by uuid (no slash) restores its files', async () => { + it("a bound book resolved by uuid (no slash) restores its files and the probed file's artwork", async () => { const user = await createTestUser(getTestTransaction()); const { series } = await seed(user.id_user); heads[objectKey('Series/01.mp3')] = ARCHIVED_COLD; heads[objectKey('Series/02.mp3')] = ARCHIVED_COLD; heads[objectKey('Series/03.mp3')] = ARCHIVED_COLD; + heads[`${PREFIX}_thumbnail/cover.jpg`] = ARCHIVED_COLD; // 01's artwork; 01 is the probe const [item] = await get(user, 'Series', series.uuid); await drain(); expect(item.relativePath).toBe('Series'); expect(item.storageState).toBeUndefined(); // a folder has no object of its own - expect(restoreObject).toHaveBeenCalledTimes(3); + expect(restoreObject.mock.calls.map((c: any[]) => c[0].key).sort()).toEqual( + [ + objectKey('Series/01.mp3'), + objectKey('Series/02.mp3'), + objectKey('Series/03.mp3'), + `${PREFIX}_thumbnail/cover.jpg`, + ].sort(), + ); }); it('a container tap whose first file cannot be checked probes the next one, and each file is HEADed once', async () => { diff --git a/src/services/GlacierRestoreService.ts b/src/services/GlacierRestoreService.ts index 4f38b1d..ed2e227 100644 --- a/src/services/GlacierRestoreService.ts +++ b/src/services/GlacierRestoreService.ts @@ -286,6 +286,13 @@ export class GlacierRestoreService { (child) => !isContainer(child) && child.id_library_item !== tapped?.id_library_item, ); const keyOf = (file: LibraryItemDB) => `${storagePrefix}/${file.source_path || file.key}`; + // Every chapter carries its own artwork too (the app shows it in the + // book's chapter list), archived under the sibling `_thumbnail/` prefix. + const thawArtwork = async (file: LibraryItemDB) => { + if (file.thumbnail) { + await this.ensureObject(user, file, `${storagePrefix}_thumbnail/${file.thumbnail}`, 'thumbnail'); + } + }; let queue = files; if (!tapped) { @@ -300,6 +307,7 @@ export class GlacierRestoreService { const outcome = await this.ensureObject(user, probe, keyOf(probe), 'object'); if (outcome.issued) { issued = true; + await thawArtwork(probe); break; } if (outcome.state !== undefined) return; // warm, thawing or ready: not a fresh freeze @@ -330,6 +338,7 @@ export class GlacierRestoreService { for (let next = queue.shift(); next; next = queue.shift()) { try { await this.ensureObject(user, next, keyOf(next), 'object'); + await thawArtwork(next); } catch (err) { this._logger.log( {